Skip to content
Submitted over 1 year ago

QR Component using HTML and CSS (2nd iteration)

Priyanka Roy Choudhury•100
@PriyankaRC16
A solution to the QR code component challenge
Preview (opens in new tab)Code (opens in new tab)

Solution retrospective

Hi everyone!

I have iterated and worked on my second version of the qr component page.

Any feedback will be appreciated!

Thanks

Code
Couldn’t fetch repository

Please log in to post a comment

Log in

Community feedback

  • Grace•32,130
    @grace-snow

    For CSS I'm going to give some essential tips and principles for you to have a go at refactoring rather than going through line by line

    1. Always use a modern css reset at the start of the styles. Andy Bell has a good one you can look up and use. Be sure to read his post(s) about it properly to understand what it is and why it includes what it does
    2. Never declare font sizes in px
    3. Remove all heights and widths in this (min height is fine on the body). Never limit the height of elements that contain text. As developers we have to build robust solutions that could consume content of a different length if editors change it, or different default font sizes which many many people change
    4. Most components, this includes, only need a max-width in rem, no widths
    5. Correct colors are provided in the style guide. The text must be readable
    6. Not everything needs to be flex. Only use flex very intentionally
    7. Line height should be set with a unitless value eg 1.5. Don't rely on keywords that can differ between browsers.
    8. Text align is inherited if placed on elements higher in the DOM
    9. give the component a little margin or the body a little padding to prevent the component from ever hitting screen edges
    10. There is no need for any media query on this challenge. You will need them on future challenges so it's worth reading about how to use media queries well
    Marked as helpful
  • Grace•32,130
    @grace-snow

    This looks great now. I don't think you need the .container div at all though. All of those styles can go on the body:

    display: flex;
  align-items: center;
  justify-content: center;
  flex-direction: column;
  min-height: 100vh;
  text-align: center;
}
    Marked as helpful
  • Grace•32,130
    @grace-snow

    First things first, you need to completely rewrite the html on this I'm afraid

    • Indent your code consistently so it's readable and easier to spot bugs. Your code editor can even do this formatting automatically for you
    • always use landmarks to contain content. This should have a main (Every page should) and a footer just for the attribution
    • never have text in divs or spans alone. Think about the meaning of the content and choose appropriate meaningful elements. This component clearly has an image, a heading and a paragraph. As you move onto other challenges there will be other elements required. Take time to think that through
    • don't use inline styles (there may be exceptions to this, but they are a long long way in the future - for now, treat that as a rule)
    • the image is the most important content in this component. Yet you have given it a description of "no image". It needs a properly written description saying what it is (QR code) and where it goes (to FrontendMentor.io)
    • do not split elements to force line breaks in specific places. Line breaks will occur naturally based off the available space and any max-widths applied
    • load fonts in the html head. Google fonts will give you the exact code to use once you select the fonts and weights you need
    Marked as helpful
  • P
    gladstone28•530
    @gladstone28

    I like your solution to the QR Code component challenge. However I notice the following error: In the HTML, you are missing the closing </div> tag for the <div class="body-card"> element.

    Marked as helpful
  • P
    Øystein Håberg•13,260
    @Islandstone89

    You should not have a div outside main. Change .container to main, and .card to div.

  • lisztomania23•170
    @lisztomania23

    Your solution seems fine, but you did not use the 'Outfit' font. Also, why the individual div for every text line, two would have been sufficient. Consider using relative properties instead of fixed for better responsiveness like min-width and max-width. Could you reduce the font size for the attribution and fix it to the bottom, or place it as preferred.

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