Merge pull request #1231 from AppFlowy-IO/fix/remote-update-scroll-jump

fix: prevent remote updates from scrolling local viewport

example/integration_test/remote_scroll_repro_test.dart

@@ -0,0 +1,33 @@
+import 'package:example/remote_scroll_repro.dart' as app;
+import 'package:flutter/widgets.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:integration_test/integration_test.dart';
+
+void main() {
+  IntegrationTestWidgetsFlutterBinding.ensureInitialized();
+
+  testWidgets('remote document updates do not move the local viewport',
+      (tester) async {
+    app.main();
+    await tester.pumpAndSettle();
+
+    await tester.tap(find.byKey(const ValueKey('reproduce_remote_scroll')));
+    await tester.pump();
+    await tester.pump(const Duration(milliseconds: 350));
+    await tester.pump(const Duration(milliseconds: 700));
+    await tester.pump(const Duration(seconds: 2));
+    await tester.pump();
+    await tester.pump(const Duration(milliseconds: 900));
+    await tester.pump();
+
+    final status = tester.widget<Text>(
+      find.byKey(const ValueKey('remote_scroll_repro_status')),
+    );
+    expect(
+      status.data,
+      'No auto-scroll request was observed.',
+      reason: 'A remote transaction may transform the local selection, but '
+          'must not make this client follow that selection.',
+    );
+  });
+}

example/lib/remote_scroll_repro.dart

