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 Challenge

P
Johnβ€’ 20

@HeavyMetalCoffee

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


---This is an updated version to create a better layout experience using the suggestions from my peers.

First challenge on Frontend Mentor. Feedback welcome as I know there are multiple ways this project could have been done and probably much better.

Community feedback

Vanza Setiaβ€’ 27,855

@vanzasetia

Posted

Hi, John! πŸ‘‹

Congratulations on completing your first Frontend Mentor challenge! πŸŽ‰

You've gotten some awesome feedback especially on solving the issues that have been reported. I have some feedback as well.

  • The alternative text of the QR code is not giving enough information. If you try a screenreader and then close your eyes, then the screenreader pronounces the image as "Frontend Mentor, image.". Then, what do you think the screenreader users would think about the image?
  • Remove all br elements from the paragraph. Let lines wrap where they need to. Screen readers will read out br elements. You can't accommodate every screen size, so it's rare you'll ever need to use them.
  • Remove width and height from the body. Never use 100vw on anything as it doesn't account for scrollbars when present. It may only ever introduce potential overflow/scroll bugs.

That's it! I hope this helps. 😊

Marked as helpful

0

P
Johnβ€’ 20

@HeavyMetalCoffee

Posted

@vanzasetia Thank you for your feedback!! Excellent, I didn't even stop to think about the little information I provided for the alternate text for the QR code. I always understood the less, most descriptive phrase as possible. I believe I have taken that just a bit too far...lol. As for the <br> tags, I did not know this, which means I need to align my paragraphs better. You make a great point about 100vw. The last challenge I did, the 3-Column preview, I ran into such an issue. Thank you again!!

0
Vanza Setiaβ€’ 27,855

@vanzasetia

Posted

@HeavyMetalCoffee You're welcome! πŸ˜‰

So, what do you think is a better alternative text for the QR code?

0
P
Johnβ€’ 20

@HeavyMetalCoffee

Posted

@vanzasetia A descriptive text such as "QR code to visit Frontend Mentor".

0
Vanza Setiaβ€’ 27,855

@vanzasetia

Posted

@HeavyMetalCoffee That's a great alternative text! So, why don't you update the alternative text on the live site? Also, if possible fix all the issues.

Improving your code or fixing bugs is one of the skills that you will need as a developer. So, it's better to get used to it. πŸ™‚

0
P
Johnβ€’ 20

@HeavyMetalCoffee

Posted

@vanzasetia I just updated and uploaded!! Thank you, I appreciate the feedback. Check it out!!

0
Vanza Setiaβ€’ 27,855

@vanzasetia

Posted

@HeavyMetalCoffee I took a look at the updated solution. One thing, that I would like to suggest is to swap the header with div instead. header is a landmark element that is used to wrap certain page content like navigation, logo, etc.

Other than that, everything looks good to me! πŸ™Œ

0
Ahmed Bayoumiβ€’ 6,800

@Bayoumi-dev

Posted

Hey John, It looks good!... Here are some suggestions:

  • Document should have one main landmark, Contain the component with <main>.
<main class="container">
   <div class="qr_code_card">
      //...
   </div>
</main>
  • Page should contain a level-one heading, Change <p class="p1"> to <h1 class="heading"> You should always have one h1 per page of the document... in this challenge, you will use h1 just to avoid the accessibility issue that appears in the challenge report... but don't use h1 on small components <h1> should represent the main heading for the whole page, and for the best practice use only one <h1> per page.

  • You have used grid to center the component on the page, but you need to set the height of the container ---> min-height: 100vh; to center it at this height:

.container {
   display: grid;
    justify-content: center;
    align-items: center;
    /* width: 1440px;        /* <---- Remove  */
    /* height: 800px;         /* <---- Remove  */
    min-height: 100vh;     /* <--- Add */
 }
  • Use REM for font size, It is a must for accessibility because px in some browsers doesn't resize when the browser settings are changed... See this article ---> CSS REM – What is REM in CSS?

Hope this help!... Keep codingπŸ‘

Marked as helpful

0

P
Johnβ€’ 20

@HeavyMetalCoffee

Posted

@Bayoumi-dev Thank you for the feedback!! I am currently stuck in the rut of watching tutorials and I am busting out that with these challenges. My next challenge, my goal is to use semantic tags properly after reading some of the feedback, especially using <h1>. Thank you for sharing the link to CSS REM, I appreciate it.

0

@vanmendez

Posted

Good job!! πŸ‘ I have a couple of observations, if you don't mind:

  • Use semantic tags instead of div's, for example the container can be a <main>
  • Each web page must have an <h1> tag; if you decide to add more <h?> make sure to follow the ascending order of these I am starting in web development, in the tutorials that I have seen they indicate that we must avoid these errors. πŸ˜‰

Marked as helpful

0

P
Johnβ€’ 20

@HeavyMetalCoffee

Posted

@vanmendez Thank you for the feedback!! Regarding the semantic tags is a great suggestion and with my next challenge I will incorporate that into my html. The use of the <h1> tag probably would have solved some layout issues. We watch so many tutorials and learn so many different ways to do things I forget some of the little things that impact greatly. Thank you again!!

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