Design comparison
Solution retrospective
Any kind of feedback will greatly be appreciated. Also, THANK YOU for taking the time to view my code.
Community feedback
- @grace-snowPosted over 3 years ago
Hi William,
This looks really broken for me on mobile, so I recommend you revisit this and see how to make it work at all screen sizes.
There are some really important (essential) changes needed in the html too which I'll try to outline
- the file needs to be called
index.html
- IDs shouldnt be used for styling. Use classes instead. IDs have to be unique per page and are heavily used by assistive technology and in navigation, so it's important to not use them for styling or javascript hooks.
- only meaningful images should have alt text, like the review profile images. All of those star images should have an intentionally empty alt attribute like
alt=""
That's as far as I got so far, but hopefully these are helpful pointers to start with.
Good luck
1@WackLanternPosted over 3 years ago@grace-snow Good afternoon grace-snow, thank you so much for your feedback. I came back to the file and fixed the problems. I added those essentials to my notes so I don't forget in the future. The only thing I am struggling with is making the website fit in a mobile viewport. But I will revisit and fix that up as well as soon as I have a better understanding on how to make websites mobile.
0@grace-snowPosted over 3 years ago@WackLantern it's all part of the learning journey ☺
Next time, try doing the mobile layout first. Your css will probably end up shorter that way too
0 - the file needs to be called
- @Comet466Posted over 3 years ago
good job William, it is truly faithful to the original design the only problem is that because you wrap the top container it acts as a column (wrap make the elements show on a single line) so if you take that flex-wrap:wrap off the #top container it will work as intended, other then that really good job
0@WackLanternPosted over 3 years ago@Comet466 Hi Comet466, thank you so much for your feedback! I came back to the file and removed the flez-wrap.
0
Please log in to post a comment
Log in with GitHubJoin 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