Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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-animations-2] Specify the animation-trigger property #10128
base: main
Are you sure you want to change the base?
[css-animations-2] Specify the animation-trigger property #10128
Changes from 4 commits
133f6e0
f3a49ba
cbd4f07
9f6464a
912426a
9ae1a13
f1d2e89
e55e476
a0e90fe
361f5d9
efa38c7
085618e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes it sound like animation triggers are separate things from animations, but I suspect we should say that as a part of updating the current time of animations we evaluate whether a trigger condition has been reached and apply a stateful change to the animation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I'll work on that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect triggers on scroll-driven animations should "just work". E.g. they could play / stop the animation when the trigger condition occurs / finishes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does each
type
affects a scroll-driven animation?Furthermore, what could be the actual use-case for this? Perhaps one that I can think of now is if we have a
hover-timeline
in the future and we want to enable/disable it on different scroll positions. Is that what you mean?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the internal control of how triggers work probably belongs in web-animations where we define things like how an updated time updates corresponding animations: https://drafts.csswg.org/web-animations-1/#update-animations-and-send-events
We should also be able to set up animation triggers from javascript.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, sure. We didn't discuss the WAAPI part of triggers in the issue, should I try fleshing that too in this PR? Or you're just stating this as a reason for this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think defining a trigger in terms of an internal animation effect's time might be overkill. E.g. I think we could define this simply in terms of the animation timeline's time with respect to the specified range
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I struggled with that quite a bit. Eventually it worked out once I defined the same hierarchy as animations', and defined a special state for the
effect
. I'll try simplifying this further.