Hi
This looks smaller than the design and the component is cut off screen on my mobile. Hopefully these suggestions will help
- Remove width 100vw. Never use that anywhere, it would only ever cause overflow bugs with horizontal scroll. You don't need a width here anyway - the element is already 100% wide by default
- Change height 100vh to min-height. This is what is causing the bugs in my mobile view because you are limiting the height
- Use landmark elements. The card needs to be in a main element, and the attribution needs to be a footer
- You don't need as many divs as this. Try to make it as simple as possible
- No media queries needed on this challenge
- Never limit the height of elements that contain text. Let the browser decide how tall it needs to be
- The component should not have a width, only a max-width. So it can shrink if it needs to
- The style guide said what size body font should be, but converted into rem. This is how large paragraphs should be (your challenge is too small).
- The img element doesnt need srcset attribute or object fit. You should always include a modern css reset at the start of the stylesheet. Amongst other things that would set img elements to display block and max-width 100%
- The image is the most important content on this challenge. It needs a proper human readable description of what the image shows, and in this case what url it goes to
Marked as helpful
@s17-git
Posted
@grace-snow thank you for your feedback. I've made some changes to my project, could you take a look of it please.
And tell me I'm use to wrapping all my content with a div like in vuejs app div#app, is it really necessary ?
I'm also using a div for my component, is it fine or should I use another semantic HTML tag like section ?
Thank you !
@s17-git much better 👏 well done
- you don't need to wrap everything in an extra div and you don't need any more semantic elements
- you don't need the div wrapping card text content
- you DO need to rewrite the image alt text still (point 10 in my first feedback)
- you DO need to update the font size so it matches what it says in the style guide but converted to rem
@s17-git
Posted
@grace-snow thank you one small.
1st, 2nd, and 3rd point done !
last point: the base font size is 15px and I converted it into rem which is 0.938rem.
I'm using 1.1rem in my component title in order to be closer to the design.
Do I really need this footer ? otherwise my markup will be much smaller .
Eg:
<main class="bee-qrcode-app"> <img alt="a QRcode to scan"> <h1 class="qrcode__title">Improve your front-end..</h1> <p class="qrcode__desc">Scan the QR code...</p> </main>
