[css-view-transitions-1] Clean up use of dynamic styles #8093

jakearchibald · 2022-11-17T13:56:04Z

css-view-transitions-1/Overview.bs

jakearchibald · 2022-11-17T14:03:42Z

css-view-transitions-1/Overview.bs

 		if it has both ''::view-transition-new()'' and ''::view-transition-old()'' as children.

+		Issue: This reads like a non-normative note, but it's normative. This rule should be specified at the same time as other styles, when the condition above can be determined.


I can fix this as part of this PR, once I've checked the details.

jakearchibald · 2022-11-17T14:05:55Z

css-view-transitions-1/Overview.bs

 		if the ancestor ''::view-transition-image-pair()'' has both ''::view-transition-new()'' and ''::view-transition-old()'' as descendants.

+		Issue: This reads like a non-normative note, but it's normative. This rule should be specified at the same time as other styles, when the condition above can be determined.


I can fix this as part of this PR, once I've checked the details.

jakearchibald · 2022-11-17T14:06:49Z

css-view-transitions-1/Overview.bs

+
+		: <dfn>new view-box rule</dfn>
+		:: A {{CSSStyleRule}} or null. Initially null.
+	</dl>


These definitions let us update/remove rules later.

Can you add some non-normative text to explain this briefly

jakearchibald · 2022-11-17T14:07:31Z

css-view-transitions-1/Overview.bs

+		:: a [=style sheet=].
+			Initially a new [=style sheet=] in the [=user-agent origin=], ordered after the [=HTML user agent style sheet=].
+
+			Note: This is used to hold dynamic styles relating to transitions.


This stylesheet is for dynamic styles involved in the transition. It's explicitly linked to a single document, clarifying the scoping.

jakearchibald · 2022-11-17T14:08:50Z

css-view-transitions-1/Overview.bs


 	<div algorithm>
-		To <dfn>perform an old capture</dfn> given a {{ViewTransition}} |transition|,
+		To <dfn>setup view transition</dfn> given a {{ViewTransition}} |transition|,


This algorithm is doing much more than capturing, so I renamed it.

jakearchibald · 2022-11-17T14:10:47Z

css-view-transitions-1/Overview.bs

@@ -828,13 +866,13 @@ urlPrefix: https://wicg.github.io/navigation-api/; type: interface;

 			Note: The ready promise would've been resolved if {{ViewTransition/skipTransition()}} is called after we start animating.

-		1. [=promise/React=] to [=ViewTransition/DOM updated promise=]:
+		1. [=promise/React=] to |transition|'s [=ViewTransition/DOM updated promise=]:


Being specific about which instance of the promise we're talking about

jakearchibald · 2022-11-17T14:12:42Z

css-view-transitions-1/Overview.bs

@@ -987,83 +1019,14 @@ urlPrefix: https://wicg.github.io/navigation-api/; type: interface;
 			Issue: Define the algorithm used to clip the snapshot when it exceeds max size.
 	</div>

-## [=Animate a view transition=] ## {#animate-a-view-transition-algorithm}


This algorithm was being called every frame, but the things it does only need to be done once ever (like non-changing UA styles), or once at the start of each transition.

I moved the steps elsewhere.

jakearchibald · 2022-11-17T14:14:36Z

css-view-transitions-1/Overview.bs

@@ -987,83 +1019,14 @@ urlPrefix: https://wicg.github.io/navigation-api/; type: interface;
 			Issue: Define the algorithm used to clip the snapshot when it exceeds max size.
 	</div>

-## [=Animate a view transition=] ## {#animate-a-view-transition-algorithm}
+## [=Setup transition pseudo-elements=] ## {#setup-transition-pseudo-elements-algorithm}


This algorithm used to be "create transition pseudo-elements", but I added the style rules for things that only need to be done once per transition, so I renamed it to reflect that.

jakearchibald · 2022-11-17T14:16:01Z

