Jump to content
Search Community

Questions on how to refactor my code to make my dropdown navigation more reliable.

samd test
Moderator Tag

Recommended Posts

Hi I built a dropdown navigation menu with gsap you can see a live example here https://sample.blinkpayment.co.uk/blink-learning-podcasts/blink-payment-insights-the-impact-of-contactless-payments-on-businesses

 

Sometimes bugs can be created either randomly or by rapidly hovering over multiple dropdowns in quick succession. 

 

What are some ways I could make this code more reliable and better manage state for all of my animations. My codebase is vanilla with typescript. 

 

Live example

https://sample.blinkpayment.co.uk/blink-learning-podcasts/blink-payment-insights-the-impact-of-contactless-payments-on-businesses

Gist example of gsap menu code

https://gist.github.com/samducker/3a1debb3a5816addddf76d3e6341927b

 

Video of issues

https://share.cleanshot.com/GJb8CW52

Link to comment
Share on other sites

It's pretty tough to troubleshoot without a minimal demo - the issue could be caused by CSS, markup, a third party library, your browser, an external script that's totally unrelated to GSAP, etc. Would you please provide a very simple CodePen or CodeSandbox that demonstrates the issue? 

 

Please don't include your whole project. Just some colored <div> elements and the GSAP code is best (avoid frameworks if possible). See if you can recreate the issue with as few dependancies as possible. If not, incrementally add code bit by bit until it breaks. Usually people solve their own issues during this process! If not, then at least we have a reduced test case which greatly increases your chances of getting a relevant answer.

 

Here's a starter CodePen that loads all the plugins. Just click "fork" at the bottom right and make your minimal demo

See the Pen aYYOdN by GreenSock (@GreenSock) on CodePen

 

If you're using something like React/Next/Vue/Nuxt or some other framework, you may find StackBlitz easier to use. We have a series of collections with different templates for you to get started on these different frameworks: React/Next/Vue/Nuxt.

 

Once we see an isolated demo, we'll do our best to jump in and help with your GSAP-specific questions. 

  • Like 1
Link to comment
Share on other sites

I don't have time to dig through all your code but from a quick glance I'd bet that the problem is that you're creating conflicting animations. Make sure you kill() existing animations that you've got running on the same element before you fire off the new ones. For example, let's say you create a timeline on mouseover that lasts 2 seconds...but then the user rolls their mouse over/out/over/out quickly. You may have created a bunch of timelines that are all fighting to control the same properties of the same element. I noticed you also create new animations inside an onComplete, so imagine you've got 3 competing animations all running simultaneously and they fire their onComplete at slightly different times...again, all competing for the same elements' properties. See the problem? 

 

One technique: 

let tl; // keeps track of the currently-running animation.
function onMouseOver() {
  if (tl) {
    tl.kill(); // kill the existing animation first!
  }
  tl = gsap.timeline(); // now create the new animation and save it to that variable...
  tl.to(...).to(...); 
}

Does that help? 

  • Like 1
Link to comment
Share on other sites

Thanks for replies so far I'm going to try to work with your solution so far Jack.
 

I guess I have a couple of additional follow ups on my implementation. 

  • Is it possible to pass params into my timeline so I can make one timeline that is reusable within a function for my different dropdown content. 
  • Should I wrap all of my animations inside a context incase I need to stop all animations at once or is killing animations individually the best approach
  • What is the best practice way to check the state of my animations (e.g. by using booleans with onCompletes updating them or something else?) So I can track reliably when the wrapper is open or not (and if the animation is finished before triggering the following animation) and if content is inside it or not and which if so.
  • Should I debounce all my event listeners? so if multiple clicks are fired only one tween is fired or is this already built in
  • Also just noting I'm using vanilla ts and not react

So I'd ideally like to create timelines for the following variables I used tweens however as I wasnt sure if I could pass down parameters from my openNavMenu function  which resulted in repetitive code 

 

Another thought is, because this issue is intermittant. Is it worth me using some sort of global isAnimating which checks for any current timelines tweens with in a context as animating and either kills them all or stops more tweens from firing whilst they finish.
 

// I ideally would have something like the below. But I'm unsure if I can pass parameters to a gsap timeline in the same way I can in a standard tween.

const openLargeContentWrapper = gsap.timeline({}) // some timeline goes here the wrapper is always the same so I do not need to pass params
const openLearnDropdown = gsap.timeline({}) // this is a seperate navigation dropdown to the others.
const fadeInLargeContentItem = gsap.timeline({}) // I'd like to pass a parameter to this so it can be reused for both the "features and the about dropdowns inner content
const fadeOutLargeContentItem = gsap.timeline({}) // some timeline animations goes here that will make the inner content item for the features or about go to display: none and also fade out.


