HTML, CSS, FLEX-BOX, RESPONSIVE, JAVASCRIPT

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, great work on this one. Desktop layout looks great and the mobile state looks great as well.

  Some suggestions would be:

  • Avoid using height: 100vh on a large container like the body tag as this limits the element's height based on the remaining screen's height. Instead use min-height: 100vh, this takes full height but lets the element expand if needed.
  • Those images that are in here are just all decoration since they are all just vector. Decorative image must be hidden at all times for screen-reader users by using alt="" and extra aria-hidden="true" attribute on the img tag.
  • Using p tags or div for the accordion toggle is not great. Right now, they are only limited for mouse users and can't be used by keyboard or other peripherals. Instead use details element. This is suited for accordions and it is really accessible and you won't have to manipulate the element's attribute.
  • The accordion's arrow as well should be hidden.
  • When an image on a site adds content to the site itself, make sure to use a descriptive alt but if the image is just decorative only and does not add any content, make sure to hide it using the method I mentioned above.

  Aside from those, great job again on this one.

  Marked as helpful

  0

  ---

  

  Parham•200

  @frontendparhamPosted about 3 years ago

  @pikamart Thanks for your excellent explanation

  1
• 

  Amzat Sheriff•300

  @Amzat19Posted about 3 years ago

  This was very helpful. Thanks a lot You rock

  1
• 

  Amzat Sheriff•300

  @Amzat19Posted about 3 years ago

  Your solution is really neat and easy to understand but please can you explain the way the iteration in your javascript works

  0

  ---

  

  Parham•200

  @frontendparhamPosted about 3 years ago

  @Amzat19 Whenever we use querySelector with a selector which actually matches multiple elements only the FIRST ONE will get selected.

  • So I used document.querySelectorAll() to select all elements that have same class.
  • The output of querySelectorAll('.question') is like Array but not actually : questionDiv(n) [div1, div2,...]
  • So we can LOOP through it with FOR loop.

  When our click event occurs on one of the question's div (questionDiv[i]) :

  • First we have to close all the previous (for (let k = i; k >= 0; k--)) and next (for (let j = i + 1; j < questionDiv.length; j++)) question divs of current div and we do this by calling makeDefault function and add the hidden class to our answer if it does not have.
  • then we can open the question's div that we clicked. (show the answer)

  I hope my explanation was clear. If there is another question, I will be happy to answer.

  0