Design comparison
Community feedback
- @VCaramesPosted almost 2 years ago
Hey there! 👋 Here are some suggestions to help improve your code:
- Your content is not fully responsive. Here is a link to Google Developer’s site that will teach you how make it 100% responsive:
- For improved accessibility 📈 for your content, it is best practice to use
rem
for yourfont-size
and other property value. Whileem
is best formedia-queries
. Using these units gives users the ability to scale elements up and down, relative to a set value.
- Move all of these properties to the
body
element
main { background-color: var(--cream); display: flex; justify-content: center; align-items: center; width: 100vw; height: 100vh; }
- The image’s
alt tag
description needs to be improved upon to better describe what it is. You will want to assume that you are describing the image to a someone.
More Info:📚
https://www.w3.org/WAI/tutorials/images/
- This component requires the use of two images 🎑 at different breakpoints. The
picture
element will facilitate this.
Here is an example of how it works: EXAMPLE
Syntax:
<picture> <source media="(min-width: )" srcset=""> <img src="" alt=""> </picture>
More Info:📚
https://www.w3schools.com/html/html_images_picture.asp
- Do not capitalize "perfume" in HTML as it is not accessible friendly. Instead you will want to style it in CSS.
- The only heading in this component, is the name of the perfume; “Gabrielle Essence Eau De Parfum” . The rest of the text should be wrapped in a
paragraph
element.
- Currently, the old price (169.99) 🏷 is not being properly announced to screen readers. To fix this, you are going to wrap the the price in a
del
element and inside it you will add aspan
element with ansr-only class
that will state something like “The previous price was…” and use CSS to make it only visible to screen readers.
More Info:📚
If you have any questions or need further clarification, feel free to reach out to me.
Happy Coding!🎄🎁
Marked as helpful1@SantiagoPoncePosted almost 2 years ago@vcarames Hello! Thank you very much for taking a little of your time to see my code and give me such a dedicated feedback, I will work on what you marked me and I will take advantage of the material you provided me :)
0 - @klduplessPosted almost 2 years ago
You should add alt text to the cart icon to fix the accessibility error. E.g.:
<img src="/images/cart.svg" alt="cart">
Instead of setting individual widths on every child element of
.preview-card__right
, you can set padding on the parent element to better match the design and make the price aligned to the left..preview-card__right { flex: 1 1 50%; display: flex; flex-direction: column; align-items: center; justify-content: space-around; padding: 1em; }
Then remove width from
..preview-card__header
,.preview-card__title
,.preview-card__description
,.preview-card__price
, and.preview-card__buyButton
Marked as helpful1@SantiagoPoncePosted almost 2 years ago@kldupless Hello! Thank you very much for taking the time to review my code and for your feedback, I think I took a wrong path and the solution becomes simpler with what you marked me :)
0
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