@MelvinAguilar
Posted
Hi @INS140 👋, good job completing this challenge! 🎉
I have some suggestions you might consider to improve your code:
- Update your <picture> element with 600px instead of 400px:
<source media="(min-width: 600px)" srcset="./assets/images/image-product-desktop.jpg">
- Use
min-height: 100vhto thebodyselector instead ofheight. This property lets you set a height and let the element grow even more if necessary.
- For specificity reasons you should work better with classes since they are reusable, and you can leave the ID when you work with Javascript.
- You could use the <del> tag to display the old price:
<del>
<span class="sr-only">Old price: </span>$169.99
</del>
Note that I added the <span> with the sr-only class to the del element, this will provide more information about what your old price is about.
The sr-only class is a class that you can add to hide content visually but is only visible to screen-readers.
Above all, the project is done well👏. I hope those tips will help you! 👍
Good job, and happy coding! 😁
Marked as helpful
@INS140
Posted
@MelvinAguilar Thank you for the great feedback! The advice you gave me fixed that bug I had with the picture element. I'll keep in mind that images need to change at the same interval as media queries. I also read the article you attached and found it very interesting. I hadn't considered the use of a screen reader when designing the pricing section. I plan to pay more attention to details like this in my future projects.
@MelvinAguilar
Posted
@INS140 You are welcome!👍 Good luck with your new projects
