Skip to content

Commit

Permalink
ScrollTimeline: make endScrollOffset inclusive only when max-scroll-p…
Browse files Browse the repository at this point in the history
…osition

Before this CL, we always treated endScrollOffset as inclusive, against
the spec. The spec was recently updated to treat endScrollOffset as
inclusive but only when it is equal to max-offset:
WICG/scroll-animations#37

Bug: 934989
Change-Id: I7cd9cf4619e804a54cef33ef8d3ec7395166bace
Reviewed-on: https://chromium-review.googlesource.com/c/1483682
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636673}
  • Loading branch information
stephenmcgruer authored and chromium-wpt-export-bot committed Mar 1, 2019
1 parent d0d5bc3 commit 480c82d
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 18 deletions.
70 changes: 61 additions & 9 deletions scroll-animations/current-time-writing-modes.html
Expand Up @@ -300,9 +300,7 @@
'Length-based timeline after the endScrollOffset point');
scroller.scrollLeft = 20;
assert_equals(
lengthScrollTimeline.currentTime,
calculateCurrentTime(
scrollerSize - 20, 0, scrollerSize - 20, scrollerSize),
lengthScrollTimeline.currentTime, null,
'Length-based timeline at the endScrollOffset point');
scroller.scrollLeft = 50;
assert_equals(
Expand All @@ -318,9 +316,7 @@
'Percentage-based timeline after the endScrollOffset point');
scroller.scrollLeft = 0.20 * scrollerSize;
assert_equals(
percentageScrollTimeline.currentTime,
calculateCurrentTime(
0.8 * scrollerSize, 0, 0.8 * scrollerSize, scrollerSize),
percentageScrollTimeline.currentTime, null,
'Percentage-based timeline at the endScrollOffset point');
scroller.scrollLeft = 0.4 * scrollerSize;
assert_equals(
Expand All @@ -336,9 +332,7 @@
'Calc-based timeline after the endScrollOffset point');
scroller.scrollLeft = 0.2 * scrollerSize - 5;
assert_equals(
calcScrollTimeline.currentTime,
calculateCurrentTime(
0.8 * scrollerSize + 5, 0, 0.8 * scrollerSize + 5, scrollerSize),
calcScrollTimeline.currentTime, null,
'Calc-based timeline at the endScrollOffset point');
scroller.scrollLeft = 0.2 * scrollerSize;
assert_equals(
Expand All @@ -347,4 +341,62 @@
0.8 * scrollerSize, 0, 0.8 * scrollerSize + 5, scrollerSize),
'Calc-based timeline before the endScrollOffset point');
}, 'currentTime handles endScrollOffset with direction: rtl correctly');

