Im fairly new to HTML and CSS and enjoyed learning flexbox. Any comments or feedback regrding code is much appreciated.
Gabriel
@ctrl-brokencodeAll comments
- @chrisdh80Submitted about 1 year ago@ctrl-brokencodePosted about 1 year ago
Hello! Congrats on uploading this challenge. I hope you don't discourage now and I hope you continue coding and learning from your mistakes. You will go far! Here are some suggestions I've made for you. Make sure to pay attention!
- On HTML, make the alt text of the image clearer. Say where the QR Code is taking you;
- Delete the
<br>
tags in<h1>
and<p>
. The text will adjust according to the size of the component;
On CSS:
- Replace the
.container
selector with the tagbody
, as it will be the body of the project. It's optional, you can continue with the div.container if you want. (If you actually replace it, take thediv.card
out of thediv.container
in HTML file); - On
display: flex
, changeflex
forgrid
to center the component later; - Remove the
width
of the body, as it can cause side scrolling on smaller devices. The component should already be centered; - Replace
justify-content: center;
andalign-items: center;
toplace-items: center
. It will only take up one line and will have the same effect
It should look like this:
body { display: grid; height: 100vh; background-color: hsl(212, 45%, 89%); place-items: center; }
As for the card:
- We can delete the following items:
-
display: flex;
-
flex-direction: column;
-
align-items: center;
- Remove
height
. Remember that the height in the component will automatically be determined by the its content;
.card { flex-direction: column; width: 300px; background-color: white; padding: 1rem; border-radius: 1rem; text-align: center; }
And you're pretty much done! Overall, your code is simple and easy to read. I wish you the best of luck!
Marked as helpful0 - @sgr-web-devSubmitted over 1 year ago@ctrl-brokencodePosted over 1 year ago
Hello! Congrats on uploading this challenge. I hope you don't discourage now and I hope you continue coding and learning from your mistakes. You will go far! Here are some suggestions I've made for you. Make sure to pay attention!
- Make sure to have a well indented code for better readability;
- para-1 should be an heading element (instead of p, use h1);
- Don't forget to delete p on .para-content p. This will apply to both the paragraph and heading;
- Make the alt text of the image clearer. Say where the QR Code is taking you;
- There's no need of having width and height in body. Delete the width and change height: 100vh to min-height: 100vh;
- On the other hand, the component (main container) should have a max-width in rem, but not a height. Remember that the height in the component will automatically be determined by the content inside;
- The component should have one padding value. This will centralize your image a bit;
- Display, flex-direction, justify-content and align-items won't be necessary;
- QR Code Image should have display: block and width or max-width: 100%. Your image should be centered now because of the width and padding.
Then make some adjustments on the paragraph content, like margin and padding, and you're done! Overall, your code is simple and easy to read. Again, careful with indentations. I wish you the best of luck!
Marked as helpful0