Skip to content

Commit

Permalink
Add undoStackModifier to UndoHistory (#138674)
Browse files Browse the repository at this point in the history
This change adds a feature to `UndoHistory` that allows the user to modify the value being pushed onto the undo stack.

This is used by the framework to ignore the composing region when pushing history entries to the Undo stack on Android. This is so an undo does not trigger an input connection restart by the Android TextInputPlugin, which occurs when the framework changes the composing region. This is also the native platform behavior observed in Google Keep app on Android, where doing an undo during composing reverts to the previous state but with composing inactive and a subsequent redo does not bring back the composing region.

Fixes #130881
Partial fix for #134398
  • Loading branch information
Renzo-Olivares authored Nov 30, 2023
1 parent 8412adb commit 75011b2
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 20 deletions.
7 changes: 7 additions & 0 deletions packages/flutter/lib/src/widgets/editable_text.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4823,6 +4823,13 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien

return oldValue.text != newValue.text || oldValue.composing != newValue.composing;
},
undoStackModifier: (TextEditingValue value) {
// On Android we should discard the composing region when pushing
// a new entry to the undo stack. This prevents the TextInputPlugin
// from restarting the input on every undo/redo when the composing
// region is changed by the framework.
return defaultTargetPlatform == TargetPlatform.android ? value.copyWith(composing: TextRange.empty) : value;
},
focusNode: widget.focusNode,
controller: widget.undoController,
child: Focus(
Expand Down
18 changes: 16 additions & 2 deletions packages/flutter/lib/src/widgets/undo_history.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class UndoHistory<T> extends StatefulWidget {
required this.value,
required this.onTriggered,
required this.focusNode,
this.undoStackModifier,
this.controller,
required this.child,
});
Expand All @@ -43,6 +44,14 @@ class UndoHistory<T> extends StatefulWidget {
/// the undo stack.
final bool Function(T? oldValue, T newValue)? shouldChangeUndoStack;

/// Called right before a new entry is pushed to the undo stack.
///
/// The value returned from this method will be pushed to the stack instead
/// of the original value.
///
/// If null then the original value will always be pushed to the stack.
final T Function(T value)? undoStackModifier;

/// Called when an undo or redo causes a state change.
///
/// If the state would still be the same before and after the undo/redo, this
Expand Down Expand Up @@ -178,9 +187,14 @@ class UndoHistoryState<T> extends State<UndoHistory<T>> with UndoManagerClient {
return;
}

_lastValue = widget.value.value;
final T nextValue = widget.undoStackModifier?.call(widget.value.value) ?? widget.value.value;
if (nextValue == _lastValue) {
return;
}

_lastValue = nextValue;

_throttleTimer = _throttledPush(widget.value.value);
_throttleTimer = _throttledPush(nextValue);
}

void _handleFocus() {
Expand Down
39 changes: 21 additions & 18 deletions packages/flutter/test/widgets/editable_text_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13127,6 +13127,13 @@ void main() {
Future<void> sendUndo(WidgetTester tester) => sendUndoRedo(tester);
Future<void> sendRedo(WidgetTester tester) => sendUndoRedo(tester, true);

TextEditingValue emptyComposingOnAndroid(TextEditingValue value) {
if (defaultTargetPlatform == TargetPlatform.android) {
return value.copyWith(composing: TextRange.empty);
}
return value;
}

Widget boilerplate() {
return MaterialApp(
home: EditableText(
Expand Down Expand Up @@ -13330,14 +13337,14 @@ void main() {

// Undo first insertion.
await sendUndo(tester);
expect(controller.value, composingStep2);
expect(controller.value, emptyComposingOnAndroid(composingStep2));

// Waiting for the throttling between undos should have no effect.
await tester.pump(const Duration(milliseconds: 500));

// Undo second insertion.
await sendUndo(tester);
expect(controller.value, composingStep1);
expect(controller.value, emptyComposingOnAndroid(composingStep1));

// On web, these keyboard shortcuts are handled by the browser.
}, variant: TargetPlatformVariant.only(TargetPlatform.android), skip: kIsWeb); // [intended]
Expand Down Expand Up @@ -13899,7 +13906,6 @@ void main() {
controller.value,
const TextEditingValue(
text: '1 nihao',
composing: TextRange(start: 2, end: 7),
selection: TextSelection.collapsed(offset: 7),
),
);
Expand All @@ -13909,7 +13915,6 @@ void main() {
controller.value,
const TextEditingValue(
text: '1 ni',
composing: TextRange(start: 2, end: 4),
selection: TextSelection.collapsed(offset: 4),
),
);
Expand All @@ -13927,7 +13932,6 @@ void main() {
controller.value,
const TextEditingValue(
text: '1 ni',
composing: TextRange(start: 2, end: 4),
selection: TextSelection.collapsed(offset: 4),
),
);
Expand All @@ -13936,7 +13940,6 @@ void main() {
controller.value,
const TextEditingValue(
text: '1 nihao',
composing: TextRange(start: 2, end: 7),
selection: TextSelection.collapsed(offset: 7),
),
);
Expand All @@ -13962,7 +13965,6 @@ void main() {
controller.value,
const TextEditingValue(
text: '1 nihao',
composing: TextRange(start: 2, end: 7),
selection: TextSelection.collapsed(offset: 7),
),
);
Expand All @@ -13971,7 +13973,6 @@ void main() {
controller.value,
const TextEditingValue(
text: '1 ni',
composing: TextRange(start: 2, end: 4),
selection: TextSelection.collapsed(offset: 4),
),
);
Expand Down Expand Up @@ -14011,7 +14012,6 @@ void main() {
controller.value,
const TextEditingValue(
text: '1 ni',
composing: TextRange(start: 2, end: 4),
selection: TextSelection.collapsed(offset: 4),
),
);
Expand All @@ -14020,7 +14020,6 @@ void main() {
controller.value,
const TextEditingValue(
text: '1 nihao',
composing: TextRange(start: 2, end: 7),
selection: TextSelection.collapsed(offset: 7),
),
);
Expand Down Expand Up @@ -14138,10 +14137,12 @@ void main() {
case TargetPlatform.android:
expect(
controller.value,
const TextEditingValue(
text: '1 2 ni',
composing: TextRange(start: 4, end: 6),
selection: TextSelection.collapsed(offset: 6),
emptyComposingOnAndroid(
const TextEditingValue(
text: '1 2 ni',
composing: TextRange(start: 4, end: 6),
selection: TextSelection.collapsed(offset: 6),
),
),
);
// Composing changes are ignored on all other platforms.
Expand Down Expand Up @@ -14195,10 +14196,12 @@ void main() {
case TargetPlatform.android:
expect(
controller.value,
const TextEditingValue(
text: '1 2 ni',
composing: TextRange(start: 4, end: 6),
selection: TextSelection.collapsed(offset: 6),
emptyComposingOnAndroid(
const TextEditingValue(
text: '1 2 ni',
composing: TextRange(start: 4, end: 6),
selection: TextSelection.collapsed(offset: 6),
),
),
);
// Composing changes are ignored on all other platforms.
Expand Down

0 comments on commit 75011b2

Please sign in to comment.