Skip to content
Submitted 3 days ago

QR code component

Abhay Ghorela•40
@AbhayGhorela
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

What are you most proud of, and what would you do differently next time?

I believed that in this section, I would learn the proper technique to use CSS by storing the colors using css variables and root properties. ''' :root { --white: hsl(0, 0%, 100%); --slate300: hsl(212, 45%, 89%); --slate500: hsl(216, 15%, 48%); --slate900: hsl(218, 44%, 22%); } ''' ''' p { font-size: 15px; color: var(--slate500); font-weight: 400; padding: 0.2rem 0.5rem; } '''

What challenges did you encounter, and how did you overcome them?

This assignment helps me recollect my CSS abilities. I had a couple months of not using pure CSS.

What specific areas of your project would you like help with?

If someone has a better answer than me, please tell me where I can improve my code. I would like to gain input, which will help me develop my skills.

Community feedback

  • Mahmoud-AbdelKarim•530
    @Mahmoud-Abdelkarim777Posted 2 days ago
    • For a good user experience, remove the max-width: 1440px; from body
    Marked as helpful
    0
  • Michał Pawlak•180
    @MichalPawlak0Posted 3 days ago

    Looking good, nice job. Not much to complain about.

    I noticed that you added max-width on body tag which could be problematic (check out what happens on screens larger than 1440px using dev tools) - usually what we tend to do when we want to keep all content within a certain width limit is we put all the content inside a separate element (eg. div). We then add max-width and margin: 0 auto (to keep it centered). But not on the body element itself.

    Also, the image alt property seems to be missing a value.

    Good use of border-radius, margins, and box-shadow.

    Marked as helpful
    0
    Abhay Ghorela•40
    @AbhayGhorelaPosted 1 day ago

    @MichalPawlak0 I appreciate your feedback and thank you for it. I give it 100% width when I'm working on it, and when I read the style guide md file after finishing, I find that it instructs me to set the desktop width to 1440 pixels. I then adjust the width 100% to 1400 pixels. However, you're correct, and I just forgot to add a value to the alt tag element.

    0
  • Kamran Kiani•1,840
    @kaamiikPosted 1 day ago
    • Add a separate file for your CSS styles.
    • Wrap all your contents in a main and the .attribution inside a footer.
    • No need for width: 100%; and max-width: 1440px;. There are pointless.
    • Use min-height: 100vh; instead of height:100vh;. height: 100vh strictly limits the height to the viewport size, potentially causing overflow issues if the content is larger than the viewport. On the other hand, min-height: 100vh allows your element to grow in height if the content exceeds the viewport size.
    • Your img only needs a max-width: 100%; and nothing else.
    • Try to use a proper CSS reset at the start of your CSS style. Andy Bell and Josh Comeau both have a good one. You can simply search on the internet to find them.
    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