Design comparison
Community feedback
- @grace-snowPosted over 3 years ago
Hi,
Looks like your desktop preview is close, but on mobile the image is cut off the top of the screen, the background ends suddenly and the faq content spills out of its card.
At a guess, id expect most of this to be caused by a height 100vh instead of min-height
I'll post some Screenshots for you
0@grace-snowPosted over 3 years agoI've had a quick look at the html and you definitely need to readdress the html on this. There are no semantics on the faqs and no interactive elements used. To fix this, I recommend you read about collapsible sections on Inclusive Components, or even just google accessible accordions and you should find some examples
0@PolThmPosted over 3 years ago@grace-snow Hey Grace, thanks a lot for your answers ! I've corrected some bugs you mentionned and I'm gonna read about collapsible sections. Thanks again for your nice advices :)
0@grace-snowPosted over 3 years ago@PolThm the alternative if you want to do a js-free version is to use the details summary elements. They are accessible html accordions by default that just need to be styled. There's a good explanation with examples on mdn docs ☺
0@PolThmPosted over 3 years ago@grace-snow Nice, thanks! :) I've read a bit about collapsible sections and it's cool but I wonder if I still prefer my way of doing... There is a lot less JS to write and it allows to make a more customizable css. What do you think about it?
0@grace-snowPosted over 3 years ago@PolThm it is unusable for me as a keyboard user, completely inaccessible to me and others. Therefore, needs changing I'm afraid
0@PolThmPosted over 3 years ago@grace-snow I've found a compromise between both, you can go to have a look if you're curious :)
0@grace-snowPosted over 3 years ago@PolThm the click event needs to be on the button, not on a div. Always use interactive elements for interactive bahaviour. At the moment the buttons do nothing but reload the page 😔
If you let them be the click target you can still toggle the active class on the parent, so it's no extra work to do, makes sense semantically and makes it accessible
0@PolThmPosted over 3 years ago@grace-snow What is your browser? It works for me, but I admit that it is a little weird not to put the "onClick" with the button, modification done! I hope it'll work for you now.
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