// Basically from there what I would need to do is check if the wrapper is open first if it was open I would just proceed to check if there was a content item already in it (if its open typically there would be) then I would need to fade out the previous item e.g. if navigating from about dropdown to features without closing the menu fully. 

// If the wrapper wasnt open I would need to first trigger than timeline animation and wait for it to complete before firing the following animation. Is there any way for me to chain multiple timelines for example with a promise/await or setTimeout wrapped.

function openNavMenu(menu: HTMLDivElement) {

  if(menu === null) return
  if(menu === aboutMenu) // do stuff related to about menu
  if(menu === learnMenu) // do stuff related to learn/resources menu
  if(menu === featuresMenu) // do stuff related to features menu (e.g. checks related to wrapper and current content then trigger respective timelines.
  
}

function closeNavMenu(menu: HTMLDivElement) { // to close a specific menu e.g. close features before opening the about menu 
}

function closeAllMenus() { // to close all menus e.g. when resizing to mobile or clicking outside the main area
}

 

Link to comment
Share on other sites

We're happy to answer GSAP-specific questions, but unfortunately logic/plumbing/architecture stuff is a bit beyond the scope of what we typically provide here (see the forum guidelines). You'll definitely have a better chance of getting answers if you break things up into one question at a time (ideally with a minimal demo) rather than one post with a whole bunch of questions crammed in :) 

 

That being said, I'll take a crack at some of your questions...

 

2 hours ago, samd said:

Is it possible to pass params into my timeline so I can make one timeline that is reusable within a function for my different dropdown content. 

Sorry, I don't understand the question. Maybe some pseudo code would help. A minimal demo is even better. 

 

2 hours ago, samd said:

Should I wrap all of my animations inside a context incase I need to stop all animations at once or is killing animations individually the best approach

A gsap.context() is only for making it easier to revert() a whole group of GSAP-related stuff (which is very different than killing it - reverting actually returns things to their original states and inline styles whereas killing an animation simply stops it from affecting things any further, leaving the state as-is). Don't think of it like a controlling mechanism. Timelines are perfect for that type of thing.

 

2 hours ago, samd said:

What is the best practice way to check the state of my animations (e.g. by using booleans with onCompletes updating them or something else?) So I can track reliably when the wrapper is open or not (and if the animation is finished before triggering the following animation) and if content is inside it or not and which if so.

It is difficult to give blanket advice for all situations. If you just need to be able to check a timeline to see if it's in-progress, you can use its isActive() method. In some cases, it can be useful to have a boolean variable that you set in an onStart/onComplete. Sometimes you need to accommodate multiple states, so it's not a boolean - the variable may store something like "about"/"learn"/"features". My personal preference is to modularize things as much as possible to avoid spaghetti code, so maybe you create a Menu object that is in charge of handling state changes, and it exposes methods like .goto(state) so you can say menu.goto("about") and internally, it handles coordinating that, tracking its own state, etc. It just seems cleaner to me to have things be object-oriented. 

 

If you need to wait until an in-progress timeline (for example) finishes before doing something else, there are several ways to do it...

  1. You could just figure out the remaining time and use that as a delay for the next animation:
    gsap.to(...{ delay: tl.duration() - tl.time(), ...}); // time left

     

  2. You could set an onComplete dynamically
    tl.eventCallback("onComplete", () => {... do stuff...});

     

  3. You could just kill() the in-progress animation and redirect values to wherever you want from there. Don't feel like you need to always reuse the same timeline instance. That has pros and cons. It's often best to just create a new timeline each time you want to animate something so that the values get redirected dynamically. 
2 hours ago, samd said:
  • Should I debounce all my event listeners? so if multiple clicks are fired only one tween is fired or is this already built in

That's up to you. In my opinion, it's best to write your code to accommodate interruptions cleanly. What if someone clicks, that creates a 2-second animation...but the user clicks again after 1 second? My pet peeve is when the app IGNORES my click, like "nope, I must finish my entire animation before I allow you to do anything". In my opinion, it should gracefully handle that. So debouncing mouse clicks may only prevent a problem if the clicks happen SUPER fast, but that's only one scenario. If you code it to handle clicks anytime gracefully, then you don't need to worry about debouncing. 

 

In my experience, I typically see people way over-complicate menus like this or they're plagued with a lot of spaghetti code or they try to create every animation ahead of time and juggle all the different instances rather than just dynamically animating the state on-demand with new animations. 

 

I hope that helps! 

Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...