@@ -0,0 +1,240 @@
+import 'dart:async';
+
+import 'package:appflowy_editor/appflowy_editor.dart';
+import 'package:flutter/material.dart';
+import 'package:flutter_localizations/flutter_localizations.dart';
+
+void main() {
+  runApp(const RemoteScrollReproApp());
+}
+
+class RemoteScrollReproApp extends StatelessWidget {
+  const RemoteScrollReproApp({super.key});
+
+  @override
+  Widget build(BuildContext context) {
+    return MaterialApp(
+      debugShowCheckedModeBanner: false,
+      localizationsDelegates: const [
+        GlobalMaterialLocalizations.delegate,
+        GlobalCupertinoLocalizations.delegate,
+        GlobalWidgetsLocalizations.delegate,
+        AppFlowyEditorLocalizations.delegate,
+      ],
+      supportedLocales: AppFlowyEditorLocalizations.delegate.supportedLocales,
+      home: const RemoteScrollReproPage(),
+    );
+  }
+}
+
+class RemoteScrollReproPage extends StatefulWidget {
+  const RemoteScrollReproPage({super.key});
+
+  @override
+  State<RemoteScrollReproPage> createState() => _RemoteScrollReproPageState();
+}
+
+class _RemoteScrollReproPageState extends State<RemoteScrollReproPage> {
+  static const _localCursorIndex = 8;
+  static const _viewportIndex = 14;
+  static const _remoteEditIndex = 30;
+
+  late final EditorState _editorState;
+  late final EditorScrollController _editorScrollController;
+
+  bool _isRunning = false;
+  int _remoteEditCount = 0;
+  String _status = 'Press “Reproduce” to run the two-client sequence.';
+  double? _offsetBeforeRemoteEdit;
+  double? _offsetAfterRemoteEdit;
+  (int, int)? _visibleRangeBeforeRemoteEdit;
+  (int, int)? _visibleRangeAfterRemoteEdit;
+  Offset? _autoScrollTarget;
+
+  @override
+  void initState() {
+    super.initState();
+    final nodes = List.generate(
+      80,
+      (index) => paragraphNode(
+        text: switch (index) {
+          _localCursorIndex =>
+            'LOCAL CURSOR — this position belongs to Client A',
+          _remoteEditIndex =>
+            'REMOTE EDIT TARGET — Client B will edit this paragraph',
+          _ => 'Paragraph $index — collaborative document content',
+        },
+      ),
+    );
+    _editorState = EditorState(
+      document: Document(root: pageNode(children: nodes)),
+    );
+    _editorScrollController = EditorScrollController(
+      editorState: _editorState,
+    );
+  }
+
+  @override
+  void dispose() {
+    _editorScrollController.dispose();
+    _editorState.dispose();
+    super.dispose();
+  }
+
+  Future<void> _reproduce() async {
+    if (_isRunning) {
+      return;
+    }
+    setState(() {
+      _isRunning = true;
+      _offsetBeforeRemoteEdit = null;
+      _offsetAfterRemoteEdit = null;
+      _visibleRangeBeforeRemoteEdit = null;
+      _visibleRangeAfterRemoteEdit = null;
+      _autoScrollTarget = null;
+      _status = 'Client A: placing the local cursor at paragraph 8…';
+    });
+
+    await _editorState.updateSelectionWithReason(
+      Selection.collapsed(Position(path: const [_localCursorIndex])),
+      reason: SelectionUpdateReason.uiEvent,
+    );
+    await Future<void>.delayed(const Duration(milliseconds: 350));
+
+    _editorScrollController.itemScrollController.jumpTo(
+      index: _viewportIndex,
+      alignment: 0,
+    );
+    await Future<void>.delayed(const Duration(milliseconds: 700));
+
+    _editorState.autoScroller?.stopAutoScroll();
+    _offsetBeforeRemoteEdit = _editorScrollController.offsetNotifier.value;
+    _visibleRangeBeforeRemoteEdit =
+        _editorScrollController.visibleRangeNotifier.value;
+    if (mounted) {
+      setState(() {
+        _status = 'Client A: viewport moved away from its cursor. '
+            'Client B edits in 2 seconds…';
+      });
+    }
+    await Future<void>.delayed(const Duration(seconds: 2));
+
+    final remotelyEditedNode =
+        _editorState.document.nodeAtPath(const [_remoteEditIndex]);
+    if (remotelyEditedNode == null) {
+      if (mounted) {
+        setState(() {
+          _isRunning = false;
+          _status = 'Unable to find the remote edit target.';
+        });
+      }
+      return;
+    }
+
+    _remoteEditCount++;
+    final transaction = _editorState.transaction
+      ..updateNode(
+        remotelyEditedNode,
+        {
+          ParagraphBlockKeys.delta: (Delta()
+                ..insert(
+                  'REMOTE EDIT #$_remoteEditCount — received from Client B',
+                ))
+              .toJson(),
+        },
+      );
+    await _editorState.apply(transaction, isRemote: true);
+    await Future<void>.delayed(const Duration(milliseconds: 900));
+
+    _offsetAfterRemoteEdit = _editorScrollController.offsetNotifier.value;
+    _visibleRangeAfterRemoteEdit =
+        _editorScrollController.visibleRangeNotifier.value;
+    _autoScrollTarget = _editorState.autoScroller?.lastOffset;
+    if (!mounted) {
+      return;
+    }
+    setState(() {
+      _isRunning = false;
+      _status = _autoScrollTarget == null
+          ? 'No auto-scroll request was observed.'
+          : 'BUG REPRODUCED: the remote edit requested local auto-scroll.';
+    });
+  }
+
+  @override
+  Widget build(BuildContext context) {
+    return Scaffold(
+      appBar: AppBar(
+        title: const Text('Remote edit viewport-jump reproducer'),
+        backgroundColor: const Color(0xFF5E2CA5),
+        foregroundColor: Colors.white,
+      ),
+      body: Column(
+        children: [
+          Material(
+            color: const Color(0xFFF1EAFE),
+            child: Padding(
+              padding: const EdgeInsets.all(16),
+              child: Row(
+                crossAxisAlignment: CrossAxisAlignment.start,
+                children: [
+                  FilledButton.icon(
+                    key: const ValueKey('reproduce_remote_scroll'),
+                    onPressed: _isRunning ? null : _reproduce,
+                    icon: const Icon(Icons.play_arrow),
+                    label: Text(_isRunning ? 'Running…' : 'Reproduce'),
+                  ),
+                  const SizedBox(width: 16),
+                  Expanded(
+                    child: Column(
+                      crossAxisAlignment: CrossAxisAlignment.start,
+                      children: [
+                        Text(
+                          key: const ValueKey('remote_scroll_repro_status'),
+                          _status,
+                          style: const TextStyle(fontWeight: FontWeight.w600),
+                        ),
+                        const SizedBox(height: 6),
+                        Text(
+                          'Visible blocks before: '
+                          '${_formatRange(_visibleRangeBeforeRemoteEdit)}  •  '
+                          'after: '
+                          '${_formatRange(_visibleRangeAfterRemoteEdit)}  •  '
+                          'scroll movement: ${_formatMovement()}  •  '
+                          'auto-scroll target: '
+                          '${_autoScrollTarget ?? 'none'}',
+                        ),
+                      ],
+                    ),
+                  ),
+                ],
+              ),
+            ),
+          ),
+          const Divider(height: 1),
+          Expanded(
+            child: AppFlowyEditor(
+              editorState: _editorState,
+              editorScrollController: _editorScrollController,
+              editorStyle: const EditorStyle.desktop(
+                padding: EdgeInsets.symmetric(horizontal: 80),
+              ),
+            ),
+          ),
+        ],
+      ),
+    );
+  }
+
+  String _formatRange((int, int)? range) =>
+      range == null ? '—' : '${range.$1}–${range.$2}';
+
+  String _formatMovement() {
+    final before = _offsetBeforeRemoteEdit;
+    final after = _offsetAfterRemoteEdit;
+    if (before == null || after == null) {
+      return '—';
+    }
+    return '${(after - before).toStringAsFixed(1)} px';
+  }
+}

