@Babajide777
Submitted
Good day all,
This is my first time using SASS, so I'll appreciate any comment on the file structure and how to make it better. Regards.
LEUILLIER
@JulienLEUILLIER
All comments
- LEUILLIER• 120
@JulienLEUILLIER
Posted
Your code works, you just need to add height to
.accordion-svg. By default backgrounds take the height of their container if I'm not mistaken, and if you use your browser Dev-tools you'll see the container has no height. Hope that helps !Marked as helpful
1 - Jesse Ma• 95
@jma26
Submitted
This challenge was a bit more difficult compared to the earlier challenges. I had trouble figuring out how to implement the desktop images with the bg-pattern and did not realize I could do this with the help of
background-image, background-position etc.(seems like I'm not thinking about the css background property and defaulting to usingimgelement). In addition, I had an issue dealing with the parent height jumping as the child element (accordion) expands and collapses. For now, I used a fixed height on the parent but am curious how this issue can be achieved or fixed.Tried to make sure the accordion complies with ADA Web Accessibility and is toggleable with a keyboard etc.
Any thoughts or opinions regarding the height issue and 1st try on ADA Web Accessibility is highly appreciated!
Thank you!
#HappyCoding
LEUILLIER• 120@JulienLEUILLIER
Posted
Hi Jesse ! I did this challenge a few days ago, nice work on this one. I'm not an expert by any means but I've got some feedback on this one :
- You should make it so the arrow is a background to the button, thus making it clickable to expand the question.
- For the expanding animations, I used this approach. But I like your use of
height: autowithvisibility. - I don't think a fixed height is really an issue, I used a dynamic height but it's really preference I guess ? With animations on the expansion the height changes are more smooth.
Great job !
Marked as helpful
1 - Al-Baraa Bakri• 330
@Al-Baraa-Bakri
Submitted
Any Feedback ?
LEUILLIER• 120@JulienLEUILLIER
Posted
Good work !
I would allow users to click on the whole
.lineelement to see the answer. I think you're also missing the mobile background image on mobile layout! Keep up the good work!Marked as helpful
0 - tmnguyen• 100
@nntt1210
Submitted
Any suggestions on improving the code would be appreciated!
LEUILLIER• 120@JulienLEUILLIER
Posted
Really nice solution, with responsiveness on the mobile layout !
You can use
classList.toggleto actually add and remove the.activeclass in your script, to save a few lines.Keep up the good job !
Marked as helpful
1
