Design comparison
Reports
All page content should be contained by landmarks
<p><strong>There isn't a GitHub Pages site here.</strong></p>
Learn more 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>
Learn more 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>
Learn more All page content should be contained by landmarks
<a href="/" class="logo logo-img-1x">
Learn more 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
main
tag and wrap the card for improve the Accessibility - add
margin: 0
to the body for remove the scroolbar of browser - use
h1
instead a simplep
for the card title - instead of using
px
use 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 GitHubJoin 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!