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

[css-transitions-1] Define before/after-change style in terms of base #6688

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

andruud
Copy link
Member

@andruud andruud commented Sep 27, 2021

[css-transitions-1] Define before/after-change style in terms of base

  • Introduce "base style", which is the computed style as it would be
    without animations/transitions.
  • Define before/after-change style in terms of that base style
    (at two different points in time).
  • There should be no need for the "completed transition" set anymore,
    since the before/after-change styles on a child are blind to any
    effects from transitions/animations happening on the parent.
  • Maintain the behavior that transitions can not start for properties
    affected by animations. Removing this behavior was considered,
    but was deemed impossible from use-counter data showing that ~5%
    of sites may rely on this behavior.

This change stems from Container Queries, which blurred the line
between the style and layout steps of the frame. The spec currently
defines the before-change style as the style of the previous
style change event, but with any animation effects updated to
current time. This is difficult, because it persisting the before-change
world in some form that is responsive to what the (newly updated)
animations actually do. (For example, we'd need to re-resolve var(),
font-relative units, be able to handle revert/revert-layer, etc.)
With Container Queries, this problem becomes substantially worse,
because now we'd also need to maintain the "before-change layout tree",
which in practice is gone at the time we need it.

#6398

@andruud
Copy link
Member Author

andruud commented Sep 27, 2021

This needs a resolution, but proposing an edit ahead of time so we know exactly what to resolve on.

Copy link
Member

