-
Notifications
You must be signed in to change notification settings - Fork 647
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
[css-view-transitions-1] domUpdated isn't the right term #8144
Comments
The developer provides a callback that updates the state of the page in some way that forms the 'after' state of a transition.
It doesn't need to be a tree change. It can be a class name changing, an inline style, a CSSOM change etc etc. Some other name ideas:
I'll try and think up more options. Although, I think |
It sounds like you and I have a different understanding of what DOM means. If the overall feature is called view transitions, viewUpdated makes some sense to me. |
The problem with Here's a diagram I've been working on to go in the spec that tries to explain the phases of a view transition. out.mp4The three promises map to those phases:
I had assumed that a change to styling counted as a DOM change, as text content of a style would have changed, or a rule in CSSOM would have changed etc etc. My assumption is that CSSOM was a subset of the DOM, since it's properties that hang off nodes that are part of the document.
|
How about something like I think there's quite distinct ideas about the scope of the term DOM and as such I'd rather also not have it as a term-of-art in APIs or even prose. |
I thought you were saying 'state' was problematic in #8143? |
Ah, ok, I think I understand what you mean in #8143 now.
|
@vmpstr suggested the name should reflect that it's linked to the callback passed to So:
I prefer the latter, in case we add other callbacks in future. |
From an API perspective:
There's nothing about the API shape right now that implies a name for the callback parameter passed to You could consider using "render," as I believe that's a term unrelated to DOM but reflects that the operation is to change the rendered content.
|
There might in future, if the API changes to a dictionary to allow for options, and perhaps other callbacks.
Yep, it will be, to reflect the name we decide here. But the developer-facing name is more important.
See the video in #8144 (comment). I don't think "render" is a good choice because the change is not rendered visually. |
|
Mutations (see |
The nice thing about If you mutate the DOM, change a style, and then (for whatever reason) await something unrelated in the callback, a name like |
I'm good with updateCallbackDone as well. This promise is tightly coupled with the execution of the developer provided callback in startViewTransition so the name makes that obvious. |
Perhaps it would be more correct to say |
Posting a summary of all the options brought up on the issue so folks can quickly skim through and pick the one they most agree with.
Please add in case I missed something. My take is that the pattern for nounVerbCallback for the callback and nounVerb for the promise is good. And If we can't agree on a noun then next best option is to drop it and go with |
Conclusion from a discussion just now, we've got 2 proposals that we'd like the group's feedback on:
No strong preference on either of the options above. |
I'd prefer 2. To me "state" sounds very vague. I do however understand that the callback argument is the "update callback". |
The CSS Working Group just discussed
The full IRC log of that discussion<JakeA> View transitions wraps a state change - we attempt generate a transition from that state change<JakeA> The developer provides a callback where they do that change, usually by modifying the DOM in some way. It can be async. <JakeA> Sometimes you just want to know when the state change is complete <JakeA> We provided a promise for that,`domUpdated`, which fulfills when the promise returned by the callback fulfills <JakeA> Anne said "we don't really expose DOM as a term to web content", so the name is bad <JakeA> So, we need a new name <JakeA> Couple of options: `stateUpdated` - avoids using DOM, but a bit vague, might be confusing due to frameworks use of 'state'. <JakeA> Vlad suggested that, since the promise resolves along with the callback, we name it `updateCallbackDone`. <JakeA> I like that, since if the developer does stuff other than DOM updating in the callback, the promise name still makes sense. <JakeA> Anne is happy with that. <bramus> JakeA: I can take up this one. posted a bunch of messages above <TabAtkins> +1 to suggested name change, it's clear and tied directly to what's actually happening, and I think will make sense for an author. <masonf> `DOMContentLoaded` ? <bramus> JakeA: I guess we are saying: any objections? <bramus> astearns: not knowing what the callback is named that seems vague. what is it? <bramus> JakeA: it does not have an developer visible name. authors pass it in as argument <bramus> JakeA: `updateCallback` and then `updateCallbackDone` <florian> have not followed deeply, but seems reasonable <bramus> TabAtkins: I like it. makes sense and matches spec language <bramus> bramus: +1 <bramus> astearns: proposed resolution: adopt suggested change to rename to `updateCallbackDone ` <bramus> RESOLVED: adopt suggested change to rename to `updateCallbackDone |
This patch renames the promise as a result of the following resolution: w3c/csswg-drafts#8144 [email protected], [email protected] Change-Id: Ic0d4f3af2569462ccbf7e30fe258f0e52b21286e
This patch renames the promise as a result of the following resolution: w3c/csswg-drafts#8144 [email protected], [email protected] Change-Id: Ic0d4f3af2569462ccbf7e30fe258f0e52b21286e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4178008 Auto-Submit: Vladimir Levin <[email protected]> Reviewed-by: Khushal Sagar <[email protected]> Commit-Queue: David Bokan <[email protected]> Commit-Queue: Vladimir Levin <[email protected]> Reviewed-by: David Bokan <[email protected]> Cr-Commit-Position: refs/heads/main@{#1094162}
This patch renames the promise as a result of the following resolution: w3c/csswg-drafts#8144 [email protected], [email protected] Change-Id: Ic0d4f3af2569462ccbf7e30fe258f0e52b21286e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4178008 Auto-Submit: Vladimir Levin <[email protected]> Reviewed-by: Khushal Sagar <[email protected]> Commit-Queue: David Bokan <[email protected]> Commit-Queue: Vladimir Levin <[email protected]> Reviewed-by: David Bokan <[email protected]> Cr-Commit-Position: refs/heads/main@{#1094162}
This patch renames the promise as a result of the following resolution: w3c/csswg-drafts#8144 [email protected], [email protected] Change-Id: Ic0d4f3af2569462ccbf7e30fe258f0e52b21286e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4178008 Auto-Submit: Vladimir Levin <[email protected]> Reviewed-by: Khushal Sagar <[email protected]> Commit-Queue: David Bokan <[email protected]> Commit-Queue: Vladimir Levin <[email protected]> Reviewed-by: David Bokan <[email protected]> Cr-Commit-Position: refs/heads/main@{#1094162}
This patch renames the promise as a result of the following resolution: w3c/csswg-drafts#8144 [email protected], [email protected] Change-Id: Ic0d4f3af2569462ccbf7e30fe258f0e52b21286e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4178008 Auto-Submit: Vladimir Levin <[email protected]> Reviewed-by: Khushal Sagar <[email protected]> Commit-Queue: David Bokan <[email protected]> Commit-Queue: Vladimir Levin <[email protected]> Reviewed-by: David Bokan <[email protected]> Cr-Commit-Position: refs/heads/main@{#1094162}
…one, a=testonly Automatic update from web-platform-tests VT: Rename domUpdated to updateCallbackDone This patch renames the promise as a result of the following resolution: w3c/csswg-drafts#8144 [email protected], [email protected] Change-Id: Ic0d4f3af2569462ccbf7e30fe258f0e52b21286e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4178008 Auto-Submit: Vladimir Levin <[email protected]> Reviewed-by: Khushal Sagar <[email protected]> Commit-Queue: David Bokan <[email protected]> Commit-Queue: Vladimir Levin <[email protected]> Reviewed-by: David Bokan <[email protected]> Cr-Commit-Position: refs/heads/main@{#1094162} -- wpt-commits: ad7fcb93363494d53af41c5ef25230e07c5b1d42 wpt-pr: 38045
…one, a=testonly Automatic update from web-platform-tests VT: Rename domUpdated to updateCallbackDone This patch renames the promise as a result of the following resolution: w3c/csswg-drafts#8144 [email protected], [email protected] Change-Id: Ic0d4f3af2569462ccbf7e30fe258f0e52b21286e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4178008 Auto-Submit: Vladimir Levin <[email protected]> Reviewed-by: Khushal Sagar <[email protected]> Commit-Queue: David Bokan <[email protected]> Commit-Queue: Vladimir Levin <[email protected]> Reviewed-by: David Bokan <[email protected]> Cr-Commit-Position: refs/heads/main@{#1094162} -- wpt-commits: ad7fcb93363494d53af41c5ef25230e07c5b1d42 wpt-pr: 38045
…one, a=testonly Automatic update from web-platform-tests VT: Rename domUpdated to updateCallbackDone This patch renames the promise as a result of the following resolution: w3c/csswg-drafts#8144 [email protected], [email protected] Change-Id: Ic0d4f3af2569462ccbf7e30fe258f0e52b21286e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4178008 Auto-Submit: Vladimir Levin <[email protected]> Reviewed-by: Khushal Sagar <[email protected]> Commit-Queue: David Bokan <[email protected]> Commit-Queue: Vladimir Levin <[email protected]> Reviewed-by: David Bokan <[email protected]> Cr-Commit-Position: refs/heads/main@{#1094162} -- wpt-commits: ad7fcb93363494d53af41c5ef25230e07c5b1d42 wpt-pr: 38045
Apart from a couple of events that are prefixed with
DOM*
, we don't really expose DOM as a term to web content and I think we should keep it that way.Either keep this somewhat more abstract or go with
nodeTreeChanged
or some such.cc @jakearchibald @domenic
The text was updated successfully, but these errors were encountered: