Skip to content

[scroll-animations-1] Also rerun style/layout for changed timeline ranges #8704

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

Merged
merged 4 commits into from
May 5, 2023

Conversation

flackr
Copy link
Contributor

@flackr flackr commented Apr 11, 2023

Fixes #8694.

@flackr flackr requested a review from fantasai April 11, 2023 19:52
Co-authored-by: fantasai <fantasai.bugs@inkedblade.net>
Copy link
Contributor

@kevers-google kevers-google left a comment

Choose a reason for hiding this comment

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

LGTM

we run and we run an additional step to recalculate styles and update layout.
As we only gather stale timelines during the first style and layout calculation,

Note: We check for layout changes after dispatching any {{ResizeObserver}}s intentionally
Copy link
Member

Choose a reason for hiding this comment

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

... but this means that ResizeObserver sees an irrelevant layout state vs. what we actually intend to render?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's correct. It's a tradeoff I think. Either:

  • Run before ResizeObserver, then any changes to sizes of content made by ResizeObserver may not be handled by scroll driven animations, but ResizeObserver will see layout driven animation results.
  • Run after ResizeObserver, then layout animations may not be fully up to date when calling ResizeObserver, but view timelines will be able to observe resizeobserver derived sizes.

Given that ResizeObservers are designed for establishing the size of elements, I feel like view timelines should be updated to reflect the final resize observer output. The converse, animating layout firing ResizeObservers in response to scroll seems like an anti-pattern.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're going to want to use the same mechanism and invalidation for anchor positioning. Chatting with @bfgeek we thought after ResizeObserver would be more in line with the use cases.

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.

[scroll-animations-1] Handling changed size/position in current frame
4 participants