Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

QR-Code - Responsive design with flex box

2div 170

@2div

Desktop design screenshot for the QR code component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


Hello my community,

kindly review my solution and share your tips and feedback on how to improve my design and code.

Thanks 2div

Community feedback

@pikapikamart

Posted

Hey, nice work on this one. I see that you somehow went on another approach on this one, it's fine but I really recommend that when you tackle other challenges, following the design itself should be the best ui approach :>

Here are some other suggestions besides Deniel helpful suggestion and don't mind me reiterating some already suggested ones:

  • Using main tag on the .container instead of div would make the site more accessible. When creating websites, you should use main tag to nest the main-content of that webpage, on this case, the whole content should be inside the main.
  • To center the contents on your solution, you don't need to use margin on the .container since it is not really consistent. What you can do is that, add these stylings on the .container:
justify-content: center;
min-height: 100vh;

This way, the item will be centered properly and dynamically.

  • The margin on the .card could be removed since its parent ( .container) is already making it centered.
  • For the img, you could just use alt="" and aria-hidden="true" on it since the image doesn't really depict anything visually and could just be left empty so that screen reader won't read it.
  • Also when you use img or svg , you don't use words that relates to graphic like "image"` since those elements are already graphics.
  • For the bold text, you can use h1 on it for now. Remember that a webpage needs to have an h1 and you can replace the p tag with it on this.
  • When wrapping up text content, use meaningful elements like a p tag, change your span into using p tag.

Aside from those, great job again on this one^^

Marked as helpful

1
Travolgi 🍕 31,480

@denielden

Posted

Hello Hersi, great job! Congratulations on completing the challenge.

I have a few advice for you:

  • add main tag and wrap the card for Accessibility
  • remove margin, max-width and min-width properties from container class
  • remove all margin from card class
  • use justify-content: center; to the container class for center the card. Read here -> flex guide
  • add heigth: 100vh to container class because Flexbox aligns to the size of the parent container.

Overall you did well :)

Hope this help and happy coding!

Marked as helpful

1

@pikapikamart

Posted

@denielden Hey, really nice feedback on this challenge. One thing to change though, instead of height: 100vh always use min-height: 100vh so that the element won't have a fixed height. That's it and really nice feedback again^^

Marked as helpful

1
Travolgi 🍕 31,480

@denielden

Posted

@pikapikamart Thank you so much! You're right :)

Marked as helpful

1
2div 170

@2div

Posted

Thank you both you @Deniel & @Raymart , your comment and feedback was very very helpful.

@Raymart thank for your deep explaining it was really helped me a lot since im in the beginning in this field.

I have revised my code and design and applied what learned from both you.

I am really amazed how my code is less and everything is good.

Only one thing on mobile display not okay, the card look too small , i could not find out why ? any idea about it?

1

@pikapikamart

Posted

@2div Hey, great that you find it useful^^

I think you just need to adjust the height and width of the .card selector on this. Also, I don't know if you coded it to be smaller but check if you are zoomed in or zoomed out when coding. Looks great by the way:>

Marked as helpful

1

Please log in to post a comment

Log in with GitHub
Discord logo

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