test(function() {
const scrollerOverrides = new Map([['direction', 'rtl']]);
const scroller = setupScrollTimelineTest(scrollerOverrides);
// Set the timeRange such that currentTime maps directly to the value
// scrolled. The contents and scroller are square, so it suffices to compute
// one edge and use it for all the timelines.
const scrollerSize = scroller.scrollHeight - scroller.clientHeight;

// When the endScrollOffset is equal to the maximum scroll offset (and there
// are no fill modes), the endScrollOffset is treated as inclusive.
const inclusiveAutoScrollTimeline = new ScrollTimeline({
scrollSource: scroller,
timeRange: scrollerSize,
orientation: 'block',
endScrollOffset: 'auto'
});
const inclusiveLengthScrollTimeline = new ScrollTimeline({
scrollSource: scroller,
timeRange: scrollerSize,
orientation: 'block',
endScrollOffset: scrollerSize + 'px'
});
const inclusivePercentageScrollTimeline = new ScrollTimeline({
scrollSource: scroller,
timeRange: scrollerSize,
orientation: 'block',
endScrollOffset: '100%'
});
const inclusiveCalcScrollTimeline = new ScrollTimeline({
scrollSource: scroller,
timeRange: scrollerSize,
orientation: 'block',
endScrollOffset: 'calc(80% + ' + (0.2 * scrollerSize) + 'px)'
});

// With direction rtl offsets are inverted, such that scrollLeft ==
// scrollerSize is the 'zero' point for currentTime. However the
// endScrollOffset is an absolute distance along the offset, so doesn't need
// adjusting.

scroller.scrollLeft = 0;
let expectedCurrentTime = calculateCurrentTime(
scroller.scrollLeft, 0, scrollerSize, scrollerSize);

assert_equals(
inclusiveAutoScrollTimeline.currentTime, expectedCurrentTime,
'Inclusive auto timeline at the endScrollOffset point');
assert_equals(
inclusiveLengthScrollTimeline.currentTime, expectedCurrentTime,
'Inclusive length-based timeline at the endScrollOffset point');
assert_equals(
inclusivePercentageScrollTimeline.currentTime, expectedCurrentTime,
'Inclusive percentage-based timeline at the endScrollOffset point');
assert_equals(
inclusiveCalcScrollTimeline.currentTime, expectedCurrentTime,
'Inclusive calc-based timeline at the endScrollOffset point');
}, 'currentTime handles endScrollOffset (inclusive case) with direction: rtl correctly');
</script>
64 changes: 55 additions & 9 deletions scroll-animations/current-time.html
Expand Up @@ -199,9 +199,7 @@
'Length-based timeline after the endScrollOffset point');
scroller.scrollTop = scrollerSize - 20;
assert_equals(
lengthScrollTimeline.currentTime,
calculateCurrentTime(
scrollerSize - 20, 0, scrollerSize - 20, scrollerSize),
lengthScrollTimeline.currentTime, null,
'Length-based timeline at the endScrollOffset point');
scroller.scrollTop = scrollerSize - 50;
assert_equals(
Expand All @@ -217,9 +215,7 @@
'Percentage-based timeline after the endScrollOffset point');
scroller.scrollTop = 0.80 * scrollerSize;
assert_equals(
percentageScrollTimeline.currentTime,
calculateCurrentTime(
scroller.scrollTop, 0, 0.8 * scrollerSize, scrollerSize),
percentageScrollTimeline.currentTime, null,
'Percentage-based timeline at the endScrollOffset point');
scroller.scrollTop = 0.50 * scrollerSize;
assert_equals(
Expand All @@ -235,9 +231,7 @@
'Calc-based timeline after the endScrollOffset point');
scroller.scrollTop = 0.8 * scrollerSize + 5;
assert_equals(
calcScrollTimeline.currentTime,
calculateCurrentTime(
scroller.scrollTop, 0, 0.8 * scrollerSize + 5, scrollerSize),
calcScrollTimeline.currentTime, null,
'Calc-based timeline at the endScrollOffset point');
scroller.scrollTop = 0.5 * scrollerSize;
assert_equals(
Expand All @@ -247,6 +241,58 @@
'Calc-based timeline before the endScrollOffset point');
}, 'currentTime handles endScrollOffset correctly');

test(function() {
const scroller = setupScrollTimelineTest();
// Set the timeRange such that currentTime maps directly to the value
// scrolled. The contents and scroller are square, so it suffices to compute
// one edge and use it for all the timelines.
const scrollerSize = scroller.scrollHeight - scroller.clientHeight;

// When the endScrollOffset is equal to the maximum scroll offset (and there
// are no fill modes), the endScrollOffset is treated as inclusive.
const inclusiveAutoScrollTimeline = new ScrollTimeline({
scrollSource: scroller,
timeRange: scrollerSize,
orientation: 'block',
endScrollOffset: 'auto'
});
const inclusiveLengthScrollTimeline = new ScrollTimeline({
scrollSource: scroller,
timeRange: scrollerSize,
orientation: 'block',
endScrollOffset: scrollerSize + 'px'
});
const inclusivePercentageScrollTimeline = new ScrollTimeline({
scrollSource: scroller,
timeRange: scrollerSize,
orientation: 'block',
endScrollOffset: '100%'
});
const inclusiveCalcScrollTimeline = new ScrollTimeline({
scrollSource: scroller,
timeRange: scrollerSize,
orientation: 'block',
endScrollOffset: 'calc(80% + ' + (0.2 * scrollerSize) + 'px)'
});

scroller.scrollTop = scrollerSize;
let expectedCurrentTime = calculateCurrentTime(
scroller.scrollTop, 0, scrollerSize, scrollerSize);

assert_equals(
inclusiveAutoScrollTimeline.currentTime, expectedCurrentTime,
'Inclusive auto timeline at the endScrollOffset point');
assert_equals(
inclusiveLengthScrollTimeline.currentTime, expectedCurrentTime,
'Inclusive length-based timeline at the endScrollOffset point');
assert_equals(
inclusivePercentageScrollTimeline.currentTime, expectedCurrentTime,
'Inclusive percentage-based timeline at the endScrollOffset point');
assert_equals(
inclusiveCalcScrollTimeline.currentTime, expectedCurrentTime,
'Inclusive calc-based timeline at the endScrollOffset point');
}, 'currentTime handles endScrollOffset correctly (inclusive cases)');

test(function() {
const scroller = setupScrollTimelineTest();
// Set the timeRange such that currentTime maps directly to the value
Expand Down

0 comments on commit 480c82d

Please sign in to comment.