example/pubspec.yaml

@@ -65,6 +65,8 @@ dependency_overrides:
 dev_dependencies:
   flutter_test:
     sdk: flutter
+  integration_test:
+    sdk: flutter
 
   # The "flutter_lints" package below contains a set of recommended lints to
   # encourage good coding practices. The lint set provided by the package is

lib/src/editor/editor_component/service/scroll_service_widget.dart

@@ -86,9 +86,12 @@ class _ScrollServiceWidgetState extends State<ScrollServiceWidget>
   void _onSelectionChanged() {
     // should auto scroll after the cursor or selection updated.
     final selection = editorState.selection;
+    final reason = editorState.selectionUpdateReason;
+    // Remote transactions rebase the local selection so it stays valid, but
+    // they must not make this client follow that selection in the viewport.
     if (selection == null ||
-        [SelectionUpdateReason.selectAll]
+        reason == SelectionUpdateReason.remote ||
-            .contains(editorState.selectionUpdateReason)) {
+        reason == SelectionUpdateReason.selectAll) {
       return;
     }
 

test/service/scroll_service_test.dart

@@ -1,5 +1,7 @@
+import 'package:appflowy_editor/appflowy_editor.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_test/flutter_test.dart';
+
 import '../new/infra/testable_editor.dart';
 
 void main() async {
@@ -28,5 +30,53 @@ void main() async {
       expect(itemFinder, findsOneWidget);
       await editor.dispose();
     });
+
+    testWidgets('remote document updates do not request local auto-scroll',
+        (tester) async {
+      final editor = tester.editor;
+      addTearDown(() async {
+        await tester.pumpWidget(const SizedBox.shrink());
+        editor.editorState.dispose();
+      });
+
+      for (var i = 0; i < 3; i++) {
+        editor.addParagraph(initialText: 'paragraph $i');
+      }
+      await editor.startTesting();
+      await editor.updateSelection(
+        Selection.collapsed(
+          Position(path: [0]),
+        ),
+      );
+
+      final localSelection = editor.selection;
+      final autoScroller = editor.editorState.autoScroller;
+      expect(autoScroller, isNotNull);
+      if (autoScroller == null) {
+        return;
+      }
+      autoScroller.stopAutoScroll();
+      expect(autoScroller.lastOffset, isNull);
+
+      final remotelyEditedNode = editor.nodeAtPath([1]);
+      expect(remotelyEditedNode, isNotNull);
+      if (remotelyEditedNode == null) {
+        return;
+      }
+      final transaction = editor.editorState.transaction
+        ..updateNode(
+          remotelyEditedNode,
+          {
+            ParagraphBlockKeys.delta:
+                (Delta()..insert('updated remotely')).toJson(),
+          },
+        );
+
+      await editor.editorState.apply(transaction, isRemote: true);
+      await tester.pump();
+
+      expect(editor.selection, localSelection);
+      expect(autoScroller.lastOffset, isNull);
+    });
   });
 }