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

[web-animations] Could commitStyles always write the end state of the animation? #5394

Open
zmajeed opened this issue Aug 4, 2020 · 12 comments

Comments

@zmajeed
Copy link

zmajeed commented Aug 4, 2020

MDN https://developer.mozilla.org/en-US/docs/Web/API/Animation/commitStyles says:

"The commitStyles() method of the Web Animations API's Animation interface commits the end styling state of an animation to the element being animated, even after that animation has been removed. It will cause the end styling state to be written to the element being animated, in the form of properties inside a style attribute."

This is exactly what I want and I like the example in which commitStyles() is called right after the animation is created. The expected behavior is very clear since the end state of an animation is well-defined

To my surprise this is not the case in Chrome 84. Looking at the spec it seems MDN got it wrong:

https://drafts.csswg.org/web-animations/#dom-animation-commitstyles says:

"Writes the current effect values produced by this animation’s animation effects to their corresponding effect targets' inline style using the commit computed styles procedure."

So an animation of transform: translate(100px), duration: 1000 may record a translation of anywhere between 0 and 100px depending on when commitStyles() is called. Sure calling it after the animation finishes insures the end state is written but seems to also require setting fill: forwards. It just seems much easier and more intuitive to have it record a predictable state no matter when it's called

Could commitStyles() have an option to always record the end state preferably without involving the fill property? Even better could it become an animation property and I wouldn't have to use fill at all?

@birtles birtles added the web-animations-2 Current Work label Aug 10, 2020
@birtles
Copy link
Contributor

birtles commented Aug 10, 2020

That's an interesting proposal. It certainly seems feasible to add a flag to commitStyles to produce the behavior you describe. Do you mind explaining the particular use case you have for it?

From my understanding, the proposal would be to effectively seek to the target effect end (but without firing events etc.), force the fill mode for all associated effects to both, and then apply the procedure to commit computed styles.

Understanding the use cases would give a better idea of whether this should be two separate flags or not (e.g. one to seek to the target end, one to force the fill mode). It would also give a better idea of the priority of this feature.

@zmajeed
Copy link
Author

zmajeed commented Aug 11, 2020

My application is a large state graph of vertices and edges. Each state transition animates the entire graph so the destination vertex (new current state) is centered in the view box. The new position of the graph needs to persist. There's a timeline slider to play the transitions from any point forward and backward.

@birtles
Copy link
Contributor

birtles commented Aug 12, 2020

It seems like what this case needs is something like:

function finishTransition(anim) {
  // Make sure finish() seeks to the target effect end
  anim.playbackRate = 1;
  anim.finish();
  anim.effect.updateTiming({ fill: 'both' }); // Or 'forwards'
  anim.commitStyles();
  anim.cancel();
}

I wonder how common this pattern is or if there are similar patterns?

@jakearchibald
Copy link
Contributor

My understanding was that commitStyles was created as an alternative to fill-forwards, with fewer gotchas. But in practice, finish happens too late to commit the styles, so you still need to use fill-forwards to persist them, then cancel the animation to avoid the gotchas of fill-forwards.

Adding a flag to commitStyles just to make its primary use-case work seems wrong, but maybe I'm wrong about the primary use-case.

@birtles
Copy link
Contributor

birtles commented Aug 27, 2020

The spec gives a bit of a summary of the motivation and alternatives here: https://drafts.csswg.org/web-animations-1/#fill-behavior

The interesting part with regard to fill modes is that most animation APIs (going back to SMIL upon which CoreAnimation etc. were built) use endpoint exclusive timing so at the exact point when an animation finishes, it has no effect--unless there is an appropriate fill mode set. That's useful for sequencing and repeating animations since it ensures they don't overlap but it's been causing problems for scroll-driven animations recently. (There a bit of explanation in the SMIL spec.)

Unfortunately, we very likely can't change this very fundamental part of the model (and other specs like SVG etc. depend on it if we ever want to rebase them on Web Animations).

However commitStyles() already has the special behavior that it applies removed animations so I suppose we could make it also have the special behavior of altering the timing model to sample a finished animation as if its (root) target effect had fill: both set. That would solve half of this issue and, I believe, the troubles @jakearchibald encountered. It's also possible this API is new enough that such a change would be Web compatible. It wouldn't change the behavior for animations which are already filling which is likely to be the majority case anyway.

@jakearchibald
Copy link
Contributor

Fwiw, my advice to developers is to avoid filling forwards, as those "stuck" animations tend to catch you out later on, so I see commitStyles as something that makes avoiding fill-forwards easier.

@flackr
Copy link
Contributor

flackr commented Aug 28, 2020

I think there are issues with immediately committing the end value when you have composite animations:

animA = element.animate(
    {transform: ['translateX(0)', 'translateX(500px)']},
    {duration: 5000, composite: 'add'});
animA.commitStyles(); // element.style.transform = 'translateX(500px)';