css-view-transitions-1/Overview.bs

+						html::view-transition-old(<var>transitionName</var>) {
+							object-view-box: <var>oldViewBox</var>;
+						}
+					</pre>


I liked the pattern of using a CSS code block to describe a rule being created, but we weren't consistent with it, so I used it for all style creation.

jakearchibald · 2022-11-17T14:16:44Z

css-view-transitions-1/Overview.bs

+						}
+					</pre>
+
+					Note: The above code example contains variables to be replaced.


These are styles that only need to be set once per transition.

jakearchibald · 2022-11-17T14:17:08Z

css-view-transitions-1/Overview.bs

 	</div>

-## [=Style transition pseudo-elements=] ## {#style-transition-pseudo-elements-algorithm}
+## [=Update pseudo-element styles=] ## {#style-transition-pseudo-elements-algorithm}


Changed the naming to suggest that it happens more than once.

jakearchibald · 2022-11-17T14:17:59Z

css-view-transitions-1/Overview.bs

+			1. [=list/For each=] |style| of |capturedElement|'s [=captured element/style definitions=]:
+
+				1. If |style| is not null,
+					then remove |style| from |document|'s [=document/view transition style sheet=].


We were missing this bit before, so styles would just hang around forever.

The thing is that I think the style removal is sufficient to cause the pseudo elements to be removed, since if no style targets a pseudo element it has default styles that are equivalent to it not being there, so I wonder if this can supersede the need to remove pseudo elements

I like that idea, but wouldn't it mean a user style like:

::view-transition { content: ''; }

…would cause the view transition pseudo to be 'always there'?

I've filed #8113 to think more about this.

vmpstr · 2022-11-17T22:29:32Z

css-view-transitions-1/Overview.bs

+
+		: <dfn>new view-box rule</dfn>
+		:: A {{CSSStyleRule}} or null. Initially null.
+	</dl>


Can you add some non-normative text to explain this briefly

vmpstr · 2022-11-17T23:57:09Z

css-view-transitions-1/Overview.bs

+			1. [=list/For each=] |style| of |capturedElement|'s [=captured element/style definitions=]:
+
+				1. If |style| is not null,
+					then remove |style| from |document|'s [=document/view transition style sheet=].


The thing is that I think the style removal is sufficient to cause the pseudo elements to be removed, since if no style targets a pseudo element it has default styles that are equivalent to it not being there, so I wonder if this can supersede the need to remove pseudo elements

vmpstr · 2022-11-18T00:00:30Z

css-view-transitions-1/Overview.bs

+
+					Note: The above code example contains variables to be replaced.
+
+					Issue: Which of ''xywh()''/''rect()''/''inset()'' should we use?


Can this be resolved at editor's discretion?

Any preferences?

Probably do inset :)

vmpstr · 2022-11-18T00:01:39Z

css-view-transitions-1/Overview.bs

@@ -1101,19 +1070,81 @@ urlPrefix: https://wicg.github.io/navigation-api/; type: interface;
 				except the [=view-transition pseudo-elements=],
 				are not painted (as if they had visibility: hidden)
 				and do not respond to hit-testing (as if they had pointer-events: none) until |new| exists.
+
+			1. If neither of |capturedElement|'s [=captured element/old image=] or [=captured element/new element=] is null:


I prefer "If both ... are not null", since it sounds more in the positive? Just a nit though.

vmpstr · 2022-11-18T00:04:26Z

css-view-transitions-1/Overview.bs

+
+			1. If neither of |capturedElement|'s [=captured element/old image=] or [=captured element/new element=] is null:
+
+				1. Let |transform| be |capturedElement|'s [=old styles=]'s 'transform' property.


Where is old styles defined? I know this has moved from elsewhere, but is this correct? We're not using the captured element's style and read its transform property, but rather find the transform it has in screen space and use that as the transform. Similarly for width/height, it could be a flex grow with no explicit width, so what does style width mean here?

