HTML, Sass, JS

Community feedback

• 

  P

  tediko•6,680

  @tedikoPosted over 3 years ago

  Hello, Charissa Ramirez! 👋

  Congratulations on finishing another challenge! In addition to Agata exhaustive feedback I would suggest you to:

  • Try not to repeat your HTML code with image for each arrow. Easier way around is to use pseudo element ::before on your .accordion__header element. Set it to position: relative and your pseudo element as position: absolute. Put your image using content: url('image.jpg').
  • Read again about BEM methodology. Overall you're doing it correctly but I found that you're using modifiers for elements and it is wrong. Modifier is an entity that defines the appearance, state, or behavior of a block or element.You used modifiers for accordion__header--icon and accordion__header--text which is wrong use of it. They're yours elements, not modifiers. For example you have accordion__header-text element and then you want to change color just of that element you use MODIFIER `accordion__header-text--red".

  Good luck with that, have fun coding! 💪

  0
• 

  Agata Liberska•3,995

  @AgataLiberskaPosted over 3 years ago

  Hi @chawissa! I think the issue with the box image is that the container div is positioned absolute, but there is no parent container with non-static position, so it is relative to the html element. In your solution the html element takes up the whole viewport, so in this case the position will depend on the size of the viewport. If you add a wrapper around the whole design which would not change dimensions, that would fix it I think. Let me know if that makes sense lol :)

  Another thing is that you listen for clicks on div elements, which are not focusable - so a person using keyboard instead of a mouse would not be able to open the accordions at all. The best option is to change those to button elements and listen on those instead. I like to share this article here:

  And one last detail - when I view the mobile version, the top of the image is cut off a little bit - you could move it down or add some extra margin-top to the card.

  Hope this helps, let me know if you have any questions and I'll do my best to help :)

  0