Design comparison
Reports
All page content should be contained by landmarks
<p><strong>There isn't a GitHub Pages site here.</strong></p>All page content should be contained by landmarks
<p>
If you're trying to publish one,
<a href="https://help.github.com/pages/">read the full documentation</a>
to learn how to set up <strong>GitHub Pages</strong>
for your repository, organization, or user account.
</p>All page content should be contained by landmarks
<div id="suggestions">
<a href="https://githubstatus.com">GitHub Status</a> ā
<a href="https://twitter.com/githubstatus">@githubstatus</a>
</div>All page content should be contained by landmarks
<a href="/" class="logo logo-img-1x">dannyguzman31ās questions for the community
This is my first submission, please let me know what I can fix. I did struggle a little with the fonts.
Community feedback
@denielden
Posted
Hi Danny, great work on this challenge! š
Here are a few tips for improve your code:
- add
maintag and wrap the card for improve the Accessibility - add
margin: 0to the body for remove the scroolbar of browser - use
h1instead a simplepfor the card title - instead of using
pxuse relative units of measurement likerem-> read here
Overall you did well š Hope this help!
Marked as helpful
@dannyguzman31
Posted
@denielden
Thanks ! will add those for sure :)
@kenreibman
Posted
Nice job on the submission! Unfortunately the preview image isn't appearing on here, but I looked at your live site and it looks great.
I noticed you were centering the div twice with your body and container. Your body could have been your container in this case, and the container could have been your items. In my opinion it was not necessary to do that.
I would also recommend when you're centering a div to use min-height: 100vh instead of height: 100vh to make responsiveness easier in future projects. Setting a fixed height like that may bring some issues in bigger projects. I would also stray away from setting a fixed width like 100vw, as well, for parent elements. The set width also applies to containers, or cards as well. I would set a max-width instead, which is more responsive friendly, and you can also reassign the dimensions in a media query when the screen gets bigger/smaller.
As you do bigger projects on here, you should also start giving your elements more "meaningful" names. Always think about someone else reviewing your work, and wondering if they would be able to understand what each line is referring to. If it was a bigger project, I would have no idea what p1 and p2 is referencing. Start using naming conventions like BEM early, and maybe start putting comments in your code as well, which will definitely help people in this community when they review your code.
I hope this helped!
Marked as helpful
@dannyguzman31
Posted
@kenreibman
Thanks for the input! This really helps a ton!
@TheCoderGuru
Posted
It appears that there is a problem with your hosting. I recommend checking your website hosting logs to make sure that it has no errors
I hope this helps
Cheers Happy coding š
@dannyguzman31
Posted
@TheCoderGuru
Yeah for some reason it did not show, but here it is https://dannyguzman31.github.io/QR-Code_Practice/
Please log in to post a comment
Log in with GitHub
Join our Slack community
Join over 180,000 people taking the challenges, talking about their code, helping each other, and chatting about all things front-end!