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

[css-logical] Logical properties should be animatable like the physical ones #2751

Closed
Loirooriol opened this issue Jun 9, 2018 · 21 comments
Closed

Comments

@Loirooriol
Copy link
Contributor

I don't see why this shouldn't be possible:

#element {
  animation: size 1s infinite alternate;
}
@keyframes size {
  from { inline-size: 100px }
  to { inline-size: 200px }
}

I expect it to animate just like if I used width. But the spec says

Animation type: discrete

On Chrome it's effectively discrete, on Firefox it doesn't animate at all. https://jsfiddle.net/45e6okrm/

@dbaron dbaron added css-animations-1 Current Work css-transitions-1 Current Work css-logical-1 Current Work labels Jun 9, 2018
@birtles
Copy link
Contributor

birtles commented Jun 11, 2018

From memory, we began implementing this in Firefox but never finished it. It's complicated by the fact that you can also animate the writing-mode etc. so that part-way through the animation the mapping for the properties changes, e.g.

#element {
  margin: 50px 100px;
  animation: yer 5s;
}
@keyframes yer {
  to { margin-inline-start: 200px; writing-mode: vertical-lr; }
}

As a result you can't just do the mapping to physical properties when you set up the animation. Instead you need to re-evaluate the mapping whenever the animated value of the writing-mode, direction etc. change.

@upsuper
Copy link
Member

upsuper commented Jun 11, 2018

It's complicated by the fact that you can also animate the writing-mode etc.

If that's where the difficulty is from, maybe we should put writing-mode and direction into the category of properties that is not animatable. I don't see how animating them can be useful, and I guess it makes sense to get rid of the animatability of them if that gets in the way of other (useful) stuff.

@birtles
Copy link
Contributor

birtles commented Jun 11, 2018

(I should clarify that responding to changes in the animated value is more difficult than responding to other changes in the computed value.

In order to avoid triggering transitions due to animations, UAs need to distinguish between non-animated style updates and animated style updates. In Gecko we do this using a separate style resolution step for animations and I suspect in order to do transitions properly with regards to inheritance, other UAs will need to do something similar, if they don't already. As a result, while non-animation related changes will get picked up naturally during the animation styling, changes due to animation itself need special handling.

We already have a similar situation with font-size and custom properties but those only affect the property values where as this affects the property name mappings.)

@fantasai
Copy link
Collaborator

@upsuper Currently writing-mode, text-orientation, and direction are all listed as not animatable. (By contrast, unicode-bidi is listed as discretely animatable.)

@fantasai
Copy link
Collaborator

