-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Improve startViewTransition() data #28541
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?
Improve startViewTransition() data #28541
Conversation
|
Tip: Review these changes grouped by change (recommended for most PRs), or grouped by feature (for large PRs). |
caugner
left a comment
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.
LGTM overall, but the nested parameters should not be nested according to BCD guidelines.
| "types_property": { | ||
| "__compat": { | ||
| "description": "`types` property", |
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.
BCD guidelines suggest to not nest these:
| "types_property": { | |
| "__compat": { | |
| "description": "`types` property", | |
| "options_types_parameter": { | |
| "__compat": { | |
| "description": "`options.types` parameter", |
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.
/cc @ddbeck Is this a special case warranting an exception? (I don't remember understanding why we don't nest these.)
Edit: Nesting has the benefit of linting the child support data against the parent support data.
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.
Yeah, not nesting them seems a little odd to me.
| "update_property": { | ||
| "__compat": { | ||
| "description": "`update` property", |
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.
| "update_property": { | |
| "__compat": { | |
| "description": "`update` property", | |
| "options_update_parameter": { | |
| "__compat": { | |
| "description": "`options.update` parameter", |
|
As an FYI, I've updated the Firefox support for these updates to "preview": see https://bugzilla.mozilla.org/show_bug.cgi?id=2001878. Update: After more testing, I'm pretty sure it should just be "147", not "preview". There doesn't appear to be a pref to flip for this. |
Summary
Chrome and Safari have both supported view transition types (see https://chromestatus.com/feature/5089552511533056) for a while, so it is high time I got them documented on MDN.
Having checked BCD, it seems that we have pretty much all of the data we need in place already, but I wanted to improve the
Document.startViewTransition()data:a.
updateCallbackmakes sense — this is the callback parameter mentioned in 1.b.
callbackOptionsdoesn't make sense. This refers to a mixin in the spec that represents the possible parameter values as one item. This is a spec construct and not a real value that developers can use. It would be better to have a sub-data point for the options object, with sub-sub-data points for the possible object properties. This last bit is maybe not totally necessary, but I thought it would be useful for developers who are specifically checking the compat table fortypessupport.This PR updates the data as described above, and also updates the spec URLs so they all point to the Level 2 of the spec
Test results and supporting details
Related issues