Commit eb839755 authored by Adam Barth's avatar Adam Barth Committed by GitHub

Overscrolls should appear below pinned headers (#8383)

With slivers, the app bar is part of the scroll view. Naively, the overscroll
appears at the beginning of the scroll view, but that's not the desired
behavior. Instead, the app bar should remained pinned to the top of the scroll
view and the overscroll should appear below the app bar, which is what this
patch does.

Fixes #8228
parent a0c3aae1
...@@ -435,6 +435,7 @@ class SliverGeometry { ...@@ -435,6 +435,7 @@ class SliverGeometry {
const SliverGeometry({ const SliverGeometry({
this.scrollExtent: 0.0, this.scrollExtent: 0.0,
this.paintExtent: 0.0, this.paintExtent: 0.0,
this.paintOrigin: 0.0,
double layoutExtent, double layoutExtent,
this.maxPaintExtent: 0.0, this.maxPaintExtent: 0.0,
double hitTestExtent, double hitTestExtent,
...@@ -456,6 +457,23 @@ class SliverGeometry { ...@@ -456,6 +457,23 @@ class SliverGeometry {
/// [SliverConstraints.remainingPaintExtent] provided during layout. /// [SliverConstraints.remainingPaintExtent] provided during layout.
final double scrollExtent; final double scrollExtent;
/// The visual location of the first visible part of this sliver relative to
/// its layout position.
///
/// For example, if the sliver wishes to paint visually before its layout
/// position, the [paintOrigin] is negative. The coordinate system this sliver
/// uses for painting is relative to this [paintOrigin].
///
/// This value does not affect the layout of subsequent slivers. The next
/// sliver is still placed at [layoutExtent] after this sliver's layout
/// position. This value does affect where the [paintExtent] extent is
/// measured from when computing the [SliverConstraints.overlap] for the next
/// sliver.
///
/// Defaults to 0.0, which means slivers start painting at their layout
/// position by default.
final double paintOrigin;
/// The amount of visual space that was taken by the sliver to render the /// The amount of visual space that was taken by the sliver to render the
/// subset of the sliver that covers all or part of the /// subset of the sliver that covers all or part of the
/// [SliverConstraints.remainingPaintExtent]. /// [SliverConstraints.remainingPaintExtent].
...@@ -513,6 +531,7 @@ class SliverGeometry { ...@@ -513,6 +531,7 @@ class SliverGeometry {
assert(scrollExtent >= 0.0); assert(scrollExtent >= 0.0);
assert(paintExtent != null); assert(paintExtent != null);
assert(paintExtent >= 0.0); assert(paintExtent >= 0.0);
assert(paintOrigin != null);
assert(layoutExtent != null); assert(layoutExtent != null);
assert(layoutExtent >= 0.0); assert(layoutExtent >= 0.0);
assert(() { assert(() {
...@@ -563,6 +582,8 @@ class SliverGeometry { ...@@ -563,6 +582,8 @@ class SliverGeometry {
} else { } else {
buffer.write('paintExtent: ${paintExtent.toStringAsFixed(1)} (!), '); buffer.write('paintExtent: ${paintExtent.toStringAsFixed(1)} (!), ');
} }
if (paintOrigin != 0.0)
buffer.write('paintOrigin: ${paintOrigin.toStringAsFixed(1)}, ');
if (layoutExtent != paintExtent) if (layoutExtent != paintExtent)
buffer.write('layoutExtent: ${layoutExtent.toStringAsFixed(1)}, '); buffer.write('layoutExtent: ${layoutExtent.toStringAsFixed(1)}, ');
buffer.write('maxPaintExtent: ${maxPaintExtent.toStringAsFixed(1)}, '); buffer.write('maxPaintExtent: ${maxPaintExtent.toStringAsFixed(1)}, ');
......
...@@ -236,21 +236,19 @@ abstract class RenderSliverPinnedPersistentHeader extends RenderSliverPersistent ...@@ -236,21 +236,19 @@ abstract class RenderSliverPinnedPersistentHeader extends RenderSliverPersistent
@override @override
void performLayout() { void performLayout() {
final double maxExtent = this.maxExtent; final double maxExtent = this.maxExtent;
layoutChild(constraints.scrollOffset + constraints.overlap, maxExtent, overlapsContent: constraints.overlap > 0.0); layoutChild(constraints.scrollOffset, maxExtent, overlapsContent: constraints.overlap > 0.0);
geometry = new SliverGeometry( geometry = new SliverGeometry(
scrollExtent: maxExtent, scrollExtent: maxExtent,
paintExtent: math.min(constraints.overlap + childExtent, constraints.remainingPaintExtent), paintOrigin: constraints.overlap,
paintExtent: math.min(childExtent, constraints.remainingPaintExtent),
layoutExtent: (maxExtent - constraints.scrollOffset).clamp(0.0, constraints.remainingPaintExtent), layoutExtent: (maxExtent - constraints.scrollOffset).clamp(0.0, constraints.remainingPaintExtent),
maxPaintExtent: constraints.overlap + maxExtent, maxPaintExtent: maxExtent,
hasVisualOverflow: true, // Conservatively say we do have overflow to avoid complexity. hasVisualOverflow: true, // Conservatively say we do have overflow to avoid complexity.
); );
} }
@override @override
double childMainAxisPosition(RenderBox child) { double childMainAxisPosition(RenderBox child) => 0.0;
assert(child == this.child);
return constraints?.overlap;
}
} }
abstract class RenderSliverFloatingPersistentHeader extends RenderSliverPersistentHeader { abstract class RenderSliverFloatingPersistentHeader extends RenderSliverPersistentHeader {
......
...@@ -120,6 +120,7 @@ abstract class RenderViewportBase<ParentDataClass extends ContainerParentDataMix ...@@ -120,6 +120,7 @@ abstract class RenderViewportBase<ParentDataClass extends ContainerParentDataMix
double layoutOneSide( double layoutOneSide(
RenderSliver child, RenderSliver child,
double scrollOffset, double scrollOffset,
double overlap,
double layoutOffset, double layoutOffset,
double remainingPaintExtent, double remainingPaintExtent,
double mainAxisExtent, double mainAxisExtent,
...@@ -129,11 +130,11 @@ abstract class RenderViewportBase<ParentDataClass extends ContainerParentDataMix ...@@ -129,11 +130,11 @@ abstract class RenderViewportBase<ParentDataClass extends ContainerParentDataMix
) { ) {
assert(scrollOffset.isFinite); assert(scrollOffset.isFinite);
assert(scrollOffset >= 0.0); assert(scrollOffset >= 0.0);
final double initialLayoutOffset = layoutOffset;
final ScrollDirection adjustedUserScrollDirection = final ScrollDirection adjustedUserScrollDirection =
applyGrowthDirecitonToScrollDirection(offset.userScrollDirection, growthDirection); applyGrowthDirecitonToScrollDirection(offset.userScrollDirection, growthDirection);
assert(adjustedUserScrollDirection != null); assert(adjustedUserScrollDirection != null);
double maxPaintOffset = layoutOffset; double maxPaintOffset = layoutOffset + overlap;
final double initialLayoutOffset = layoutOffset;
while (child != null) { while (child != null) {
assert(scrollOffset >= 0.0); assert(scrollOffset >= 0.0);
child.layout(new SliverConstraints( child.layout(new SliverConstraints(
...@@ -146,19 +147,19 @@ abstract class RenderViewportBase<ParentDataClass extends ContainerParentDataMix ...@@ -146,19 +147,19 @@ abstract class RenderViewportBase<ParentDataClass extends ContainerParentDataMix
crossAxisExtent: crossAxisExtent, crossAxisExtent: crossAxisExtent,
viewportMainAxisExtent: mainAxisExtent, viewportMainAxisExtent: mainAxisExtent,
), parentUsesSize: true); ), parentUsesSize: true);
// collect the child's objects
final SliverGeometry childLayoutGeometry = child.geometry;
final SliverGeometry childLayoutGeometry = child.geometry;
assert(childLayoutGeometry.debugAssertIsValid); assert(childLayoutGeometry.debugAssertIsValid);
// first check that there isn't a correction to apply. If there is we'll // If there is a correction to apply, we'll have to start over.
// have to start over.
if (childLayoutGeometry.scrollOffsetCorrection != 0.0) if (childLayoutGeometry.scrollOffsetCorrection != 0.0)
return childLayoutGeometry.scrollOffsetCorrection; return childLayoutGeometry.scrollOffsetCorrection;
// geometry // We use the child's paint origin in our coordinate system as the
updateChildLayoutOffset(child, layoutOffset, growthDirection); // layoutOffset we store in the child's parent data.
maxPaintOffset = math.max(layoutOffset + childLayoutGeometry.paintExtent, maxPaintOffset); final double effectiveLayoutOffset = layoutOffset + childLayoutGeometry.paintOrigin;
updateChildLayoutOffset(child, effectiveLayoutOffset, growthDirection);
maxPaintOffset = math.max(effectiveLayoutOffset + childLayoutGeometry.paintExtent, maxPaintOffset);
scrollOffset -= childLayoutGeometry.scrollExtent; scrollOffset -= childLayoutGeometry.scrollExtent;
layoutOffset += childLayoutGeometry.layoutExtent; layoutOffset += childLayoutGeometry.layoutExtent;
...@@ -670,24 +671,30 @@ class RenderViewport extends RenderViewportBase<SliverPhysicalContainerParentDat ...@@ -670,24 +671,30 @@ class RenderViewport extends RenderViewportBase<SliverPhysicalContainerParentDat
final double clampedForwardCenter = math.max(0.0, math.min(mainAxisExtent, centerOffset)); final double clampedForwardCenter = math.max(0.0, math.min(mainAxisExtent, centerOffset));
final double clampedReverseCenter = math.max(0.0, math.min(mainAxisExtent, mainAxisExtent - centerOffset)); final double clampedReverseCenter = math.max(0.0, math.min(mainAxisExtent, mainAxisExtent - centerOffset));
// negative scroll offsets final RenderSliver leadingNegativeChild = childBefore(center);
double result = layoutOneSide(
childBefore(center), if (leadingNegativeChild != null) {
math.max(mainAxisExtent, mainAxisExtent * anchor - correctedOffset) - mainAxisExtent, // negative scroll offsets
clampedReverseCenter, double result = layoutOneSide(
clampedForwardCenter, leadingNegativeChild,
mainAxisExtent, math.max(mainAxisExtent, centerOffset) - mainAxisExtent,
crossAxisExtent, 0.0,
GrowthDirection.reverse, clampedReverseCenter,
childBefore, clampedForwardCenter,
); mainAxisExtent,
if (result != 0.0) crossAxisExtent,
return -result; GrowthDirection.reverse,
childBefore,
);
if (result != 0.0)
return -result;
}
// positive scroll offsets // positive scroll offsets
return layoutOneSide( return layoutOneSide(
center, center,
math.max(0.0, correctedOffset - mainAxisExtent * anchor), math.max(0.0, -centerOffset),
leadingNegativeChild == null ? math.min(0.0, -centerOffset) : 0.0,
clampedForwardCenter, clampedForwardCenter,
clampedReverseCenter, clampedReverseCenter,
mainAxisExtent, mainAxisExtent,
...@@ -952,6 +959,7 @@ class RenderShrinkWrappingViewport extends RenderViewportBase<SliverLogicalConta ...@@ -952,6 +959,7 @@ class RenderShrinkWrappingViewport extends RenderViewportBase<SliverLogicalConta
return layoutOneSide( return layoutOneSide(
firstChild, firstChild,
math.max(0.0, correctedOffset), math.max(0.0, correctedOffset),
math.min(0.0, correctedOffset),
0.0, 0.0,
mainAxisExtent, mainAxisExtent,
mainAxisExtent, mainAxisExtent,
......
...@@ -51,7 +51,7 @@ void main() { ...@@ -51,7 +51,7 @@ void main() {
expect(position.maxScrollExtent, max); expect(position.maxScrollExtent, max);
verifyPaintPosition(key1, const Offset(0.0, 0.0), false); verifyPaintPosition(key1, const Offset(0.0, 0.0), false);
verifyPaintPosition(key2, const Offset(0.0, 0.0), true); verifyPaintPosition(key2, const Offset(0.0, 0.0), true);
verifyPaintPosition(key3, const Offset(0.0, 0.0), true); verifyPaintPosition(key3, const Offset(0.0, 100.0), true);
verifyPaintPosition(key4, const Offset(0.0, 0.0), true); verifyPaintPosition(key4, const Offset(0.0, 0.0), true);
verifyPaintPosition(key5, const Offset(0.0, 50.0), true); verifyPaintPosition(key5, const Offset(0.0, 50.0), true);
}); });
...@@ -102,44 +102,44 @@ void main() { ...@@ -102,44 +102,44 @@ void main() {
await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 400)); await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 400));
verifyPaintPosition(key1, const Offset(0.0, 0.0), false); verifyPaintPosition(key1, const Offset(0.0, 0.0), false);
verifyPaintPosition(key2, const Offset(0.0, 0.0), true); verifyPaintPosition(key2, const Offset(0.0, 0.0), true);
verifyPaintPosition(key3, const Offset(0.0, 50.0), true); verifyPaintPosition(key3, const Offset(0.0, 100.0), true);
verifyActualBoxPosition(tester, find.byType(Container), 1, new Rect.fromLTWH(0.0, 100.0, 800.0, 150.0)); verifyActualBoxPosition(tester, find.byType(Container), 1, new Rect.fromLTWH(0.0, 100.0, 800.0, 200.0));
verifyPaintPosition(key4, const Offset(0.0, 250.0), true); verifyPaintPosition(key4, const Offset(0.0, 250.0), true);
verifyPaintPosition(key5, const Offset(0.0, 600.0), false); verifyPaintPosition(key5, const Offset(0.0, 600.0), false);
position.animateTo(750.0, curve: Curves.linear, duration: const Duration(minutes: 1)); position.animateTo(750.0, curve: Curves.linear, duration: const Duration(minutes: 1));
await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 500)); await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 500));
verifyPaintPosition(key1, const Offset(0.0, 0.0), false); verifyPaintPosition(key1, const Offset(0.0, 0.0), false);
verifyPaintPosition(key2, const Offset(0.0, 0.0), true); verifyPaintPosition(key2, const Offset(0.0, 0.0), true);
verifyPaintPosition(key3, const Offset(0.0, 0.0), true); verifyPaintPosition(key3, const Offset(0.0, 100.0), true);
verifyActualBoxPosition(tester, find.byType(Container), 1, new Rect.fromLTWH(0.0, 100.0, 800.0, 100.0)); verifyActualBoxPosition(tester, find.byType(Container), 1, new Rect.fromLTWH(0.0, 100.0, 800.0, 200.0));
verifyPaintPosition(key4, const Offset(0.0, 200.0), true); verifyPaintPosition(key4, const Offset(0.0, 200.0), true);
verifyPaintPosition(key5, const Offset(0.0, 600.0), false); verifyPaintPosition(key5, const Offset(0.0, 600.0), false);
position.animateTo(800.0, curve: Curves.linear, duration: const Duration(minutes: 1)); position.animateTo(800.0, curve: Curves.linear, duration: const Duration(minutes: 1));
await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 60)); await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 60));
verifyPaintPosition(key1, const Offset(0.0, 0.0), false); verifyPaintPosition(key1, const Offset(0.0, 0.0), false);
verifyPaintPosition(key2, const Offset(0.0, 0.0), true); verifyPaintPosition(key2, const Offset(0.0, 0.0), true);
verifyPaintPosition(key3, const Offset(0.0, 0.0), true); verifyPaintPosition(key3, const Offset(0.0, 100.0), true);
verifyPaintPosition(key4, const Offset(0.0, 150.0), true); verifyPaintPosition(key4, const Offset(0.0, 150.0), true);
verifyPaintPosition(key5, const Offset(0.0, 600.0), false); verifyPaintPosition(key5, const Offset(0.0, 600.0), false);
position.animateTo(850.0, curve: Curves.linear, duration: const Duration(minutes: 1)); position.animateTo(850.0, curve: Curves.linear, duration: const Duration(minutes: 1));
await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 70)); await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 70));
verifyPaintPosition(key1, const Offset(0.0, 0.0), false); verifyPaintPosition(key1, const Offset(0.0, 0.0), false);
verifyPaintPosition(key2, const Offset(0.0, 0.0), true); verifyPaintPosition(key2, const Offset(0.0, 0.0), true);
verifyPaintPosition(key3, const Offset(0.0, 0.0), true); verifyPaintPosition(key3, const Offset(0.0, 100.0), true);
verifyPaintPosition(key4, const Offset(0.0, 100.0), true); verifyPaintPosition(key4, const Offset(0.0, 100.0), true);
verifyPaintPosition(key5, const Offset(0.0, 600.0), false); verifyPaintPosition(key5, const Offset(0.0, 600.0), false);
position.animateTo(900.0, curve: Curves.linear, duration: const Duration(minutes: 1)); position.animateTo(900.0, curve: Curves.linear, duration: const Duration(minutes: 1));
await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 80)); await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 80));
verifyPaintPosition(key1, const Offset(0.0, 0.0), false); verifyPaintPosition(key1, const Offset(0.0, 0.0), false);
verifyPaintPosition(key2, const Offset(0.0, 0.0), true); verifyPaintPosition(key2, const Offset(0.0, 0.0), true);
verifyPaintPosition(key3, const Offset(0.0, 0.0), true); verifyPaintPosition(key3, const Offset(0.0, 100.0), true);
verifyPaintPosition(key4, const Offset(0.0, 50.0), true); verifyPaintPosition(key4, const Offset(0.0, 50.0), true);
verifyPaintPosition(key5, const Offset(0.0, 600.0), false); verifyPaintPosition(key5, const Offset(0.0, 600.0), false);
position.animateTo(950.0, curve: Curves.linear, duration: const Duration(minutes: 1)); position.animateTo(950.0, curve: Curves.linear, duration: const Duration(minutes: 1));
await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 90)); await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 90));
verifyPaintPosition(key1, const Offset(0.0, 0.0), false); verifyPaintPosition(key1, const Offset(0.0, 0.0), false);
verifyPaintPosition(key2, const Offset(0.0, 0.0), true); verifyPaintPosition(key2, const Offset(0.0, 0.0), true);
verifyPaintPosition(key3, const Offset(0.0, 0.0), true); verifyPaintPosition(key3, const Offset(0.0, 100.0), true);
verifyActualBoxPosition(tester, find.byType(Container), 1, new Rect.fromLTWH(0.0, 100.0, 800.0, 100.0)); verifyActualBoxPosition(tester, find.byType(Container), 1, new Rect.fromLTWH(0.0, 100.0, 800.0, 100.0));
verifyPaintPosition(key4, const Offset(0.0, 0.0), true); verifyPaintPosition(key4, const Offset(0.0, 0.0), true);
verifyPaintPosition(key5, const Offset(0.0, 550.0), true); verifyPaintPosition(key5, const Offset(0.0, 550.0), true);
...@@ -173,10 +173,51 @@ void main() { ...@@ -173,10 +173,51 @@ void main() {
expect(position.maxScrollExtent, max); expect(position.maxScrollExtent, max);
verifyPaintPosition(key1, const Offset(0.0, 0.0), false); verifyPaintPosition(key1, const Offset(0.0, 0.0), false);
verifyPaintPosition(key2, const Offset(0.0, 0.0), true); verifyPaintPosition(key2, const Offset(0.0, 0.0), true);
verifyPaintPosition(key3, const Offset(0.0, 0.0), true); verifyPaintPosition(key3, const Offset(0.0, 100.0), true);
verifyPaintPosition(key4, const Offset(0.0, 0.0), false); verifyPaintPosition(key4, const Offset(0.0, 0.0), false);
verifyPaintPosition(key5, const Offset(0.0, 0.0), true); verifyPaintPosition(key5, const Offset(0.0, 0.0), true);
}); });
testWidgets('Sliver appbars - overscroll gap is below header', (WidgetTester tester) async {
await tester.pumpWidget(
new CustomScrollView(
physics: const BouncingScrollPhysics(),
slivers: <Widget>[
new SliverPersistentHeader(delegate: new TestDelegate(), pinned: true),
new SliverList(
delegate: new SliverChildListDelegate(<Widget>[
new SizedBox(
height: 300.0,
child: new Text('X'),
),
]),
),
],
),
);
expect(tester.getTopLeft(find.byType(Container)), Point.origin);
expect(tester.getTopLeft(find.text('X')), const Point(0.0, 200.0));
ScrollPosition position = tester.state<ScrollableState>(find.byType(Scrollable)).position;
position.jumpTo(-50.0);
await tester.pump();
expect(tester.getTopLeft(find.byType(Container)), Point.origin);
expect(tester.getTopLeft(find.text('X')), const Point(0.0, 250.0));
position.jumpTo(50.0);
await tester.pump();
expect(tester.getTopLeft(find.byType(Container)), Point.origin);
expect(tester.getTopLeft(find.text('X')), const Point(0.0, 150.0));
position.jumpTo(150.0);
await tester.pump();
expect(tester.getTopLeft(find.byType(Container)), Point.origin);
expect(tester.getTopLeft(find.text('X')), const Point(0.0, 50.0));
});
} }
class TestDelegate extends SliverPersistentHeaderDelegate { class TestDelegate extends SliverPersistentHeaderDelegate {
......
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