@Loirooriol Bikeshed is auto-filling the Animatable line in the propdef tables. :( It should really stop doing that. If the spec left out the Animatable line, it should either leave it out of the output or post an error.

@fantasai
Copy link
Collaborator

fantasai commented Jun 11, 2018

Fixed the Animatable lines to match their corresponding physical properties (as was always intended) in bfba9a7

@birtles Does there need to be any further clarifications anywhere else?

@birtles
Copy link
Contributor

birtles commented Jun 14, 2018

Yes, we should update Writing Modes spec to say "Animation type: none" since "Animatable: no" is often taken to mean "Animation type: discrete" (after we resolved that properties should be animatable by default and only a very small subset of properties should be marked as explicitly not animatable).

@Loirooriol
Copy link
Contributor Author

Just as a reminder, display should also be updated: #72 (comment).

@css-meeting-bot
Copy link
Member

The Working Group just discussed animating writing-mode/direction, and agreed to the following:

  • RESOLVED: Fix unicode-bidi to be non-animatable, and make sure all propdefs are using the proper terminology.
The full IRC log of that discussion <TabAtkins> Topic: animating writing-mode/direction
<birtles> Github: https://github.com//issues/2751
<TabAtkins> birtles: Like françois says, animation is tricky because they share the same computed properties
<TabAtkins> birtles: and animations works on computed values, so you need to do mapping to computed proeprties befor eyou animate
<TabAtkins> birtles: This is different to font-size
<fantasai> Actually, writing-mode, direction, and text-orientation aren't currently animatable anyway
<TabAtkins> birtles: There the dependency is between property *values*, but here it's a dependency between properties themselves
<fantasai> https://www.w3.org/TR/css-writing-modes-3/#direction
<TabAtkins> birtles: In gecko, we distinguish between properties from animation, and changes from OM.
<fantasai> https://www.w3.org/TR/css-writing-modes-3/#propdef-writing-mode
<TabAtkins> birtles: Re-resolving that mapping in response to aniation change is harder than in response to OM change
<TabAtkins> birtles: So it does add significant complexity
<TabAtkins> birtles: I think if we say they're not we still need to update the mapping from OM, and that's some complexity, but much less.
<TabAtkins> florian: Any use-case for animating?
<fantasai> [tab tries to imagine a use case, other ppl say it's not very credible use case]
<fantasai> heycam: Why are direction and unicode-bidi excluded from all?
<fantasai> TabAtkins: They shouldn't have been in CSS in the first place, they're really content attributes
<TabAtkins> fantasai: They're already described as "animatable: no"
<TabAtkins> florian: It's unclear whether that's the old meaning of "no" (meaning "discrete"), or the new sense (meaning not animatable)
<TabAtkins> fantasai: Currently unicode-bidi says "discrete" and the rest say "no", which doesn' tmake a lot of sense
<TabAtkins> astearns: So the proposed resolution is to take direciton/writing-mode/unicode-bidi and make them not animatable
<TabAtkins> shane: This'll be confusing for authors if they do put them in keyframes
<TabAtkins> fantasai: Authors already shoudln't be using direction or unicode-bidi in stylesheets
<TabAtkins> shane: That's a good argument in general, then!
<TabAtkins> RESOLVED: Fix unicode-bidi to be non-animatable, and make sure all propdefs are using the proper terminology.
<TabAtkins> frremy: With this resolution, I"m much happier about the previous topic.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 6, 2018
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 6, 2018
Per w3c/csswg-drafts#2751

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1473779
gecko-commit: 8e815e6e442be7255528d81891cc70401e3f07ce
gecko-integration-branch: mozilla-inbound
gecko-reviewers: heycam
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 7, 2018
Per w3c/csswg-drafts#2751

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1473779
gecko-commit: 8e815e6e442be7255528d81891cc70401e3f07ce
gecko-integration-branch: mozilla-inbound
gecko-reviewers: heycam
emilio added a commit to emilio/servo that referenced this issue Jul 9, 2018
@majido
Copy link
Contributor

majido commented Jul 9, 2018

@emilio your FF patch only makes direction and writing-mode non-animatable.

But my reading of that csswg thread was that we needed to more properties non-animatable namely:
direction, text-orientation, unicode-bidi, writing-mode.

Since the specifications have not been updated yet can someone please clarify the exact change?

PS: I just found our about your patch since my Chromium patch conflicted with your wpt test changes. Yay for wpt despite more conflict 😉

@emilio
Copy link
Collaborator

emilio commented Jul 9, 2018

Heh, indeed, yay for WPT :-)

I updated those because those are the only properties that affect the logical -> physical property mapping. I did leave a TODO re. text-orientation, explaining the reasoning here:

mozilla/gecko-dev@c6c50bd#diff-2bcfd81b55b37cd491bddf83a0a71842R43

I think those two are the only ones needed for logical properties themselves, though I'm happy to change text-orientation and unicode-bidi as well. As the minutes say they're not really useful. But they didn't block my work in https://bugzilla.mozilla.org/show_bug.cgi?id=1309752 :)

@emilio
Copy link
Collaborator

emilio commented Jul 9, 2018

To be clear, the resolution says clearly that the four properties shouldn't be animatable, just two of them weren't blocking my immediate work :P

I'm happy to send a patch of the other two, though I guess I'll have the same problem as soon as your WPT changes land.

@majido I just filed https://bugzilla.mozilla.org/show_bug.cgi?id=1474317 to update the other two properties, I'm happy to change them as soon as your test changes land (or before if you want, I don't particularly mind :P).

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 9, 2018
…atable

Match the recent CSSWG resolutions:
w3c/csswg-drafts#2737 (comment)
w3c/csswg-drafts#2751 (comment)

Make following properties none animatable:

* contain
* direction
* display
* text-orientation
* unicode-bidi
* will-change
* writing-mode

Note that wpt tests have already been updated for direction and writing-mode
here [1]

[1] 41f4ab6

Bug: 860359
Change-Id: I3e7296e3c28ff494eddbc6f011621dd29ba7d2aa
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 9, 2018
…atable

Match the recent CSSWG resolutions:
w3c/csswg-drafts#2737 (comment)
w3c/csswg-drafts#2751 (comment)

Make following properties none animatable:

* contain
* direction
* display
* text-orientation
* unicode-bidi
* will-change
* writing-mode

Note that wpt tests have already been updated for direction and writing-mode
here [1]

[1] 41f4ab6

Bug: 860359
Change-Id: I3e7296e3c28ff494eddbc6f011621dd29ba7d2aa
@birtles
Copy link
Contributor

birtles commented Jul 9, 2018

I've been writing the Web Animations wpt and CSS Animations wpt (in progress) for this and I got stuck working out how to test that text-orientation affects the mapping.

In a couple of places the spec says,

The mapping depends on the element’s writing-mode, direction, and text-orientation.

but I can't find where the mapping involving text-orientation is specified.

@Loirooriol
Copy link
Contributor Author

I can't find where the mapping involving text-orientation is specified.

I was also wondering about this. https://drafts.csswg.org/css-writing-modes/#propdef-text-orientation says text-orientation: upright

causes the used value of direction to be ltr,

but according to CSS Logical the dependency is with the

element’s computed values of writing-mode, direction, and text-orientation

so the used value shouldn't matter? I tested and neither Firefox nor Chromium considered this to affect the resolution of logical properties when I specified direction: rtl.

@upsuper
Copy link
Member

upsuper commented Jul 9, 2018

Based on Abstract-to-Physical Mappings I don't think text-orientation affects any property mapping.

aarongable pushed a commit to chromium/chromium that referenced this issue Jul 9, 2018
…atable

Match the recent CSSWG resolutions:
w3c/csswg-drafts#2737 (comment)
w3c/csswg-drafts#2751 (comment)


Make following properties none animatable:

* contain
* direction
* display
* text-orientation
* unicode-bidi
* will-change
* writing-mode


Note that wpt tests have already been updated for direction and writing-mode
here [1]

[1] web-platform-tests/wpt@41f4ab6

Bug: 860359
Change-Id: I3e7296e3c28ff494eddbc6f011621dd29ba7d2aa
Reviewed-on: https://chromium-review.googlesource.com/1127062
Commit-Queue: Majid Valipour <majidvp@chromium.org>
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573509}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 9, 2018
…atable

