diff --git a/lib/src/common/models/observe_scroll_to_index_result_model.dart b/lib/src/common/models/observe_scroll_to_index_result_model.dart index 85d6fcf..ab8abe9 100644 --- a/lib/src/common/models/observe_scroll_to_index_result_model.dart +++ b/lib/src/common/models/observe_scroll_to_index_result_model.dart @@ -38,9 +38,18 @@ class ObservePrepareScrollToIndexModel { /// The offset of the target child widget on the main axis. double targetChildLayoutOffset; + /// Whether the remaining scrollable extent is enough to scroll to the + /// target child widget. + /// + /// It will be [false] when [calculateTargetLayoutOffset] has been clamped to + /// the maximum scrollable offset, which means the target child widget cannot + /// be reached by this scrolling. + bool isEnoughScroll; + ObservePrepareScrollToIndexModel({ required this.calculateTargetLayoutOffset, required this.precedingScrollExtent, required this.targetChildLayoutOffset, + this.isEnoughScroll = true, }); } diff --git a/lib/src/common/observer_controller.dart b/lib/src/common/observer_controller.dart index fdd968a..c367820 100644 --- a/lib/src/common/observer_controller.dart +++ b/lib/src/common/observer_controller.dart @@ -5,6 +5,7 @@ */ import 'dart:async'; +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; import 'package:scrollview_observer/scrollview_observer.dart'; @@ -249,6 +250,10 @@ mixin ObserverControllerForScroll on ObserverControllerForInfo { static const Duration _findingDuration = Duration(milliseconds: 1); static const Curve _findingCurve = Curves.ease; + /// The maximum number of times to correct the target offset when the + /// scrolling has been clamped by an outdated [SliverGeometry.scrollExtent]. + static const int _maxScrollToIndexCorrectionCount = 5; + /// Whether to cache the offset when jump to a specified index position. /// Defaults to true. bool cacheJumpIndexOffset = true; @@ -495,12 +500,17 @@ mixin ObserverControllerForScroll on ObserverControllerForInfo { padding: padding, offset: offset, ); - await _scrollTo( + await _scrollToWithCorrection( isAnimateTo: isAnimateTo, duration: duration, curve: curve, controller: _controller, + obj: obj, calcResult: calcResult, + childSize: targetScrollChildModel.size, + alignment: alignment, + padding: padding, + offset: offset, onPrepareScrollToIndex: onPrepareScrollToIndex, ); @@ -627,12 +637,17 @@ mixin ObserverControllerForScroll on ObserverControllerForInfo { offset: offset, ); childLayoutOffset = calcResult.calculateTargetLayoutOffset; - await _scrollTo( + await _scrollToWithCorrection( isAnimateTo: isAnimateTo, duration: isAnimateTo ? duration : null, curve: isAnimateTo ? curve : null, controller: _controller, + obj: obj, calcResult: calcResult, + childSize: childMainAxisSize, + alignment: alignment, + padding: padding, + offset: offset, onPrepareScrollToIndex: onPrepareScrollToIndex, ); _handleScrollEnd(context: ctx, completer: completer); @@ -878,6 +893,85 @@ mixin ObserverControllerForScroll on ObserverControllerForInfo { } } + /// Scrolling to the target offset, then correcting it when the scrolling has + /// been clamped by an outdated [SliverGeometry.scrollExtent]. + /// + /// A [RenderSliverMultiBoxAdaptor] may skip its layout phase when the number + /// of children changes but none of the existing children needs to be laid + /// out again. In that case its [SliverGeometry.scrollExtent] is still the + /// old one, so the target offset is clamped to the outdated + /// [ScrollPosition.maxScrollExtent] and the target child widget cannot be + /// reached. + /// + /// Fortunately, the scrolling itself changes [ScrollPosition.pixels], which + /// makes the sliver receive different [SliverConstraints] and be laid out + /// again, so a fresh [SliverGeometry.scrollExtent] is available in the next + /// frame. By recalculating and scrolling again until the target offset no + /// longer changes, the target child widget can be reached without asking the + /// developer to scroll twice. + /// https://github.com/fluttercandies/flutter_scrollview_observer/issues/150 + Future _scrollToWithCorrection({ + required bool isAnimateTo, + required Duration? duration, + required Curve? curve, + required ScrollController controller, + required RenderSliverMultiBoxAdaptor obj, + required ObservePrepareScrollToIndexModel calcResult, + required double childSize, + required double alignment, + required EdgeInsets padding, + required ObserverLocateIndexOffsetCallback? offset, + required ObserverOnPrepareScrollToIndex? onPrepareScrollToIndex, + }) async { + await _scrollTo( + isAnimateTo: isAnimateTo, + duration: duration, + curve: curve, + controller: controller, + calcResult: calcResult, + onPrepareScrollToIndex: onPrepareScrollToIndex, + ); + // The scrolling has been handled by the developer externally. + if (onPrepareScrollToIndex != null) return; + var result = calcResult; + var lastTargetOffset = result.calculateTargetLayoutOffset; + var correctionCount = 0; + while (!result.isEnoughScroll && + correctionCount < _maxScrollToIndexCorrectionCount) { + correctionCount++; + // Waiting for the sliver to be laid out again. + await WidgetsBinding.instance.endOfFrame; + if (!controller.hasClients || !obj.attached || obj.geometry == null) { + return; + } + // The layout offset of the target child widget never changes, only the + // scrollExtent of the sliver does. + result = _calculateTargetLayoutOffset( + obj: obj, + childLayoutOffset: result.targetChildLayoutOffset, + childSize: childSize, + alignment: alignment, + padding: padding, + offset: offset, + ); + // The target offset is stable, which means the current offset is already + // the real maximum scrollable offset. + if ((result.calculateTargetLayoutOffset - lastTargetOffset).abs() < + precisionErrorTolerance) { + return; + } + lastTargetOffset = result.calculateTargetLayoutOffset; + await _scrollTo( + isAnimateTo: isAnimateTo, + duration: duration, + curve: curve, + controller: controller, + calcResult: result, + onPrepareScrollToIndex: onPrepareScrollToIndex, + ); + } + } + /// Getting target safety layout offset for scrolling to index. /// This can avoid jitter. ObservePrepareScrollToIndexModel _calculateTargetLayoutOffset({ @@ -948,6 +1042,7 @@ mixin ObserverControllerForScroll on ObserverControllerForInfo { calculateTargetLayoutOffset: calculateTargetLayoutOffset, precedingScrollExtent: precedingScrollExtent, targetChildLayoutOffset: childLayoutOffset, + isEnoughScroll: isEnoughScroll, ); } diff --git a/test/grid_observer_test.dart b/test/grid_observer_test.dart index ca1fab1..ba5ff55 100644 --- a/test/grid_observer_test.dart +++ b/test/grid_observer_test.dart @@ -209,6 +209,96 @@ void main() { }); }); + group('Scroll to index after the item count changed', () { + // https://github.com/fluttercandies/flutter_scrollview_observer/issues/150 + // + // The sliver may skip its layout phase when the item count changes but + // none of the existing children needs to be laid out again, which makes + // its scrollExtent outdated, so the scrolling would be clamped to the + // outdated maxScrollExtent. + Widget getGridViewWithItemCount({ + required ScrollController scrollController, + required int itemCount, + required double itemExtent, + required int crossAxisCount, + required double spacing, + }) { + return Directionality( + textDirection: TextDirection.ltr, + child: GridView.custom( + controller: scrollController, + gridDelegate: SliverGridDelegateWithFixedCrossAxisCount( + crossAxisCount: crossAxisCount, + mainAxisSpacing: spacing, + crossAxisSpacing: spacing, + mainAxisExtent: itemExtent, + ), + childrenDelegate: SliverChildBuilderDelegate( + // Building the same widget for the same index on purpose, so that + // no existing child needs to be laid out again after the rebuild. + (ctx, index) => Center(child: Text('index -- $index')), + childCount: itemCount, + ), + ), + ); + } + + testWidgets('Jump to the last index', (tester) async { + const crossAxisCount = 4; + const spacing = 5.0; + const itemExtent = 150.0; + const viewportHeight = 360.0; + tester.view.physicalSize = const Size(800, viewportHeight); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.reset); + + final scrollController = ScrollController(); + final observerController = GridObserverController( + controller: scrollController, + ); + + var itemCount = 20; + late StateSetter setStateFn; + Widget widget = StatefulBuilder(builder: (context, setState) { + setStateFn = setState; + return getGridViewWithItemCount( + scrollController: scrollController, + itemCount: itemCount, + itemExtent: itemExtent, + crossAxisCount: crossAxisCount, + spacing: spacing, + ); + }); + widget = GridViewObserver( + child: widget, + controller: observerController, + ); + await tester.pumpWidget(widget); + await tester.pumpAndSettle(); + + final maxScrollExtentBefore = scrollController.position.maxScrollExtent; + + itemCount = 40; + setStateFn(() {}); + await tester.pumpAndSettle(); + + // The maxScrollExtent is outdated here, that is the root cause. + expect(scrollController.position.maxScrollExtent, maxScrollExtentBefore); + + observerController.jumpTo(index: itemCount - 1, isFixedHeight: true); + await tester.pumpAndSettle(); + + const rowCount = 10; + const realMaxScrollExtent = + rowCount * (itemExtent + spacing) - spacing - viewportHeight; + expect(scrollController.position.maxScrollExtent, realMaxScrollExtent); + expect(scrollController.offset, realMaxScrollExtent); + expect(find.text('index -- ${itemCount - 1}'), findsOneWidget); + + scrollController.dispose(); + }); + }); + testWidgets('Check displayPercentage', (tester) async { final scrollController = ScrollController(); final observerController = GridObserverController( diff --git a/test/list_observer_test.dart b/test/list_observer_test.dart index 94e8b13..7e16fd8 100644 --- a/test/list_observer_test.dart +++ b/test/list_observer_test.dart @@ -408,6 +408,102 @@ void main() { }); }); + group('Scroll to index after the item count changed', () { + // https://github.com/fluttercandies/flutter_scrollview_observer/issues/150 + // + // The sliver may skip its layout phase when the item count changes but + // none of the existing children needs to be laid out again, which makes + // its scrollExtent outdated, so the scrolling would be clamped to the + // outdated maxScrollExtent. + const double viewportHeight = 360; + const double itemHeight = 60; + const double separatorHeight = 8; + + // Building the same widget for the same index on purpose, so that no + // existing child needs to be laid out again after the rebuild. + Widget buildItem(BuildContext ctx, int index) => const SizedBox( + height: itemHeight, + child: Center(child: Text('item')), + ); + + Future testJumpToTheLastIndex( + WidgetTester tester, { + required bool isSeparated, + required double realMaxScrollExtent, + }) async { + tester.view.physicalSize = const Size(800, viewportHeight); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.reset); + + final scrollController = ScrollController(); + final observerController = ListObserverController( + controller: scrollController, + ); + + var itemCount = 20; + late StateSetter setStateFn; + Widget widget = StatefulBuilder(builder: (context, setState) { + setStateFn = setState; + return Directionality( + textDirection: TextDirection.ltr, + child: isSeparated + ? ListView.separated( + controller: scrollController, + itemBuilder: buildItem, + separatorBuilder: (ctx, index) => + const SizedBox(height: separatorHeight), + itemCount: itemCount, + ) + : ListView.builder( + controller: scrollController, + itemBuilder: buildItem, + itemCount: itemCount, + ), + ); + }); + widget = ListViewObserver( + child: widget, + controller: observerController, + ); + await tester.pumpWidget(widget); + await tester.pumpAndSettle(); + + final maxScrollExtentBefore = scrollController.position.maxScrollExtent; + + itemCount = 40; + setStateFn(() {}); + await tester.pumpAndSettle(); + + // The maxScrollExtent is outdated here, that is the root cause. + expect(scrollController.position.maxScrollExtent, maxScrollExtentBefore); + + observerController.jumpTo(index: itemCount - 1, isFixedHeight: true); + await tester.pumpAndSettle(); + + expect(scrollController.position.maxScrollExtent, realMaxScrollExtent); + expect(scrollController.offset, realMaxScrollExtent); + + scrollController.dispose(); + } + + testWidgets('Jump to the last index in ListView', (tester) async { + await testJumpToTheLastIndex( + tester, + isSeparated: false, + realMaxScrollExtent: 40 * itemHeight - viewportHeight, + ); + }); + + testWidgets('Jump to the last index in separated ListView', (tester) async { + await testJumpToTheLastIndex( + tester, + isSeparated: true, + realMaxScrollExtent: + 40 * itemHeight + 39 * separatorHeight - viewportHeight, + ); + }); + }); + testWidgets('Check displayPercentage', (tester) async { final scrollController = ScrollController(); final observerController = ListObserverController(