Design comparison
Solution retrospective
Hi! The is my first contribute of my Solution and feel free to put your feedback
Community feedback
- @frank-itachiPosted over 1 year ago
Hello there π. You did a good job!
I have some suggestions about your code that might interest you.
HTML π:
- Wrap the page's whole main content in the
<main>
tag. - The heading order is important in the html structure so try to always start your headings with an
<h1>
tag and then you can decrease by one if you need to use more heading in your html code.
CSS π¨:
You can use grid or flexbox to center the content no matter the viewport size. Since I use grid to achieve such purpose, you can do the following:
body { min-height: 100vh; display: grid; align-items: center; justify-content: center; }
As I said, you can use flexbox to center the content and it will work as well.
body { min-height: 100vh; display: flex; align-items: center; justify-content: center; }
I hope you find it useful! ππ Above all, the solution you submitted is greatπ!
Happy
<coding />
π!Marked as helpful0 - Wrap the page's whole main content in the
- @vanzasetiaPosted over 1 year ago
Hi, C-Shadow! π
Congratulations on completing your first Frontend Mentor challenge! π
There are a few changes you can do to make your solution better.
- Alternative text for the QR code should describe the purpose of the QR code.
- Wrap "Improve Your Front-End Skills By Building Projects" with a
<h1>
instead of<p>
element. Each page should have at one<h1>
. - Fix the issues that have been reported.
- Use flexbox or grid to place the card in the center of the page. These modern techniques are more robust than absolute positioning and have less code to write.
- Use a CSS reset whenever you start a new project. This can help you set the styling foundation easily. My recommendation β A Modern CSS Reset | Andy Bell
- Never limit the height of the
<body>
. It will not allow the users to scroll the page when the page content needs more height. Try to look at the site on a mobile landscape view to see the issue. So, usemin-height
instead. - Remove the media query. You should only set a
max-width
to the card. Make the<img>
element as a block element and setmax-width: 100%
to prevent it from overflowing.
I hope you find this useful. Happy coding! π
Marked as helpful0 - @PvvirgPosted over 1 year ago
It was very cool
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