@Higokian
Posted
Great job on your first project! I just have a few suggestions.
#bold id in <p> element
I suggest making this a class instead of an id, this would be great for reusability. Another option would be to use the <strong></strong> tag to bold.
main class
Rather than giving this div a class of main, you could change the div to a <main></main> element. This would be good for structure and accessibility for things such as screen readers.
attribution class
You could change this div into a <footer></footer>. This has similar benefits to using the main element.
External CSS
To aid with code readability and file structure, I recommend using an external css file rather than using internal css. Not only does this benefit file structure, it's also good for reusability. This would allow you to link the css file to other html files rather than having to copy a lot of css into each html file.
Centering QR Code Component
I love your use of flexbox! It seems to me you have a pretty good grasp of it. One thing that may help with getting it to center within the viewport without having to scroll down is to give the body a height: 100vh. This will make the body take up 100% of the viewport no matter the device. If you then get rid of the 670px margin in your main class and the -700px margin in your attribution class, this should make all of your content center without scrolling!
I really hope that this is helps! Keep up the great work!
Happy coding!!
Marked as helpful
@utkarsh95-dev
Posted
@Higokian Hey thanks for the review man, i will try and improve the code by rectifying whatever inputs you gave.
about the body height being set to 100vh is it because of the layout on the small screen size because i was using an code editor on my mobile phone to code it down and in that app as i was running my site it perfectly scaled down to how it was supposed to look but somehow now it looks awfully placed do you know how can i fix that, will placing body height:100vh alone solve this problem??
@Higokian
Posted
@utkarsh95-dev No problem at all!
Oh that's interesting. I'm not sure if height:100vh alone would fix it... I think that and getting rid of those margins I mentioned in my previous comment would though.
Your flexboxes will handle the placement based on the size of it's container. So with a height:100vh and no extra margins, it should keep it sitting where you want across all devices. You already set the height and widths of the card, so getting rid of the margins shouldn't affect anything!
I've never used my phone for coding before, I'd imagine it's tough, so kudos to you for doing it!
