From e9ea41a094d0837e0949024646de594b4c140ca3 Mon Sep 17 00:00:00 2001 From: LinXunFeng Date: Sat, 22 Aug 2026 14:52:52 +0800 Subject: [PATCH] fix(ObserverController): correct the scrolling clamped by an outdated 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, which leaves its SliverGeometry.scrollExtent outdated. The target offset is then clamped to the outdated ScrollPosition.maxScrollExtent, so the target child widget cannot be reached until scrolling to index a second time. The scrolling itself changes ScrollPosition.pixels, which makes the sliver be laid out again and report a fresh scrollExtent in the next frame. So expose isEnoughScroll from ObservePrepareScrollToIndexModel to tell whether the scrolling has been clamped, then recalculate and scroll again until the target offset no longer changes. It is applied to the two one-shot paths, the fixed height one and the one hitting the indexOffsetMap cache. The path of gradually scrolling around the target index location already converges by itself. Closes #150 --- .../observe_scroll_to_index_result_model.dart | 9 ++ lib/src/common/observer_controller.dart | 99 ++++++++++++++++++- test/grid_observer_test.dart | 90 +++++++++++++++++ test/list_observer_test.dart | 96 ++++++++++++++++++ 4 files changed, 292 insertions(+), 2 deletions(-) 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(