Skip to content
Submitted over 1 year ago

-not-responsive

blue-knight99•40
@blue-knight99
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...QR code component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Solution retrospective

The center issue was solved

Community feedback

  • Chamu•13,130
    @ChamuMutezvaPosted over 1 year ago

    Hi BLUE-KNIGHT99

    So far so good with your solution, but I will add some issues that needs attention:

    1. There are some divs that I think are not necessary. Whilst you have the important main element, this is followed by a div with the class main. There are other divs surrounding the text elements as well which I think should be done away with.
    2. A site should have a heading element - all the text content you have assigned the paragraph element, one of them should be a heading element.
    3. The image in this case should have an alt value with text, I think it has a message for the user (this is just a personal opinion)
    4. For font sizes, it is recommended not to use px values. The following article will guide with the reasons. Why font sizes should never be in px
    5. Using explicit sizes on elements can cause you some issues, especially when working on large projects, hence I would advise you to use padding or/and margins when you need that extra space.
    Marked as helpful
    1
    blue-knight99•40
    @blue-knight99Posted over 1 year ago

    @ChamuMutezva

    Thanks a lot for suggestions

    0
  • VHugo632•220
    @VHugo632Posted over 1 year ago

    Hello ! Good job for your solution :)

    Concerning your problem about the center of your component, you can put into <body> with CSS :

    • "witdh: 100%"
    • "height: 100vh"
    • "display: flex"
    • "justify-content: center"
    • "align-items: center"

    Moreover, delete "margin-top: 5%" from <main> to have a perfectly centered component.

    Finally, I have noticed that your <header> (the main text of the card) and in particular his content (so <p> with "Improve your front-end ...") is not centered. To fix this, you can put into <p> : "text-align: center".

    I hope that my message will help you, so tell me if it works or if you still need help :)

    Marked as helpful
    1
    blue-knight99•40
    @blue-knight99Posted over 1 year ago

    @VHugo632

    That's what I'm trying to do

    Thanks Hugo 🔥

    1
    VHugo632•220
    @VHugo632Posted over 1 year ago

    @blue-knight99

    No problem :)

    I'm happy to have helped you !

    1
  • Zubair Abid•280
    @ZubairAbid101Posted over 1 year ago

    Hello!!!

    Congratulations on completing the challenge!

    There are some things that you can improve in the code,

    • An efficient way to dynamically center elements is to use display: flex, justify-content: center, align-items:center.
    • Another way to do this is to assign the element in question a specific width and height value and then use margin: auto auto.

    I hope you find this comment helpful!

    Did you know that there's a mark as helpful and an up-vote option?

    There's even a Follow button!!!

    Happy Coding!!!

    Marked as helpful
    0
    blue-knight99•40
    @blue-knight99Posted over 1 year ago

    @ZubairAbid101 Thanks for advice Zubair.

    Actually I tried to use align-items:center but it did not work. So at this point I don't know how I solve this issue

    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