Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Cannot read properties of null (reading 'code')
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Product Preview Card Component with Vanilla CSS

@joecobb

Desktop design screenshot for the Product preview card component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


I would love to know what you think and how I can improve it.

Community feedback

kxnzx 870

@kxnzx

Posted

Hello @joecobb,

Your mobile version looks great! To finish your desktop version use in your div wrapper: display: grid and grid-template-columns: 1fr 1fr;

  • In your div wrapper us max-width instead of width, because with a max-width your element can shrink when the screen is resized.

  • Instead of <div id="wrapper"></div> use <main id="wrapper"></main>. The main tag is considered an accessibility landmark and ensures that a screen reader and people with visual impairment have the ability to jump to sections of the page. Examples of landmarks are: header, nav, main, aside, section, footer.

  • It is good to use div’s for the purpose of styling and arranging components on the page. Div’s are like empty containers, but they do not have any semantic meaning. So instead of <div id="content"></div> use <section id="content"></section>. You can use the section tag when the content within is related. As you can see the image with the perfume bottle and the description are related.

  • Wrap “Gabrielle Essence Eau De Parfum” into an <h1></h1> tag. This is very important for search engines, screen readers, SEO and people with visual visual impairment to know what your page is about. You always need ONE h1 in your page. Then (when needed) you follow it up with subheadings such as an h2, h3, h4 etc. You can wrap “PERFUME” in an <h2></h2> tag. Even though it starts earlier on the page, this does not matter. It’s the hierarchy that matters. Just remember that the most important title on the page is an h1.

Marked as helpful

1

kxnzx 870

@kxnzx

Posted

@joecobb Yrw! :)

0

@toonchavez8

Posted

Great job on submitting a solution! That's always a big step!

I've reviewed your code and I have some suggestions that might help.

Firstly I highly recommend creating a .css file and linking it using the <link> tag like this <link rel="stylesheet" href="style.css"> and place all of your CSS there to make your HTML more readable.

In terms of HTML regarding accessibility, there are some good semantic articles I would recommend you take a look at like this one... Web.dev/landmarks

the jist of the article is that HTML provides landmark tags that will help users identify what section of content they are reading, just as the <main></main> tag, I would wrap all of the components within that.

Another point of the article is declaring titles or headings as their respective heading case, I would recommend changing some of your <divs> for H1 and H2 and then styling them according to the goal.

in terms of css i see that you declared your #content with a display:relative and im not if you meant to go with a position:relative or a Display:Flex if its that latter you would need to declare it as display:flex; flex-direction:column; in for it to not change your current flow

Marked as helpful

0

Please log in to post a comment

Log in with GitHub
Discord logo

Join 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