Product Preview Card Component (FlexBox + Mobile-First)
Design comparison
Community feedback
- @MelvinAguilarPosted almost 2 years ago
Hello there π. Good job on completing the challenge !
I have some suggestions about your code that might interest you.
HTML π:
- It is generally not recommended to use multiple h1 tags on a single webpage, as the h1 tag is typically used to indicate the main heading of a document or section. Having multiple h1 tags can create confusion for users and search engines as to which text is the main heading.
- Always avoid skipping heading levels; You can try to use different html headings like h2, h3, h4 in order to create a clear hierarchy of headings and subheadings on the page, which will make the content easier to understand and navigate.
-
You could use the
<del>
tag to indicate the price that was before the discount. Additionally, you can use asr-only
class to describe the discount. This will help screen reader users to understand that the price was discounted.Example:
<del><span class="sr-only">Old price: </span>$169.99</del>
-
You can use the
<picture>
tag when you have different versions of the same image πΌ. Using the<picture>
tag will help you to load the correct image for the user's device saving bandwidth and improving performance. You can read more about this here π.Example:
<picture> <source media="(max-width: 460px)" srcset="./images/image-product-mobile.jpg"> <img src="./images/image-product-desktop.jpg" alt="{your alt text goes here}"> </picture>
Alt text π·:
- The
alt
attribute should not contain the words "image", "photo", or "picture", because the image tag already conveys that information.
-
Not all images should have alt text. The cart-icon is a decorative image, it does not add any information to the page. You should use an empty
alt
attribute instead of a descriptive one. You can read more about this here π.If you want to learn more about the
alt
attribute, you can read this article. π.
CSS π¨:
- Instead of using pixels in font-size, use relative units like
em
orrem
. The font-size in absolute units like pixels does not scale with the user's browser settings. This can cause accessibility issues for users who have set their browser to use a larger font size. You can read more about this here π.
- The
width: 100%
property in themain
tag is not necessary. Themain
tag is a block element and it will take the full width of the page by default.
- Use
min-height: 100vh
instead ofheight: 100vh
. Theheight
property will not work if the content of the page grows beyond the height of the viewport.
I hope you find it useful! π Above all, the solution you submitted is great!
Happy coding!
Marked as helpful1
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