Looking to hire developers?
@Deolabest
@Einaroks
Submitted
@Deolabest
Posted
Hey , Congratulations on completing this challenge!
Here is my feedback:
You're using the span element too much.
Use <main> instead of a simple <div> to improve the semantics and accessibility on the page. Remember that every page should have a <main> block and that <div> doesn't have any semantic meaning.
To answer your questions.
*1. The Add to Cart text and icon are both in the button. Just put them in it like this:
<button class="btn" type="button"><img src="images/icon-cart.svg" alt="">Add to Cart</button>
Then you can give the img some margins to create space.
*2. For the smaller old price, you have to make sure both prices are under one parent elements. i.e.
<div class="prices">
<strong>$149.99</strong>
<p class="original-price">$169.99</p>
</div>
Then use this CSS:
.prices {
display: flex;
margin: 20px;
}
strong {
font-size: 2rem;
margin-right: 15px;
}
.prices p{
font-size: 0.8rem;
margin-top: 12px;
}
*3. Already answered in the first question.
Keep doing a good job!
Marked as helpful
@0xabdulkhalid
Submitted
Hello everyone..👨💻
grid & flex layout.Things i tweaked up..🚀
scrollreveal.js on dom elements to animate themFeel free to leave comments on how I can improve my code.
Peace be upon you..✨
@Deolabest
Posted
Hey @0xAbdulKhalid, Congratulations on completing this challenge!
Here is my feedback:
Your design is so amazing. The animation makes it more attractive.
The project is not responsive enough for iPad and Tablet users (481px — 768px).
Some of the design is cutting out for the mobile users (320px — 480px) too.
Keep doing a good job!
Marked as helpful
@Kuraanal
Submitted
@Deolabest
Posted
https://www.youtube.com/watch?v=BW0FCFV323s
Hey @Kuraanal, Congratulations on completing this challenge!
Here is my feedback:
Firstly, you did not upload your project properly on Github. Please watch this video for guidance https://www.youtube.com/watch?v=BW0FCFV323s
It's not a good practice to use px to set font-size. Instead use rem units, they are great since they adapt better to the font-size the user will set in the browser settings. You can convert from px to rem here: https://pixelsconverter.com/px-to-rem.
The project is not responsive enough. The width of the main element should determine where the media query should begin.
Keep doing a good job!
@xdcron
Submitted
Very happy with how this came out .would love to hear some feedback.
@Deolabest
Posted
Hey @xdcron, Congratulations on completing this challenge!
Here is my feedback:
Keep doing a good job!
Marked as helpful
@Hermodesign
Submitted
@Deolabest
Posted
Hey @Hermodesign, Congratulations on completing this challenge!
Here is my feedback:
It's not a good practice to use px to set font-size. Instead use rem units, they are great since they adapt better to the font-size the user will set in the browser settings. You can convert from px to rem here: https://pixelsconverter.com/px-to-rem.
Use <main> instead of a simple <div> to improve the semantics and accessibility on the page. Remember that every page should have a <main> block and that <div> doesn't have any semantic meaning.
This property background-color: hsl(212, 45%, 89%); should be under the body element to avoid the white space in the body.
i.e.
body {
height: 100vh;
display: flex;
align-items: center;
justify-content: center;
background-color: hsl(212, 45%, 89%);
}
Keep doing a good job!
Marked as helpful
@Spsingh0005
Submitted
@Deolabest
Posted
Hey @Spsingh0005, Congratulations on completing this challenge!
Here is my feedback:
You're using about 3 different font-family that's making the required one for the project Montserrat not to work.
Keep doing a good job!
Marked as helpful
@ecemgo
Submitted
@Deolabest
Posted
Hey @ecemgo, Congratulations on completing this challenge!
Your solutions are top notch and I've learnt a lot from them.
I noticed the top border-radius is missing from the mobile version.
Keep doing a good job!
@visualdenniss
Submitted
It starts with a loader animation, once you get to Thank You page, you can click on Thank you again to go back and forth between two pages/states.
A challenge i've done while ago, so i'vent changed the old code, (e.g. conversion from px to rem etc) but i've just added some animations today.
@Deolabest
Posted
Hey , Congratulations on completing this challenge!
Here is my feedback:
The thanks page doesn't have the option to rate again.
5 star rating is selected by default which shouldn't be the case.
NB: I just started learning JS and I'm checking out some related projects here.
Keep doing a good job!
@jmjules
Submitted
I tried to use radio buttons so that my HTML is more semantic. For some Reason the content is slightly more to the left for Desktop in the given designs, which gave me difficulties in recreating the layout.
@Deolabest
Posted
Great work!
You forgot to add the JS file on Github.
@simplysabir
Submitted
@Deolabest
Posted
Hey @simplysabir, Congratulations on completing this challenge!
Here is my feedback:
It's not a good practice to use px to set font-size. Instead use rem units, they are great since they adapt better to the font-size the user will set in the browser settings. You can convert from px to rem here: https://pixelsconverter.com/px-to-rem.
Use <main> instead of a simple <div> to improve the semantics and accessibility on the page. Remember that every page should have a <main> block and that <div> doesn't have any semantic meaning.
Keep doing a good job!
Marked as helpful
@dragonanson
Submitted
@Deolabest
Posted
Hey @dragonanson, Congratulations on completing this challenge!
Here is my feedback:
It's not a good practice to use px to set font-size. Instead use rem units, they are great since they adapt better to the font-size the user will set in the browser settings. You can convert from px to rem here: https://pixelsconverter.com/px-to-rem.
Use <main> instead of a simple <div> to improve the semantics and accessibility on the page. Remember that every page should have a <main> block and that <div> doesn't have any semantic meaning.
Keep doing a good job!
@YaswanthSaiCh
Submitted
@Deolabest
Posted
Hey @YaswanthSaiCh, Congratulations on completing this challenge!
Here is my feedback:
It's not a good practice to use px to set font-size. Instead use rem units, they are great since they adapt better to the font-size the user will set in the browser settings. You can convert from px to rem here: https://pixelsconverter.com/px-to-rem.
Use <main> instead of a simple <div> to improve the semantics and accessibility on the page. Remember that every page should have a <main> block and that <div> doesn't have any semantic meaning.
Also add a little margin to the bottom of the original price.
Keep doing a good job!
@nicolas-lukita
Submitted
@Deolabest
Posted
Hey @nicolas-lukita, Congratulations on completing this challenge!
Here is my feedback:
Keep doing a good job!
@31-wtcr
Submitted
@Deolabest
Posted
Hey @31-wtcr, Congratulations on completing this challenge!
Here is my feedback:
https://pixelsconverter.com/px-to-rem.Keep doing a good job!
Marked as helpful
@Amadievans
Submitted
All feedbacks are needed, thanks in advance.
@Deolabest
Posted
Hey @Amadievans, Congratulations on completing this challenge!
Here is my feedback:
It's not a good practice to use px to set font-size. Instead use rem units, they are great since they adapt better to the font-size the user will set in the browser settings. You can convert from px to rem here: https://pixelsconverter.com/px-to-rem.
Use <main> instead of a simple <div> to improve the semantics and accessibility on the page. Remember that every page should have a <main> block and that <div> doesn't have any semantic meaning.
Keep doing a good job!
Marked as helpful
@Mecha-Mind
Submitted
@Deolabest
Posted
Hey @Mecha-Mind, Congratulations on completing this challenge!
Here is my feedback:
It's not a good practice to use px to set font-size. Instead use rem units, they are great since they adapt better to the font-size the user will set in the browser settings. You can convert from px to rem here: https://pixelsconverter.com/px-to-rem.
Use <main> instead of a simple <div> to improve the semantics and accessibility on the page. Remember that every page should have a <main> block and that <div> doesn't have any semantic meaning.
Keep doing a good job!
Marked as helpful
@JoshuaAsistio
Submitted
@Deolabest
Posted
Hey @JoshuaAsistio, Congratulations on completing this challenge!
Here is my feedback:
It's not a good practice to use px to set font-size. Instead use rem units, they are great since they adapt better to the font-size the user will set in the browser settings. You can convert from px to rem here: https://pixelsconverter.com/px-to-rem.
Use <main> instead of a simple <div> to improve the semantics and accessibility on the page. Remember that every page should have a <main> block and that <div> doesn't have any semantic meaning.
Keep doing a good job!
Marked as helpful
@DoyeDesigns
Submitted
I found putting the elements in the middle of the screen a bit challenging. I'm sure of all areas of my code
@Deolabest
Posted
Hey @DoyeDesigns, Congratulations on completing this challenge!
Here is my feedback:
body {
min-height: 100vh;
align-items: center;
justify-content: center;
display: flex;
}
https://pixelsconverter.com/px-to-rem.Keep doing a good job!
Marked as helpful
@imbranatoInformatico
Submitted
@Deolabest
Posted
Hey @imbranatoInformatico, Congratulations on completing this challenge!
Here is my feedback:
It's not a good practice to use px to set font-size. Instead use rem units, they are great since they adapt better to the font-size the user will set in the browser settings. You can convert from px to rem here: https://pixelsconverter.com/px-to-rem.
Use <main> instead of a simple <div> to improve the semantics and accessibility on the page. Remember that every page should have a <main> block and that <div> doesn't have any semantic meaning.
Keep doing a good job!
Marked as helpful
@mlevrat
Submitted
@Deolabest
Posted
Hey @mlevrat, Congratulations on completing this challenge!
Here is my feedback:
It's not a good practice to use px to set font-size. Instead use rem units, they are great since they adapt better to the font-size the user will set in the browser settings. You can convert from px to rem here: https://pixelsconverter.com/px-to-rem.
Use <main> instead of a simple <div> to improve the semantics and accessibility on the page. Remember that every page should have a <main> block and that <div> doesn't have any semantic meaning.
Keep doing a good job!
@LeLeoSK88
Submitted
My first challenge done, it took me some time to do it, I've been learning html and css for 2 weeks, and trying to do some challenges here, if anyone has any tips, any advice to improve, would be most welcome and thanks
@Deolabest
Posted
Hey @LeLeoSK88, Congratulations on completing this challenge!
Here is my feedback:
It's not a good practice to use px to set font-size. Instead use rem units, they are great since they adapt better to the font-size the user will set in the browser settings. You can convert from px to rem here: https://pixelsconverter.com/px-to-rem.
The button doesn't need any border. You need to remove it. Solution:
.butt { border: none; }
Keep doing a good job!
@naitiklodha
Submitted
Kindly give your suggestions on making the code better.
@Deolabest
Posted
Hey @naitik-lodha, Congratulations on completing this challenge!
Here is my feedback:
It's not a good practice to use px to set font-size. Instead use rem units, they are great since they adapt better to the font-size the user will set in the browser settings. You can convert from px to rem here: https://pixelsconverter.com/px-to-rem.
Use <main> instead of a simple <div> to improve the semantics and accessibility on the page. Remember that every page should have a <main> block and that <div> doesn't have any semantic meaning.
Keep doing a good job!
@Matze9
Submitted
@Deolabest
Posted
Hey @Matze9, Congratulations on completing this challenge!
Here is my feedback:
It's not a good practice to use px to set font-size. Instead use rem units, they are great since they adapt better to the font-size the user will set in the browser settings. You can convert from px to rem here: https://pixelsconverter.com/px-to-rem.
Use <main> instead of a simple <div> to improve the semantics and accessibility on the page. Remember that every page should have a <main> block and that <div> doesn't have any semantic meaning.
Keep doing a good job!
Marked as helpful
@Vinzz34
Submitted
@Deolabest
Posted
Hey @Vinzz34, Congratulations on completing this challenge!
Here is my feedback:
It's not a good practice to use px to set font-size. Instead use rem units, they are great since they adapt better to the font-size the user will set in the browser settings. You can convert from px to rem here: https://pixelsconverter.com/px-to-rem.
Use <main> instead of a simple <div> to improve the semantics and accessibility on the page. Remember that every page should have a <main> block and that <div> doesn't have any semantic meaning.
Keep doing a good job!
Marked as helpful
