@TanDevv
Posted
Hi, riteshkumarldh! Great work on this one! Here are a few suggestions regarding your code. :)
šHTML:
- In regards to your
<img src="./images/icon-cart.svg" alt="cart">
, I would suggest leaving any icons/decorative images (eg: pattern backgrounds) that hold no meaningful content with an empty alt attribute, so screen-readers will ignore it and not confuse the users depending on them.
(For more information regarding <alt>, you can read this article also by MDN)
šØ CSS:
- Try to avoid using
px
units for everything. Pixel units do not scale well once you take into account users who may increase the font size via their browser for accessibility, if the website explicitly sets font-sizes in pixels, a heading set at 30px will always be 30px. Which may cause issues for users with visual impairment.rem
orem
units both respectfully scale when needed and would be much preferred when it comes to font sizing, padding/margins and media queries.
If you want something to be a constant size that does not need to be responsive to changes in other elements or little things such as borders, border radius or box shadows then pixel units are the way to go.
(For more information on using units and which are best for what, Kevin Powell does a great video explaining a few of them)
.btn {
display: inline-block;
width: 100%;
background-color: var(--dark-cyan);
border: none;
outline: none;
height: 60px;
border-radius: 10px;
font-size: 18px;
font-weight: 700;
color: var(--white);
display: flex;
align-items: center;
justify-content: center;
gap: 10px;
}
- Regarding to your code above, you have two duplicate
display
rules, I would suggest removing one of them as the one below the other will just overwrite it, making it redundant.
I hope you find this information helpful. Above all, your solution is great, well done! š
Marked as helpful
@riteshkumarldh
Posted
@YorkieLT Hi, very helpful feedback will keep in mind all the mentioned points thanks.