FAQ accordion

Community feedback

• 

  T

  Chamu•13,340

  @ChamuMutezvaPosted 4 months ago

  Hi Nasif Fuad

  So far so good with your project, but here is my take:

  1. One of the building blocks of a website are Landmark elements, which provide a powerful way to identify the organization and structure of a web page - this is very useful for assistive technology users as information will be conveyed. Such elements include main, header and footer . For this project I would use the div with the class wrapper as the main element.
  2. This challenge requires interactive elements since it involves what is known as the disclosure pattern. EventListeners can only work effectively with elements such as buttons, an image is not an interactive element - hence it should not be associated with eventListeners. In other words , use semantic elements.
  3. Separation of concerns - let your html , your css and your JS be on separate files . You have some inline styles in your html and that can complicate maintainability if bigger projects. The use of the !important is something that should be avoided as well.
  4. For font sizes , at least you should use rems , using px has some disadvantages as explained in the attached article - Why font-size must NEVER be in pixels

  Hope that helps

  Marked as helpful

  1

  ---

  

  Nasif Fuad•260

  @NasifuadPosted 4 months ago

  @ChamuMutezva Thank you so much for pointing out this to me.

  0
• 

  Abdellah Abadou•270

  @abdellah-abadouPosted 4 months ago

  Your JavaScript codes are working and this is the most important, but there are better ways to implement the task

  1

  ---

  

  Nasif Fuad•260

  @NasifuadPosted 4 months ago

  @abdellah-abadou I have made a function for the toggleDisplay to reduce JS

  1