Skip to content
Submitted over 2 years ago

Product Review Card Component

Ali Hafidz•40
@Hanzz14
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Product preview card component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Solution retrospective

I wish someone can correct my project so this is was my very first project i got a problem in dekstop view i'm trying to use display:grid on container but it wont display as a grid so im trying to use float. everything is good when the window is a half my laptop screen. but it ruined when i fullscreen it

Community feedback

  • Chamu•13,130
    @ChamuMutezvaPosted over 2 years ago

    Hi Ali . Welcome to Frontend Mentor

    HTML

    • Semantic elements such as the main are considered important in a site, i would further say that at least the main element should be present in your site , here is what MDN says: The <main> HTML element represents the dominant content of the <body> of a document. The main content area consists of content that is directly related to or expands upon the central topic of a document, or the central functionality of an application.
    • heading elements should follow a sequential order, with the h1 being the first heading element and the next heading following an h1 must be an h2. That is an h2 should not come first ahead of an h1 element.
    • The picture element is displaying the images correctly, well done.

    CSS

    • font sizes , use rems instead of px , read the following Why designers should move from px to rem (and how to do that in Figma)
    • generally , you need to do away with floats, there are rarely used these days. Grid does work on this solution but i would have preferred to use flexbox. Since you had opted for Grid, for the desktop design, to get the image and the content side by side use grid-template-columns: 1fr 1fr; on the container element. The container element should have a max-width in order to match the design
    • the content article should be centered as well, that can be done using flexbox for example
    • Let your elements have classes for styling.
    Marked as helpful
    1

Please log in to post a comment

Log in with GitHub

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