fix: support multi-step redo by refactoring undo/redo recording (#1192)

Multi-step redo was broken because `getUndoHistoryItem()` cleared the
redo stack whenever the last undo item was sealed — even during redo
operations. After the first redo, the 50ms debounce timer would seal
the item, causing the next redo to wipe the remaining redo history.

Refactored the undo/redo system with:
- `TransactionSource` enum (userEdit, undo, redo, none) replacing
  boolean flag combinations in ApplyOptions
- `UndoManager.record()` with symmetric paths for each source:
  - userEdit: groups into unsealed items, clears redo on branch
  - undo: pushes directly to redo stack
  - redo: pushes directly to undo stack (sealed), no redo clear
- Deprecated `getUndoHistoryItem()` and legacy boolean flags
This commit is contained in:
Lucas 2026-03-25 15:25:00 +08:00 • committed by GitHub
commit 37521c205f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 580 additions and 28 deletions

View file

@ -14,6 +14,8 @@ export 'src/editor/l10n/appflowy_editor_l10n.dart';
export 'src/editor/selection_menu/selection_menu.dart';
// editor state
export 'src/editor_state.dart';
// history
export 'src/history/undo_manager.dart' show TransactionSource;
// extension
export 'src/extensions/extensions.dart';
export 'src/infra/clipboard.dart';

View file

@ -36,17 +36,41 @@ class ApplyOptions {
const ApplyOptions({
this.recordUndo = true,
this.recordRedo = false,
this.source,
this.inMemoryUpdate = false,
});
/// This flag indicates that
/// whether the transaction should be recorded into
/// the undo stack
/// Whether the transaction should be recorded into the undo stack.
@Deprecated('Use [source] instead')
final bool recordUndo;
@Deprecated('Use [source] instead')
final bool recordRedo;
/// The source of the transaction. When set, takes precedence over
/// the legacy `recordUndo` and `recordRedo` flags for determining
/// how the transaction is recorded in the undo/redo history.
final TransactionSource? source;
/// This flag used to determine whether the transaction is in-memory update.
final bool inMemoryUpdate;
/// Returns the resolved [TransactionSource].
/// Prefers explicit [source], falls back to legacy boolean flags.
///
/// Legacy mapping (for backward compatibility):
/// - `recordRedo: true` → [TransactionSource.undo] (records *to* redo stack)
/// - `recordUndo: true` → [TransactionSource.userEdit]
/// - both false → [TransactionSource.none]
TransactionSource get resolvedSource {
if (source != null) return source!;
// ignore: deprecated_member_use_from_same_package
if (recordRedo) return TransactionSource.undo;
// ignore: deprecated_member_use_from_same_package
if (recordUndo) return TransactionSource.userEdit;
return TransactionSource.none;
}
}
@Deprecated('use SelectionUpdateReason instead')
@ -671,14 +695,11 @@ class EditorState {
Transaction transaction,
bool skipDebounce,
) {
if (options.recordUndo) {
final undoItem = undoManager.getUndoHistoryItem();
undoItem.addAll(transaction.operations);
if (undoItem.beforeSelection == null &&
transaction.beforeSelection != null) {
undoItem.beforeSelection = transaction.beforeSelection;
}
undoItem.afterSelection = transaction.afterSelection;
final source = options.resolvedSource;
undoManager.record(transaction, source);
// Only debounce-seal for user edits (grouping consecutive keystrokes).
if (source == TransactionSource.userEdit) {
if (skipDebounce && undoManager.undoStack.isNonEmpty) {
AppFlowyEditorLog.editor.debug('Seal history item');
final last = undoManager.undoStack.last;
@ -686,12 +707,6 @@ class EditorState {
} else {
_debouncedSealHistoryItem();
}
} else if (options.recordRedo) {
final redoItem = HistoryItem();
redoItem.addAll(transaction.operations);
redoItem.beforeSelection = transaction.beforeSelection;
redoItem.afterSelection = transaction.afterSelection;
undoManager.redoStack.push(redoItem);
}
}

View file

@ -6,6 +6,22 @@ import 'package:appflowy_editor/src/core/transform/transaction.dart';
import 'package:appflowy_editor/src/editor_state.dart';
import 'package:appflowy_editor/src/infra/log.dart';
/// Describes the origin of a transaction for undo/redo recording.
enum TransactionSource {
/// A normal user edit. Records to undo stack and clears redo stack
/// when branching (new edit after undo).
userEdit,
/// An undo operation. Records to redo stack.
undo,
/// A redo operation. Records to undo stack without clearing redo stack.
redo,
/// Not recorded in either stack (e.g. remote or in-memory updates).
none,
}
/// A [HistoryItem] contains list of operations committed by users.
/// If a [HistoryItem] is not sealed, operations can be added sequentially.
/// Otherwise, the operations should be added to a new [HistoryItem].
@ -84,6 +100,8 @@ class FixedSizeStack {
bool get isEmpty => _list.isEmpty;
bool get isNonEmpty => _list.isNotEmpty;
int get length => _list.length;
}
class UndoManager {
@ -95,6 +113,82 @@ class UndoManager {
: undoStack = FixedSizeStack(stackSize),
redoStack = FixedSizeStack(stackSize);
/// Record a transaction into the appropriate stack based on [source].
///
/// Returns the [HistoryItem] that was created or updated, or null
/// if the source is [TransactionSource.none].
HistoryItem? record(
Transaction transaction,
TransactionSource source,
) {
switch (source) {
case TransactionSource.userEdit:
return _recordUserEdit(transaction);
case TransactionSource.undo:
return _recordFromUndo(transaction);
case TransactionSource.redo:
return _recordFromRedo(transaction);
case TransactionSource.none:
return null;
}
}
/// User edit: add to current unsealed undo item or create a new one.
/// Clears redo stack when branching (new sealed item means new edit
/// after a previous undo).
HistoryItem _recordUserEdit(Transaction transaction) {
HistoryItem undoItem;
if (undoStack.isEmpty) {
undoItem = HistoryItem();
undoStack.push(undoItem);
} else {
final last = undoStack.last;
if (last.sealed) {
redoStack.clear();
undoItem = HistoryItem();
undoStack.push(undoItem);
} else {
undoItem = last;
}
}
undoItem.addAll(transaction.operations);
if (undoItem.beforeSelection == null &&
transaction.beforeSelection != null) {
undoItem.beforeSelection = transaction.beforeSelection;
}
undoItem.afterSelection = transaction.afterSelection;
return undoItem;
}
/// Undo operation: push directly to redo stack.
HistoryItem _recordFromUndo(Transaction transaction) {
final redoItem = HistoryItem();
redoItem.addAll(transaction.operations);
redoItem.beforeSelection = transaction.beforeSelection;
redoItem.afterSelection = transaction.afterSelection;
redoStack.push(redoItem);
return redoItem;
}
/// Redo operation: push directly to undo stack, sealed immediately.
/// Does NOT clear redo stack.
HistoryItem _recordFromRedo(Transaction transaction) {
final undoItem = HistoryItem();
undoItem.addAll(transaction.operations);
undoItem.beforeSelection = transaction.beforeSelection;
undoItem.afterSelection = transaction.afterSelection;
undoItem.seal();
undoStack.push(undoItem);
return undoItem;
}
@Deprecated('Use record() with TransactionSource instead')
HistoryItem getUndoHistoryItem() {
if (undoStack.isEmpty) {
final item = HistoryItem();
@ -127,10 +221,7 @@ class UndoManager {
final transaction = historyItem.toTransaction(s);
s.apply(
transaction,
options: const ApplyOptions(
recordUndo: false,
recordRedo: true,
),
options: const ApplyOptions(source: TransactionSource.undo),
);
}
@ -147,10 +238,7 @@ class UndoManager {
final transaction = historyItem.toTransaction(s);
s.apply(
transaction,
options: const ApplyOptions(
recordUndo: true,
recordRedo: false,
),
options: const ApplyOptions(source: TransactionSource.redo),
);
}

View file

@ -45,6 +45,401 @@ void main() async {
historyItem.seal();
expect(historyItem.sealed, true);
});
test('Multi-step redo preserves redo stack', () {
final editorState = EditorState(
document: Document(
root: pageNode(
children: [
paragraphNode(text: ''),
],
),
),
);
editorState.disableSealTimer = true;
// Apply 3 separate transactions, sealing each one
for (var i = 0; i < 3; i++) {
final transaction = editorState.transaction;
transaction.insertText(
editorState.document.nodeAtPath([0])!,
i, // offset
String.fromCharCode(65 + i), // 'A', 'B', 'C'
);
editorState.apply(transaction);
editorState.undoManager.undoStack.last.seal();
}
// Document should be "ABC"
expect(
editorState.document.nodeAtPath([0])!.delta!.toPlainText(),
'ABC',
);
expect(editorState.undoManager.undoStack.length, 3);
expect(editorState.undoManager.redoStack.length, 0);
// Undo 3 times → back to empty
editorState.undoManager.undo();
expect(
editorState.document.nodeAtPath([0])!.delta!.toPlainText(),
'AB',
);
expect(editorState.undoManager.undoStack.length, 2);
expect(editorState.undoManager.redoStack.length, 1);
editorState.undoManager.undo();
expect(
editorState.document.nodeAtPath([0])!.delta!.toPlainText(),
'A',
);
expect(editorState.undoManager.undoStack.length, 1);
expect(editorState.undoManager.redoStack.length, 2);
editorState.undoManager.undo();
expect(
editorState.document.nodeAtPath([0])!.delta!.toPlainText(),
'',
);
expect(editorState.undoManager.undoStack.length, 0);
expect(editorState.undoManager.redoStack.length, 3);
// Redo 3 times → should restore each step
editorState.undoManager.redo();
expect(
editorState.document.nodeAtPath([0])!.delta!.toPlainText(),
'A',
);
expect(editorState.undoManager.undoStack.length, 1);
expect(editorState.undoManager.redoStack.length, 2);
// Manually seal to simulate the 50ms debounce timer
editorState.undoManager.undoStack.last.seal();
editorState.undoManager.redo();
expect(
editorState.document.nodeAtPath([0])!.delta!.toPlainText(),
'AB',
);
expect(editorState.undoManager.undoStack.length, 2);
expect(editorState.undoManager.redoStack.length, 1);
editorState.undoManager.undoStack.last.seal();
editorState.undoManager.redo();
expect(
editorState.document.nodeAtPath([0])!.delta!.toPlainText(),
'ABC',
);
expect(editorState.undoManager.undoStack.length, 3);
expect(editorState.undoManager.redoStack.length, 0);
});
test('New edit after undo still clears redo stack', () {
final editorState = EditorState(
document: Document(
root: pageNode(
children: [
paragraphNode(text: ''),
],
),
),
);
editorState.disableSealTimer = true;
// Insert "A"
final t1 = editorState.transaction;
t1.insertText(editorState.document.nodeAtPath([0])!, 0, 'A');
editorState.apply(t1);
editorState.undoManager.undoStack.last.seal();
// Insert "B"
final t2 = editorState.transaction;
t2.insertText(editorState.document.nodeAtPath([0])!, 1, 'B');
editorState.apply(t2);
editorState.undoManager.undoStack.last.seal();
// Undo once → "A", redo stack should have 1 item
editorState.undoManager.undo();
expect(
editorState.document.nodeAtPath([0])!.delta!.toPlainText(),
'A',
);
expect(editorState.undoManager.redoStack.isNonEmpty, true);
// Make a NEW edit (not redo) → redo stack should be cleared
final t3 = editorState.transaction;
t3.insertText(editorState.document.nodeAtPath([0])!, 1, 'X');
editorState.apply(t3);
expect(editorState.undoManager.redoStack.isEmpty, true);
});
test('Undo after redo restores previous state', () {
final editorState = EditorState(
document: Document(
root: pageNode(
children: [
paragraphNode(text: ''),
],
),
),
);
editorState.disableSealTimer = true;
// Insert A, B, C
for (var i = 0; i < 3; i++) {
final transaction = editorState.transaction;
transaction.insertText(
editorState.document.nodeAtPath([0])!,
i,
String.fromCharCode(65 + i),
);
editorState.apply(transaction);
editorState.undoManager.undoStack.last.seal();
}
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'ABC');
// Undo all
editorState.undoManager.undo();
editorState.undoManager.undo();
editorState.undoManager.undo();
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), '');
// Redo 2
editorState.undoManager.redo();
editorState.undoManager.redo();
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'AB');
expect(editorState.undoManager.undoStack.length, 2);
expect(editorState.undoManager.redoStack.length, 1);
// Undo 1 — should undo the last redo
editorState.undoManager.undo();
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'A');
expect(editorState.undoManager.undoStack.length, 1);
expect(editorState.undoManager.redoStack.length, 2);
// Redo all remaining
editorState.undoManager.redo();
editorState.undoManager.redo();
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'ABC');
});
test('Partial undo then redo', () {
final editorState = EditorState(
document: Document(
root: pageNode(
children: [
paragraphNode(text: ''),
],
),
),
);
editorState.disableSealTimer = true;
// Insert A, B, C, D
for (var i = 0; i < 4; i++) {
final transaction = editorState.transaction;
transaction.insertText(
editorState.document.nodeAtPath([0])!,
i,
String.fromCharCode(65 + i),
);
editorState.apply(transaction);
editorState.undoManager.undoStack.last.seal();
}
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'ABCD');
// Undo 2 of 4
editorState.undoManager.undo();
editorState.undoManager.undo();
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'AB');
expect(editorState.undoManager.undoStack.length, 2);
expect(editorState.undoManager.redoStack.length, 2);
// Redo 2
editorState.undoManager.redo();
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'ABC');
editorState.undoManager.redo();
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'ABCD');
expect(editorState.undoManager.undoStack.length, 4);
expect(editorState.undoManager.redoStack.length, 0);
});
test('Interleaved undo and redo', () {
final editorState = EditorState(
document: Document(
root: pageNode(
children: [
paragraphNode(text: ''),
],
),
),
);
editorState.disableSealTimer = true;
// Insert A, B, C
for (var i = 0; i < 3; i++) {
final transaction = editorState.transaction;
transaction.insertText(
editorState.document.nodeAtPath([0])!,
i,
String.fromCharCode(65 + i),
);
editorState.apply(transaction);
editorState.undoManager.undoStack.last.seal();
}
// undo → redo → undo → redo cycle
editorState.undoManager.undo(); // ABC → AB
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'AB');
editorState.undoManager.redo(); // AB → ABC
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'ABC');
editorState.undoManager.undo(); // ABC → AB
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'AB');
editorState.undoManager.undo(); // AB → A
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'A');
editorState.undoManager.redo(); // A → AB
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'AB');
editorState.undoManager.redo(); // AB → ABC
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'ABC');
});
test('New edit mid-redo clears remaining redo stack', () {
final editorState = EditorState(
document: Document(
root: pageNode(
children: [
paragraphNode(text: ''),
],
),
),
);
editorState.disableSealTimer = true;
// Insert A, B, C
for (var i = 0; i < 3; i++) {
final transaction = editorState.transaction;
transaction.insertText(
editorState.document.nodeAtPath([0])!,
i,
String.fromCharCode(65 + i),
);
editorState.apply(transaction);
editorState.undoManager.undoStack.last.seal();
}
// Undo all 3
editorState.undoManager.undo();
editorState.undoManager.undo();
editorState.undoManager.undo();
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), '');
expect(editorState.undoManager.redoStack.length, 3);
// Redo 1
editorState.undoManager.redo();
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'A');
expect(editorState.undoManager.redoStack.length, 2);
// Make a NEW edit → remaining redo should be cleared
final t = editorState.transaction;
t.insertText(editorState.document.nodeAtPath([0])!, 1, 'X');
editorState.apply(t);
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'AX');
expect(editorState.undoManager.redoStack.length, 0);
// Redo should be no-op now
editorState.undoManager.redo();
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), 'AX');
});
test('Selection is preserved during multi-step redo', () {
final editorState = EditorState(
document: Document(
root: pageNode(
children: [
paragraphNode(text: ''),
],
),
),
);
editorState.disableSealTimer = true;
// Insert A at offset 0 with selection
final t1 = editorState.transaction;
t1.insertText(editorState.document.nodeAtPath([0])!, 0, 'A');
t1.afterSelection = Selection.collapsed(Position(path: [0], offset: 1));
editorState.apply(t1);
editorState.undoManager.undoStack.last.seal();
// Insert B at offset 1 with selection
final t2 = editorState.transaction;
t2.insertText(editorState.document.nodeAtPath([0])!, 1, 'B');
t2.afterSelection = Selection.collapsed(Position(path: [0], offset: 2));
editorState.apply(t2);
editorState.undoManager.undoStack.last.seal();
// Undo both
editorState.undoManager.undo();
editorState.undoManager.undo();
expect(editorState.getNodeAtPath([0])!.delta!.toPlainText(), '');
// Redo first — selection should be at offset 1
editorState.undoManager.redo();
expect(
editorState.selection,
Selection.collapsed(Position(path: [0], offset: 1)),
);
// Redo second — selection should be at offset 2
editorState.undoManager.redo();
expect(
editorState.selection,
Selection.collapsed(Position(path: [0], offset: 2)),
);
});
test('TransactionSource.none does not record to either stack', () {
final editorState = EditorState(
document: Document(
root: pageNode(
children: [
paragraphNode(text: ''),
],
),
),
);
editorState.disableSealTimer = true;
// Apply with source: none
final transaction = editorState.transaction;
transaction.insertText(
editorState.document.nodeAtPath([0])!,
0,
'A',
);
editorState.apply(
transaction,
options: const ApplyOptions(
recordUndo: false,
recordRedo: false,
source: TransactionSource.none,
),
);
// Document is modified but neither stack is affected
expect(
editorState.document.nodeAtPath([0])!.delta!.toPlainText(),
'A',
);
expect(editorState.undoManager.undoStack.length, 0);
expect(editorState.undoManager.redoStack.length, 0);
});
}
bool isInsertAndPathEqual(Operation operation, Path path, [String? content]) {

View file

@ -184,11 +184,63 @@ void main() async {
expect(editor.nodeAtPath([1])!.delta!.toPlainText(), isEmpty);
expect(editor.nodeAtPath([1, 0])!.delta!.toPlainText(), text01);
// second redo
// second redo - should restore the tab indent
await _pressRedoCommand(editor);
expect(editor.nodeAtPath([0])!.delta!.toPlainText(), text0);
expect(editor.nodeAtPath([1])!.delta!.toPlainText(), isEmpty);
expect(editor.nodeAtPath([1, 0])!.delta!.toPlainText(), text01);
expect(editor.nodeAtPath([0, 0])!.delta!.toPlainText(), isEmpty);
expect(editor.nodeAtPath([0, 0, 0])!.delta!.toPlainText(), text01);
await editor.dispose();
});
// Type A, B, C, D with delays between each,
// undo 4 times to empty, then redo 4 times.
// Each redo should restore one character.
testWidgets('Multi-step typing then multi-step undo and redo',
(tester) async {
final editor = tester.editor..addParagraph(initialText: '');
await editor.startTesting();
// Place cursor at [0] offset 0
await editor.updateSelection(
Selection.collapsed(Position(path: [0], offset: 0)),
);
// Type 4 characters with pumpAndSettle between each
// to ensure debounce sealing
await editor.ime.typeText('A');
await tester.pumpAndSettle();
await editor.ime.typeText('B');
await tester.pumpAndSettle();
await editor.ime.typeText('C');
await tester.pumpAndSettle();
await editor.ime.typeText('D');
await tester.pumpAndSettle();
expect(
editor.nodeAtPath([0])!.delta!.toPlainText(),
'ABCD',
);
// Undo 4 times
await _pressUndoCommand(editor);
expect(editor.nodeAtPath([0])!.delta!.toPlainText(), 'ABC');
await _pressUndoCommand(editor);
expect(editor.nodeAtPath([0])!.delta!.toPlainText(), 'AB');
await _pressUndoCommand(editor);
expect(editor.nodeAtPath([0])!.delta!.toPlainText(), 'A');
await _pressUndoCommand(editor);
expect(editor.nodeAtPath([0])!.delta!.toPlainText(), '');
// Redo 4 times - each should restore one character
await _pressRedoCommand(editor);
expect(editor.nodeAtPath([0])!.delta!.toPlainText(), 'A');
await _pressRedoCommand(editor);
expect(editor.nodeAtPath([0])!.delta!.toPlainText(), 'AB');
await _pressRedoCommand(editor);
expect(editor.nodeAtPath([0])!.delta!.toPlainText(), 'ABC');
await _pressRedoCommand(editor);
expect(editor.nodeAtPath([0])!.delta!.toPlainText(), 'ABCD');
await editor.dispose();
});