Page MenuHomePhabricator

Bug 1808410 - Part 2: Add animation-timeline: view() in style system.
ClosedPublic

Authored by boris on Mar 28 2023, 11:18 PM.
Referenced Files
Unknown Object (File)
Sat, Nov 1, 8:59 PM
Unknown Object (File)
Mon, Oct 13, 4:07 AM
Unknown Object (File)
Oct 11 2025, 11:30 AM
Unknown Object (File)
Jun 18 2025, 8:25 PM
Unknown Object (File)
Jun 16 2025, 4:48 AM
Unknown Object (File)
May 26 2025, 5:20 PM
Unknown Object (File)
May 24 2025, 2:14 AM
Unknown Object (File)
May 22 2025, 7:51 AM
Subscribers

Details

Summary

Support view() notation for animation-timeline:

`<view()> = view( [ <axis> || <'view-timeline-inset'> ]? )`

We move AnimationTimeline and its related types into the generics folder,
and define two new structs for scroll() and view().

Note:

  1. The syntax of scroll() doesn't match the current version of the spec. I will update it in Bug 1814444.
  2. We will handle the creation/usage of the Anonymous View Progress Timelines in the next patch.

Diff Detail

Event Timeline

phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".
phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".
phab-bot removed a project: secure-revision.

Fix clang-format warnings

emilio requested changes to this revision.Apr 20 2023, 12:09 PM
emilio added inline comments.
servo/components/style/values/generics/ui.rs
223 ↗(On Diff #706201)

This is not generic, maybe should remain in specified?

249 ↗(On Diff #706201)

We generally don't put the bounds here and put them on the specific implementations, is there any reason that doesn't work?

279 ↗(On Diff #706201)

Same comment here.

299 ↗(On Diff #706201)

That way it's not needed here.

338 ↗(On Diff #706201)

Random change?

servo/components/style/values/specified/ui.rs
405 ↗(On Diff #706201)

Maybe just parse_arguments?

488 ↗(On Diff #706201)

Do you really need the turbofish?

This revision now requires changes to proceed.Apr 20 2023, 12:09 PM
boris added inline comments.
servo/components/style/values/generics/ui.rs
249 ↗(On Diff #706201)

Because I use is_default() for ToCss, and is_default() requires these bounds. Perhaps a better way is to implement ToCss manually, and add #[css(field_bound)] to AnimationTimeline because these bounds are only for ToCss.

servo/components/style/values/specified/ui.rs
488 ↗(On Diff #706201)

After removing the bounds from the generic type, we can remove ::<LengthPercentage>.

boris marked 2 inline comments as done.

Addressed comments

emilio requested changes to this revision.Apr 22 2023, 10:14 PM
emilio added inline comments.
servo/components/style/values/generics/ui.rs
249 ↗(On Diff #706201)

I don't know why you'd need the ToCss bound in any case (is_default doesn't require it does it?)

What about instead of an over-generic is_default we do something like adding GenericViewTimelineInset::is_auto, and use it there instead? Seems like that should work?

This revision now requires changes to proceed.Apr 22 2023, 10:14 PM
servo/components/style/values/generics/ui.rs
249 ↗(On Diff #706201)

Sorry. Right. It works. :)

emilio added a project: testing-approved.
emilio removed reviewers: layout-reviewers, Restricted Project.
This revision is now accepted and ready to land.May 1 2023, 1:00 PM