@dbaron dbaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good to me (although it has some compatibility risks... though perhaps low given that current implementations don't agree very much), but I think this either needs working group consensus or at least approval from the other implementors of transitions, in #6398.

It's possible the list could be further simplified at this point, but it will be easier to tell once this lands.

css-transitions-1/Overview.bs Outdated Show resolved Hide resolved
@birtles
Copy link
Contributor

birtles commented Oct 1, 2021

I think this represents a change to case 11 from https://bug1192592.bmoattachments.org/attachment.cgi?id=8843824. That is, I believe with this proposal it would be possible to trigger a transition while an animation is running. Maybe that's ok but it does seem a bit risky in terms of compatibility, since browsers appear to agree on that at the moment.

Also, I'm really rusty on how all this fits together but I believe in Gecko at least SMIL animated styles are written to an override stylesheet separate to the animations level of the cascade whereas this PR seems to assume they are written to the animations level of the cascade.

@andruud
Copy link
Member Author

andruud commented Oct 5, 2021

case 11

I think you're right. Although while the animation is running, the transition would add no effect value to the cascade. https://drafts.csswg.org/css-transitions-1/#application. But it would be observable via events, and when the animation ends sooner than the transition.

  • You'd think that it's uncommon for authors to specify a transition and an animation on the same property. If you like I can add a (Chrome) use-counter to see how often a would-be transition is "blocked" by a running animation, or
  • We could explicitly specify in https://drafts.csswg.org/css-transitions-1/#starting that a transition should not start if there's currently an animation on the same property. I don't think that behavior must arise from the current definition of before/after-change style?

SMIL, cascade

Oh, that's unfortunate. Thanks for pointing that out. I'll have to investigate more to know what makes sense there.

@birtles
Copy link
Contributor

birtles commented Oct 5, 2021

  • You'd think that it's uncommon for authors to specify a transition and an animation on the same property. If you like I can add a (Chrome) use-counter to see how often a would-be transition is "blocked" by a running animation, or

  • We could explicitly specify in https://drafts.csswg.org/css-transitions-1/#starting that a transition should not start if there's currently an animation on the same property. I don't think that behavior must arise from the current definition of before/after-change style?

Thanks. I'm not sure which is preferable. If there's time, adding the use counter would be safest.

If we decide to change this behavior, I guess it would be little odd that transition events are fired even when there's no visual change. Maybe it's ok though.

@andruud
Copy link
Member Author

andruud commented Oct 29, 2021

@birtles I have now added the use-counter to Chrome to investigate. I'll report back in a few months when good data comes in.

Meanwhile, could you have a look at this as well @smfr?

@birtles
Copy link
Contributor

birtles commented Oct 30, 2021

@birtles I have now added the use-counter to Chrome to investigate. I'll report back in a few months when good data comes in.

Thank you!

@mirisuzanne
Copy link
Contributor

@andruud any updates on this?

@andruud
Copy link
Member Author

andruud commented Feb 17, 2022

Yes, but not the update I wanted. The use-counter is at almost 4%, which is way higher than expected. I've been waiting for sample pages to come in so I could check if there's any typical usage patterns we could use to argue that the change doesn't matter, but at 4% it might be futile to find the "typical" usage patterns anyway.

So as it stands, the use-counter did not help in making a confident decision here, unfortunately.

@birtles In light of this, do you think we should do the following?

We could explicitly specify in https://drafts.csswg.org/css-transitions-1/#starting that a transition should not start if there's currently an animation on the same property. I don't think that behavior must arise from the current definition of before/after-change style?

@birtles
Copy link
Contributor

birtles commented Feb 18, 2022

@birtles In light of this, do you think we should do the following?

We could explicitly specify in https://drafts.csswg.org/css-transitions-1/#starting that a transition should not start if there's currently an animation on the same property. I don't think that behavior must arise from the current definition of before/after-change style?

I think so. Transition events are quite important to a lot of apps -- often various state changes are keyed off getting a transitionend event. I think any change to the circumstances under which transition events are dispatched is risky and we should try to match the current (interoperable) behavior there.

(Anecdotally, many many years ago when working on Firefox OS, a recurring cause for apps breaking was that they didn't receive expected transitionend events which is what motivated specifying the transitioncancel event.)

@andruud
Copy link
Member Author

andruud commented Feb 21, 2022

OK, thanks @birtles. I'll update this PR with new proposed text as soon as I can.

@flackr
Copy link
Contributor

flackr commented Apr 12, 2023

Sounds good to me to make transitions not start if there is an animation on that property. I think you should look for animations in the after-change style so that we avoid starting a transition in the case in #8701.

As an extreme use case, consider a page which makes use of both transitions and animations to move an element around, e.g.

<style>
#target {
  transition: transform 1s;
}
</style>
<script>
moveTo(transform) {
  let animation = target.animate([{transform}], {duration: 1000, fill: 'forwards'});
  animation.finished.then(() => {
    // Changes the base style to match the animated position.
    animation.commitStyles();
    animation.cancel();
  });
}
</script>

The call to commitStyles should not start a transition (this would cause the element to move back to its old location and slide back to the target). I think this would be fine right now because the animation is still technically active, however we should also make sure that with #5394 you can omit the fill and have the animation technically not active in the final frame, i.e.

bounceTo(transform) {
  let animation = target.animate([{transform}], 1000);
  animation.finished.then(() => {
    // Changes the base style to match the animated position.
    animation.commitStyles();
  });
}

In this case the animation is technically no longer active but was previously - so perhaps this is just justification to consider animations in the before or after change style?

@andruud
Copy link
Member Author

andruud commented Nov 1, 2024

Trying to pick this back up (at the request of @dbaron), I have updated this PR:

  • Excluded SMIL explicitly from the base style.
  • Blocked transitions from starting when they are "affected by animations".
  • @flackr's commitStyles() example seemed quite convincing to me, so I added a version of that as an example.

Please review closely.

 - Introduce "base style", which is the computed style as it would be
   without animations/transitions.
 - Define before/after-change style in terms of that base style
   (at two different points in time).
 - There should be no need for the "completed transition" set anymore,
   since the before/after-change styles on a child are blind to any
   effects from transitions/animations happening on the parent.
 - Maintain the behavior that transitions can not start for properties
   affected by animations. Removing this behavior was considered,
   but was deemed impossible from use-counter data showing that ~5%
   of sites may rely on this behavior.

This change stems from Container Queries, which blurred the line
between the style and layout steps of the frame. The spec currently
defines the before-change style as the style of the previous
style change event, but with any animation effects updated to
current time. This is difficult, because it persisting the before-change
world in some form that is responsive to what the (newly updated)
animations actually do. (For example, we'd need to re-resolve var(),
font-relative units, be able to handle revert/revert-layer, etc.)
With Container Queries, this problem becomes substantially worse,
because now we'd also need to maintain the "before-change layout tree",
which in practice is gone at the time we need it.

w3c#6398
@birtles
Copy link
Contributor

birtles commented Nov 11, 2024

I started reviewing this today, just briefly, but I'm still trying to get my head around how this will work with #5394 (and whether we can truly do without #11085).

@birtles
Copy link
Contributor

birtles commented Nov 14, 2024

I started reviewing this today, just briefly, but I'm still trying to get my head around how this will work with #5394 (and whether we can truly do without #11085).

So to capture my working, in #5394 we're proposing that calling commitStyles on an animation that doesn't fill and has just finished captures the final value (i.e. ignores the endpoint exclusive timing).

Suppose we have an animation from width 100px to 200px, no fill mode, and we call commitStyles as part of its finished event.

Let's assume the unanimated width (i.e. base style width) is 50px (just to make it easier to distinguish the animation style from the base style).

  1. commitStyles is called.
  2. We flush styles to ensure everything is up-to-date.
  3. At this point the inline style has yet to be updated so there is no change to the base value (it's still 50px) and no transition is triggered.
  4. We calculate the final value of the animation as 200px and update the inline style to 200px.
  5. On the next style change event, we notice that the base width has gone from 50px to 200px. Furthermore, during the previous style change event (2~3) width was not "affected by an animation" and hence we trigger a transition—which I don't think we intend to.

Does that seem right?

Comment on lines +895 to +900
The call to <code>commitStyles()</code>
will trigger a <a>style change event</a>,
with the <a href="#before-change-style">before-change</a>
<code>width</code> being <code>100px</code>,
and the <a href="#after-change-style">after-change</a>
<code>width</code> being <code>200px</code>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is right? The style change event occurs before the inline style is updated so the before-change and after-change width at this point should both be 100px?

@andruud
Copy link
Member Author

andruud commented Nov 14, 2024

@birtles Ah, I was not aware that commitStyles is specified to "applying any pending style changes" before doing its thing. That is indeed a problem.

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

Successfully merging this pull request may close these issues.

5 participants