Skip to content
OverviewSolutions
5
Comments
3
Jhonatan
@JhonyDomingos

All comments

  • Jhonatan•90
    @JhonyDomingosPosted almost 2 years ago

    Hey congratulations for your solution.. Hey I have some tips if you wanna test for your solution

    1. Adding to the body min-height 100vh the elements should occupy the web browser viewport height.

    2. .container you add text-aling:center; set the width with a value in rem not in %, I think 18.5rem will awsome, you can declare margin 30px to the top and auto ex:. margin:30px(top) auto;

    3. h1 do not need a width and you can declare the padding like this padding:(top)20px (right)30px (left)30px 0(bottom); and check this padding I think it's a little too much, I think the padding 15px its enough

    4. p tag I think you can remove the paddings because already and put in the container it will be aplied to h1 and p tags

    5. on .img do not need the height:100% and you can add max-width:100% and set the width:100% and check the paddings maybe you won't need them

    6. You can change all the sizes px to rem to resize better when the screen gets smaller

    I think that's all!!! there are a few tips that can improve on your solution!! I'm here to help anything, I'm gald in help ya !!! if it was useful for ya I'll be glad

    Marked as helpful
    1
  • Jhonatan•90
    @JhonyDomingosPosted almost 2 years ago

    Hey, Great Job buddy @Sudheer1918, Good Solution I like the way that you worked with the flexbox It's awesome.. I would recommend use mainfor the container landmark but your solution is great... if you wanna take a look at my solution, I appreciate your feed-back in anything that I can improve my solution!! congrats

    0
  • Jhonatan•90
    @JhonyDomingosPosted almost 2 years ago

    Hey congratulations for your solution 🎉🎉 Here are a few tips that I've received from my solution that works very well

    • Replace the <h2> containing the main title with <h1> note that this title is the main heading for this page and every page needs one h1 to show which is the most important heading. Use the sequence h1 h2 h3 h4 h5 to show the hierarchy of your titles in the level of importance, never jump a level.

    • Use relative units as rem or em instead of pxto improve your performance by resizing fonts between different screens and devices. These units are better to make your website more accessible. REM does not just apply to font size, but to all sizes as well.

    • Use <main> instead of <div> to wrap the card container. This way you show that this is the main block of content and also replace the div with a semantic tag.

    • Put an alt value <img> tag to improve the accessibility.

    • There are a few things that you can put on your code as you wish!!

    Great solution 👍

    Marked as helpful
    2