Match the recent CSSWG resolutions:
w3c/csswg-drafts#2737 (comment)
w3c/csswg-drafts#2751 (comment)

Make following properties none animatable:

* contain
* direction
* display
* text-orientation
* unicode-bidi
* will-change
* writing-mode

Note that wpt tests have already been updated for direction and writing-mode
here [1]

[1] 41f4ab6

Bug: 860359
Change-Id: I3e7296e3c28ff494eddbc6f011621dd29ba7d2aa
Reviewed-on: https://chromium-review.googlesource.com/1127062
Commit-Queue: Majid Valipour <majidvp@chromium.org>
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573509}
bors-servo pushed a commit to servo/servo that referenced this issue Jul 16, 2018
Per w3c/csswg-drafts#2751

Bug: 1473779
Reviewed-by: heycam
MozReview-Commit-ID: GCG3vJWNPfC
ewilligers pushed a commit to ewilligers/csswg-drafts that referenced this issue Jul 19, 2018
Make it clear that display is not animatable (as opposed to supporting
discrete animation.) Clarification discussed for other properties in
w3c#2751 (comment)

Already tested in WPT
web-animations/interfaces/KeyframeEffect/processing-a-keyframes-argument-001.html
ewilligers pushed a commit to ewilligers/csswg-drafts that referenced this issue Jul 19, 2018
w3c#2751 (comment)
direction/writing-mode/unicode-bidi are not animatable
tabatkins pushed a commit that referenced this issue Jul 19, 2018
Make it clear that display is not animatable (as opposed to supporting
discrete animation.) Clarification discussed for other properties in
#2751 (comment)

Already tested in WPT
web-animations/interfaces/KeyframeEffect/processing-a-keyframes-argument-001.html
tabatkins pushed a commit that referenced this issue Jul 19, 2018
#2751 (comment)
direction/writing-mode/unicode-bidi are not animatable
@nigelmegitt
Copy link

For information, TTWG discussed yesterday how we change TTML2 to match this resolution and noted that in TTML there are two distinct concepts that we think are not present in CSS, i.e. discrete animation vs continuous animation.

Discrete animation means a step change in a property value at a defined point in time, i.e. analogous to "render this new document with these style properties now, in place of what was there before", with a syntactic construct to make that easier than actually providing a whole new chunk of content, which is the set element.

Continuous animation means a continually varying change in a property value along a curve with defined begin and end times and an interpolation calculation mode, defined using the animate element.

We decided to retain the option for discretely animating direction, writing-mode etc and interpreted the resolution made in this issue as relating only to continuous animation, which we already decided to remove from TTML2 for the relevant style attributes.

@Loirooriol
Copy link
Contributor Author

