Skip to content
Submitted over 1 year ago

QR code with CSS color variables

Aleš Zima•420
@Esosek
A solution to the QR code component challenge
Preview (opens in new tab)Code (opens in new tab)

Solution retrospective

Hi, I'm new to CSS and would love to get your opinion on my choices and suggestions for cleaner code.

  1. Is using Flexbox on body with justify-content: center and align-items: center the best way to center elements vertically? I initially thought of using margin: auto but it works for horizontal axis only.
  2. I struggled moving the attribution to the bottom of the page. My solution uses position: absolute and bottom: 0. Is this a clean solution? Trying different attributes using flex ended up to mess with centered container.

Thanks! Aleš

Code

Please log in to post a comment

Log in

Community feedback

  • P
    Daniel 🛸•44,810
    @danielmrz-dev

    Hello @Esosek!

    Your solution looks great!

    I have a suggestion for improvement:

    • For semantic reasons, use main as a wrapper tag for the main content instead of div.

    This tag change has no visual impact but they make your HTML code more semantic and improve SEO optimization as well as the accessibility of your project.

    I hope it helps!

    Other than that, great job!

    Marked as helpful
  • raya•2,870
    @rayaatta

    👋Hi Ales Zim congratulations on completing your first FED challenge 🎉 Addressing your questions.

    Is using Flexbox on body with justify-content: center and align-items: center the best way to center elements vertically? There are several methods but the best methods are using grid and flex.Centering using margins works but it pushes against other content so it's not advisable.

    I struggled moving the attribution to the bottom of the page. My solution uses position: absolute and bottom: 0. Is this a clean solution? Yes,it is but it blocks content on short devices or mobile devices in landscape mode it would be better if you add position: relative To the body so the .attribution is absolutely positioned relative to the parent(body) instead of the view port hence no content blocked.

    I also have some tips you might find useful 1.change <div class="container"> to <main class="container">. Divs carry no semantic value so it is better if you use a more Semantic tag. The main tag indicates to browsers what the main page content is.check out this article 📰. 2.change <div class="attribution"> to <footer class="attribution"><p></p> This is more semantic than a div. If you wish to know more about semantics click on this link.

    I hope this helps Other than that,your solution is awesome 👏 ✌️

    Marked as helpful
  • P
    Ruben de Man•760
    @developer-ruben

    Hi!

    Nice first solution, welcome to Frontend Mentor! Here are my tips:

    • Try avoiding pixels for font-sizes, this makes that the font size is not reacting to the user's browser settings
    • Look in to BEM to better organise your css
    • In my opinion it's better to import the fonts in the HTML rather than in the CSS, but that's depending on ones preference :)

    Best, Ruben

    Marked as helpful
  • Mohamed Mostafa•520
    @Mohamedkazafy

    your code is awesome but the image has to get smaller , the justify-content: center and align-items: center will center your div if you want to center it at the center of body you can add min-height:100vh; it will make in the center of all page . Keep going bro

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