diff --git a/lib/example/sticky_header.dart b/lib/example/sticky_header.dart index b1dadd479e..57c7022123 100644 --- a/lib/example/sticky_header.dart +++ b/lib/example/sticky_header.dart @@ -17,8 +17,10 @@ /// so as to set the app ID differently. library; -import 'package:flutter/material.dart'; +// ignore: undefined_hidden_name // anticipates https://github.com/flutter/flutter/pull/164818 +import 'package:flutter/material.dart' hide SliverPaintOrder; +import '../widgets/scrolling.dart'; import '../widgets/sticky_header.dart'; /// Example page using [StickyHeaderListView] and [StickyHeaderItem] in a @@ -126,11 +128,13 @@ class ExampleVerticalDouble extends StatelessWidget { required this.title, // this.reverse = false, required this.headerPlacement, + required this.topSliverGrowsUpward, }); final String title; // final bool reverse; final HeaderPlacement headerPlacement; + final bool topSliverGrowsUpward; @override Widget build(BuildContext context) { @@ -144,19 +148,16 @@ class ExampleVerticalDouble extends StatelessWidget { HeaderPlacement.scrollingEnd => true, }; - // Choose the "center" sliver so that the sliver which might need to paint - // a header overflowing the other header is the sliver that paints last. - final centerKey = headerAtBottom ? + final centerKey = topSliverGrowsUpward ? const ValueKey('bottom') : const ValueKey('top'); - // This is a side effect of our choice of centerKey. - final topSliverGrowsUpward = headerAtBottom; - return Scaffold( appBar: AppBar(title: Text(title)), - body: CustomScrollView( + body: CustomPaintOrderScrollView( semanticChildCount: numSections, center: centerKey, + paintOrder: headerAtBottom ? + SliverPaintOrder.lastIsTop : SliverPaintOrder.firstIsTop, slivers: [ SliverStickyHeaderList( key: const ValueKey('top'), @@ -345,11 +346,19 @@ class MainPage extends StatelessWidget { title: 'Double slivers, headers at top', page: ExampleVerticalDouble( title: 'Double slivers, headers at top', + topSliverGrowsUpward: false, + headerPlacement: HeaderPlacement.scrollingStart)), + _buildButton(context, + title: 'Split slivers, headers at top', + page: ExampleVerticalDouble( + title: 'Split slivers, headers at top', + topSliverGrowsUpward: true, headerPlacement: HeaderPlacement.scrollingStart)), _buildButton(context, - title: 'Double slivers, headers at bottom', + title: 'Split slivers, headers at bottom', page: ExampleVerticalDouble( - title: 'Double slivers, headers at bottom', + title: 'Split slivers, headers at bottom', + topSliverGrowsUpward: true, headerPlacement: HeaderPlacement.scrollingEnd)), ]; return Scaffold( diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index c949602824..5f2a059a69 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -22,6 +22,7 @@ import 'inset_shadow.dart'; import 'lightbox.dart'; import 'message_list.dart'; import 'poll.dart'; +import 'scrolling.dart'; import 'store.dart'; import 'text.dart'; @@ -796,33 +797,6 @@ class _CodeBlockContainer extends StatelessWidget { } } -class SingleChildScrollViewWithScrollbar extends StatefulWidget { - const SingleChildScrollViewWithScrollbar( - {super.key, required this.scrollDirection, required this.child}); - - final Axis scrollDirection; - final Widget child; - - @override - State createState() => - _SingleChildScrollViewWithScrollbarState(); -} - -class _SingleChildScrollViewWithScrollbarState - extends State { - final ScrollController controller = ScrollController(); - - @override - Widget build(BuildContext context) { - return Scrollbar( - controller: controller, - child: SingleChildScrollView( - controller: controller, - scrollDirection: widget.scrollDirection, - child: widget.child)); - } -} - class MathBlock extends StatelessWidget { const MathBlock({super.key, required this.node}); diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 7a95d10177..1e95a1be49 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -1,7 +1,8 @@ import 'dart:math'; import 'package:collection/collection.dart'; -import 'package:flutter/material.dart'; +// ignore: undefined_hidden_name // anticipates https://github.com/flutter/flutter/pull/164818 +import 'package:flutter/material.dart' hide SliverPaintOrder; import 'package:flutter_color_models/flutter_color_models.dart'; import 'package:intl/intl.dart' hide TextDirection; @@ -21,6 +22,7 @@ import 'emoji_reaction.dart'; import 'icons.dart'; import 'page.dart'; import 'profile.dart'; +import 'scrolling.dart'; import 'sticky_header.dart'; import 'store.dart'; import 'text.dart'; @@ -629,7 +631,7 @@ class _MessageListState extends State with PerAccountStoreAwareStat sliver = SliverSafeArea(sliver: sliver); } - return CustomScrollView( + return CustomPaintOrderScrollView( // TODO: Offer `ScrollViewKeyboardDismissBehavior.interactive` (or // similar) if that is ever offered: // https://github.com/flutter/flutter/issues/57609#issuecomment-1355340849 @@ -645,6 +647,7 @@ class _MessageListState extends State with PerAccountStoreAwareStat semanticChildCount: length + 2, anchor: 1.0, center: centerSliverKey, + paintOrder: SliverPaintOrder.firstIsTop, slivers: [ sliver, diff --git a/lib/widgets/scrolling.dart b/lib/widgets/scrolling.dart new file mode 100644 index 0000000000..7ceff8b745 --- /dev/null +++ b/lib/widgets/scrolling.dart @@ -0,0 +1,246 @@ +import 'package:flutter/material.dart'; +import 'package:flutter/rendering.dart'; + +/// A [SingleChildScrollView] that always shows a Material [Scrollbar]. +/// +/// This differs from the behavior provided by [MaterialScrollBehavior] in that +/// (a) the scrollbar appears even when [scrollDirection] is [Axis.horizontal], +/// and (b) the scrollbar appears on all platforms, rather than only on +/// desktop platforms. +// TODO(upstream): SingleChildScrollView should have a scrollBehavior field +// and pass it on to Scrollable, just like ScrollView does; then this would +// be covered by using that. +// TODO: Maybe show scrollbar only on mobile platforms, like MaterialScrollBehavior +// and the base ScrollBehavior do? +class SingleChildScrollViewWithScrollbar extends StatefulWidget { + const SingleChildScrollViewWithScrollbar( + {super.key, required this.scrollDirection, required this.child}); + + final Axis scrollDirection; + final Widget child; + + @override + State createState() => + _SingleChildScrollViewWithScrollbarState(); +} + +class _SingleChildScrollViewWithScrollbarState + extends State { + final ScrollController controller = ScrollController(); + + @override + Widget build(BuildContext context) { + return Scrollbar( + controller: controller, + child: SingleChildScrollView( + controller: controller, + scrollDirection: widget.scrollDirection, + child: widget.child)); + } +} + +/// Specifies an order in which to paint the slivers of a [CustomScrollView]. +/// +/// Whichever order the slivers are painted in, +/// they will be hit-tested in the opposite order. +/// +/// This can also be thought of as an ordering in the z-direction: +/// whichever sliver is painted last (and hit-tested first) is on top, +/// because it will paint over other slivers if there is overlap. +/// Similarly, whichever sliver is painted first (and hit-tested last) +/// is on the bottom. +enum SliverPaintOrder { + /// The first sliver paints on top, and the last sliver on bottom. + /// + /// The slivers are painted in the reverse order of [CustomScrollView.slivers], + /// and hit-tested in the same order as [CustomScrollView.slivers]. + firstIsTop, + + /// The last sliver paints on top, and the first sliver on bottom. + /// + /// The slivers are painted in the same order as [CustomScrollView.slivers], + /// and hit-tested in the reverse order. + lastIsTop, + + /// The default order for [CustomScrollView]: the center sliver paints on top, + /// and the first sliver paints on bottom. + /// + /// If [CustomScrollView.center] is null or corresponds to the first sliver + /// in [CustomScrollView.slivers], this order is equivalent to [firstIsTop]. + /// Otherwise, the [CustomScrollView.center] sliver paints on top; + /// it's followed in the z-order by the slivers after it to the end + /// of the list, then the slivers before the center in reverse order, + /// with the first sliver in the list at the bottom in the z-direction. + centerTopFirstBottom, +} + +/// A [CustomScrollView] with control over the paint order, or z-order, +/// between slivers. +/// +/// This is just like [CustomScrollView] except it adds the [paintOrder_] field. +/// +/// (Actually there's one [CustomScrollView] feature this doesn't implement: +/// [shrinkWrap] always has its default value of false. That feature would be +/// easy to add if desired.) +// TODO(upstream): Pending PR: https://github.com/flutter/flutter/pull/164818 +// Notes from before sending that PR: +// Add an option [ScrollView.zOrder]? (An enum, or possibly +// a delegate.) Or at minimum document on [ScrollView.center] the +// existing behavior, which is counterintuitive. +// Nearest related upstream feature requests I find are for a "z-index", +// for CustomScrollView, Column, Row, and Stack respectively: +// https://github.com/flutter/flutter/issues/121173#issuecomment-1712825747 +// https://github.com/flutter/flutter/issues/121173 +// https://github.com/flutter/flutter/issues/121173#issuecomment-1914959184 +// https://github.com/flutter/flutter/issues/70836 +// A delegate would give enough flexibility for that and much else, +// but I'm not sure how many use cases wouldn't be covered by a small enum. +// +// Ah, and here's a more on-point issue (more recently): +// https://github.com/flutter/flutter/issues/145592 +// +// TODO: perhaps sticky_header should configure a CustomPaintOrderScrollView automatically? +class CustomPaintOrderScrollView extends CustomScrollView { + const CustomPaintOrderScrollView({ + super.key, + super.scrollDirection, + super.reverse, + super.controller, + super.primary, + super.physics, + super.scrollBehavior, + // super.shrinkWrap, // omitted, always false + super.center, + super.anchor, + super.cacheExtent, + super.slivers, + super.semanticChildCount, + super.dragStartBehavior, + super.keyboardDismissBehavior, + super.restorationId, + super.clipBehavior, + super.hitTestBehavior, + SliverPaintOrder paintOrder = SliverPaintOrder.centerTopFirstBottom, + }) : paintOrder_ = paintOrder; + + /// The order in which to paint the slivers; + /// equivalently, the order in which to arrange them in the z-direction. + /// + /// Whichever order the slivers are painted in, + /// they will be hit-tested in the opposite order. + /// + /// To think of this as an ordering in the z-direction: + /// whichever sliver is painted last (and hit-tested first) is on top, + /// because it will paint over other slivers if there is overlap. + /// Similarly, whichever sliver is painted first (and hit-tested last) + /// is on the bottom. + /// + /// This defaults to [SliverPaintOrder.centerTopFirstBottom], + /// the behavior of the [CustomScrollView] base class. + final SliverPaintOrder paintOrder_; + + @override + Widget buildViewport(BuildContext context, ViewportOffset offset, + AxisDirection axisDirection, List slivers) { + return CustomPaintOrderViewport( + axisDirection: axisDirection, + offset: offset, + slivers: slivers, + cacheExtent: cacheExtent, + center: center, + anchor: anchor, + clipBehavior: clipBehavior, + paintOrder_: paintOrder_, + ); + } +} + +/// The viewport configured by a [CustomPaintOrderScrollView]. +class CustomPaintOrderViewport extends Viewport { + CustomPaintOrderViewport({ + super.key, + super.axisDirection, + super.crossAxisDirection, + super.anchor, + required super.offset, + super.center, + super.cacheExtent, + super.cacheExtentStyle, + super.slivers, + super.clipBehavior, + required this.paintOrder_, + }); + + final SliverPaintOrder paintOrder_; + + @override + RenderViewport createRenderObject(BuildContext context) { + return RenderCustomPaintOrderViewport( + axisDirection: axisDirection, + crossAxisDirection: crossAxisDirection + ?? Viewport.getDefaultCrossAxisDirection(context, axisDirection), + anchor: anchor, + offset: offset, + cacheExtent: cacheExtent, + cacheExtentStyle: cacheExtentStyle, + clipBehavior: clipBehavior, + paintOrder_: paintOrder_, + ); + } +} + +/// The render object configured by a [CustomPaintOrderViewport]. +class RenderCustomPaintOrderViewport extends RenderViewport { + RenderCustomPaintOrderViewport({ + super.axisDirection, + required super.crossAxisDirection, + required super.offset, + super.anchor, + super.children, + super.center, + super.cacheExtent, + super.cacheExtentStyle, + super.clipBehavior, + required this.paintOrder_, + }); + + final SliverPaintOrder paintOrder_; + + Iterable get _lastToFirst { + final List children = []; + RenderSliver? child = lastChild; + while (child != null) { + children.add(child); + child = childBefore(child); + } + return children; + } + + Iterable get _firstToLast { + final List children = []; + RenderSliver? child = firstChild; + while (child != null) { + children.add(child); + child = childAfter(child); + } + return children; + } + + @override + Iterable get childrenInPaintOrder { + return switch (paintOrder_) { + SliverPaintOrder.firstIsTop => _lastToFirst, + SliverPaintOrder.lastIsTop => _firstToLast, + SliverPaintOrder.centerTopFirstBottom => super.childrenInPaintOrder, + }; + } + + @override + Iterable get childrenInHitTestOrder { + return switch (paintOrder_) { + SliverPaintOrder.firstIsTop => _firstToLast, + SliverPaintOrder.lastIsTop => _lastToFirst, + SliverPaintOrder.centerTopFirstBottom => super.childrenInHitTestOrder, + }; + } +} diff --git a/lib/widgets/sticky_header.dart b/lib/widgets/sticky_header.dart index d9d9a738dc..8547229030 100644 --- a/lib/widgets/sticky_header.dart +++ b/lib/widgets/sticky_header.dart @@ -318,6 +318,8 @@ enum _HeaderGrowthPlacement { /// When the list item that controls the sticky header has /// [StickyHeaderItem.allowOverflow] true, the header will be permitted /// to overflow not only the item but this whole sliver. +/// (This provides seamless behavior if, for example, two back-to-back slivers +/// are used for implementing a double-ended scrollable list.) /// /// The caller is responsible for arranging the paint order between slivers /// so that this works correctly: a sliver that might overflow must be painted @@ -327,12 +329,11 @@ enum _HeaderGrowthPlacement { /// then this [SliverStickyHeaderList] must paint after any slivers that appear /// to the right of this sliver. /// -/// At present there's no off-the-shelf way to fully control the paint order -/// between slivers. -/// See the implementation of [RenderViewport.childrenInPaintOrder] for the -/// paint order provided by [CustomScrollView]; it meets the above needs -/// for some arrangements of slivers and values of [headerPlacement], -/// but not others. +/// To control the viewport's paint order, +/// consider using [CustomPaintOrderScrollView] instead of [CustomScrollView]. +/// Then [SliverPaintOrder.firstIsTop] for [HeaderPlacement.scrollingStart], +/// or [SliverPaintOrder.lastIsTop] for [HeaderPlacement.scrollingEnd], +/// suffices for meeting the needs above. class SliverStickyHeaderList extends RenderObjectWidget { SliverStickyHeaderList({ super.key, diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index bd54412a46..6d2fdeef27 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -103,9 +103,11 @@ void main() { .findsOne(); } + ScrollView findScrollView(WidgetTester tester) => + tester.widget(find.bySubtype()); + ScrollController? findMessageListScrollController(WidgetTester tester) { - final scrollView = tester.widget(find.byType(CustomScrollView)); - return scrollView.controller; + return findScrollView(tester).controller; } group('MessageListPage', () { @@ -368,7 +370,7 @@ void main() { group('fetch older messages on scroll', () { int? itemCount(WidgetTester tester) => - tester.widget(find.byType(CustomScrollView)).semanticChildCount; + findScrollView(tester).semanticChildCount; testWidgets('basic', (tester) async { await setupMessageListPage(tester, foundOldest: false, diff --git a/test/widgets/scrolling_test.dart b/test/widgets/scrolling_test.dart new file mode 100644 index 0000000000..bfb010ccf0 --- /dev/null +++ b/test/widgets/scrolling_test.dart @@ -0,0 +1,191 @@ +import 'package:checks/checks.dart'; +// ignore: undefined_hidden_name // anticipates https://github.com/flutter/flutter/pull/164818 +import 'package:flutter/rendering.dart' hide SliverPaintOrder; +// ignore: undefined_hidden_name // anticipates https://github.com/flutter/flutter/pull/164818 +import 'package:flutter/widgets.dart' hide SliverPaintOrder; +import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/widgets/scrolling.dart'; + +void main() { + group('CustomPaintOrderScrollView paint order', () { + final paintLog = []; + + Widget makeSliver(int i) { + return SliverToBoxAdapter( + key: ValueKey(i), + child: CustomPaint( + painter: TestCustomPainter() + ..onPaint = (_, _) => paintLog.add(i), + child: Text('Item $i'))); + } + + testWidgets('firstIsTop', (tester) async { + addTearDown(paintLog.clear); + await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr, + child: CustomPaintOrderScrollView( + paintOrder: SliverPaintOrder.firstIsTop, + center: ValueKey(2), anchor: 0.5, + slivers: List.generate(5, makeSliver)))); + + // First sliver paints last, over other slivers; last sliver paints first. + check(paintLog).deepEquals([4, 3, 2, 1, 0]); + }); + + testWidgets('lastIsTop', (tester) async { + addTearDown(paintLog.clear); + await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr, + child: CustomPaintOrderScrollView( + paintOrder: SliverPaintOrder.lastIsTop, + center: ValueKey(2), anchor: 0.5, + slivers: List.generate(5, makeSliver)))); + + // Last sliver paints last, over other slivers; first sliver paints first. + check(paintLog).deepEquals([0, 1, 2, 3, 4]); + }); + + // This test will fail if a corresponding upstream PR lands: + // https://github.com/flutter/flutter/pull/164818 + // because that eliminates the quirky centerTopFirstBottom behavior. + // In that case, skip this test for a quick fix; or go ahead and + // rip out CustomPaintOrderScrollView in favor of CustomScrollView. + // (Greg has a draft commit ready which does the latter.) + testWidgets('centerTopFirstBottom', (tester) async { + addTearDown(paintLog.clear); + await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr, + child: CustomPaintOrderScrollView( + paintOrder: SliverPaintOrder.centerTopFirstBottom, + center: ValueKey(2), anchor: 0.5, + slivers: List.generate(5, makeSliver)))); + + // The particular order CustomScrollView paints in. + check(paintLog).deepEquals([0, 1, 4, 3, 2]); + + // Check that CustomScrollView indeed paints in the same order. + final result = paintLog.toList(); + paintLog.clear(); + await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr, + child: CustomScrollView( + center: ValueKey(2), anchor: 0.5, + slivers: List.generate(5, makeSliver)))); + check(paintLog).deepEquals(result); + }); + }); + + group('CustomPaintOrderScrollView hit-test order', () { + Widget makeSliver(int i) { + return _AllOverlapSliver(key: ValueKey(i), id: i); + } + + List sliverIds(Iterable path) => [ + for (final e in path) + if (e.target case _RenderAllOverlapSliver(:final id)) + id, + ]; + + testWidgets('firstIsTop', (WidgetTester tester) async { + await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr, + child: CustomPaintOrderScrollView( + paintOrder: SliverPaintOrder.firstIsTop, + center: const ValueKey(2), anchor: 0.5, + slivers: List.generate(5, makeSliver)))); + + final result = tester.hitTestOnBinding(const Offset(400, 300)); + check(sliverIds(result.path)).deepEquals([0, 1, 2, 3, 4]); + }); + + testWidgets('lastIsTop', (WidgetTester tester) async { + await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr, + child: CustomPaintOrderScrollView( + paintOrder: SliverPaintOrder.lastIsTop, + center: const ValueKey(2), anchor: 0.5, + slivers: List.generate(5, makeSliver)))); + + final result = tester.hitTestOnBinding(const Offset(400, 300)); + check(sliverIds(result.path)).deepEquals([4, 3, 2, 1, 0]); + }); + + // This test will fail if the upstream PR 164818 lands. + // In that case the test is no longer needed and we'll take it out; + // see comment on other centerTopFirstBottom test above. + testWidgets('centerTopFirstBottom', (tester) async { + await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr, + child: CustomPaintOrderScrollView( + paintOrder: SliverPaintOrder.centerTopFirstBottom, + center: const ValueKey(2), anchor: 0.5, + slivers: List.generate(5, makeSliver)))); + + final result = tester.hitTestOnBinding(const Offset(400, 300)); + // The particular order CustomScrollView hit-tests in. + check(sliverIds(result.path)).deepEquals([2, 3, 4, 1, 0]); + + // Check that CustomScrollView indeed hit-tests in the same order. + await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr, + child: CustomScrollView( + center: const ValueKey(2), anchor: 0.5, + slivers: List.generate(5, makeSliver)))); + check(sliverIds(tester.hitTestOnBinding(const Offset(400, 300)).path)) + .deepEquals(sliverIds(result.path)); + }); + }); +} + +class TestCustomPainter extends CustomPainter { + void Function(Canvas canvas, Size size)? onPaint; + + @override + void paint(Canvas canvas, Size size) { + if (onPaint != null) onPaint!(canvas, size); + } + + @override + bool shouldRepaint(covariant CustomPainter oldDelegate) { + return true; + } +} + +/// A sliver that overlaps with other slivers as far as possible, +/// and does nothing else. +class _AllOverlapSliver extends LeafRenderObjectWidget { + const _AllOverlapSliver({super.key, required this.id}); + + final int id; + + @override + RenderObject createRenderObject(BuildContext context) => _RenderAllOverlapSliver(id); +} + +class _RenderAllOverlapSliver extends RenderSliver { + _RenderAllOverlapSliver(this.id); + + final int id; + + @override + void performLayout() { + geometry = SliverGeometry( + paintExtent: constraints.remainingPaintExtent, + maxPaintExtent: constraints.remainingPaintExtent, + layoutExtent: 0.0, + ); + } + + @override + bool hitTest( + SliverHitTestResult result, { + required double mainAxisPosition, + required double crossAxisPosition, + }) { + if (mainAxisPosition >= 0.0 && + mainAxisPosition < geometry!.hitTestExtent && + crossAxisPosition >= 0.0 && + crossAxisPosition < constraints.crossAxisExtent) { + result.add( + SliverHitTestEntry( + this, + mainAxisPosition: mainAxisPosition, + crossAxisPosition: crossAxisPosition, + ), + ); + } + return false; + } +} diff --git a/test/widgets/sticky_header_test.dart b/test/widgets/sticky_header_test.dart index affe75e8c6..6ff94c02d5 100644 --- a/test/widgets/sticky_header_test.dart +++ b/test/widgets/sticky_header_test.dart @@ -4,9 +4,12 @@ import 'package:checks/checks.dart'; import 'package:collection/collection.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/gestures.dart'; -import 'package:flutter/rendering.dart'; -import 'package:flutter/widgets.dart'; +// ignore: undefined_hidden_name // anticipates https://github.com/flutter/flutter/pull/164818 +import 'package:flutter/rendering.dart' hide SliverPaintOrder; +// ignore: undefined_hidden_name // anticipates https://github.com/flutter/flutter/pull/164818 +import 'package:flutter/widgets.dart' hide SliverPaintOrder; import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/widgets/scrolling.dart'; import 'package:zulip/widgets/sticky_header.dart'; void main() { @@ -230,19 +233,24 @@ void main() { }); testWidgets('hit-testing for header overflowing sliver', (tester) async { + const centerKey = ValueKey('center'); final controller = ScrollController(); await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr, - child: CustomScrollView( + child: CustomPaintOrderScrollView( controller: controller, + anchor: 0.0, + center: centerKey, + paintOrder: SliverPaintOrder.firstIsTop, slivers: [ SliverStickyHeaderList( headerPlacement: HeaderPlacement.scrollingStart, delegate: SliverChildListDelegate( List.generate(100, (i) => StickyHeaderItem( allowOverflow: true, - header: _Header(i, height: 20), - child: _Item(i, height: 100))))), + header: _Header(99 - i, height: 20), + child: _Item(99 - i, height: 100))))), SliverStickyHeaderList( + key: centerKey, headerPlacement: HeaderPlacement.scrollingStart, delegate: SliverChildListDelegate( List.generate(100, (i) => StickyHeaderItem( @@ -251,9 +259,8 @@ void main() { child: _Item(100 + i, height: 100))))), ]))); - const topExtent = 100 * 100; for (double topHeight in [5, 10, 15, 20]) { - controller.jumpTo(topExtent - topHeight); + controller.jumpTo(-topHeight); await tester.pump(); // The top sliver occupies height [topHeight]. // Its header overhangs by `20 - topHeight`. @@ -327,49 +334,34 @@ Future _checkSequence( ]; final double anchor; - bool paintOrderGood; if (reverseGrowth) { slivers.reverseRange(0, slivers.length); anchor = 1.0; - paintOrderGood = switch (sliverConfig) { - _SliverConfig.single => true, - // The last sliver will paint last. - _SliverConfig.backToBack => headerPlacement == HeaderPlacement.scrollingEnd, - // The last sliver will paint last. - _SliverConfig.followed => headerPlacement == HeaderPlacement.scrollingEnd, - }; } else { anchor = 0.0; - paintOrderGood = switch (sliverConfig) { - _SliverConfig.single => true, - // The last sliver will paint last. - _SliverConfig.backToBack => headerPlacement == HeaderPlacement.scrollingEnd, - // The first sliver will paint last. - _SliverConfig.followed => headerPlacement == HeaderPlacement.scrollingStart, - }; } - final skipBecausePaintOrder = allowOverflow && !paintOrderGood; - if (skipBecausePaintOrder) { - // TODO need to control paint order of slivers within viewport in order to - // make some configurations behave properly when headers overflow slivers - markTestSkipped('sliver paint order'); - // Don't return yet; we'll still check layout, and skip specific affected checks below. + SliverPaintOrder paintOrder = SliverPaintOrder.centerTopFirstBottom; + if (!allowOverflow || (sliverConfig == _SliverConfig.single)) { + // The paint order doesn't matter. + } else { + paintOrder = headerPlacement == HeaderPlacement.scrollingStart + ? SliverPaintOrder.firstIsTop : SliverPaintOrder.lastIsTop; } - final controller = ScrollController(); await tester.pumpWidget(Directionality( textDirection: textDirection ?? TextDirection.rtl, - child: CustomScrollView( + child: CustomPaintOrderScrollView( controller: controller, scrollDirection: axis, reverse: reverse, anchor: anchor, center: center, + paintOrder: paintOrder, slivers: slivers))); - final overallSize = tester.getSize(find.byType(CustomScrollView)); + final overallSize = tester.getSize(find.bySubtype()); final extent = overallSize.onAxis(axis); assert(extent % 100 == 0); assert(sliverScrollExtent - extent > 100); @@ -418,7 +410,6 @@ Future _checkSequence( check(insetExtent(find.byType(_Header))).equals(expectedHeaderInsetExtent); // Check the header gets hit when it should, and not when it shouldn't. - if (skipBecausePaintOrder) return; await tester.tapAt(headerInset(1)); await tester.tapAt(headerInset(expectedHeaderInsetExtent - 1)); check(_TapLogged.takeTapLog())..length.equals(2)