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

HTML, Sass, JS

@chawissa

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


I noticed that this looks a bit different on my live-server and github pages. The positioning of the "@" box is a bit off in github pages and the container is bigger but not on the live-server.

Any feedback that you think can improve my code is appreciated. Thanks.

I started practicing using BEM method in naming classes and also Sass.

Community feedback

P
tediko 6,580

@tediko

Posted

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

@AgataLiberska

Posted

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

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