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

Challenge #2 - FAQ accordion by Pau Ferrer

@PaulOCastle

Desktop design screenshot for the FAQ accordion card coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


It was challenging, specially the JS coding but I got it working and I think very similar to the original design. Please, let me know your opinion and give me as much feedback as possible!! Thanks in advance!!

Community feedback

T
Grace 29,310

@grace-snow

Posted

Hi Paul,

Definitely use buttons in this to add keyboard accessibility.

Also, I recommend you read up more on alt text, how to write alt descriptions on images and when they are/are not needed. At the moment, this is less than ideal for screenreader users.

Other than that though, this looks really nice, very close to the design and nice transitions

1

@PaulOCastle

Posted

Thanks @grace-snow!! I definitely will be changing the code to use the buttons!! Also, thanks for pointing at the importance of the alt text, I will look into it and will improve my coding to be more screenreader friendly!! BTW, did you check the JS code?? I'd love some feedback on it. Thanks again!!

0
T
Grace 29,310

@grace-snow

Posted

@PaulOCastle the js looks fine and works OK but could definitely be a lot simpler.

I usually advise against manipulating inline styles with javascript. Instead, I would do everything with css. All the js needs to do is toggle one class on each collapsible. Wrapper element. Just adding an is-open class (or changing that data attribute) would be a hook you can use in css to do everything else you need.

Altering height is a performance heavy change as it triggers a dom repaint (as opposed to opacity or transform, which do not). But I don't think there's any way around that if you want a nice effect on a collapsible element. I would still change height on the faq content, but from 0 to auto rather than setting a pixel value (again, there is no need to set the height with js, let css handle it)

1

@PaulOCastle

Posted

Hi @grace-snow, Thanks for that, I definitely didn't got it right then, as in my search on how to code the JS for the collapsible object, I found that height is not possible to transition unless the specific height (in px or rem, or any other size measurement) is set. And transitioning to auto didn't work either, so I definitely didn't understand nor found the correct advice up to date. Every site I looked into mentioned that to do a collapsible element that would vary its height, the only way to have a correct transition is by injecting inline styles with JS. Please, if there is any web or place you know I can learn how to work this "problem" around I would be very pleased!! Thanks for all you help and feedback, I am really learning lots!!

0

@PaulOCastle

Posted

Hi again @grace-snow!! Just letting you know that I have done the changes to use the button and to improve the alt attributes. Please, don't get me wrong or as a problematic person asking all the time. I really appreciate all you inputs and will continue to if you keep giving me feedback!! Do not have to check the code again, but just letting you know I have done it. I'll definitely will look into the JS and the setting the height using CSS/SCSS only and not as an inline style injection. Thanks again!!

0
T
Grace 29,310

@grace-snow

Posted

@PaulOCastle take a look at Heydon pickerings inclusive components, see if that helps with anything. His patterns are usually very good

1

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