Skip to content

F
Front-End
Unverified Commit 5fd9ef42 authored by Tae Hyung Kim's avatar Tae Hyung Kim Committed by GitHub
Browse files
Options

Fix SliverPersistentHeader interactions with SliverCrossAxisGroup (#127338) 

This PR fixes `SliverCrossAxisGroup` so that once we calculate the total `scrollExtent` of the `SliverCrossAxisGroup`, we ensure that any child sliver is not painted outside of the "scroll extent" of the sliver group.

https://github.com/flutter/flutter/assets/2004742/670dc6f9-a3c6-4bcc-85d3-576cf7f33c6a

https://github.com/flutter/flutter/assets/2004742/0c9cd951-c133-4a8b-9e5e-89d0a69a4f59

Fixes #126958.
Fixes #126957.
parent 25360aed
Hide whitespace changes
Inline Side-by-side
Showing with 350 additions and 22 deletions
packages/flutter/lib/src/rendering/sliver_group.dart
View file @ 5fd9ef42
......@@ -76,7 +76,6 @@ class RenderSliverCrossAxisGroup extends RenderSliver with ContainerRenderObject
final double extentPerFlexValue = remainingExtent / totalFlex;
child = firstChild;
double offset = 0.0;
// At this point, all slivers with constrained cross axis should already be laid out.
// Layout the rest and keep track of the child geometry with greatest scrollExtent.
......@@ -94,22 +93,35 @@ class RenderSliverCrossAxisGroup extends RenderSliver with ContainerRenderObject
} else {
childExtent = child.geometry!.crossAxisExtent!;
}
final SliverGeometry childLayoutGeometry = child.geometry!;
if (geometry!.scrollExtent < childLayoutGeometry.scrollExtent) {
geometry = childLayoutGeometry;
}
child = childAfter(child);
}
// Go back and correct any slivers using a negative paint offset if it tries
// to paint outside the bounds of the sliver group.
child = firstChild;
double offset = 0.0;
while (child != null) {
final SliverPhysicalParentData childParentData = child.parentData! as SliverPhysicalParentData;
final SliverGeometry childLayoutGeometry = child.geometry!;
final double remainingExtent = geometry!.scrollExtent - constraints.scrollOffset;
final double paintCorrection = childLayoutGeometry.paintExtent > remainingExtent
? childLayoutGeometry.paintExtent - remainingExtent
: 0.0;
final double childExtent = child.geometry!.crossAxisExtent ?? extentPerFlexValue * (childParentData.crossAxisFlex ?? 0);
// Set child parent data.
switch (constraints.axis) {
case Axis.vertical:
childParentData.paintOffset = Offset(offset, 0.0);
childParentData.paintOffset = Offset(offset, -paintCorrection);
case Axis.horizontal:
childParentData.paintOffset = Offset(0.0, offset);
childParentData.paintOffset = Offset(-paintCorrection, offset);
}
offset += childExtent;
if (geometry!.scrollExtent < child.geometry!.scrollExtent) {
geometry = child.geometry;
}
child = childAfter(child);
}
// Set the geometry with the proper crossAxisExtent.
geometry = geometry!.copyWith(crossAxisExtent: constraints.crossAxisExtent);
}
@override
......
packages/flutter/test/widgets/sliver_cross_axis_group_test.dart
View file @ 5fd9ef42
......@@ -511,6 +511,301 @@ void main() {
expect(first.localToGlobal(Offset.zero), Offset.zero);
expect(second.localToGlobal(Offset.zero), const Offset(VIEWPORT_WIDTH / 2, 0));
});
testWidgets('SliverPinnedPersistentHeader is painted within bounds of SliverCrossAxisGroup', (WidgetTester tester) async {
final ScrollController controller = ScrollController();
await tester.pumpWidget(_buildSliverCrossAxisGroup(
controller: controller,
slivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 600)),
SliverPersistentHeader(
delegate: TestDelegate(),
pinned: true,
),
],
otherSlivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 2400)),
],
));
final RenderSliverCrossAxisGroup renderGroup = tester.renderObject(find.byType(SliverCrossAxisGroup)) as RenderSliverCrossAxisGroup;
expect(renderGroup.geometry!.scrollExtent, equals(600));
controller.jumpTo(560);
await tester.pumpAndSettle();
final RenderSliverPersistentHeader renderHeader = tester.renderObject(find.byType(SliverPersistentHeader)) as RenderSliverPersistentHeader;
// Paint extent after header's layout is 60.0, so we must offset by -20.0 to fit within the 40.0 remaining extent.
expect(renderHeader.geometry!.paintExtent, equals(60.0));
expect((renderHeader.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(-20.0));
});
testWidgets('SliverFloatingPersistentHeader is painted within bounds of SliverCrossAxisGroup', (WidgetTester tester) async {
final ScrollController controller = ScrollController();
await tester.pumpWidget(_buildSliverCrossAxisGroup(
controller: controller,
slivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 600)),
SliverPersistentHeader(
delegate: TestDelegate(),
floating: true,
),
],
otherSlivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 2400)),
],
));
await tester.pumpAndSettle();
final RenderSliverCrossAxisGroup renderGroup = tester.renderObject(find.byType(SliverCrossAxisGroup)) as RenderSliverCrossAxisGroup;
expect(renderGroup.geometry!.scrollExtent, equals(600));
controller.jumpTo(600.0);
await tester.pumpAndSettle();
final TestGesture gesture = await tester.startGesture(const Offset(150.0, 300.0));
await gesture.moveBy(const Offset(0.0, 40));
await tester.pump();
final RenderSliverPersistentHeader renderHeader = tester.renderObject(find.byType(SliverPersistentHeader)) as RenderSliverPersistentHeader;
// Paint extent after header's layout is 40.0, so no need to correct the paintOffset.
expect(renderHeader.geometry!.paintExtent, equals(40.0));
expect((renderHeader.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(0.0));
});
testWidgets('SliverPinnedPersistentHeader is painted within bounds of SliverCrossAxisGroup with different minExtent/maxExtent', (WidgetTester tester) async {
final ScrollController controller = ScrollController();
await tester.pumpWidget(_buildSliverCrossAxisGroup(
controller: controller,
slivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 600)),
SliverPersistentHeader(
delegate: TestDelegate(minExtent: 40.0),
pinned: true,
),
],
otherSlivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 2400)),
],
));
final RenderSliverCrossAxisGroup renderGroup = tester.renderObject(find.byType(SliverCrossAxisGroup)) as RenderSliverCrossAxisGroup;
final RenderSliverPersistentHeader renderHeader = tester.renderObject(find.byType(SliverPersistentHeader)) as RenderSliverPersistentHeader;
expect(renderGroup.geometry!.scrollExtent, equals(600));
controller.jumpTo(570);
await tester.pumpAndSettle();
// Paint extent of the header is 40.0, so we must provide an offset of -10.0 to make it fit in the 30.0 remaining paint extent of the group.
expect(renderHeader.geometry!.paintExtent, equals(40.0));
expect((renderHeader.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(-10.0));
// Pinned headers should not expand to the maximum extent unless the scroll offset is at the top of the sliver group.
controller.jumpTo(550);
await tester.pumpAndSettle();
expect(renderHeader.geometry!.paintExtent, equals(40.0));
expect((renderHeader.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(0.0));
});
testWidgets('SliverFloatingPersistentHeader is painted within bounds of SliverCrossAxisGroup with different minExtent/maxExtent', (WidgetTester tester) async {
final ScrollController controller = ScrollController();
await tester.pumpWidget(_buildSliverCrossAxisGroup(
controller: controller,
slivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 600)),
SliverPersistentHeader(
delegate: TestDelegate(minExtent: 40.0),
floating: true,
),
],
otherSlivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 2400)),
],
));
await tester.pumpAndSettle();
final RenderSliverCrossAxisGroup renderGroup = tester.renderObject(find.byType(SliverCrossAxisGroup)) as RenderSliverCrossAxisGroup;
final RenderSliverPersistentHeader renderHeader = tester.renderObject(find.byType(SliverPersistentHeader)) as RenderSliverPersistentHeader;
expect(renderGroup.geometry!.scrollExtent, equals(600));
controller.jumpTo(600);
await tester.pumpAndSettle();
final TestGesture gesture = await tester.startGesture(const Offset(150.0, 300.0));
await gesture.moveBy(const Offset(0.0, 30.0));
await tester.pump();
// Paint extent after header's layout is 30.0, so no need to correct the paintOffset.
expect(renderHeader.geometry!.paintExtent, equals(30.0));
expect((renderHeader.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(0.0));
// Floating headers should expand to maximum extent as we continue scrolling.
await gesture.moveBy(const Offset(0.0, 20.0));
await tester.pump();
expect(renderHeader.geometry!.paintExtent, equals(50.0));
expect((renderHeader.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(0.0));
});
testWidgets('SliverPinnedFloatingPersistentHeader is painted within bounds of SliverCrossAxisGroup with different minExtent/maxExtent', (WidgetTester tester) async {
final ScrollController controller = ScrollController();
await tester.pumpWidget(_buildSliverCrossAxisGroup(
controller: controller,
slivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 600)),
SliverPersistentHeader(
delegate: TestDelegate(minExtent: 40.0),
pinned: true,
floating: true,
),
],
otherSlivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 2400)),
],
));
await tester.pumpAndSettle();
final RenderSliverCrossAxisGroup renderGroup = tester.renderObject(find.byType(SliverCrossAxisGroup)) as RenderSliverCrossAxisGroup;
final RenderSliverPersistentHeader renderHeader = tester.renderObject(find.byType(SliverPersistentHeader)) as RenderSliverPersistentHeader;
expect(renderGroup.geometry!.scrollExtent, equals(600));
controller.jumpTo(600);
await tester.pumpAndSettle();
final TestGesture gesture = await tester.startGesture(const Offset(150.0, 300.0));
await gesture.moveBy(const Offset(0.0, 30.0));
await tester.pump();
// Paint extent after header's layout is 40.0, so we need to adjust by -10.0.
expect(renderHeader.geometry!.paintExtent, equals(40.0));
expect((renderHeader.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(-10.0));
// Pinned floating headers should expand to maximum extent as we continue scrolling.
await gesture.moveBy(const Offset(0.0, 20.0));
await tester.pump();
expect(renderHeader.geometry!.paintExtent, equals(50.0));
expect((renderHeader.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(0.0));
});
testWidgets('SliverAppBar with floating: false, pinned: false, snap: false is painted within bounds of SliverCrossAxisGroup', (WidgetTester tester) async {
final ScrollController controller = ScrollController();
await tester.pumpWidget(_buildSliverCrossAxisGroup(
controller: controller,
slivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 600)),
const SliverAppBar(
toolbarHeight: 30,
expandedHeight: 60,
),
],
otherSlivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 2400)),
],
));
await tester.pumpAndSettle();
final RenderSliverCrossAxisGroup renderGroup = tester.renderObject(find.byType(SliverCrossAxisGroup)) as RenderSliverCrossAxisGroup;
expect(renderGroup.geometry!.scrollExtent, equals(600));
controller.jumpTo(600);
await tester.pumpAndSettle();
controller.jumpTo(570);
await tester.pumpAndSettle();
// At a scroll offset of 570, a normal scrolling header should be out of view.
final RenderSliverPersistentHeader renderHeader = tester.renderObject(find.byType(SliverPersistentHeader)) as RenderSliverPersistentHeader;
expect(renderHeader.geometry!.paintExtent, equals(0.0));
});
testWidgets('SliverAppBar with floating: true, pinned: false, snap: true is painted within bounds of SliverCrossAxisGroup', (WidgetTester tester) async {
final ScrollController controller = ScrollController();
await tester.pumpWidget(_buildSliverCrossAxisGroup(
controller: controller,
slivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 600)),
const SliverAppBar(
toolbarHeight: 30,
expandedHeight: 60,
floating: true,
snap: true,
),
],
otherSlivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 2400)),
],
));
await tester.pumpAndSettle();
final RenderSliverCrossAxisGroup renderGroup = tester.renderObject(find.byType(SliverCrossAxisGroup)) as RenderSliverCrossAxisGroup;
final RenderSliverPersistentHeader renderHeader = tester.renderObject(find.byType(SliverPersistentHeader)) as RenderSliverPersistentHeader;
expect(renderGroup.geometry!.scrollExtent, equals(600));
controller.jumpTo(600);
await tester.pumpAndSettle();
final TestGesture gesture = await tester.startGesture(const Offset(150.0, 300.0));
await gesture.moveBy(const Offset(0.0, 10));
await tester.pump();
// The snap animation does not go through until the gesture is released.
expect(renderHeader.geometry!.paintExtent, equals(10));
expect((renderHeader.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(0.0));
// Once it is released, the header's paint extent becomes the maximum and the group sets an offset of -50.0.
await gesture.up();
await tester.pumpAndSettle();
expect(renderHeader.geometry!.paintExtent, equals(60));
expect((renderHeader.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(-50.0));
});
testWidgets('SliverAppBar with floating: true, pinned: true, snap: true is painted within bounds of SliverCrossAxisGroup', (WidgetTester tester) async {
final ScrollController controller = ScrollController();
await tester.pumpWidget(_buildSliverCrossAxisGroup(
controller: controller,
slivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 600)),
const SliverAppBar(
toolbarHeight: 30,
expandedHeight: 60,
floating: true,
pinned: true,
snap: true,
),
],
otherSlivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 2400)),
],
));
await tester.pumpAndSettle();
final RenderSliverCrossAxisGroup renderGroup = tester.renderObject(find.byType(SliverCrossAxisGroup)) as RenderSliverCrossAxisGroup;
final RenderSliverPersistentHeader renderHeader = tester.renderObject(find.byType(SliverPersistentHeader)) as RenderSliverPersistentHeader;
expect(renderGroup.geometry!.scrollExtent, equals(600));
controller.jumpTo(600);
await tester.pumpAndSettle();
final TestGesture gesture = await tester.startGesture(const Offset(150.0, 300.0));
await gesture.moveBy(const Offset(0.0, 10));
await tester.pump();
expect(renderHeader.geometry!.paintExtent, equals(30.0));
expect((renderHeader.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(-20.0));
// Once we lift the gesture up, the animation should finish.
await gesture.up();
await tester.pumpAndSettle();
expect(renderHeader.geometry!.paintExtent, equals(60.0));
expect((renderHeader.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(-50.0));
});
testWidgets('SliverFloatingPersistentHeader scroll direction is not affected by controller.jumpTo', (WidgetTester tester) async {
final ScrollController controller = ScrollController();
await tester.pumpWidget(_buildSliverCrossAxisGroup(
controller: controller,
slivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 600)),
SliverPersistentHeader(
delegate: TestDelegate(),
floating: true,
),
],
otherSlivers: <Widget>[
const SliverToBoxAdapter(child: SizedBox(height: 2400)),
],
));
await tester.pumpAndSettle();
final RenderSliverCrossAxisGroup renderGroup = tester.renderObject(find.byType(SliverCrossAxisGroup)) as RenderSliverCrossAxisGroup;
final RenderSliverPersistentHeader renderHeader = tester.renderObject(find.byType(SliverPersistentHeader)) as RenderSliverPersistentHeader;
expect(renderGroup.geometry!.scrollExtent, equals(600));
controller.jumpTo(600);
await tester.pumpAndSettle();
controller.jumpTo(570);
await tester.pumpAndSettle();
// If renderHeader._lastStartedScrollDirection is not ScrollDirection.forward, then we shouldn't see the header at all.
expect((renderHeader.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(0.0));
});
}
Widget _buildSliverList({
......@@ -550,21 +845,42 @@ Widget _buildSliverCrossAxisGroup({
double viewportWidth = VIEWPORT_WIDTH,
Axis scrollDirection = Axis.vertical,
bool reverse = false,
List<Widget> otherSlivers = const <Widget>[],
}) {
return Directionality(
textDirection: TextDirection.ltr,
child: Align(
alignment: Alignment.topLeft,
child: SizedBox(
height: viewportHeight,
width: viewportWidth,
child: CustomScrollView(
scrollDirection: scrollDirection,
reverse: reverse,
controller: controller,
slivers: <Widget>[SliverCrossAxisGroup(slivers: slivers)],
return MaterialApp(
home: Directionality(
textDirection: TextDirection.ltr,
child: Align(
alignment: Alignment.topLeft,
child: SizedBox(
height: viewportHeight,
width: viewportWidth,
child: CustomScrollView(
scrollDirection: scrollDirection,
reverse: reverse,
controller: controller,
slivers: <Widget>[SliverCrossAxisGroup(slivers: slivers), ...otherSlivers],
),
),
),
),
)
);
}
class TestDelegate extends SliverPersistentHeaderDelegate {
TestDelegate({ this.maxExtent = 60.0, this.minExtent = 60.0 });
@override
final double maxExtent;
@override
final double minExtent;
@override
Widget build(BuildContext context, double shrinkOffset, bool overlapsContent) {
return Container(height: maxExtent);
}
@override
bool shouldRebuild(TestDelegate oldDelegate) => true;
}
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

HIAST - GitLab