fix: prevent remote updates from scrolling local viewport
This commit is contained in:
parent
41335c54ae
commit
a9697798f6
5 changed files with 330 additions and 2 deletions
33
example/integration_test/remote_scroll_repro_test.dart
Normal file
33
example/integration_test/remote_scroll_repro_test.dart
Normal file
|
|
@ -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.',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
}
|
||||||
240
example/lib/remote_scroll_repro.dart
Normal file
240
example/lib/remote_scroll_repro.dart
Normal file
|
|
@ -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';
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -65,6 +65,8 @@ dependency_overrides:
|
||||||
dev_dependencies:
|
dev_dependencies:
|
||||||
flutter_test:
|
flutter_test:
|
||||||
sdk: flutter
|
sdk: flutter
|
||||||
|
integration_test:
|
||||||
|
sdk: flutter
|
||||||
|
|
||||||
# The "flutter_lints" package below contains a set of recommended lints to
|
# The "flutter_lints" package below contains a set of recommended lints to
|
||||||
# encourage good coding practices. The lint set provided by the package is
|
# encourage good coding practices. The lint set provided by the package is
|
||||||
|
|
|
||||||
|
|
@ -86,9 +86,12 @@ class _ScrollServiceWidgetState extends State<ScrollServiceWidget>
|
||||||
void _onSelectionChanged() {
|
void _onSelectionChanged() {
|
||||||
// should auto scroll after the cursor or selection updated.
|
// should auto scroll after the cursor or selection updated.
|
||||||
final selection = editorState.selection;
|
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 ||
|
if (selection == null ||
|
||||||
[SelectionUpdateReason.selectAll]
|
reason == SelectionUpdateReason.remote ||
|
||||||
.contains(editorState.selectionUpdateReason)) {
|
reason == SelectionUpdateReason.selectAll) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,7 @@
|
||||||
|
import 'package:appflowy_editor/appflowy_editor.dart';
|
||||||
import 'package:flutter/material.dart';
|
import 'package:flutter/material.dart';
|
||||||
import 'package:flutter_test/flutter_test.dart';
|
import 'package:flutter_test/flutter_test.dart';
|
||||||
|
|
||||||
import '../new/infra/testable_editor.dart';
|
import '../new/infra/testable_editor.dart';
|
||||||
|
|
||||||
void main() async {
|
void main() async {
|
||||||
|
|
@ -28,5 +30,53 @@ void main() async {
|
||||||
expect(itemFinder, findsOneWidget);
|
expect(itemFinder, findsOneWidget);
|
||||||
await editor.dispose();
|
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);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue