@asbhogal
Posted
Hi Isidora,
Great work! The accordion seems to function nicely. I've noted some things down below worth considering for refactoring:
- The accordion isn't fully responsive - between
481px
and640px
you have some overflow issues and spacing caused by the explicitheight
value set on the parent container. Ideally you should avoid this, as it constrains the child elements. It's best to let them occupy the space they need naturally. - I'd recommend using
grid
for this to keep the intrinsic values of the left and right columns consistent as the viewport changes. This ensures the content isn't compressed. Here's a good link explaining the difference between thegrid
andflex
and when to use one instead of the other Link - Use a CSS reset to ensure it removes any preset styling by browsers and defaults to a completely unstyled 'canvas'. Here's a link to a good modern one Link
- You can refactor your JS logic by replacing the
if
statements with a ternary operator, for e.g.addSupp.addEventListener("click", () => { paraFive.style.display = (paraFive.style.display === "none") ? "block" : "none"; hideParaTwo(); hideParaThree(); hideParaOne(); hideParaFour(); })
This says that if paraFive.style.display is equal to "none", it sets paraFive.style.display to "block", otherwise, it sets it to "none", and is great for single-line parsing for when the condition is evaluated (ie. if it is truthy, it returns the "block" for thedisplay
and if its falsy, it returns "none") - Your code also has a lot of gaps and spaces, particularly the stylesheet. I'd recommend using an extension like Prettier if you use VS Code, which formats your code nicely and removes extraneous spacing. Here's the link to it Link
Hope this helps!
Marked as helpful
@moonrose93
Posted
@asbhogal Ineed you as my mentor π
@asbhogal
Posted
@moonrose93 haha of course βΊοΈ