Design comparison
Solution retrospective
Thank you for taking interest in my first ever submitted frontend mentor project. What I found difficult: Animating the opening/closing effect, making the site fully responsive, adding keyboard navigation. Question: I would appreciate any help in making the site fully responsive for devices less than 400px.
Community feedback
- @stueybrockPosted 10 months ago
Hey,
First of all, great job, it's looking pretty similar to the designs.
In terms of code feedback:
- With this type of project, I would always firstly focus on the HTML before touching anything else. You're building an accordion here with clickable sections but semantically you're using divs. To make it accessible, you'll want to use something like a <button>. I would strongly recommend search for accessible accordion examples to get some solid HTML markup, that includes all of the accessibility attributes you'll need such as
aria-expanded
,aria-controls
etc. - You're getting an issue with height on your smallest screens because you're hard-coding a
max-height
in your Javascript, which is not unset for smaller screens. As a rule of thumb, I would always separate my concerns, so CSS is done CSS files, Javascript is done in Javascript files etc. - I don't see a reason why you'd separate your media queries into a different file. It's easier to maintain and review code if all of the CSS is in one place. Splitting up CSS into separate components is good too!
- It's a little bit jumpy on open and close, worth investigating that further.
Marked as helpful1@Ryan1731Posted 9 months agoThank you so much for the reply. This is the first challenge I submitted and it's such a great feeling to receive feedback from the community. I will definitely take a look at your suggestions and work on the changes.
Once again, thank you for the feedback.
0 - With this type of project, I would always firstly focus on the HTML before touching anything else. You're building an accordion here with clickable sections but semantically you're using divs. To make it accessible, you'll want to use something like a <button>. I would strongly recommend search for accessible accordion examples to get some solid HTML markup, that includes all of the accessibility attributes you'll need such as
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