Commit e76547af authored by Sandra Sun's avatar Sandra Sun Committed by Commit Bot

scroll-snap-align should specify inline and block.

Currently, we parse the two values in scroll-snap-align for x and y.
However, according to the spec,
https://drafts.csswg.org/css-scroll-snap-1/#scroll-snap-align, the two
values specify the alignment in the inline axis and block axis. This
patch renames the two values and handles the vertical writing mode for
inline and block alignments.

Bug: 821645
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I78acff0df4698b3f9c0f7443e61e3bad75379b14
Reviewed-on: https://chromium-review.googlesource.com/967890
Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Reviewed-by: default avatarAli Juma <ajuma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544055}
parent a1608bb0
......@@ -62,24 +62,26 @@ struct ScrollSnapType {
struct ScrollSnapAlign {
ScrollSnapAlign()
: alignmentX(SnapAlignment::kNone), alignmentY(SnapAlignment::kNone) {}
: alignment_inline(SnapAlignment::kNone),
alignment_block(SnapAlignment::kNone) {}
explicit ScrollSnapAlign(SnapAlignment alignment)
: alignmentX(alignment), alignmentY(alignment) {}
: alignment_inline(alignment), alignment_block(alignment) {}
ScrollSnapAlign(SnapAlignment x, SnapAlignment y)
: alignmentX(x), alignmentY(y) {}
ScrollSnapAlign(SnapAlignment i, SnapAlignment b)
: alignment_inline(i), alignment_block(b) {}
bool operator==(const ScrollSnapAlign& other) const {
return alignmentX == other.alignmentX && alignmentY == other.alignmentY;
return alignment_inline == other.alignment_inline &&
alignment_block == other.alignment_block;
}
bool operator!=(const ScrollSnapAlign& other) const {
return !(*this == other);
}
SnapAlignment alignmentX;
SnapAlignment alignmentY;
SnapAlignment alignment_inline;
SnapAlignment alignment_block;
};
// Snap area is a bounding box that could be snapped to when a scroll happens in
......
<!DOCTYPE html>
<link rel="help" href="https://drafts.csswg.org/css-scroll-snap-1" />
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>
div {
position: absolute;
margin: 0px;
}
#scroller {
width: 400px;
height: 350px;
overflow: scroll;
scroll-snap-type: both mandatory;
}
#space {
width: 1000px;
height: 1000px;
}
#target {
width: 200px;
height: 200px;
left: 300px;
top: 300px;
scroll-snap-align: start end;
}
</style>
<div id="scroller">
<div id="space"></div>
<div id="target"></div>
</div>
<script>
var scroller = document.getElementById("scroller");
var width = scroller.clientWidth;
var height = scroller.clientHeight;
[
["horizontal-tb", 300, 500 - height],
["vertical-lr", 500 - width, 300],
["vertical-rl", 300, 300]
].forEach(([writing_mode, left, top]) => {
test(() => {
scroller.style.writingMode = writing_mode;
if (writing_mode == "vertical-rl")
document.getElementById("target").style.left = (width - 700) + "px";
scroller.scrollTo(0, 0);
assert_equals(scroller.scrollLeft, left, "aligns correctly on x");
assert_equals(scroller.scrollTop, top, "aligns correctly on y");
}, "Snaps correctly for " + writing_mode +
" writing mode with 'inline' and 'block' alignments");
})
</script>
\ No newline at end of file
......@@ -2023,8 +2023,9 @@ CSSValue* ComputedStyleUtils::ValueForScrollSnapType(
CSSValue* ComputedStyleUtils::ValueForScrollSnapAlign(
const ScrollSnapAlign& align,
const ComputedStyle& style) {
return CSSValuePair::Create(CSSIdentifierValue::Create(align.alignmentX),
CSSIdentifierValue::Create(align.alignmentY),
return CSSValuePair::Create(
CSSIdentifierValue::Create(align.alignment_inline),
CSSIdentifierValue::Create(align.alignment_block),
CSSValuePair::kDropIdenticalValues);
}
......
......@@ -1523,14 +1523,14 @@ ScrollSnapAlign StyleBuilderConverter::ConvertSnapAlign(StyleResolverState&,
ComputedStyleInitialValues::InitialScrollSnapAlign();
if (value.IsValuePair()) {
const CSSValuePair& pair = ToCSSValuePair(value);
snapAlign.alignmentX =
snapAlign.alignment_inline =
ToCSSIdentifierValue(pair.First()).ConvertTo<SnapAlignment>();
snapAlign.alignmentY =
snapAlign.alignment_block =
ToCSSIdentifierValue(pair.Second()).ConvertTo<SnapAlignment>();
} else {
snapAlign.alignmentX =
snapAlign.alignment_inline =
ToCSSIdentifierValue(value).ConvertTo<SnapAlignment>();
snapAlign.alignmentY = snapAlign.alignmentX;
snapAlign.alignment_block = snapAlign.alignment_inline;
}
return snapAlign;
}
......
......@@ -47,8 +47,8 @@ static LayoutBox* FindSnapContainer(const LayoutBox& snap_area) {
void SnapCoordinator::SnapAreaDidChange(LayoutBox& snap_area,
ScrollSnapAlign scroll_snap_align) {
LayoutBox* old_container = snap_area.SnapContainer();
if (scroll_snap_align.alignmentX == SnapAlignment::kNone &&
scroll_snap_align.alignmentY == SnapAlignment::kNone) {
if (scroll_snap_align.alignment_inline == SnapAlignment::kNone &&
scroll_snap_align.alignment_block == SnapAlignment::kNone) {
snap_area.SetSnapContainer(nullptr);
if (old_container)
UpdateSnapContainerData(*old_container);
......@@ -82,8 +82,6 @@ static ScrollableArea* ScrollableAreaForSnapping(const LayoutBox& layout_box) {
: layout_box.GetScrollableArea();
}
// TODO(sunyunjia): Needs to add layout test for vertical writing mode.
// https://crbug.com/821645
static ScrollSnapType GetPhysicalSnapType(const LayoutBox& snap_container) {
ScrollSnapType scroll_snap_type = snap_container.Style()->GetScrollSnapType();
if (scroll_snap_type.axis == SnapAxis::kInline) {
......@@ -274,11 +272,18 @@ static ScrollSnapAlign GetPhysicalAlignment(
const ComputedStyle& area_style,
const ComputedStyle& container_style) {
ScrollSnapAlign align = area_style.GetScrollSnapAlign();
if (container_style.IsHorizontalWritingMode())
return align;
SnapAlignment tmp = align.alignment_inline;
align.alignment_inline = align.alignment_block;
align.alignment_block = tmp;
if (container_style.IsFlippedBlocksWritingMode()) {
if (align.alignmentX == SnapAlignment::kStart) {
align.alignmentX = SnapAlignment::kEnd;
} else if (align.alignmentX == SnapAlignment::kEnd) {
align.alignmentX = SnapAlignment::kStart;
if (align.alignment_inline == SnapAlignment::kStart) {
align.alignment_inline = SnapAlignment::kEnd;
} else if (align.alignment_inline == SnapAlignment::kEnd) {
align.alignment_inline = SnapAlignment::kStart;
}
}
return align;
......@@ -294,12 +299,12 @@ static FloatRect GetVisibleRegion(const LayoutRect& container,
}
static SnapAxis ToSnapAxis(ScrollSnapAlign align) {
if (align.alignmentX != SnapAlignment::kNone &&
align.alignmentY != SnapAlignment::kNone)
if (align.alignment_inline != SnapAlignment::kNone &&
align.alignment_block != SnapAlignment::kNone)
return SnapAxis::kBoth;
if (align.alignmentX != SnapAlignment::kNone &&
align.alignmentY == SnapAlignment::kNone)
if (align.alignment_inline != SnapAlignment::kNone &&
align.alignment_block == SnapAlignment::kNone)
return SnapAxis::kX;
return SnapAxis::kY;
......@@ -373,9 +378,9 @@ SnapAreaData SnapCoordinator::CalculateSnapAreaData(
ScrollSnapAlign align = GetPhysicalAlignment(*area_style, *container_style);
snap_area_data.snap_position.set_x(CalculateSnapPosition(
align.alignmentX, SearchAxis::kX, container, max_position, area));
align.alignment_inline, SearchAxis::kX, container, max_position, area));
snap_area_data.snap_position.set_y(CalculateSnapPosition(
align.alignmentY, SearchAxis::kY, container, max_position, area));
align.alignment_block, SearchAxis::kY, container, max_position, area));
snap_area_data.snap_axis = ToSnapAxis(align);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment