QR code with CSS color variables

Community feedback

• 

  Daniel 🛸•44,310

  @danielmrz-devPosted 11 months ago

  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

  0
• 

  raya•2,870

  @rayaattaPosted 11 months ago

  👋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

  0
• 

  P

  Ruben de Man•460

  @developer-rubenPosted 11 months ago

  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

  0
• 

  Mohamed Mostafa•480

  @MohamedkazafyPosted 11 months ago

  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

  0