Frontend Mentor | FAQ Accordion Card

Community feedback

• 

  Aman Singh Bhogal•1,010

  @asbhogalPosted over 1 year ago

  Hi Isidora,

  Great work! The accordion seems to function nicely. I've noted some things down below worth considering for refactoring:

  • The accordion isn't fully responsive - between 481px and 640px you have some overflow issues and spacing caused by the explicit height value set on the parent container. Ideally you should avoid this, as it constrains the child elements. It's best to let them occupy the space they need naturally.
  • I'd recommend using grid for this to keep the intrinsic values of the left and right columns consistent as the viewport changes. This ensures the content isn't compressed. Here's a good link explaining the difference between the grid and flex and when to use one instead of the other Link
  • Use a CSS reset to ensure it removes any preset styling by browsers and defaults to a completely unstyled 'canvas'. Here's a link to a good modern one Link
  • You can refactor your JS logic by replacing the if statements with a ternary operator, for e.g. addSupp.addEventListener("click", () => { paraFive.style.display = (paraFive.style.display === "none") ? "block" : "none"; hideParaTwo(); hideParaThree(); hideParaOne(); hideParaFour(); }) This says that if paraFive.style.display is equal to "none", it sets paraFive.style.display to "block", otherwise, it sets it to "none", and is great for single-line parsing for when the condition is evaluated (ie. if it is truthy, it returns the "block" for the display and if its falsy, it returns "none")
  • Your code also has a lot of gaps and spaces, particularly the stylesheet. I'd recommend using an extension like Prettier if you use VS Code, which formats your code nicely and removes extraneous spacing. Here's the link to it Link

  Hope this helps!

  Marked as helpful

  1

  ---

  

  moonrose93•370

  @moonrose93Posted over 1 year ago

  @asbhogal Ineed you as my mentor 💃

  1

  

  Aman Singh Bhogal•1,010

  @asbhogalPosted over 1 year ago

  @moonrose93 haha of course ☺️

  0
• 

  Sham Sunder•240

  @ShamSutherPosted over 1 year ago

  Great job on your work! I really like the design and the accordion functionality. I noticed that you used position absolute on the box, which is quite precise. However, I think it would be better if you set it relative to the overall component wrapper and used CSS transform property to align it according to the screen size. Keep up the amazing work, and I wish you happy coding! 😊💻✨

  0