Now the effect is animating from 500px to 1000px?

This is why in #5475 I proposed we don't commit immediately but rather commit the style change when the animation finishes (on an infinitely precise timeline which is conceptually identical to filling forwards).

@birtles
Copy link
Contributor

birtles commented Aug 28, 2020

The proposal here is that for a finished animation you would apply the final value. It wouldn't affect animation's that are not finished. (More precisely, we'd simply opt out of the endpoint exclusive behaviour for the root target effect such that this would still apply to un-finished animations that are in their end delay.)

@birtles
Copy link
Contributor

birtles commented Aug 28, 2020

The part about jumping to the end is separate (hence why this proposal only addresses half of this issue).

@birtles
Copy link
Contributor

birtles commented Jan 13, 2021

I started trying to specify how the fill mode part of this works and came across a few possibilities for how this could work. I'll list them out as questions.

1. Does this behavior apply to just the animation on which commitStyles is called? Or all the animations below it in the stack?

Suggested answer: It should be just the animation itself.

Rationale: The special behavior where we re-add removed animations to the stack already only applies to the animation on which commitStyles is called.

For use cases where you have a lot of animations finishing at the same time that would be removed, we rely on the fact that simultaneous remove events are going to be dispatched in their composite order so we'd work our way up the stack and everything should behave as expected.

We should follow the same behavior for this adjustment too.

2. Is the adjustment to the fill mode? Or to the endpoint-exclusive timing?

This matters for a few cases.

One is if you have an animation in its delay phase with no fill mode (or a forwards fill mode). Does commitStyles act as if it has a fill mode of both/backwards?

Another is if you seek an animation past its end time. Does commitStyles act as if it it has a fill mode of both/forwards then too?

Suggested answer: The adjustment applies only to endpoint-exclusive timing.

Rationale:

  • It's less likely to produce surprises when applied to animations in their delay phase
  • As a result it's more likely to be Web compatible if we make this the default behavior (see question 4 below)
  • It's more likely to produce a sane result when we introduce group effects (see question 3 below)

Unfortunately, it's a little more bit invasive than tweaking the fill mode.

3. Does it apply to just the root target effect, or to all descendant effects?

Suggested answer: All children

The previous suggestion was that this only applies to the root target effect but I think we want it to apply to all descendants.

For example if you have:

  • Group effect (fillMode = "auto" → "both")
    • Child keyframe effect (fillMode = "auto" → "none", end time of keyframe effect == end time of group effect)

If we only tweak the fill mode on the root target effect, we'll fail to apply the styles from the keyframe effect, but I think the author would expect the keyframe effect to fill in this case.

Conversely, if the child keyframe effect finished before the parent group effect, I think if we call commitStyles on the animation when it has finished, we would not expect it to apply to any child keyframe effects that finished before the end of the group (or else you'd get very odd results for sequence groups).

4. Does this apply by default? Is there an opt-out mechanism?

There are a few possibilities here:

  1. Make it opt-in with a parameter, e.g. commitStyles({ endMode: 'extend' })
  2. Make it opt-out with a parameter, e.g. commitStyles({ endMode: 'finish' })
  3. Make it apply only to effects whose specified fillMode is "auto" (i.e. make it opt-out but use fillMode as the mechanism)
  4. Make it the default and wait until people complain

Suggested answer: 4? Keep it simple and take a risk that this is Web-compatible? 3 is also interesting as the author is making an explicit signal that the animation should not continue past the end? 3 would also allow a means of opting-out on a per-effect basis.

@flackr
Copy link
Contributor

flackr commented Jan 13, 2021

I strongly agree with all of your suggested answers. I think this is going to be the least surprising and most compatible way to implement it.

One thought about point 1 (doesn't affect this proposal), given that commitStyles applies styles at the bottom of the animation stack (i.e. because inline style applies before animation styles), it seems as if commitStyles could only apply the current animation rather than all animations beneath it with the expectation that you would have already had an opportunity to commit the animations below it. This may be less surprising than committing the effect of multiple animations with one call.

@birtles
Copy link
Contributor

birtles commented Jan 14, 2021

I strongly agree with all of your suggested answers. I think this is going to be the least surprising and most compatible way to implement it.

Great, thanks Rob!

One thought about point 1 (doesn't affect this proposal), given that commitStyles applies styles at the bottom of the animation stack (i.e. because inline style applies before animation styles), it seems as if commitStyles could only apply the current animation rather than all animations beneath it with the expectation that you would have already had an opportunity to commit the animations below it. This may be less surprising than committing the effect of multiple animations with one call.

Yeah, I'm a little confused about this part. I think the idea was that by compositing the lower animations, the result would be correct even if some of the animations lower in the cascade had not yet been removed but, as you point out, any such animations would clobber the inline style.

The WPT for commitStyles includes a few cases where it makes a difference, but I don't know if any of them represent real use cases.

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

No branches or pull requests

4 participants