Submitted over 1 year ago

QR Code Challenge

MohammedHussain23•10

@MohammedHussain23

Preview site View code

This is a solution for...QR code component

HTML
CSS

1newbie

Design comparison

SolutionDesign

Solution retrospective

Hello everyone,

I am not sure about the sizes and positioning aspect while resizing the window but I did what I can using the knowledge I gained. Would like it if someone could give me feedback and be a mentor for me. Especially in the CSS part where I need help in the responsive designs. Appreciate all honest feedback. Thanks!

Hussain

Community feedback

Siraj Sharma•150
@sirajsharmaPosted over 1 year ago
Try to use semantic tags instead of divs.

Use h1 tag instead of h2 for level headings. Click here for more information on this.

To center the component you can use flexbox, grid or position. The most simple way is: -

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

For more ways, you can visit here.

You don't need to wrap every tag with div. Try to understand the difference between block, inline and inline-block elements. Click here to go to the video.

Try to use <link> tag instead CSS @import. For more information visit here.

Use relative units (em, rem, vw, vh, % etc) instead of absolute units (px, in, cm etc). Click here for more information.
Marked as helpful
0
MohammedHussain23•10
@MohammedHussain23Posted over 1 year ago
@sirajsharma Thanks for the feedback, will go through the resources and make necessary changes as mentioned.

Hussain
0
Siraj Sharma•150
@sirajsharmaPosted over 1 year ago
@MohammedHussain23 Keep it up bro.
1
MohammedHussain23•10
@MohammedHussain23Posted over 1 year ago
@sirajsharma I guess you have also started using this app recently?
0
Siraj Sharma•150
@sirajsharmaPosted over 1 year ago
@MohammedHussain23 Yes😅😅
1
Abdul Khaliq 🚀•72,560
@0xabdulkhaliqPosted 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.

HEADINGS ⚠️:

This solution has generated accessibility error report due to lack of level-one heading <h1>

Every site must want at least one h1 element identifying and describing the main content of the page.

An h1 heading provides an important navigation point for users of assistive technologies, allowing them to easily find the main content of the page.

So we want to add a level-one heading to improve accessibility by reading aloud the heading by screen readers, you can achieve this by adding a sr-only class to hide it from visual users (it will be useful for visually impaired users)

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

Happy coding!
Marked as helpful
0
MohammedHussain23•10
@MohammedHussain23Posted over 1 year ago
Hello @0xAbdulKhalid ,

Thanks a lot for the feedback Khalid. Hope to do better and include h1 tags hereafter.

Hussain
0
Atif Iqbal•3,200
@atif-devPosted over 1 year ago
Hi MohammedHussain23, congrats🎉 on completing the challenge. Better take care about following points.

Always check Frontendmentor Report Generator issues after submitting the project for removing errors and warnings. To avoid accessibility issue "All page content should be contained by landmarks" use code as :

<body> <main> ---your code here---- </main> <footer> </footer> </body>

(why main matters? Read here)

For proper centering the container(whole card) vertically and horizontally you can use code as:

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

Or you can learn centering from here.

In .card selector use max-width as max-width: 300px; and also in img selector use width: 100%; for proper display.

If you don't know much about responsiveness, you can learn from FreeCodeCamp or you can check out this free intro course for getting started in responsiveness.

When we open GitHub repository link, at right side you will find an About Section. There, also include live preview link of your project. It is better for someone to check your live project while interacting with code.

Write more in your GitHub project's README file. Like, write about your working flow, findings, new learned things, useful resources, etc.

(Have any questions🧪?reply to this comment😇)

Hope you will find this Feedback Helpful.
Marked as helpful
0
MohammedHussain23•10
@MohammedHussain23Posted over 1 year ago
Hey @atif-dev, Thanks !! I am totally new to this front end development so all of this kind of seems overwhelming to be honest but I will surely take your feedback and work on it. So the display should be grid rather than flex? Can we achieve the same output using flex or is grid easier to achieve the solution?

Regards, Hussain
1
Atif Iqbal•3,200
@atif-devPosted over 1 year ago
@MohammedHussain23 We can achieve the centering using flex also as:

body { min-height: 100vh; display: flex; align-items: center; justify-content: center; }
1
MohammedHussain23•10
@MohammedHussain23Posted over 1 year ago
@atif-dev thanks, how do I make it responsive ?
1
Atif Iqbal•3,200
@atif-devPosted over 1 year ago
@MohammedHussain23 If you implement the things that I have highlighted in my first comment then your solution will be almost fine for different screen sizes I think(can't say for sure because I have only tested on chrome dev tools). Further, digging deep...There are some things that you need to learn for responsiveness. I have also mentioned a short course, previously.That course is kind of getting started in learning responsiveness. Check out that course to find out how responsiveness work and what are the units that we need to use in responsiveness.

Hope that Help,Friend. 👍😇
Marked as helpful
0
MohammedHussain23•10
@MohammedHussain23Posted over 1 year ago
@atif-dev thanks for the quick response.
0

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