Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Janky menu closing transition when entering a route with heavy rendering calculations. #185

Open
johanrd opened this issue Sep 21, 2021 · 3 comments

Comments

@johanrd
Copy link
Contributor

johanrd commented Sep 21, 2021

Hi. I experience janky menu closing transitions whenever I'm entering a route that kicks off heavy rendering calculations. (the menu stops halfway until the entered route is finished with its heavy calculations)

I've thought of different ways to work around this with different ways to block heavy rendering until the menu is closed. All feels quite hackish:
a) keep track of the isOpen state whenever onToggle fires, e.g. by tracking mobileMenuIsOpen on the application controller
b) Listen for transitionend event or similar on the mobile menu DOM-element (if possible?)
c) hard code a delay for safety

Then I saw that https://motion.dev/ promotes the safety of HW-accelerated transitions when doing heavy work on the main thread. @nickschot I know that you have some oversight on the Web Animations API and wanted to ask you if there are some simple tricks to be used to avoid the closing transition issue mentioned above? If not, maybe you know a more elegant way to use the ember-mobile-menu API make sure its transitions can finish before other blocking JS can start?

Thanks a lot,
Johan

@nickschot
Copy link
Owner

You are right in your findings! ember-mobile-menu's current iteration uses older style javascript based animations with requestAnimationFrame (like for example framer-motion, ember-animated also do) which can be interrupted/become janky if other heavy calculations happen. Some libraries implement a frameskip in this case where the animation would not hang, but simply skip ahead to where it should be at that time. With heavy JS work happening this usually results in only a couple of visible frames of the animation.

Ideally we would (and will) indeed move towards the WAAPI in the future which should prevent this from occurring.

The easiest "robust" fix until then with an overlay menu in my opinion is to indeed simply defer the route transition until after the animation has finished if possible. This may or may not look nice depending on what you are rendering on the next page and how much time that takes. Unfortunately I don't think I've made it easily possible 🤔 . The only publicly available hint is the yielded position/relativePosition values which would be 0 if the menu is closed. I think if we return the running task from this action (or a promise of some kind) https://github.com/nickschot/ember-mobile-menu/blob/master/addon/components/mobile-menu-wrapper.js#L410 you could work around this by awaiting this action before transitioning to your target route.

b) this event won't be called as we don't use a CSS transition/animation nor WAAPI. I suppose we could, instead of the above, also fire some kind of event here if that might help? Not sure if that is less confusing than the above option.

c) this is always an option, though you won't know exactly how long you'd have to wait of course. Even more so because we use spring physics rather than a fixed duration easing.

Let me know what would work for you.

@johanrd
Copy link
Contributor Author

johanrd commented Oct 21, 2021

@nickschot thanks for the reply!

Deriving the animating state based on relativePosition might be an option alternative to onToggle, yes. I'll have a look. Waiting with transitioning is not very tempting: it seems better to provide cheaper loading states while transitioning.

For what it is worth, when stumbling upon the same issue with ember-animated I found that they have an isAnimating property on a (somewhat hidden) motion service (https://github.com/ember-animation/ember-animated/blob/master/addon/services/motion.ts#L187) which makes it possible access it directly from the template.

As you write, the ideal is a rewrite to WAAPI to not need to manage other rendering states at all.

@johanrd
Copy link
Contributor Author

johanrd commented Oct 21, 2021

Update: Heres a rough working sketch of keeping transitioning states in a service if restricted to only using the yielded relativePosition :

<MobileMenuWrapper 
  as |mmw|>
  <mmw.MobileMenu
    {{did-update (fn this.mobileMenu.setRelativePosition mmw.relativePosition) mmw.relativePosition}}
  as |mm|>
  </mmw.MobileMenu>
  <mmw.Content>
      {{outlet}}
  </mmw.Content>
</MobileMenuWrapper>
import Service from '@ember/service';
import { action } from '@ember/object'
import { tracked } from '@glimmer/tracking';

export default class MobileMenuService extends Service.extend() {

  @tracked relativePosition : number = 0
  
  get isTransitioning() {
    return this.relativePosition > 0 && this.relativePosition < 1
  }

  @action
  setRelativePosition(relativePosition: number) {
    this.relativePosition = relativePosition
  }
}

It would of course be better to not listen for yielded properties, and instead have states managed directly by finishTransitionTask. Open for adding such a service? isTransitioning isOpen isClosed isClosing isOpening are states that could be a start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants