Fix bug with 'visibilitychange' event and paused instances (fixes: #560) #701
Conversation
… updated on tick Details: when i-th element was removed from `activeInstances`, next `i+1`-th element was skipped because of unconditional `i++` Also noticed more edge cases, left notes as comments.
minor improvement: removes extra rAF when there were only paused (or completed) instances left after latest tick
fljot
force-pushed
the
bugfix/document-visibilitychange
branch
from
fe15c0c
to
670a00e
Compare
Bug description:
animation instances, previously paused by user, were activated again after returning back to browser tab ('visibilitychange' event).
This could cause unwanted animations and memory leaks.
Demo:
https://codepen.io/fljot/pen/oNbEdBj
Solution:
Instead of dealing with animations' public API (pause, play methods), which are used in userland code, we suspend engine itself (rAF ticks).
…juliangarnier#335, juliangarnier#711) because in some projects we want animations to play with throttled rAF (it's up to browser to optimize)
fljot
force-pushed
the
bugfix/document-visibilitychange
branch
from
670a00e
to
1ecb301
Compare
| raf = cancelAnimationFrame(raf); | ||
| } else { // is back to active tab | ||
| // first adjust animations to consider the time that ticks were suspended | ||
| activeInstances.forEach( |
There was a problem hiding this comment.
Question to @juliangarnier
API changes: animations are now paused when tab of window is not active
What's the point of pause animation when the tab is inactive? As I see, Anime.js works through RAF and nowadays everywhere it automatically pauses itself. As far as I understood, this was done due to the support of old browsers such as Сhrome 24+ in 1, 2 versions of Anime.js. We can see in these versions that Anime.js added browser vendor prefixes.
As I understand, version 3 does not support these old browsers, because browser vendor prefixes are not set in the styles and they are not set in thevisibilitychange event.
Also, i checked GSAP, PIXI and they are handling this with nativeRAF pause.
I found that only Phaser handling this.
I propose to remove the pausing animations in an inactive tab in version 3, it is simply not needed since it is not aimed at such old browsers where it does not automatically pause.
What do you think @juliangarnier @fljot
There was a problem hiding this comment.
The only reason to force the RAF to pause is to restore the animations state where they were just before switching tab.
Otherwise, the animations currentTime will skip to where it should be if the tab didn't change.
| @@ -1244,6 +1254,8 @@ function timeline(params = {}) { | |||
|
|
|||
| anime.version = '3.2.0'; | |||
| anime.speed = 1; | |||
| // TODO:#review: naming, documentation | |||
| anime.suspendWhenDocumentHidden = true; | |||
There was a problem hiding this comment.
I think due to this being a feature point of the v3 release and so that there is consistency with the minor updates, the default should be set to false. A major could introduce this as default behaviour.
There was a problem hiding this comment.
But true behavior is exactly the current "master" (3.2.0) one, where stuff gets paused – so I did care about semver and preserving behavior)
... though, again, overall this behavior nowdays looks already unnecessary according to @Ridvanovskyy in #701 (comment)
|
Great PR @fljot, I'll merge this into 3.2.1 for now, and will create the doc for it soon. |
|
@juliangarnier thnx!
I often advocate for verbose and explicit naming, especially when it's a top-level configuration with questionable (as in "unclear") purpose 😎 |
@fljot @Ridvanovskyy's reasoning makes sense, but it doesn't take into account that the current behaviour do not only pauses the RAF, but also re-sync the animations where they were currently playing before switching tab. I think having both options make sense: Regarding the name, I can't find anything better for now, so will probably stick with "suspendWhenDocumentHidden". |
Few fixes and improvements, please review by individual commits.
Main point was to fix
"visibilitychange"event handling (d6f096b) – looks similar to closed #560, but still wasn't working properly in3.2.0.Bug demo: https://codepen.io/fljot/pen/oNbEdBj
CC: @juliangarnier @chrisbrown-io @elvira1112
...but during fixing that I also noticed a couple other issues and things to improve around engine ticks. Fixes and improvements are commited separately, with respective comments.
Finally there's also suggestion to be able to configure
"visibilitychange"event handling strategy (1ecb301) – whether to suspend rAF ticks or not (as suggested in #335 and recent #711)