Skip to content
OverviewSolutions
2
Comments
5
Tobias
@ToHX

All comments

  • Tobias•100
    @ToHXPosted almost 3 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 helpful
    1
  • 👾 Ekaterine Mitagvaria 👾•7,600
    @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

    QR code component

    20
    Tobias•100
    @ToHXPosted almost 3 years ago

    Hi,

    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 helpful
    1
  • Tobias•100
    @ToHXPosted almost 3 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 helpful
    0
  • Tobias•100
    @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 :)

    Profile card component challenge

    2
    Tobias•100
    @ToHXPosted almost 3 years ago

    Hi, 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
  • Harish Pillarisetti•20
    @hpsettiSubmitted almost 3 years ago

    1. 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)

    2. 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 :).

    QR code Component using basic HTML, CSS, and flexbox

    3
    Tobias•100
    @ToHXPosted almost 3 years ago

    Hi 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 helpful
    2