@nigelmegitt CSS differentiates between "Animation type: dicrete" and "Animation type: not animatable". See https://drafts.csswg.org/css-transitions/#animatable-types and https://drafts.csswg.org/css-transitions/#animatable-properties

The problem was that if you animate direction and writing-mode discretely, then you also need to update the logical-to-physical mapping halfway the animation, which adds complexity. So it was resolved that they don't animate at all.

@nigelmegitt
Copy link

Thanks @Loirooriol that's very useful. In the context of TTML, where timed updates to the physical output are the general desired outcome , the incremental increase in complexity caused by discretely animating properties that cause a new logical-to-physical mapping at a predictable time is minimal.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 20, 2018
…/writing modes props not animatable, a=testonly

Automatic update from web-platform-tests[web-animation] Make contain/will-change/writing modes props not animatable

Match the recent CSSWG resolutions:
w3c/csswg-drafts#2737 (comment)
w3c/csswg-drafts#2751 (comment)

Make following properties none animatable:

* contain
* direction
* display
* text-orientation
* unicode-bidi
* will-change
* writing-mode

Note that wpt tests have already been updated for direction and writing-mode
here [1]

[1] web-platform-tests/wpt@41f4ab6

Bug: 860359
Change-Id: I3e7296e3c28ff494eddbc6f011621dd29ba7d2aa
Reviewed-on: https://chromium-review.googlesource.com/1127062
Commit-Queue: Majid Valipour <majidvp@chromium.org>
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573509}

--

wpt-commits: 7f017e863fec0f00ab1eb531b1bc12864d850d06
wpt-pr: 11813
@ewilligers
Copy link
Contributor

All properties in the spec should clearly describe animation with "Animation type: not animatable" or "Animation type: discrete".

The minutes didn't explicitly mention text-orientation. I thought that was intended to also be "not animatable", but should it be "discrete"? Note that text-orientation and glyph-orientation-vertical should be consistent with each other.

text-combine-upright is the other property in the spec whose animation behavior should be described more clearly ("not animatable" or "discrete").

@birtles
Copy link
Contributor

birtles commented Jul 22, 2018

The minutes didn't explicitly mention text-orientation. I thought that was intended to also be "not animatable", but should it be "discrete"? Note that text-orientation and glyph-orientation-vertical should be consistent with each other.

See the earlier comments in this thread, but the reason for considering text-orientation not animatable is that the Logical Properties spec says, "The mapping depends on the element’s writing-mode, direction, and text-orientation." However, neither Xidorn or I could find how text-orientation affects the mapping. If it doesn't affect the mapping, then I think it would probably be ok to make it animatable.

jankeromnes pushed a commit to jankeromnes/gecko that referenced this issue Jul 23, 2018
…/writing modes props not animatable, a=testonly

Automatic update from web-platform-tests[web-animation] Make contain/will-change/writing modes props not animatable

Match the recent CSSWG resolutions:
w3c/csswg-drafts#2737 (comment)
w3c/csswg-drafts#2751 (comment)

Make following properties none animatable:

* contain
* direction
* display
* text-orientation
* unicode-bidi
* will-change
* writing-mode

Note that wpt tests have already been updated for direction and writing-mode
here [1]

[1] web-platform-tests/wpt@41f4ab6

Bug: 860359
Change-Id: I3e7296e3c28ff494eddbc6f011621dd29ba7d2aa
Reviewed-on: https://chromium-review.googlesource.com/1127062
Commit-Queue: Majid Valipour <majidvp@chromium.org>
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573509}

--

wpt-commits: 7f017e863fec0f00ab1eb531b1bc12864d850d06
wpt-pr: 11813
@fantasai
Copy link
Collaborator

@birtles It does affect the mapping: writing-mode: vertical-rl; direction: rtl; text-orientation: upright has a used direction of ltr. Added some clarifying notes in #691 (comment)

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 22, 2018
jankeromnes pushed a commit to jankeromnes/gecko that referenced this issue Aug 22, 2018
emilio added a commit to emilio/servo that referenced this issue Sep 3, 2018
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 3, 2019
Per w3c/csswg-drafts#2751

MozReview-Commit-ID: GCG3vJWNPfC

UltraBlame original commit: 8e815e6e442be7255528d81891cc70401e3f07ce
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 3, 2019
Per w3c/csswg-drafts#2751

MozReview-Commit-ID: GCG3vJWNPfC

UltraBlame original commit: 8e815e6e442be7255528d81891cc70401e3f07ce
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 3, 2019
…/writing modes props not animatable, a=testonly

