Submitted over 1 year ago

Container with QR Code using HTML and CSS

zMitchC• 30

@zMitchC

Preview site(opens in new window)View code(opens in new window)

This is a solution for...

QR code component

HTML
CSS

1newbie

View challenge

Design comparison

SolutionDesign

Solution retrospective

Hi guys! This is my solution.

There are two things I'm note sure about: The first thing is the code optimization, to be more simple avoiding repeat in the code.

Secondly, I don't know if I've written well the README file. Is that clear to understand?

Feel free to give me feedbacks and tips. I'm happy to learn new things that can improve my works! :)

Community feedback

Ecem Gokdogan• 9,380

@ecemgo

Posted over 1 year ago

Some recommendations regarding your code that could be of interest to you.

In order to center the card correctly, you'd better add flexbox and min-height: 100vh to the body
You'd better remove the width: 100%; you gave to the body in HTML.

body{
   display: flex;
   flex-direction: column;
   justify-content: center;
   align-items: center;
   min-height: 100vh;
}

If you use flexbox in the body, you don't need to use margin, top, left and right to center the card.
Also, if you use max-width instead of width, the card will be responsive.

.qrcode-ctnr {
  max-width: 250px;
  /* width: 250px; */
  /* height: 400px; */
  /* position: absolute; */
  /* top: 0; */
  /* bottom: 0; */
  /* left: 0; */
  /* right: 0; */
  /* margin: auto; */
}

If you want to make the card responsive and the image will be positioned completely on the card, you'd better add width: 100% and display: block for the img in this way:

img {
  /* width: 250px; */
  /* height: 250px; */
  width: 100%;
  display: block;
}

Hope I am helpful. :)

Marked as helpful

zMitchC• 30

@zMitchC

Posted over 1 year ago

Thank you @ecemgo! Your advice really helped me.

I submit the new solution if you want to check it out :)

Ecem Gokdogan• 9,380

@ecemgo

Posted over 1 year ago

@zMitchC

I've just checked your new solution. I viewed that you fixed the codes and it is more awesome now :)

I also want to mention about accessibility issue a little bit. Each main content needs to include at least h1 element so you'd better add one <h1> element in the <main> tag. If you want, you can replace your <p class="qrcode-title"> Improve your front-end skills by building projects </p> element with the <h1 class="qrcode-title"> Improve your front-end skills by building projects </h1> element.

After committing the changes on GitHub and you need to deploy it as a live site. Finally, you should click generate a new report on this solution page to clear the warnings.

Marked as helpful

Abdul Khaliq 🚀• 72,660

@0xabdulkhaliq

Posted over 1 year ago

Hello there 👋. Congratulations on successfully completing the challenge! 🎉

I have other recommendations regarding your code that I believe will be of great interest to you.

CSS 🎨:

Let me explain, How you can easily center the component for better layout without usage of absolute positioning.

We don't need to use absolute to center the component both horizontally & vertically. Because using absolute will not dynamical centers our component at all states

To properly center the component in the page, you should use Flexbox or Grid layout. You can read more about centering in CSS here 📚.

For this demonstration we use css Grid to center the component

body {
    min-height: 100vh;
    display: grid;
    place-items: center;
}

Now remove these styles, after removing you can able to see the changes

.qrcode-ctnr {
  position: absolute;
  top: 0;
  bottom: 0;
  left: 0;
  right: 0;
  margin: auto;
}

Along with that you want to remove,

<body style="width: 100%; background-color: hsl(212, 45%, 89%);">
</body>

Hereafter avoid inline styling it may affect code reusability, instead you can apply these styles from the seperate css file

Now your component has been properly centered with better structured code

I hope you find this helpful 😄 Above all, the solution you submitted is great !

Happy coding!

Marked as helpful

zMitchC• 30

@zMitchC

Posted over 1 year ago

Thank you so much @0xAbdulKhalid for linking that CSS guide and for your advices!

I tryed to correct the work if you want to check it out!

dimar hanung• 560

@dimar-hanung

Posted over 1 year ago

Hi.. 👋, Congratulations on completing the challenge 🎉 .

I have some interest and feedback with your code

That i like:

I appreciate the similarity of your results with the design, a bit different in scale but still good
html is pretty good, not too nested 👍
Responsive until galaxy fold screen size 👌
CSS Naming is also good, represent what is it for, but in my opinion don't use abbreviations, because the important thing is easy to understand, instead class="qrcode-ctnr" i’ts ok to be class="qrcode-container" , it looks a bit long, but the main thing is easy for others to understand

My Feedback:

I suggest you use html semantic convention, add <main class="container">, it will make it clearer, and will improve seo if you want to submit your website to google, i recomended this article: here
you can seperate file by folder to be more structured, for example:

public/
├─ images/
│  ├─ qr-code-image.png
├─ styles/
│  ├─ main.css
├─ favicon.png
index.html

About your question

You handle it well, like in here:

.qrcode-title, .qrcode-par{
    font-family: 'Outfit', 'sans-serif';
    text-align: center;    
}

so it more less repetitive, nice 👍

The README.md was clear, just little mistake like whitespace in image too long and some typo like in section Afer that, to get rounded corners I use border-radius: 10px., but I understand what you mean like progress, technology used, etc

and finally overall is good, nice solution, hope it useful 🙌

Marked as helpful

zMitchC• 30

@zMitchC

Posted over 1 year ago

Hi @dimar-hanung, thank you for your feedback and for giving me these advices! I won't use no more abbrevations and I will be more clear naming elements, using semantic elements as well and thank you for linking that page.

I forgot to structure this repository, but I'll surely do also in future works!

If you want to check it out, I submit the corrected version!

Please log in to post a comment

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