Product preview card component using Tailwind
Design comparison
Community feedback
- @grace-snowPosted about 1 month ago
Hi, there are a few suggestions to improve this:
- use the picture element instead of loading both images. It's much better for performance as well. The mobile image should be the default and the desktop in the source tag with a media query.
- look up how and when to write alt text on images. The alt should never say words like "image" because it's already on an image role. The alt on the main image should describe what the image is showing, the key visible features of this product.
- the shopping cart image is decorative though so should not have a filled in alt description. The alt should be left intentionally blank. There are some good posts about alt text in the resources channel on discord if you want to take a look.
- when building single component demos like this it's important to consider how they would be used in a real site. This is a simple product card, it would never be used to serve the main heading on a page, so you know it shouldn't have a h1. Use a lower importance heading level like h2 instead.
- the old/original price needs wrapping in a strike through element (
s
). - for people using a screen reader it could be difficult to understand the two prices. I recommend you add some screenreader-only text in a span to make this clearer. Eg
<span class="sr-only">Original price: </span>
. - divs are not allowed as a child of button. Use a span there.
- instead of adding custom classes inline I recommend you include the things you need like colours and fonts in the tailwind config.
- make sure the font sizes are accurate. The body font size (which is also the paragraph size) should match the style guide except converted to rem. You shouldn't ever end up with strange values like 14.4px, which is what the 0.9rem value you've used equates to.
- it's not wrong at all using flex for this challenge but it would be slightly shorter css to use css grid in this case. The elements would stack by default in display grid, then larger screens only need two 1fr columns on larger screens. There's no need for any widths.
- this solution is hitting my screen edges on all sides on my phone. Add a little padding to the body so this can't happen.
1@mehrnaz98Posted about 1 month agoHi @grace-snow! Thank you for your valuable suggestions. I truly appreciate your insights on improving the project, particularly regarding the use of the <picture> element, proper alt text for images, and accessibility considerations for the pricing display, including the use of sr-only for clearer screen reader navigation. I appreciate your suggestions and will keep them in mind as I work on the project. Thanks again!
0 - @kaamiikPosted about 1 month ago
Hi. Just a quick note. Instead of using two
img
inside your html, you can usepicture
element that is more robust and change your image based on screen size. It also reduces the load time. You can read more aboutpicture
here inside MDN0@mehrnaz98Posted about 1 month agoThanks for the suggestion @kaamiik ! I appreciate the insight on using the <picture> element for better responsiveness. I'll definitely check out the MDN article!
1
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord