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

Queue the long animation frame entry when presentation is ready #109

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

noamr
Copy link
Contributor

@noamr noamr commented Nov 13, 2024

Together with w3c/long-animation-frames#22

When presentation timing is ready, we queue the entry with the paint timing info.


Preview | Diff

Together with #22

When presentation timing is ready, we queue the entry with the paint timing info.
@noamr noamr requested a review from clelland November 13, 2024 09:13
@noamr noamr requested a review from mmocny November 25, 2024 19:18
Copy link

@mmocny mmocny left a comment

Choose a reason for hiding this comment

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

As discussed in person: I think that this solution means that Paint Timing will have to have explicit references to every specific entry type that requires paint timing. An alternative would have been to use a common hook/callback to request and receive just the timings. But I know you thought this was a good choice and did it by design.

@noamr
Copy link
Contributor Author

noamr commented Nov 26, 2024

As discussed in person: I think that this solution means that Paint Timing will have to have explicit references to every specific entry type that requires paint timing. An alternative would have been to use a common hook/callback to request and receive just the timings. But I know you thought this was a good choice and did it by design.

Yes! I think hooks are brittle and make the order unclear and bug-prone. They're not necessary when we have a constant set of APIs that need this.

For reference, the HTML spec does not use hooks/observers as it was causing this kind of issue. See for example https://html.spec.whatwg.org/multipage/interaction.html#update-the-visibility-state.

@noamr noamr merged commit b5204cf into main Nov 26, 2024
2 checks passed
@noamr noamr deleted the pres-time branch November 26, 2024 19:45
github-actions bot added a commit that referenced this pull request Nov 26, 2024
SHA: b5204cf
Reason: push, by noamr

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

2 participants