@kabir-afk
Posted
Hey !! Congrats on clearing the challenge! Great page. I had some concerns regarding your code and thought I should address them.
-
Reducing verbosity : Instead of making separate functions for opening and closing each accordion element , you could have declared classes inside your custom CSS and then later used
classlist.toggleto add/remove them depending on the state of your variable, which you could have changed when user clicks the particular element. It reduces verbosity and makes for an efficient code. Then, based on the conditions of your state, you could have integratedgsapaccordingly. -
Breakpoint too high : You could have settled for a lower breakpoint , somewhere around 748px , instead of 1024px. I mean, it still looks good, but when you open the sidebar menu, it feels too wide. You can ignore this point if you wish. I guess it's subjective.
Everything else feels top notch . . .hope this helps you. Cheers !! 🥂
@xup60521
Posted
@kabir-afk Thx for your advice.
First I want to share my thought on the accordion. Using css class seems to be more trivial than gsap functions, but it has some potential drawbacks.
Take the accordion as an example, you might assume it should have 2 states, open and close. While it is true, when it comes to animation, things get complicated.
In my case, there are 3 states, init, open and close. init state represents the initial sytle the element should have, which doesn't contain any animation.
open and close states, however, are bind with expand and collapse animation respectively. Even though user would not notice those additional movements since the nav menu is close by default, it's not a great pattern in my opinion.
So GSAP actually solves this problem. I only need to track if the accordion is open or not, and apply the animation without updating the classlist. GSAP is able to compute the animation based on the current position, so two gsap.to functions are enough to represent expand and collapse animation.
Not to mention that because vanilla css animation breaks when height: auto, it's almost necessary to use libraries like GSAP to achieve this type of animated movement.
As for the breakpoint, since I use TailwindCSS in this challenge, I change from lg: to md: so that the breakpoint is 768px instead of 1024px. It's not a breaking change though, I still appreciate your indication on this defect.
