Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Qr Code Component

P

@Cinquantesix

Desktop design screenshot for the QR code component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


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

I tried to use gradient background. Because in the Figma file, the background change between desktop and mobile and I tried to make it. I'm pretty happy because gradient was always a little bit scary for me to use.

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

I will try to have a better html and css structure for the next project. I also want to add some comment to be clear wy I'm doing this way.

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

I'm not sure to use some unit like I must do. Like, for exemple, rem instead of px or else. DI you know some kind of "good" rules about it?

Community feedback

T

@gmagnenat

Posted

Hi, congrats on completing the challenge !

I noticed a few issues in your solution that you may want to check and fix before moving to the next challenge.

Does the solution include semantic HTML?

  • It's a detail but this component is probably part of a larger website. So the heading would not be a <h1>. You can add a visually hidden <h1> to your page if you want.
  • A performance tip. You may want to move the title in the head higher just under <meta name="viewport...". there's a great ressource shared on the #-ressources channel on discord I recommand.

Is it accessible, and what improvements could be made?

  • Currently this solution is easily broken if the default font-size of the browser is changed by the user. I recommand to read this great article -> Why font-size must NEVER be in pixels. You need to use relative unit for everything related to font-size so it can scale correctly and respect the user settings.
  • You want to define a min-height of 100svh (the value you used) instead of a height the reason is : you don't want to limit the height of your element containing any text that can scale. If you force a height at a fixed value, you can have overflow.
  • Fixing a width in pixel on the figure and the img force this image in one size. if the content scale the image doesn't fill its container. Remove the pixel values and add a width:100% on the image. You need to remove also the padding on the image as it breakes the layout, it isn't necessary as the image is contained in the card. Just add a padding to the card itself. remove the heigh of the image also as it needs to be free to scale.
  • You need to remove the position absolute on the attribution. It get placed over the layout when the font-size is increased.

Does the layout look good on a range of screen sizes? yes

Is the code well-structured, readable, and reusable?

  • Take a good habit to add a modern css reset at the beginning of your stylesheet. Check out Josh Comeau or Andy Bell. They both have good ones.
  • Your approach for the circles is interesting and good eye catching this!. You need to keep in mind that design isn't always 100% accurate I'm not sure this is supposed to change and it looks more like an opacity issue on the design. In practice you would exchange with the designer if you have a doubt on something. The design should be approach as a guide for the implementation.
  • It's a good practice to take from the beginning to not style the html component directly unless you are doing a global style or a reset. Think of this challenge as a component and not a full page. If you style the h1, it applies to all h1 of your website.

Does the solution differ considerably from the design?

  • It respects the design intention but needs to be fixed for better accessibility.

Sorry for the long review but I hope you find something useful in my comments that will help you improve your solution and answers some of your questions.

Happy Coding !

Marked as helpful

0

P

@Cinquantesix

Posted

Thanks @gmagnenat

I've made some changes !

Does the solution include semantic HTML? -Okay for the title. I will look for some explain.

  • I understand, but if FM need a h1 for the report, I will use this instead of, like you say, an other title more logical.

Is it accessible, and what improvements could be made?

  • thanks I will read it!
  • I know I need to use more min-… max… so I change these everwhere it make sens for me.
  • For the figure, I deleted the width properties but as I didn't take the picture with the background, I can't remove the padding…
  • I changed the z-index for attribution.

Is the code well-structured, readable, and reusable?

  • I add some reset.
  • Yeah, I know I take this too far, but I really wanted to try and understand bg gradient so I take this like a challenge.
  • I understand. For now, I just don't really know where I need to be more or less specific with my selector. I add a class to isolate the card from maybe other element.

Thank again for your comments !

0

Please log in to post a comment

Log in with GitHub
Discord logo

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