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

Responsive Page using FlexBox and Media Query

@PriyanshuSaxena2612

Desktop design screenshot for the Order summary component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


This is my first solution, please pass on the feedback on the following:

  1. How much did you like the project?
  2. Was the code written properly or any improvements can be done?
  3. Some suggestions from your side. Thank You.

Community feedback

P
Remus D. Buhaianuβ€’ 3,145

@Miculino

Posted

Hey Priyanshu!

First of all, congrats on successfully completing your first Frontend Mentor challenge - you did a great job all things considered and learned quite a lot ;)

Since you've asked for feedback, I'd like to offer a few suggestions. (in no particular order):

  1. In the Readme, you state that the project was built using HTML5 semantic markup. Unfortunately, there isn't any semantic HTML element present in your index.html file. Your code consists solely of <p>, <div>, and <img/> tags - no semantics whatsoever. Consider this resource https://developer.mozilla.org/en-US/docs/Glossary/Semantics for more info on semantic markup.

  2. Some of your classes don't have the best names. You've used emojis such as πŸ’ͺ and 🎡 as values for the class attribute. Even though you can, in fact, target these elements using emojis classes, they're definitely not a great clean code practice. You might reconsider the names of certain classes and replace them with more meaningful options.

  3. There are some issues with the wavey background image on the iPad and iPhone screen sizes.

  4. You can set a background image for the entire body element, instead of using the <img /> element. It's a more simple and efficient solution.

  5. Your media queries code is rather repetitive and unnecessary. You don't need to set the same properties over and over again for the body element.

  6. Since you use rem units (which is a good practice), you'd want to target the html element and set the font-size to 62.5%. This way, the default font size will become 10px and 1rem will amount to 10px - it's easier to use rem units.

  7. Avoid using px units. Absolute units such as px don't allow the user to make the desired adjustments if needed, thus making the overall project less accessible and responsive. Setting the html's font-size to an absolute value is a bad practice.

Hope my suggestions will be useful to you!

Once again, well done on completing this challenge. Celebrate every victory you achieve and keep up the good work.

Looking forward to seeing more of your work here!

Marked as helpful

3

@PriyanshuSaxena2612

Posted

@Remus432 Thank you, sir, for your feedback and suggestions. I would surely work on my mistakes and rectify them.

0
P
David Turnerβ€’ 4,150

@brodiewebdt

Posted

Download AXE DevTools and you can clear accessibility warnings while you code. https://www.deque.com/axe/devtools/

Hope this helps.

Marked as helpful

1

@PriyanshuSaxena2612

Posted

@brodiewebdt Thank you, sir, I would surely use this tool for my upcoming challenges.

0
P
David Turnerβ€’ 4,150

@brodiewebdt

Posted

Your welcome. Glad I could help.

0

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