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

Update README.md for video #119

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Update README.md for video #119

wants to merge 2 commits into from

Conversation

tunetheweb
Copy link
Member

No description provided.

@clelland
Copy link
Collaborator

#83 and #85 are still open, while I write up the spec changes for this -- should we coordinate the spec + explainer changes, or do you think it's worth updating the explainer even when the spec doesn't account for animated content at all?

@tunetheweb
Copy link
Member Author

tunetheweb commented Oct 30, 2023

Yeah I was thinking about that (and checked the spec to see if it needed updating too). The spec doesn’t mention video elements at all whereas the README does but with poster (which is now out of date).

Similarly, only the README mentions “fully loaded” which is now also outdated.

But agree it’s less than ideal the spec doesn’t cover this so could understand not wanting to merge this. Or could change it to be more vague but still not be wrong? Or closer to the spec and update later.

WDYT?

@clelland
Copy link
Collaborator

The spec only includes it by reference to element-timing, when it says "The Largest Contentful Paint API will only expose element types that are already exposed by the Element Timing API." -- that spec includes video poster images.

That said, I have a refactor of this spec, as well as Element Timing and Paint Timing, which I hope to land soon, to resolve a bunch of inappropriate spec layering, and we should be able to land this with it.

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