Design comparison
Solution retrospective
I am a beginner to front-end development. Tried my best to make the page responsive to small display sizes. Feedbacks and suggestions to optimize the code are welcome.
Community feedback
- @k-stopczynskaPosted over 2 years ago
Hi!
Congratulations on finishing the project!
I've noticed two things that could improve user's experience:
- For the main container: try to use margin top and bottom or setup height lesser than viewports height for mobile devices. It would be much smoother.
- You used media-queries for device's max width 485px, but your main container for larger resolutions has min-width of 800px so you cannot see it properly in the range 485-800px (it overflows the body). So either get another media-query for this situation or change the actual one to min-width: 800px.
Overall nice one, very similar to design:)
Happy coding!
Marked as helpful1@Kaushaljoshi29Posted over 2 years ago@k-stopczynska The problem that you mentioned in point number 2, will that be solved if I remove the min-width property from main container ?
0@k-stopczynskaPosted over 2 years ago@Kaushaljoshi29
Yeah, it should, but from the user's experience point of view the cards will be very narrow then and it would be hard to read the descriptions (e.g for resolutions like 500px).
Marked as helpful0 - @vanzasetiaPosted over 2 years ago
Hello, Kaushal Joshi! 👋
Congratulations on completing your first challenge! 🎉
You have gotten some incredible feedback. I am just going to add three more recommendations on this solution.
- If you see the site on the design comparison, you may notice that the page has a gray background color. So, I recommend trying to add this to make your solution looks more similar to the design.
- I would make the "Learn More" buttons as link elements. I think if this is a real website those links would navigate the users to another webpage (instead of doing an "action").
- Alternative text should not be hyphenated (like code). Also, those car icons are decorative images. So, I would recommend leaving the
alt=""empty to prevent the screenreaders from pronouncing the images.
That's it! Hope you find this useful! 😁
Marked as helpful0 - @PraiseImmanuelPosted over 2 years ago
Hello Kaushai Joshi, You did a really nice job. But I just have a few corrections you can improve with. You should put a lang attribute in your <html> tag like <html lang="en"> because of accessibility issues. I would advise you to use your developers tool (F12 on your browser) in order to view the responsiveness of your web app on different devices. Happy Coding!
Marked as helpful0 - @MavreonPosted over 2 years ago
Hey Joshi! Congratulations on completing this challenge. It's really responsive and functional, I think you nailed most part of the design.
Here's a few pointers to help with your HTML validation issues and accessibility issues:
In order to resolve some of your accessibility issues, you might wanna use landmarks in your html code, these help browsers easily navigate your code. So you might consider wrapping your divs in landmarks like
<main>or<header>or<footer>you need to do this according to how your page is structured. Incase you want to know more about landmarks, follow this link. You might want to add labels to your form input elements, this enables the browser to properly identify the input elements within your form and it's also beneficial to people with using screen readers. Happy coding and keep up the good work👍Marked as helpful0@Kaushaljoshi29Posted over 2 years ago@Mavreon Thanks for the suggestions, really appreciate it!
1
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