Skip to content
OverviewSolutions
4
Comments
8
Funsho Ayobanjo
@ayobanjo

All comments

  • Funsho Ayobanjo•150
    @ayobanjoPosted about 2 years ago

    Hello @Yusif70, you did well on this challenge

    I just have a few suggestions.

    To make the main responsive, on all screen sizes, change the width: 300px to max-width: 300px

    Also on the body, set the justify-content: centre and min-height: 100vh, with this, you wouldn't need to set the hard-coded margin-top

    I hope this helps.

    1
  • Funsho Ayobanjo•150
    @ayobanjoPosted about 2 years ago

    Hi @kagiso101 Good Job on this.

    Just a few corrections to make it more responsive and less rigid. Mark as helpful if you like it

    HTML

    Change the div to the more semantic main tag

    CSS

    1. Set the main div to a max-width instead of width to make it responsive

    2. Set the img to max-width: 100%, to always make sure it retains the width of the container

    3. Remove the width and height on the h1, and p tags. They are naturally responsive. Then add a padding to the main div

    Marked as helpful
    0
  • Funsho Ayobanjo•150
    @ayobanjoPosted about 2 years ago

    Hi @IsuruAkalanka, Good job on this. Just a few corrections to make it more responsive. Mark as helpful if you like it

    HTML

    Make the div a main for a more semantic structure.

    CSS

    1. On the div change the width to max-width: 300px for responsiveness

    2. Set the img element to max-width: 100% to always keep it within its container

    Marked as helpful
    1
  • Funsho Ayobanjo•150
    @ayobanjoPosted about 2 years ago

    Hi @IndraSaputraIdrus Good job on your first project, lovely

    The only observation I have is that for mobile responsiveness, you should use a max-width instead of width on the card div

    max-width : 260px or its equivalent in rem

    0
  • Funsho Ayobanjo•150
    @ayobanjoPosted about 2 years ago

    Hello @villAsh. Congratulations on your first submission.

    I do have a few corrections you could apply

    HTML

    1. For SEO, and proper semantics, you should use the <picture> tag. This will help you to also change from either mobile img to desktop img depending on what your approach is (mobile-first or not) picture tag

    2. The Gabrielle Essence Eau De Parfum should be an h1 element, and the perfume a paragraph

    3. For the former price, you can simply use the del tag

    CSS

    1. The height of the img should be 100%, so it simply fills the available space in its container

    2. The perfume has a letter spacing, which you have omitted, try

      letter-spacing: 0.25rem, or whichever value you find desirable

    3. You can set a max-width for your main container on the desktop view.

    Here is my own solution. I hope you find this helpful my submission

    Marked as helpful
    0
  • Funsho Ayobanjo•150
    @ayobanjoPosted about 2 years ago

    Hello @josephmadras, good Job, I do have a few corrections.

    For the perfume, there is letter spacing, which you have omitted letter-spacing: 0.25rem or whatever spacing you find desirable.....

    Hope this helps

    Marked as helpful
    0
  • Funsho Ayobanjo•150
    @ayobanjoPosted about 2 years ago

    Your solution is elegant, I like it. Just completed this same task myself. The only thing is the margins added/ gap in the mobile view, use gap or margins instead on space-evenly

    0
  • aljacly•20
    @aljaclySubmitted about 2 years ago

    This is my first time creating something with HTML and CSS. Any advice on best practices is appreciated.

    I struggle with understanding how to use layouts more effectively, hence why "px" is used a lot when positioning elements.

    I am also unsure if I am using CSS selectors correctly with the div tag.

    QR code component using basic HTML and CSS

    3
    Funsho Ayobanjo•150
    @ayobanjoPosted about 2 years ago

    Hello Aljacly, I just submitted this myself, few observations though #CSS

    1. You should create a CSS file to make your HTML file cleaner
    2. You should use flex on your body tag to centre its children(main or the div containing the code)
    3. The div class border is breaking the code. You should remove it once the flex is added

    #HTML

    1. Your text should be in a paragraph tag, or heading tag, not in a div

    Hopefully, I have been of help. Still a lot to be done though

    Marked as helpful
    1