Automatic update from web-platform-tests[web-animation] Make contain/will-change/writing modes props not animatable

Match the recent CSSWG resolutions:
w3c/csswg-drafts#2737 (comment)
w3c/csswg-drafts#2751 (comment)

Make following properties none animatable:

* contain
* direction
* display
* text-orientation
* unicode-bidi
* will-change
* writing-mode

Note that wpt tests have already been updated for direction and writing-mode
here [1]

[1] web-platform-tests/wpt@41f4ab6

Bug: 860359
Change-Id: I3e7296e3c28ff494eddbc6f011621dd29ba7d2aa
Reviewed-on: https://chromium-review.googlesource.com/1127062
Commit-Queue: Majid Valipour <majidvpchromium.org>
Reviewed-by: Eric Willigers <ericwilligerschromium.org>
Cr-Commit-Position: refs/heads/master{#573509}

--

wpt-commits: 7f017e863fec0f00ab1eb531b1bc12864d850d06
wpt-pr: 11813

UltraBlame original commit: 531f5352385b287a160bea990a478460c249e523
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
Per w3c/csswg-drafts#2751

MozReview-Commit-ID: GCG3vJWNPfC

UltraBlame original commit: 8e815e6e442be7255528d81891cc70401e3f07ce
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 3, 2019
…/writing modes props not animatable, a=testonly

Automatic update from web-platform-tests[web-animation] Make contain/will-change/writing modes props not animatable

Match the recent CSSWG resolutions:
w3c/csswg-drafts#2737 (comment)
w3c/csswg-drafts#2751 (comment)

Make following properties none animatable:

* contain
* direction
* display
* text-orientation
* unicode-bidi
* will-change
* writing-mode

Note that wpt tests have already been updated for direction and writing-mode
here [1]

[1] web-platform-tests/wpt@41f4ab6

Bug: 860359
Change-Id: I3e7296e3c28ff494eddbc6f011621dd29ba7d2aa
Reviewed-on: https://chromium-review.googlesource.com/1127062
Commit-Queue: Majid Valipour <majidvpchromium.org>
Reviewed-by: Eric Willigers <ericwilligerschromium.org>
Cr-Commit-Position: refs/heads/master{#573509}

--

wpt-commits: 7f017e863fec0f00ab1eb531b1bc12864d850d06
wpt-pr: 11813

UltraBlame original commit: 531f5352385b287a160bea990a478460c249e523
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
…/writing modes props not animatable, a=testonly

Automatic update from web-platform-tests[web-animation] Make contain/will-change/writing modes props not animatable

Match the recent CSSWG resolutions:
w3c/csswg-drafts#2737 (comment)
w3c/csswg-drafts#2751 (comment)

Make following properties none animatable:

* contain
* direction
* display
* text-orientation
* unicode-bidi
* will-change
* writing-mode

Note that wpt tests have already been updated for direction and writing-mode
here [1]

[1] web-platform-tests/wpt@41f4ab6

Bug: 860359
Change-Id: I3e7296e3c28ff494eddbc6f011621dd29ba7d2aa
Reviewed-on: https://chromium-review.googlesource.com/1127062
Commit-Queue: Majid Valipour <majidvpchromium.org>
Reviewed-by: Eric Willigers <ericwilligerschromium.org>
Cr-Commit-Position: refs/heads/master{#573509}

--

wpt-commits: 7f017e863fec0f00ab1eb531b1bc12864d850d06
wpt-pr: 11813

UltraBlame original commit: 531f5352385b287a160bea990a478460c249e523
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 3, 2019
…ange non-animatable. r=heycam

Per recent CSSWG resolutions:

  w3c/csswg-drafts#2737
  w3c/csswg-drafts#2751

Differential Revision: https://phabricator.services.mozilla.com/D3888

UltraBlame original commit: 4bb2acbf4eb0d386046945e52bdd546fc41ae8e5
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 3, 2019
…ange non-animatable. r=heycam

Per recent CSSWG resolutions:

  w3c/csswg-drafts#2737
  w3c/csswg-drafts#2751

Differential Revision: https://phabricator.services.mozilla.com/D3888

UltraBlame original commit: 4bb2acbf4eb0d386046945e52bdd546fc41ae8e5
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
…ange non-animatable. r=heycam

Per recent CSSWG resolutions:

  w3c/csswg-drafts#2737
  w3c/csswg-drafts#2751

Differential Revision: https://phabricator.services.mozilla.com/D3888

UltraBlame original commit: 4bb2acbf4eb0d386046945e52bdd546fc41ae8e5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants