Skip to content
Submitted almost 3 years ago

[Desktop first] Stats preview card component

Sandra Ise•20
@sandraise
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Stats preview card component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Community feedback

  • PhoenixDev22•16,950
    @PhoenixDev22Posted almost 3 years ago

    Hello , I have some suggestions regarding your solution :

    • You can use <ul> to wrap the stats class="figures" . Don't use <span> and <div> for meaningful content. Numbers don't make sense as h2s.

    • The number and word have to be read together to make sense so need to be in the same meaningful element. so only a span or maybe strong tag needs to wrap the numbers. the words like companies should not be in paragraph tags. They don't need to be wrapped in anything as they are already inside a meaningful element (list item).

    • To improve the image overlay effect, I would prefer to leave the image in html and use blend modes. you can use background color, Use mix- blend mode and opacity to make it more like the design.

    • To center the component on the middle of the page , you can use the flexbox properties and min-height: 100vh for the <body>.

    body{

   /* max-width: 1440px;*/you don't need to do that as the body represents your viewport. You can have a wrapper div that will be the only child of the body containing all the other elements, you can then set the max-width on that container. In  this  challenge , you can set max-width to the card. */ 
 /* margin: 0 auto;*/ no need for margin 
display:flex;
align-items: center;
justify-content: center;
width: 100%;
min-height: 100vh;/*using vh (viewport height) units to allow the body to set a minimum height value based upon the full height of the viewport.This also allows the body to to grow taller if the content outgrows the visible page.*/
}

    • Using percentages % for width can cause a lot of problem , consider using max-width to the component.

    • Remove the height height: 400px; you almost never want to set it . let the content define the height .

    • Use min-height instead on the background image half.

    • In line-height: 30px ;, use unitless value for the line-height , this is the preferred way to set line-height and avoid unexpected results due to inheritance.Read more in MDN.

    • Last , check the responsiveness and the content is out the card..

    Overall , your solution is good, Hopefully this feedback helps.

    Marked as helpful
    0
    Sandra Ise•20
    @sandraisePosted almost 3 years ago

    @PhoenixDev22 Wow thanks for these comments, I appreciate it. I'll implement those changes now.

    1
  • Momen Hakim•120
    @MomenHakimPosted almost 3 years ago

    good job, check the image violet color, needs to be a bit darker.

    Marked as helpful
    0
    Sandra Ise•20
    @sandraisePosted almost 3 years ago

    @MomenHakim Thank you, I'll look into that now. :)

    0

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