From f79ad055bcce232ca700de14ea5b81aa74209aae Mon Sep 17 00:00:00 2001 From: Darren Austin Date: Mon, 12 Apr 2021 12:44:22 -0700 Subject: [PATCH] Make the ReorderableListView padding scroll with the list. (#80251) --- .../lib/src/material/reorderable_list.dart | 109 ++++++++++-------- .../test/material/reorderable_list_test.dart | 55 ++++++++- 2 files changed, 111 insertions(+), 53 deletions(-) diff --git a/packages/flutter/lib/src/material/reorderable_list.dart b/packages/flutter/lib/src/material/reorderable_list.dart index 5d93d9d1ea1..8fbd37d1f05 100644 --- a/packages/flutter/lib/src/material/reorderable_list.dart +++ b/packages/flutter/lib/src/material/reorderable_list.dart @@ -487,63 +487,70 @@ class _ReorderableListViewState extends State { assert(debugCheckHasOverlay(context)); // If there is a header we can't just apply the padding to the list, - // so we wrap the CustomScrollView in the padding for the top, left and right - // and only add the padding from the bottom to the sliver list (or the equivalent - // for other axis directions). + // so we break it up into padding for the header and padding for the list. final EdgeInsets padding = widget.padding ?? EdgeInsets.zero; - late EdgeInsets outerPadding; - late EdgeInsets listPadding; - switch (widget.scrollDirection) { + late final EdgeInsets headerPadding; + late final EdgeInsets listPadding; - case Axis.horizontal: - if (widget.reverse) { - outerPadding = EdgeInsets.fromLTRB(0, padding.top, padding.right, padding.bottom); - listPadding = EdgeInsets.fromLTRB(padding.left, 0, 0, 0); - } else { - outerPadding = EdgeInsets.fromLTRB(padding.left, padding.top, 0, padding.bottom); - listPadding = EdgeInsets.fromLTRB(0, 0, padding.right, 0); - } - break; - case Axis.vertical: - if (widget.reverse) { - outerPadding = EdgeInsets.fromLTRB(padding.left, 0, padding.right, padding.bottom); - listPadding = EdgeInsets.fromLTRB(0, padding.top, 0, 0); - } else { - outerPadding = EdgeInsets.fromLTRB(padding.left, padding.top, padding.right, 0); - listPadding = EdgeInsets.fromLTRB(0, 0, 0, padding.bottom); - } - break; + if (widget.header == null) { + headerPadding = EdgeInsets.zero; + listPadding = padding; + } else { + switch (widget.scrollDirection) { + case Axis.horizontal: + if (widget.reverse) { + // Header on the right + headerPadding = EdgeInsets.fromLTRB(0, padding.top, padding.right, padding.bottom); + listPadding = EdgeInsets.fromLTRB(padding.left, padding.top, 0, padding.bottom); + } else { + // Header on the left + headerPadding = EdgeInsets.fromLTRB(padding.left, padding.top, 0, padding.bottom); + listPadding = EdgeInsets.fromLTRB(0, padding.top, padding.right, padding.bottom); + } + break; + case Axis.vertical: + if (widget.reverse) { + // Header on the bottom + headerPadding = EdgeInsets.fromLTRB(padding.left, 0, padding.right, padding.bottom); + listPadding = EdgeInsets.fromLTRB(padding.left, padding.top, padding.right, 0); + } else { + // Header on the top + headerPadding = EdgeInsets.fromLTRB(padding.left, padding.top, padding.right, 0); + listPadding = EdgeInsets.fromLTRB(padding.left, 0, padding.right, padding.bottom); + } + break; + } } - return Padding( - padding: outerPadding, - child: CustomScrollView( - scrollDirection: widget.scrollDirection, - reverse: widget.reverse, - controller: widget.scrollController, - primary: widget.primary, - physics: widget.physics, - shrinkWrap: widget.shrinkWrap, - anchor: widget.anchor, - cacheExtent: widget.cacheExtent, - dragStartBehavior: widget.dragStartBehavior, - keyboardDismissBehavior: widget.keyboardDismissBehavior, - restorationId: widget.restorationId, - clipBehavior: widget.clipBehavior, - slivers: [ - if (widget.header != null) - SliverToBoxAdapter(child: widget.header!), + return CustomScrollView( + scrollDirection: widget.scrollDirection, + reverse: widget.reverse, + controller: widget.scrollController, + primary: widget.primary, + physics: widget.physics, + shrinkWrap: widget.shrinkWrap, + anchor: widget.anchor, + cacheExtent: widget.cacheExtent, + dragStartBehavior: widget.dragStartBehavior, + keyboardDismissBehavior: widget.keyboardDismissBehavior, + restorationId: widget.restorationId, + clipBehavior: widget.clipBehavior, + slivers: [ + if (widget.header != null) SliverPadding( - padding: listPadding, - sliver: SliverReorderableList( - itemBuilder: _itemBuilder, - itemCount: widget.itemCount, - onReorder: widget.onReorder, - proxyDecorator: widget.proxyDecorator ?? _proxyDecorator, - ), + padding: headerPadding, + sliver: SliverToBoxAdapter(child: widget.header!), ), - ], - ), + SliverPadding( + padding: listPadding, + sliver: SliverReorderableList( + itemBuilder: _itemBuilder, + itemCount: widget.itemCount, + onReorder: widget.onReorder, + proxyDecorator: widget.proxyDecorator ?? _proxyDecorator, + ), + ), + ], ); } } diff --git a/packages/flutter/test/material/reorderable_list_test.dart b/packages/flutter/test/material/reorderable_list_test.dart index 5224f691b2d..1b136a09207 100644 --- a/packages/flutter/test/material/reorderable_list_test.dart +++ b/packages/flutter/test/material/reorderable_list_test.dart @@ -33,6 +33,8 @@ void main() { Widget build({ Widget? header, Axis scrollDirection = Axis.vertical, + bool reverse = false, + EdgeInsets padding = EdgeInsets.zero, TextDirection textDirection = TextDirection.ltr, TargetPlatform? platform, }) { @@ -48,6 +50,8 @@ void main() { children: listItems.map(listItemToWidget).toList(), scrollDirection: scrollDirection, onReorder: onReorder, + reverse: reverse, + padding: padding, ), ), ), @@ -276,7 +280,7 @@ void main() { // first with a gap between the first and third and a drop shadow on // the dragged item. await expectLater( - find.byType(ReorderableListView), + find.byType(Overlay).last, matchesGoldenFile('reorderable_list_test.vertical.drop_area.png'), ); debugDisableShadows = true; @@ -888,7 +892,7 @@ void main() { // first with a gap between the first and third and a drop shadow on // the dragged item. await expectLater( - find.byType(ReorderableListView), + find.byType(Overlay).last, matchesGoldenFile('reorderable_list_test.horizontal.drop_area.png'), ); debugDisableShadows = true; @@ -1238,6 +1242,53 @@ void main() { expect(itemsCreated, {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17}); }); + group('Padding', () { + testWidgets('Padding with no header', (WidgetTester tester) async { + const EdgeInsets padding = EdgeInsets.fromLTRB(10, 20, 30, 40); + + // Vertical + await tester.pumpWidget(build(padding: padding)); + expect(tester.getRect(find.byKey(const Key('Item 1'))), const Rect.fromLTRB(10, 20, 770, 68)); + expect(tester.getRect(find.byKey(const Key('Item 4'))), const Rect.fromLTRB(10, 164, 770, 212)); + + // Horizontal + await tester.pumpWidget(build(padding: padding, scrollDirection: Axis.horizontal)); + expect(tester.getRect(find.byKey(const Key('Item 1'))), const Rect.fromLTRB(10, 20, 58, 560)); + expect(tester.getRect(find.byKey(const Key('Item 4'))), const Rect.fromLTRB(154, 20, 202, 560)); + }); + + testWidgets('Padding with header', (WidgetTester tester) async { + const EdgeInsets padding = EdgeInsets.fromLTRB(10, 20, 30, 40); + const Key headerKey = Key('Header'); + const Widget verticalHeader = SizedBox(key: headerKey, height: 10); + const Widget horizontalHeader = SizedBox(key: headerKey, width: 10); + + // Vertical + await tester.pumpWidget(build(padding: padding, header: verticalHeader)); + expect(tester.getRect(find.byKey(headerKey)), const Rect.fromLTRB(10, 20, 770, 30)); + expect(tester.getRect(find.byKey(const Key('Item 1'))), const Rect.fromLTRB(10, 30, 770, 78)); + expect(tester.getRect(find.byKey(const Key('Item 4'))), const Rect.fromLTRB(10, 174, 770, 222)); + + // Vertical, reversed + await tester.pumpWidget(build(padding: padding, header: verticalHeader, reverse: true)); + expect(tester.getRect(find.byKey(headerKey)), const Rect.fromLTRB(10, 550, 770, 560)); + expect(tester.getRect(find.byKey(const Key('Item 1'))), const Rect.fromLTRB(10, 502, 770, 550)); + expect(tester.getRect(find.byKey(const Key('Item 4'))), const Rect.fromLTRB(10, 358, 770, 406)); + + // Horizontal + await tester.pumpWidget(build(padding: padding, header: horizontalHeader, scrollDirection: Axis.horizontal)); + expect(tester.getRect(find.byKey(headerKey)), const Rect.fromLTRB(10, 20, 20, 560)); + expect(tester.getRect(find.byKey(const Key('Item 1'))), const Rect.fromLTRB(20, 20, 68, 560)); + expect(tester.getRect(find.byKey(const Key('Item 4'))), const Rect.fromLTRB(164, 20, 212, 560)); + + // Horizontal, reversed + await tester.pumpWidget(build(padding: padding, header: horizontalHeader, scrollDirection: Axis.horizontal, reverse: true)); + expect(tester.getRect(find.byKey(headerKey)), const Rect.fromLTRB(760, 20, 770, 560)); + expect(tester.getRect(find.byKey(const Key('Item 1'))), const Rect.fromLTRB(712, 20, 760, 560)); + expect(tester.getRect(find.byKey(const Key('Item 4'))), const Rect.fromLTRB(568, 20, 616, 560)); + }); + }); + testWidgets('ReorderableListView can be reversed', (WidgetTester tester) async { final Widget reorderableListView = ReorderableListView( children: const [