I've replaced [=old styles=] with [=captured element/old styles=]. It's set in "setup view transition".

I think the confusion here is it feels like a direct reference to the old styles. Would it be better if the prose was:

1. Let |transform| be |capturedElement|'s [=captured element/old offset transform=].

vmpstr · 2022-11-18T00:25:12Z

css-view-transitions-1/Overview.bs

+
+					Note: The above code example contains variables to be replaced.
+
+					Issue: Which of ''xywh()''/''rect()''/''inset()'' should we use?


jakearchibald added 4 commits November 17, 2022 13:23

tmp

65cc54b

Set up UA styles

8152680

Progress on handling styles correctly

4e8335a

Clean up use of style modifications

3953af9

jakearchibald commented Nov 17, 2022

View reviewed changes

css-view-transitions-1/Overview.bs Show resolved Hide resolved

Add issues for a couple of incorrectly applied styles

8b0a2a6

jakearchibald commented Nov 17, 2022

View reviewed changes

Fix heading id

1b9fc28

jakearchibald commented Nov 17, 2022

View reviewed changes

Apply mix-blend-mode and isolation in algo steps

1e1e534

vmpstr reviewed Nov 18, 2022

View reviewed changes

jakearchibald added 2 commits November 21, 2022 13:07

Feedback from review

847a808

Use inset()

d3ac589

vmpstr approved these changes Nov 22, 2022

View reviewed changes

vmpstr merged commit f6bd34b into w3c:main Nov 22, 2022

jakearchibald deleted the ua-stylesheet branch November 22, 2022 15:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[css-view-transitions-1] Clean up use of dynamic styles #8093

[css-view-transitions-1] Clean up use of dynamic styles #8093

jakearchibald commented Nov 17, 2022

jakearchibald Nov 17, 2022

jakearchibald Nov 17, 2022

jakearchibald Nov 17, 2022

vmpstr Nov 17, 2022

jakearchibald Nov 17, 2022

jakearchibald Nov 17, 2022

jakearchibald Nov 17, 2022

jakearchibald Nov 17, 2022

jakearchibald Nov 17, 2022

jakearchibald Nov 17, 2022

jakearchibald Nov 17, 2022

jakearchibald Nov 17, 2022

jakearchibald Nov 17, 2022

vmpstr Nov 17, 2022

jakearchibald Nov 21, 2022

jakearchibald Nov 21, 2022

vmpstr Nov 17, 2022

vmpstr Nov 17, 2022

vmpstr Nov 18, 2022

jakearchibald Nov 21, 2022

vmpstr Nov 21, 2022

jakearchibald Nov 21, 2022

vmpstr Nov 18, 2022

vmpstr Nov 18, 2022

jakearchibald Nov 21, 2022 •

edited

Loading

vmpstr Nov 18, 2022

		if it has both ''::view-transition-new()'' and ''::view-transition-old()'' as children.

		Issue: This reads like a non-normative note, but it's normative. This rule should be specified at the same time as other styles, when the condition above can be determined.

		if the ancestor ''::view-transition-image-pair()'' has both ''::view-transition-new()'' and ''::view-transition-old()'' as descendants.

		Issue: This reads like a non-normative note, but it's normative. This rule should be specified at the same time as other styles, when the condition above can be determined.


		Note: The above code example contains variables to be replaced.

		Issue: Which of ''xywh()''/''rect()''/''inset()'' should we use?


		1. If neither of \|capturedElement\|'s [=captured element/old image=] or [=captured element/new element=] is null:

		1. Let \|transform\| be \|capturedElement\|'s [=old styles=]'s 'transform' property.

[css-view-transitions-1] Clean up use of dynamic styles #8093

[css-view-transitions-1] Clean up use of dynamic styles #8093

Conversation

jakearchibald commented Nov 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakearchibald Nov 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakearchibald Nov 21, 2022 •

edited

Loading