Skip to content
OverviewSolutions
27
Comments
26

Sarah

@sarahc-dev835 points

I’m a mysterious individual who has yet to fill out my bio. One thing’s for certain: I love writing front-end code!

Latest solutions

View more solutions

Latest comments

  • Sarah•835
    @sarahc-dev

    This looks great!

    I love the image zoom animation at the bottom! Really nice touch.

  • Sarah•835
    @sarahc-dev

    Hey!

    I think the problem is you are using % for the width of your cards - so they grow and shrink with the screen and the flex-wrap can never kick in. If you change you cards to a width or max-width in rem, the flex wrap will then work as it should.

    Also - if you want to use flexbox and not grid - I would create a separate container for the time tracking cards - and apply flex only to these (and not the initial profile card). Then you don't have the issue of trying to move the second row across.

    Hope that helps :)

  • Sarah•835
    @sarahc-dev

    Hey! This looks good - so well done! A few tips that might help you on future challenges -

    • Use min-height: 100vh instead of 100% on the body and don't set a height on the html tag.
    • You don't need so many divs! If it only contains one element - you probably don't need it.
    • Avoid setting a height and width on everything - try to use padding and margin to create the spacing instead.
    • For accessibility, avoid using px values for font sizes, heights and widths - use rem or em instead

    Hope that helps. Keep going!

    Marked as helpful
  • Sarah•835
    @sarahc-dev

    This looks great - congrats!

    Just one tip - you should write your code mobile first and use a media query with min-width to make the amendments for larger screen sizes. It's best to get used to this way from the beginning :)

    Hope that helps!

    Marked as helpful
  • Sarah•835
    @sarahc-dev

    Hey! This looks good - I wouldn't say you have spaghetti code at all!

    • Using h1 instead of h2 in this instance would stop the accessibility report shouting at you - you wouldn't typically have more than one h1 - but in this design where the 3 main headers are of equal importance it doesn't cause a html error I don't believe.
    • Use rem for your font sizes instead of px for better accessibility (allows a user to set browser to a larger font). But if you do this, you also largely need to set heights and widths if using in rem or em so everything's container can also grow if font is set to large.
    • I wouldn't use justify-content: space-around to create the spacing between the text - i have a thin screen and everything gets squashed for me. Better to use padding or margins to ensure the space is always there on different screen sizes.

    Hope that helps :)

  • Sarah•835
    @sarahc-dev

    Hey!

    • My understanding is that if you are using semantic HTML like <article> or <section> and the heading for each is of equal importance such as in this design and there is no other main heading - it is ok to use multiple h1s.
    • Using margin under the paragraph is what is giving you the height of the container. An alternative would be to set a height in rem or vh and use margin-top:auto on the button to push it to the bottom. You have to be careful of the text overflowing at different screen sizes if you do this - but it does keep the button in the same place.

    Hope that helps!

    Marked as helpful
View more comments