-
Notifications
You must be signed in to change notification settings - Fork 9
Experiments with observing changes caused by animations #90
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for style-observer ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
src/element-style-observer.js
Outdated
if (!this.properties.has(event.propertyName)) { | ||
if (event.type === "animationstart") { | ||
let animation = document.getAnimations().find(a => a.effect.target === this.target); | ||
let properties = this.getObservedAnimationProperties(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.
I’m not sure if this needs to be a separate function. It's only called once, and it seems to niche to be useful outside of this (but perhaps you plan to call it from somewhere else too?)
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.
Yes, you are right. We don't need a separate function for this. I just thought it would make the code a bit cleaner. But it looks OK even with the inlined code.
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 looks very promising!
- Inline the code called once - Add support for `animationThrottle` option
I addressed your feedback (thank you!). What our next steps will be? |
0b927c0
to
631ad31
Compare
|
||
// Get the observed properties that are being animated by the animation | ||
let keyframes = animation.effect.getKeyframes(); | ||
let properties = new Set(keyframes.flatMap(frame => |
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.
There's a caveat: We can't get their names from the keyframes if we animate custom properties. 🤷♂️ See https://codepen.io/dmitrysharabin/pen/mydPGPR
On my local branch, I tried to add all observed custom properties to the properties
array properties.push(...this.propertyNames.filter(property => property.startsWith("--")));
.
It worked. But I still wonder, what if it's wrong... In that case, we will observe changes in computed properties even if the animation on the observed element changes properties we don't actually observe. I need to think about it a bit more. 🤔
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.
Interestingly, Firefox does return custom properties when getting an animation's keyframes with getKeyframes().
No description provided.