Skip to content

Commit

Permalink
[scroll-animations] Rename vertical/horizontal => y/x
Browse files Browse the repository at this point in the history
Fixed: 1440971
Change-Id: I83fc60031eb1725b195676e73141879d1cebccf3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4547320
Reviewed-by: Kevin Ellis <kevers@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1146573}
  • Loading branch information
andruud authored and Chromium LUCI CQ committed May 19, 2023
1 parent 5aa87bb commit 9ebed4a
Show file tree
Hide file tree
Showing 34 changed files with 226 additions and 239 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -897,10 +897,10 @@ ScrollTimeline::ScrollAxis ComputeAxis(TimelineAxis axis) {
return ScrollTimeline::ScrollAxis::kBlock;
case TimelineAxis::kInline:
return ScrollTimeline::ScrollAxis::kInline;
case TimelineAxis::kVertical:
return ScrollTimeline::ScrollAxis::kVertical;
case TimelineAxis::kHorizontal:
return ScrollTimeline::ScrollAxis::kHorizontal;
case TimelineAxis::kX:
return ScrollTimeline::ScrollAxis::kX;
case TimelineAxis::kY:
return ScrollTimeline::ScrollAxis::kY;
}

NOTREACHED();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ TEST_F(CSSScrollTimelineTest, ViewTimelineHost) {
animation-timeline: timeline;
}
.scroller > div {
view-timeline: timeline horizontal;
view-timeline: timeline x;
}
</style>
<div class=scroller>
Expand All @@ -260,7 +260,7 @@ TEST_F(CSSScrollTimelineTest, ViewTimelineHost) {
<template shadowroot=open>
<style>
:host {
view-timeline: timeline vertical;
view-timeline: timeline y;
}
</style>
</template>
Expand All @@ -273,8 +273,7 @@ TEST_F(CSSScrollTimelineTest, ViewTimelineHost) {
ASSERT_TRUE(target);
HeapVector<Member<Animation>> animations = target->getAnimations();
ASSERT_EQ(1u, animations.size());
ASSERT_EQ(ScrollTimeline::ScrollAxis::kHorizontal,
GetTimelineAxis(*animations[0]))
ASSERT_EQ(ScrollTimeline::ScrollAxis::kX, GetTimelineAxis(*animations[0]))
<< "Outer animation can not see view timeline defined by :host";
}

Expand All @@ -293,15 +292,15 @@ TEST_F(CSSScrollTimelineTest, ViewTimelineSlotted) {
animation-timeline: timeline;
}
.host {
view-timeline: timeline horizontal;
view-timeline: timeline x;
}
</style>
<div class=scroller>
<div class=host>
<template shadowroot=open>
<style>
::slotted(.target) {
view-timeline: timeline vertical;
view-timeline: timeline y;
}
</style>
<slot></slot>
Expand All @@ -315,8 +314,7 @@ TEST_F(CSSScrollTimelineTest, ViewTimelineSlotted) {
ASSERT_TRUE(target);
HeapVector<Member<Animation>> animations = target->getAnimations();
ASSERT_EQ(1u, animations.size());
ASSERT_EQ(ScrollTimeline::ScrollAxis::kHorizontal,
GetTimelineAxis(*animations[0]))
ASSERT_EQ(ScrollTimeline::ScrollAxis::kX, GetTimelineAxis(*animations[0]))
<< "Outer animation can not see view timeline defined by ::slotted";
}

Expand All @@ -327,10 +325,10 @@ TEST_F(CSSScrollTimelineTest, ViewTimelinePart) {
->setInnerHTMLWithDeclarativeShadowDOMForTesting(R"HTML(
<style>
.host {
view-timeline: timeline vertical;
view-timeline: timeline y;
}
.host::part(foo) {
view-timeline: timeline horizontal;
view-timeline: timeline x;
}
</style>
<div class=host>
Expand Down Expand Up @@ -361,8 +359,7 @@ TEST_F(CSSScrollTimelineTest, ViewTimelinePart) {
ASSERT_TRUE(target);
HeapVector<Member<Animation>> animations = target->getAnimations();
ASSERT_EQ(1u, animations.size());
ASSERT_EQ(ScrollTimeline::ScrollAxis::kHorizontal,
GetTimelineAxis(*animations[0]))
ASSERT_EQ(ScrollTimeline::ScrollAxis::kX, GetTimelineAxis(*animations[0]))
<< "Inner animation can see view timeline defined by ::part";
}

Expand All @@ -381,7 +378,7 @@ TEST_F(CSSScrollTimelineTest, ScrollTimelineHost) {
animation-timeline: timeline;
}
main > .scroller {
scroll-timeline: timeline horizontal;
scroll-timeline: timeline x;
}
</style>
<main>
Expand All @@ -390,7 +387,7 @@ TEST_F(CSSScrollTimelineTest, ScrollTimelineHost) {
<template shadowroot=open>
<style>
:host {
scroll-timeline: timeline vertical;
scroll-timeline: timeline y;
}
</style>
<slot></slot>
Expand All @@ -405,8 +402,7 @@ TEST_F(CSSScrollTimelineTest, ScrollTimelineHost) {
ASSERT_TRUE(target);
HeapVector<Member<Animation>> animations = target->getAnimations();
ASSERT_EQ(1u, animations.size());
ASSERT_EQ(ScrollTimeline::ScrollAxis::kHorizontal,
GetTimelineAxis(*animations[0]))
ASSERT_EQ(ScrollTimeline::ScrollAxis::kX, GetTimelineAxis(*animations[0]))
<< "Outer animation can not see scroll timeline defined by :host";
}

Expand All @@ -425,14 +421,14 @@ TEST_F(CSSScrollTimelineTest, ScrollTimelineSlotted) {
animation-timeline: timeline;
}
.host {
scroll-timeline: timeline horizontal;
scroll-timeline: timeline x;
}
</style>
<div class=host>
<template shadowroot=open>
<style>
::slotted(.scroller) {
scroll-timeline: timeline vertical;
scroll-timeline: timeline y;
}
</style>
<slot></slot>
Expand All @@ -447,8 +443,7 @@ TEST_F(CSSScrollTimelineTest, ScrollTimelineSlotted) {
ASSERT_TRUE(target);
HeapVector<Member<Animation>> animations = target->getAnimations();
ASSERT_EQ(1u, animations.size());
ASSERT_EQ(ScrollTimeline::ScrollAxis::kHorizontal,
GetTimelineAxis(*animations[0]))
ASSERT_EQ(ScrollTimeline::ScrollAxis::kX, GetTimelineAxis(*animations[0]))
<< "Outer animation can not see scroll timeline defined by ::slotted";
}

Expand All @@ -459,10 +454,10 @@ TEST_F(CSSScrollTimelineTest, ScrollTimelinePart) {
->setInnerHTMLWithDeclarativeShadowDOMForTesting(R"HTML(
<style>
.host {
scroll-timeline: timeline vertical;
scroll-timeline: timeline y;
}
.host::part(foo) {
scroll-timeline: timeline horizontal;
scroll-timeline: timeline x;
}
</style>
<div class=host>
Expand Down Expand Up @@ -493,8 +488,7 @@ TEST_F(CSSScrollTimelineTest, ScrollTimelinePart) {
ASSERT_TRUE(target);
HeapVector<Member<Animation>> animations = target->getAnimations();
ASSERT_EQ(1u, animations.size());
ASSERT_EQ(ScrollTimeline::ScrollAxis::kHorizontal,
GetTimelineAxis(*animations[0]))
ASSERT_EQ(ScrollTimeline::ScrollAxis::kX, GetTimelineAxis(*animations[0]))
<< "Inner animation can see scroll timeline defined by ::part";
}

Expand Down
4 changes: 2 additions & 2 deletions third_party/blink/renderer/core/animation/scroll_timeline.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ ScrollOrientation ToPhysicalScrollOrientation(ScrollAxis axis,
return is_horizontal ? kVerticalScroll : kHorizontalScroll;
case ScrollAxis::kInline:
return is_horizontal ? kHorizontalScroll : kVerticalScroll;
case ScrollAxis::kHorizontal:
case ScrollAxis::kX:
return kHorizontalScroll;
case ScrollAxis::kVertical:
case ScrollAxis::kY:
return kVerticalScroll;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

enum ScrollAxis { "block", "inline", "horizontal", "vertical" };
enum ScrollAxis { "block", "inline", "x", "y" };

// https://wicg.github.io/scroll-animations/#scrolltimeline-interface
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class TestScrollTimeline : public ScrollTimeline {
TimelineAttachment::kLocal,
ScrollTimeline::ReferenceType::kSource,
source,
ScrollAxis::kVertical) {
ScrollAxis::kY) {
if (snapshot) {
UpdateSnapshot();
}
Expand Down Expand Up @@ -116,7 +116,7 @@ class TestViewTimeline : public ViewTimeline {
: ViewTimeline(document,
TimelineAttachment::kLocal,
subject,
ScrollAxis::kVertical,
ScrollAxis::kY,
TimelineInset()) {
if (snapshot) {
UpdateSnapshot();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,12 @@ CompositorScrollTimeline::ScrollDirection ConvertOrientation(
ScrollAxis axis,
const ComputedStyle* style) {
// Easy cases; physical is always physical.
if (axis == ScrollAxis::kHorizontal)
if (axis == ScrollAxis::kX) {
return CompositorScrollTimeline::ScrollRight;
if (axis == ScrollAxis::kVertical)
}
if (axis == ScrollAxis::kY) {
return CompositorScrollTimeline::ScrollDown;
}

// Harder cases; first work out which axis is which, and then for each check
// which edge we start at.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,9 @@ TEST_F(ScrollTimelineUtilTest, ConvertOrientationPhysicalCases) {
style_builder.SetWritingMode(writing_mode);
style_builder.SetDirection(direction);
scoped_refptr<const ComputedStyle> style = style_builder.TakeStyle();
EXPECT_EQ(ConvertOrientation(ScrollTimeline::ScrollAxis::kVertical,
style.get()),
EXPECT_EQ(ConvertOrientation(ScrollTimeline::ScrollAxis::kY, style.get()),
CompositorScrollTimeline::ScrollDown);
EXPECT_EQ(ConvertOrientation(ScrollTimeline::ScrollAxis::kHorizontal,
style.get()),
EXPECT_EQ(ConvertOrientation(ScrollTimeline::ScrollAxis::kX, style.get()),
CompositorScrollTimeline::ScrollRight);
}
}
Expand Down Expand Up @@ -197,11 +195,10 @@ TEST_F(ScrollTimelineUtilTest, ConvertOrientationLogical) {
TEST_F(ScrollTimelineUtilTest, ConvertOrientationNullStyle) {
// When the style is nullptr we assume horizontal-tb and ltr direction. This
// means that block is ScrollDown and inline is ScrollRight
EXPECT_EQ(ConvertOrientation(ScrollTimeline::ScrollAxis::kVertical, nullptr),
EXPECT_EQ(ConvertOrientation(ScrollTimeline::ScrollAxis::kY, nullptr),
CompositorScrollTimeline::ScrollDown);
EXPECT_EQ(
ConvertOrientation(ScrollTimeline::ScrollAxis::kHorizontal, nullptr),
CompositorScrollTimeline::ScrollRight);
EXPECT_EQ(ConvertOrientation(ScrollTimeline::ScrollAxis::kX, nullptr),
CompositorScrollTimeline::ScrollRight);
EXPECT_EQ(ConvertOrientation(ScrollTimeline::ScrollAxis::kBlock, nullptr),
CompositorScrollTimeline::ScrollDown);
EXPECT_EQ(ConvertOrientation(ScrollTimeline::ScrollAxis::kInline, nullptr),
Expand Down
4 changes: 2 additions & 2 deletions third_party/blink/renderer/core/animation/view_timeline.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ bool IsBlockDirection(ViewTimeline::ScrollAxis axis, WritingMode writing_mode) {
return true;
case ViewTimeline::ScrollAxis::kInline:
return false;
case ViewTimeline::ScrollAxis::kHorizontal:
case ViewTimeline::ScrollAxis::kX:
return !blink::IsHorizontalWritingMode(writing_mode);
case ViewTimeline::ScrollAxis::kVertical:
case ViewTimeline::ScrollAxis::kY:
return blink::IsHorizontalWritingMode(writing_mode);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1772,11 +1772,11 @@ inline CSSIdentifierValue::CSSIdentifierValue(TimelineAxis axis)
case TimelineAxis::kInline:
value_id_ = CSSValueID::kInline;
break;
case TimelineAxis::kVertical:
value_id_ = CSSValueID::kVertical;
case TimelineAxis::kX:
value_id_ = CSSValueID::kX;
break;
case TimelineAxis::kHorizontal:
value_id_ = CSSValueID::kHorizontal;
case TimelineAxis::kY:
value_id_ = CSSValueID::kY;
break;
}
}
Expand All @@ -1788,10 +1788,10 @@ inline TimelineAxis CSSIdentifierValue::ConvertTo() const {
return TimelineAxis::kBlock;
case CSSValueID::kInline:
return TimelineAxis::kInline;
case CSSValueID::kVertical:
return TimelineAxis::kVertical;
case CSSValueID::kHorizontal:
return TimelineAxis::kHorizontal;
case CSSValueID::kX:
return TimelineAxis::kX;
case CSSValueID::kY:
return TimelineAxis::kY;
default:
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4226,8 +4226,7 @@ CSSValue* ConsumeScrollFunction(CSSParserTokenRange& range,
}
if (!axis) {
if ((axis = ConsumeIdent<CSSValueID::kBlock, CSSValueID::kInline,
CSSValueID::kVertical, CSSValueID::kHorizontal>(
block))) {
CSSValueID::kX, CSSValueID::kY>(block))) {
continue;
}
}
Expand Down Expand Up @@ -4264,8 +4263,7 @@ CSSValue* ConsumeViewFunction(CSSParserTokenRange& range,
}
if (!axis) {
if ((axis = ConsumeIdent<CSSValueID::kBlock, CSSValueID::kInline,
CSSValueID::kVertical, CSSValueID::kHorizontal>(
block))) {
CSSValueID::kX, CSSValueID::kY>(block))) {
continue;
}
}
Expand Down Expand Up @@ -4456,8 +4454,8 @@ CSSValue* ConsumeSingleTimelineAttachment(CSSParserTokenRange& range) {
}

CSSValue* ConsumeSingleTimelineAxis(CSSParserTokenRange& range) {
return ConsumeIdent<CSSValueID::kBlock, CSSValueID::kInline,
CSSValueID::kVertical, CSSValueID::kHorizontal>(range);
return ConsumeIdent<CSSValueID::kBlock, CSSValueID::kInline, CSSValueID::kX,
CSSValueID::kY>(range);
}

CSSValue* ConsumeSingleTimelineName(CSSParserTokenRange& range,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ enum class ViewportUnitFlag {
kDynamic = 0x2,
};

enum class TimelineAxis { kBlock, kInline, kVertical, kHorizontal };
enum class TimelineAxis { kBlock, kInline, kX, kY };
enum class TimelineAttachment {
// The timeline is not attached to another timeline, and other timelines
// can not be attached to this timeline.
Expand Down
12 changes: 4 additions & 8 deletions third_party/blink/renderer/core/style/computed_style_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1846,10 +1846,8 @@ TEST_F(ComputedStyleTest, ScrollTimelineAxisNoDiff) {
ComputedStyleBuilder builder1(*InitialComputedStyle());
ComputedStyleBuilder builder2(*InitialComputedStyle());

builder1.SetScrollTimelineAxis(
Vector<TimelineAxis>(1u, TimelineAxis::kVertical));
builder2.SetScrollTimelineAxis(
Vector<TimelineAxis>(1u, TimelineAxis::kVertical));
builder1.SetScrollTimelineAxis(Vector<TimelineAxis>(1u, TimelineAxis::kY));
builder2.SetScrollTimelineAxis(Vector<TimelineAxis>(1u, TimelineAxis::kY));

scoped_refptr<const ComputedStyle> style1 = builder1.TakeStyle();
scoped_refptr<const ComputedStyle> style2 = builder2.TakeStyle();
Expand Down Expand Up @@ -1898,10 +1896,8 @@ TEST_F(ComputedStyleTest, ViewTimelineAxisNoDiff) {
ComputedStyleBuilder builder1(*InitialComputedStyle());
ComputedStyleBuilder builder2(*InitialComputedStyle());

builder1.SetViewTimelineAxis(
Vector<TimelineAxis>(1u, TimelineAxis::kVertical));
builder2.SetViewTimelineAxis(
Vector<TimelineAxis>(1u, TimelineAxis::kVertical));
builder1.SetViewTimelineAxis(Vector<TimelineAxis>(1u, TimelineAxis::kY));
builder2.SetViewTimelineAxis(Vector<TimelineAxis>(1u, TimelineAxis::kY));

scoped_refptr<const ComputedStyle> style1 = builder1.TakeStyle();
scoped_refptr<const ComputedStyle> style2 = builder2.TakeStyle();
Expand Down

0 comments on commit 9ebed4a

Please sign in to comment.