Tobias
@ToHXAll comments
- @BerlinWebDevSubmitted over 2 years ago@ToHXPosted over 2 years ago
Good job! Some things which I did notice:
- You're missing an h1 in your HTML
- Class naming, do you use a specific format like BEM?
- font-size always in REM
- Fallback for the font-family is missing -Instead of for example Flexbox and padding you did work with fixed width and height which makes it not responsible (not an issue in this project though)
Marked as helpful1 - @catherineisonlineSubmitted almost 3 years ago
Hello, Frontend Mentor community! This is my solution to the QR code component.
I have read all the feedback on this project and improved my code. Due to the fact that I published this project very long ago, I am no longer updating it and changing its status to Public Archive on my Github.
You are still free to download or use the code for reference in your projects but I longer update it or accept any feedback.
Thank you
@ToHXPosted over 2 years agoHi,
you already have many comments but let me add one too :D
- font-size: 15px; should be in REM
- box-sizing: border-box is on the <body> but I think most CSS Reset will use it on the * selector
- body min-height: 100vh;
- I did use the <main> and then an <article> element,
Marked as helpful1 - @haikalmolSubmitted over 2 years ago@ToHXPosted over 2 years ago
Hi there,
as I was handing in my solution shortly after yours I was taking a look at your code as the solution does look really good.
I have some things I noticed as I am curious about your decisions :-)
- You didn't use box-sizing: border-box;
- You did give the container a vw and vh, but from what I learned width 100vw introduces overflow and instead of height 100vh you should use min height so the content can scroll when needed
- You did use a <div> before the <main> where instead I would've guessed you use it after the main
- You could give the card a max-width instead of the width
Marked as helpful0 - @ToHXSubmitted almost 3 years ago
I'm finally done with my first challenge. It took me actually 2 weeks for finishing it with phases on and off. I would be glad if somebody can point out some mistakes of mine :)
@ToHXPosted almost 3 years agoHi, thanks for you feedback!
I'm actually a bit confused, as I did learn that I shouldn't use
width
with this card, because this will give it a fixed width which will introduce problems for the responsibility.Also, for font you should never use px, always use rem. This won't make your side inclusive.
1 - @hpsettiSubmitted almost 3 years ago
-
I would like to know if my project is in-line with the best practices (E.g. CSS class naming convention, file structure, readme, etc)
-
I would like to know if any features could be added, I have few in my mind like generating custom QR code whenever a user enters their own website, but I'm open to any suggestions :).
@ToHXPosted almost 3 years agoHi there 👋
Some issues I saw
- your CSS class naming could be improved with BEM
- Your img has no alt Attribute
- You have a h3 but no h1
- Font size always in rem
Marked as helpful2 -