Skip to content
Submitted over 2 years ago

HTML CSS SASS

Saleh•390
@SalehAbuhussein
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...QR code component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Solution retrospective

1 - is there a better way to make the Qr Card size more dynamic I am unsure of the way I made it. 2 - is there is a best Practice for specifying max-width and min-width I would appreciate that.

Community feedback

  • PhoenixDev22•16,950
    @PhoenixDev22Posted over 2 years ago

    Greeting @Honko-o,

    Congratulation on completing your Frontend mentor challenge,

    I have few suggestions regarding your solution:

    • Page should contain a level-one heading . As this is not a whole webpage , you can use <h1> with class="sr-only" (Hidden visually, but present for assistive tech).

    • Use the main landmark to wrap the body content which is the card .HTML5 landmark elements are used to improve navigation.

    • When you write alternative text, it should not be hyphenated. The alternative text should be human-readable ex : QR code for frontend mentor

    • Never use <span> alone for meaningful content.

    • To center the card on the middle of the page , you can use the flexbox properties and min-height: 100vh for the <body> add a little the margin to the card or a little padding to the body .(No need for position absolute)

    body {
    display: flex;
    align-items: center;
    justify-content: center;
    min-height: 100vh;
}

    • Consider using max-width instead to the card , let it grow to a point. Then a little margin on the card or padding on the body .

    • It's recommended to use em and rem units .Both em and rem are relative units , Using px won't allow the user to control the font size based on their needs.

    • In this challenge no need for any media query , as the card's width is the same in different layouts.

    Overall , your solution is good. Hopefully this feedback helps.

    Marked as helpful
    1
    Saleh•390
    @SalehAbuhusseinPosted over 2 years ago

    @PhoenixDev22 Thank You for Your Comment ❤❤🥰 Your Comment Helped me a lot I even Removed the Media Queries and Centered The Card with no position and made Function to Calculate from Pixel to Rem. I Made the Sass code way more Clear with the points you gave me and I will update the solution.

    0
    Saleh•390
    @SalehAbuhusseinPosted over 2 years ago

    @PhoenixDev22 The point about: "When you write alternative text, it should not be hyphenated. The alternative text should be human-readable ex : QR code for frontend mentor." Did you mean the Image Name ? and if it is. Is there a better way to rename it

    1
    PhoenixDev22•16,950
    @PhoenixDev22Posted over 2 years ago

    @Honko-o You are welcome . for your question : the alt attribute of the <img> , you can read more in here hopefully it's clear.

    Marked as helpful
    1
    Saleh•390
    @SalehAbuhusseinPosted over 2 years ago

    @PhoenixDev22 Thanks Bro ❤🥰

    0
  • CyrusKabir•1,885
    @CyrusKabirPosted over 2 years ago

    hello my dear friend ♥ you did good on this challenge and here some improvements :

    • it's better to use max-width when you don't want your component or container have more than one absolute or relative value (in this case it was good to set max-width:300px you shouldn't have more than 300px but you use min-width and this property is for when you don't want something have height or width less than one absolute or relative value)
    • it's better to have some good file structure when using sass it's make your code reusable, maintainable, ... a lot of more benefits and also you can check this links to learn about those structures in sass 2 ways to structure sass , how structure your sass code base, a simple sass file strucute
    Marked as helpful
    1
    Saleh•390
    @SalehAbuhusseinPosted over 2 years ago

    @CyrusKabir Thanks for your comment colleague 🥰 Your comment was very helpful to me But I didn't get that part (you use min-width and this property is for when you don't want something have height or width less than one absolute or relative value) can I'd appreciate if you can clarify that point .

    0
    CyrusKabir•1,885
    @CyrusKabirPosted over 2 years ago

    @Honko-o your welcome I am glad it was helpful and for that min width point I just explained about min width property it wasn`t something important i just explain that property

    Marked as helpful
    1
    Saleh•390
    @SalehAbuhusseinPosted over 2 years ago

    @CyrusKabir Thanks Bro 🥰❤

    1
  • iago•400
    @iagohenrique2009Posted over 2 years ago

    Hi! My answers for yours question are: 1- The true size i think you can only get by being premium, but if you are unsure of that just resize it and give some border-radius to the card 2-The max-width is always the best choice for most of cases especially for when you want somenthing to have to center os sides(left,right). Obs:You could give some read here if https://developer.mozilla.org/en-US/docs/Web/CSS/min-width https://developer.mozilla.org/en-US/docs/Web/CSS/max-width

    1
    Saleh•390
    @SalehAbuhusseinPosted over 2 years ago

    @iagohenrique2009 hi thanks for your Comment I wonder doesn't setting max-width: 100vw makes the card spans all the way to the left and the right or did you mean something else? sorry if I didn't get what you mean 😅

    1
    iago•400
    @iagohenrique2009Posted over 2 years ago

    @Honko-o oops i said wrong but alread corrected

    0

Please log in to post a comment

Log in with GitHub

Join 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