Design comparison
Solution retrospective
Centering the card both vertically and horizontally was a bit difficult to do but did it with flex anyway. Is there any other way of doing it?
Community feedback
- @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.
HTML π·οΈ:
- This solution generates accessibility error reports, "All page content should be contained by landmarks" is due to
non-semantic
markup, which causes lacking of landmark for a webpage
- So fix it by replacing the
<div class="container flex">
element with the semantic element<main>
in yourindex.html
file to improve accessibility and organization of your page.
- What is meant by landmark ?, They used to define major sections of your page instead of relying on generic elements like
<div>
or<span>
- They convey the structure of your page. For example, the
<main>
element should include all content directly related to the page's main idea, so there should only be one per page
HEADINGS β οΈ:
- And, this solution has also 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)
iMAGES πΈ:
- Since this component involves scanning the QR code, the image is not a decoration, so it must have an
alt
attribute.
- The
alt
attribute should explain the purpose of theimage
.
- E.g.
alt="QR code to frontendmentor.io"
.
I hope you find this helpful π Above all, the solution you submitted is great !
Happy coding!
Marked as helpful0 - @visualdennissPosted over 1 year ago
You don't need to have height: 100vh for the container, in fact, avoid fixed height for text container elements.
Instead use grid or flexbox on the parent, in this case the body:
- body { font-family: 'Outfit', sans-serif; background-color: hsl(212, 45%, 89%); min-height: 100vh; display: grid; place-items: center; }
or
- body { font-family: 'Outfit', sans-serif; background-color: hsl(212, 45%, 89%); min-height: 100vh; display: flex; justify-content: center; align-items: center; }
notice the min-height:100vh, this ensures that it covers the entire screen so you have space to use for centering.
Hope you find this feedback helpful!
Marked as helpful0 - @MooseCowBearPosted over 1 year ago
You can also use absolute positioning, top and left properties, and then a transform translate combination to center things, but it is a lot uglier than either flex or grid so I wouldn't recommend it.
0 - @MrinmoyHaloiPosted over 1 year ago
Flex is a great way of centering things. But you can also do it using grid. You can use the following snippet. It takes only two lines. Shorter then flex. I prefer to center things this way.
display: grid; place-items: center;
0
Please log in to post a comment
Log in with GitHubJoin 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