diff --git a/lib/example/sticky_header.dart b/lib/example/sticky_header.dart index 57c7022123..fc6f42cd64 100644 --- a/lib/example/sticky_header.dart +++ b/lib/example/sticky_header.dart @@ -17,10 +17,8 @@ /// so as to set the app ID differently. library; -// ignore: undefined_hidden_name // anticipates https://github.com/flutter/flutter/pull/164818 -import 'package:flutter/material.dart' hide SliverPaintOrder; +import 'package:flutter/material.dart'; -import '../widgets/scrolling.dart'; import '../widgets/sticky_header.dart'; /// Example page using [StickyHeaderListView] and [StickyHeaderItem] in a @@ -153,7 +151,7 @@ class ExampleVerticalDouble extends StatelessWidget { return Scaffold( appBar: AppBar(title: Text(title)), - body: CustomPaintOrderScrollView( + body: CustomScrollView( semanticChildCount: numSections, center: centerKey, paintOrder: headerAtBottom ? diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 5f4c24c77c..03ae763bdc 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -1,8 +1,7 @@ import 'dart:math'; import 'package:collection/collection.dart'; -// ignore: undefined_hidden_name // anticipates https://github.com/flutter/flutter/pull/164818 -import 'package:flutter/material.dart' hide SliverPaintOrder; +import 'package:flutter/material.dart'; import 'package:flutter_color_models/flutter_color_models.dart'; import 'package:intl/intl.dart' hide TextDirection; diff --git a/lib/widgets/scrolling.dart b/lib/widgets/scrolling.dart index d5a1447763..69f46082b3 100644 --- a/lib/widgets/scrolling.dart +++ b/lib/widgets/scrolling.dart @@ -43,212 +43,6 @@ class _SingleChildScrollViewWithScrollbarState } } -/// 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, - }; - } -} - /// A version of [ScrollPosition] adapted for the Zulip message list, /// used by [MessageListScrollController]. class MessageListScrollPosition extends ScrollPositionWithSingleContext { @@ -403,7 +197,7 @@ class MessageListScrollController extends ScrollController { /// /// This lets us customize behavior in ways that aren't currently supported /// by the fields of [CustomScrollView] itself. -class MessageListScrollView extends CustomPaintOrderScrollView { +class MessageListScrollView extends CustomScrollView { const MessageListScrollView({ super.key, super.scrollDirection, @@ -435,13 +229,13 @@ class MessageListScrollView extends CustomPaintOrderScrollView { cacheExtent: cacheExtent, center: center, clipBehavior: clipBehavior, - paintOrder_: paintOrder_, + paintOrder: paintOrder, ); } } /// The version of [Viewport] that underlies [MessageListScrollView]. -class MessageListViewport extends CustomPaintOrderViewport { +class MessageListViewport extends Viewport { MessageListViewport({ super.key, super.axisDirection, @@ -452,7 +246,7 @@ class MessageListViewport extends CustomPaintOrderViewport { super.cacheExtentStyle, super.slivers, super.clipBehavior, - required super.paintOrder_, + required super.paintOrder, }); @override @@ -465,7 +259,7 @@ class MessageListViewport extends CustomPaintOrderViewport { cacheExtent: cacheExtent, cacheExtentStyle: cacheExtentStyle, clipBehavior: clipBehavior, - paintOrder_: paintOrder_, + paintOrder: paintOrder, ); } } @@ -474,7 +268,7 @@ class MessageListViewport extends CustomPaintOrderViewport { /// and [MessageListScrollView]. // TODO(upstream): Devise upstream APIs to obviate the duplicated code here; // use `git log -L` to see what edits we've made locally. -class RenderMessageListViewport extends RenderCustomPaintOrderViewport { +class RenderMessageListViewport extends RenderViewport { RenderMessageListViewport({ super.axisDirection, required super.crossAxisDirection, @@ -484,7 +278,7 @@ class RenderMessageListViewport extends RenderCustomPaintOrderViewport { super.cacheExtent, super.cacheExtentStyle, super.clipBehavior, - required super.paintOrder_, + required super.paintOrder, }); @override diff --git a/lib/widgets/sticky_header.dart b/lib/widgets/sticky_header.dart index 8547229030..fcf3fdb4b3 100644 --- a/lib/widgets/sticky_header.dart +++ b/lib/widgets/sticky_header.dart @@ -329,9 +329,8 @@ enum _HeaderGrowthPlacement { /// then this [SliverStickyHeaderList] must paint after any slivers that appear /// to the right of this sliver. /// -/// To control the viewport's paint order, -/// consider using [CustomPaintOrderScrollView] instead of [CustomScrollView]. -/// Then [SliverPaintOrder.firstIsTop] for [HeaderPlacement.scrollingStart], +/// To control the viewport's paint order, use [ScrollView.paintOrder]. +/// There [SliverPaintOrder.firstIsTop] for [HeaderPlacement.scrollingStart], /// or [SliverPaintOrder.lastIsTop] for [HeaderPlacement.scrollingEnd], /// suffices for meeting the needs above. class SliverStickyHeaderList extends RenderObjectWidget { diff --git a/pubspec.lock b/pubspec.lock index 191c48a987..f9401bbaf3 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -650,26 +650,26 @@ packages: dependency: transitive description: name: leak_tracker - sha256: "6bb818ecbdffe216e81182c2f0714a2e62b593f4a4f13098713ff1685dfb6ab0" + sha256: "8dcda04c3fc16c14f48a7bb586d4be1f0d1572731b6d81d51772ef47c02081e0" url: "https://pub.dev" source: hosted - version: "10.0.9" + version: "11.0.1" leak_tracker_flutter_testing: dependency: transitive description: name: leak_tracker_flutter_testing - sha256: f8b613e7e6a13ec79cfdc0e97638fddb3ab848452eff057653abd3edba760573 + sha256: "1dbc140bb5a23c75ea9c4811222756104fbcd1a27173f0c34ca01e16bea473c1" url: "https://pub.dev" source: hosted - version: "3.0.9" + version: "3.0.10" leak_tracker_testing: dependency: transitive description: name: leak_tracker_testing - sha256: "6ba465d5d76e67ddf503e1161d1f4a6bc42306f9d66ca1e8f079a47290fb06d3" + sha256: "8d5a2d49f4a66b49744b23b018848400d23e54caf9463f4eb20df3eb8acb2eb1" url: "https://pub.dev" source: hosted - version: "3.0.1" + version: "3.0.2" legacy_checks: dependency: "direct dev" description: @@ -1355,5 +1355,5 @@ packages: source: path version: "0.0.1" sdks: - dart: ">=3.8.0-265.0.dev <4.0.0" - flutter: ">=3.32.0-1.0.pre.87" + dart: ">=3.9.0-55.0.dev <4.0.0" + flutter: ">=3.32.0-1.0.pre.257" diff --git a/pubspec.yaml b/pubspec.yaml index 59117b3ab8..95d05d41c8 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -14,8 +14,8 @@ environment: # We use a recent version of Flutter from its main channel, and # the corresponding recent version of the Dart SDK. # Feel free to update these regularly; see README.md for instructions. - sdk: '>=3.8.0-265.0.dev <4.0.0' - flutter: '>=3.32.0-1.0.pre.87' # 427f8c8e1569e1d2dba4583ef27cc06baa744108 + sdk: '>=3.9.0-55.0.dev <4.0.0' + flutter: '>=3.32.0-1.0.pre.257' # b90818ec53ac6f774cdeebd2acd9ab8e71b5c7b5 # To update dependencies, see instructions in README.md. dependencies: diff --git a/test/widgets/scrolling_test.dart b/test/widgets/scrolling_test.dart index 9454ef4fe3..8f1820e6ff 100644 --- a/test/widgets/scrolling_test.dart +++ b/test/widgets/scrolling_test.dart @@ -1,132 +1,11 @@ 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/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:zulip/widgets/scrolling.dart'; import '../flutter_checks.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]); - }); - - 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); - }, skip: true, // TODO(upstream): once PR 164818 lands, cut our CustomPaintOrderScrollView - // in favor of CustomScrollView; this test was checking that - // the former matched the latter's old default behavior. - ); - }); - - 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]); - }); - - 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)); - }, skip: true, // TODO(upstream): once PR 164818 lands, cut our CustomPaintOrderScrollView - // in favor of CustomScrollView; this test was checking that - // the former matched the latter's old default behavior. - ); - }); - group('MessageListScrollView', () { Widget buildList({ MessageListScrollController? controller, @@ -309,64 +188,3 @@ void main() { }); }); } - -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 6ff94c02d5..ac7ae4819a 100644 --- a/test/widgets/sticky_header_test.dart +++ b/test/widgets/sticky_header_test.dart @@ -4,12 +4,9 @@ import 'package:checks/checks.dart'; import 'package:collection/collection.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/gestures.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/rendering.dart'; +import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'package:zulip/widgets/scrolling.dart'; import 'package:zulip/widgets/sticky_header.dart'; void main() { @@ -236,7 +233,7 @@ void main() { const centerKey = ValueKey('center'); final controller = ScrollController(); await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr, - child: CustomPaintOrderScrollView( + child: CustomScrollView( controller: controller, anchor: 0.0, center: centerKey, @@ -341,7 +338,7 @@ Future _checkSequence( anchor = 0.0; } - SliverPaintOrder paintOrder = SliverPaintOrder.centerTopFirstBottom; + SliverPaintOrder paintOrder = SliverPaintOrder.firstIsTop; if (!allowOverflow || (sliverConfig == _SliverConfig.single)) { // The paint order doesn't matter. } else { @@ -352,7 +349,7 @@ Future _checkSequence( final controller = ScrollController(); await tester.pumpWidget(Directionality( textDirection: textDirection ?? TextDirection.rtl, - child: CustomPaintOrderScrollView( + child: CustomScrollView( controller: controller, scrollDirection: axis, reverse: reverse,