[css-view-transitions-1] Rearrange algorithms, add notes #3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jakearchibald
commented
Jan 20, 2023
css-view-transitions-1/Overview.bs
Outdated
| Issue: Define the algorithm used to clip the snapshot when it exceeds max size. | ||
| </div> | ||
|
|
||
| ## [=Handle transition frame=] ## {#update-transition-dom-algorithm} |
There was a problem hiding this comment.
Renamed this. It used to be "update transition DOM", but it's doing more than that.
jakearchibald
commented
Jan 20, 2023
|
|
||
| Note: This algorithm captures the current state of the document, | ||
| calls the transition's {{UpdateCallback}}, | ||
| then captures the new state of the document. |
jakearchibald
commented
Jan 20, 2023
Comment on lines
+1100
to
+1103
| 1. [=Capture the old state=] for |transition|. | ||
|
|
||
| If failure is returned, then [=skip the view transition=] for |transition| with an "{{InvalidStateError}}" {{DOMException}} in |transition|'s [=relevant Realm=], | ||
| and return. |
There was a problem hiding this comment.
This used to be inline, but the algorithm was getting a bit large. This felt like a unit that could be moved to its own sub-algorithm.
jakearchibald
commented
Jan 20, 2023
Comment on lines
+1151
to
+1154
| 1. [=Capture the new state=] for |transition|. | ||
|
|
||
| If failure is returned, then [=skip the view transition=] for |transition| with an "{{InvalidStateError}}" {{DOMException}} in |transition|'s [=relevant Realm=], | ||
| and return. |
There was a problem hiding this comment.
Another bit that used to be inline, but now moved into its own algorithm.
jakearchibald
commented
Jan 20, 2023
| 1. [=Resolve=] |transition|'s [=ViewTransition/ready promise=]. | ||
| </div> | ||
|
|
||
| ### [=Capture the old state=] ### {#capture-old-state-algorithm} |
There was a problem hiding this comment.
I've groups some algorithms with a parent algorithm, if they're only ever called by the parent. This gives us a more hierarchical heading structure.
jakearchibald
commented
Jan 20, 2023
| </div> | ||
|
|
||
| 1. Set |document|'s [=document/transition suppressing rendering=] to true. | ||
| ### [=Capture the new state=] ### {#capture-new-state-algorithm} |
There was a problem hiding this comment.
This is just extracted from the "Setup view transition" algorithm. No changes other than returning failure, rather than throwing an error.
jakearchibald
commented
Jan 20, 2023
| Issue: The link for "paint order" doesn't seem right. | ||
| Is there a more canonical definition? | ||
| <div algorithm> | ||
| To <dfn>setup transition pseudo-elements</dfn> for a {{ViewTransition}} |transition|: |
There was a problem hiding this comment.
This is just moved. No changes, aside from those noted.
jakearchibald
commented
Jan 20, 2023
| Would it be simpler to always add it and try to optimize in the implementation? | ||
| </div> | ||
|
|
||
| ## [=Call the update callback=] ## {#call-dom-update-callback-algorithm} |
There was a problem hiding this comment.
Not changed, just moved.
jakearchibald
commented
Jan 20, 2023
|
|
||
| Note: The |interestRectangle| is the subset of |element|'s [=ink overflow rectangle=] that should be captured. | ||
| This is required for cases where an element's ink overflow rectangle needs to be clipped because of hardware constraints. | ||
| For example, if it exceeds the maximum texture size. |
There was a problem hiding this comment.
Moved this note to the "computing the interest rectangle" algorithm
jakearchibald
commented
Jan 20, 2023
Comment on lines
+1292
to
+1294
| Note: This algorithm constructs the pseudo-element tree for the transition, | ||
| and generates initial styles. | ||
| The structure of the pseudo-tree is covered at a higher level in [[#view-transition-pseudos]]. |
jakearchibald
commented
Jan 20, 2023
Comment on lines
+1421
to
+1423
| Note: This is guaranteed to happen for every {{ViewTransition}}, | ||
| even if the transition is [=Skip the view transition|skipped=]. | ||
| The reasons for this are discussed in [[#transitions-as-enhancements]]. |
93388ed to
4782752
Compare
e4cdc46 to
c97c2d9
Compare
khushalsagar
reviewed
Feb 7, 2023
khushalsagar
approved these changes
Feb 7, 2023
Co-authored-by: Khushal Sagar <63884798+khushalsagar@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I moved some algorithms around, as such the diff is trash, sorry. I'll try and make it easier to read in the comments.
Don't merge this. I'll move it to the CSSWG when #2 lands.