Design comparison
Solution retrospective
What did you find difficult while building the project?
- just re-familiarizing myself with HTML, CSS, and having it responsive
Which areas of your code are you unsure of?
as dumb as this sounds, the shorthand for 'font` in CSS
Do you have any questions about best practices?
Since this was written in basic HTML and CSS (without frameworks), is there anything more that I could do to tighten down my code? Whether it be refactoring, or sizing or... anything. For example, could I have made it without even using a media query..?
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. -
If your code has different sections that have a specific purpose like articles , sections or footer, itโs a good practice to enclose those parts with HTML5 semantic tags. For example, you could use a
<footer>
tag to wrap a footer section.
I hope you find it useful! ๐๐ Above all, the solution you submitted is great๐!
Happy
<coding />
๐!Marked as helpful1@jcampbell18Posted over 1 year ago@frank-itachi Thank you so much Francisco! That made total sense, and it cleaned up the warnings as well. I appreciate your feedback on this!
1 -
- @MelvinAguilarPosted over 1 year ago
Hello there ๐. Good job on completing the challenge !
The solution looks much better now, I just have a few small suggestions.
-
"could I have made it without even using a media query..?" - Yes, Even now there is no div with class
qr-container
, so the media query does not apply. If you want to prevent it from touching the edges of the screen, add a small margin to the component and remove itswidth: 100%
.* { box-sizing: border-box; /* see next note*/ } main { /* width: 100%; */ max-width: 300px; margin: 15px; /* Convert to em :) */ . . . }
-
You should use the
box-sizing: border-box
property to make thewidth
andheight
properties include the padding and border of the element. This will make it easier to calculate the size of an element. You can read more about this here ๐.You have used
max-width: 300px
, but if you inspect the element, it measures332px
, you have not taken into account the padding and the border.
-
Use
min-height: 100vh
instead ofheight
. Setting the height to 100vh may result in the component being cut off on smaller screens, such as a mobile phone in landscape orientation.
I hope you find it useful! ๐
Happy coding!
Marked as helpful0@jcampbell18Posted over 1 year ago@MelvinAguilar Wow! Melvin, I appreciate your recommendations, and education references. I'll be honest, I had never come across box-sizing until you said something. Thanks again!
0 -
- @JesnerWPosted over 1 year ago
Hello there ๐. You did a good job!
I have some suggestions about your code that might interest you.
Try to have a better look to have a more accurate solution, the width of the container is about 295px.
Marked as helpful0
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