fix(ObserverController): correct the scrolling clamped by an outdated scrollExtent

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,
   });
 }

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<void> _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,
     );
   }
 

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(

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<void> 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(