@joshjavier
Posted
Hi Gabriel! :)
Nice work on your solution. I like that you used descriptive names for classes, which makes the code more readable.
Syntax-wise, your HTML is okay. You can improve this by using HTML5 semantic tags, e.g., you can use <main>
instead of <div class="wrapper">
which is more descriptive (and also solves the accessibility issues that show up in your solution's report).
For the image, your solution of using two <img>
tags and hiding one depending on the viewport size works fine. As an alternative, I recommend looking into the <picture>
element, which allows you to specify multiple versions/sizes of an image for different screen sizes. This way, you don't have to write additional media queries. You can check out my solution to see how I implemented it.
In terms of accessibility, good job adding descriptive alt text to the main images. However, I noticed you also added alt text to the cart icon in the button. It's actually best practice to use an empty alt text attribute for images that are purely decorative, which applies in this case since there's already a text in the button that says "Add to Cart". In fact, screen readers would read this as "cart icon Add to Cart, button" which is repetitive. You can use an empty alt text for the cart icon so that screen readers will read it as "Add to Cart, button" which is better. :)
Happy coding!
Marked as helpful
@Gabriel4PF
Posted
@joshjavier I see what you are saying, great explanation I actually did not think that's how it would be read to the screen readers. I wont forget Thank you