Unverified Commit 6e224ee0 authored by LongCatIsLooong's avatar LongCatIsLooong Committed by GitHub

[EditableText] call `onSelectionChanged` only when there're actual selection/cause changes (#87971)

parent fbc4e9bc
......@@ -593,7 +593,6 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
bool _wasSelectingVerticallyWithKeyboard = false;
void _setTextEditingValue(TextEditingValue newValue, SelectionChangedCause cause) {
textSelectionDelegate.textEditingValue = newValue;
textSelectionDelegate.userUpdateTextEditingValue(newValue, cause);
}
......@@ -613,11 +612,11 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
extentOffset: math.min(nextSelection.extentOffset, textLength),
);
}
_handleSelectionChange(nextSelection, cause);
_setTextEditingValue(
textSelectionDelegate.textEditingValue.copyWith(selection: nextSelection),
cause,
);
_handleSelectionChange(nextSelection, cause);
}
void _handleSelectionChange(
......
......@@ -1226,7 +1226,7 @@ class TextSelectionGestureDetectorBuilder {
@protected
void onDoubleTapDown(TapDownDetails details) {
if (delegate.selectionEnabled) {
renderEditable.selectWord(cause: SelectionChangedCause.tap);
renderEditable.selectWord(cause: SelectionChangedCause.doubleTap);
if (shouldShowSelectionToolbar)
editableText.showToolbar();
}
......
......@@ -2381,6 +2381,7 @@ void main() {
await gesture.moveBy(const Offset(600, 0));
// To the edge of the screen basically.
await tester.pump();
await tester.pumpAndSettle();
expect(
controller.selection,
const TextSelection.collapsed(offset: 54, affinity: TextAffinity.upstream),
......@@ -2388,12 +2389,14 @@ void main() {
// Keep moving out.
await gesture.moveBy(const Offset(1, 0));
await tester.pump();
await tester.pumpAndSettle();
expect(
controller.selection,
const TextSelection.collapsed(offset: 61, affinity: TextAffinity.upstream),
);
await gesture.moveBy(const Offset(1, 0));
await tester.pump();
await tester.pumpAndSettle();
expect(
controller.selection,
const TextSelection.collapsed(offset: 66, affinity: TextAffinity.upstream),
......
......@@ -3871,7 +3871,7 @@ void main() {
editableTextState.textEditingValue.copyWith(
selection: TextSelection.collapsed(offset: longText.length),
),
null,
SelectionChangedCause.tap,
);
await tester.pump(); // TODO(ianh): Figure out why this extra pump is needed.
......@@ -3908,7 +3908,7 @@ void main() {
editableTextState.textEditingValue.copyWith(
selection: const TextSelection.collapsed(offset: tallText.length),
),
null,
SelectionChangedCause.tap,
);
await tester.pump();
await skipPastScrollingAnimation(tester);
......@@ -7709,6 +7709,7 @@ void main() {
await gesture.moveBy(const Offset(600, 0));
// To the edge of the screen basically.
await tester.pump();
await tester.pumpAndSettle();
expect(
controller.selection,
const TextSelection.collapsed(offset: 56, affinity: TextAffinity.downstream),
......@@ -7716,12 +7717,14 @@ void main() {
// Keep moving out.
await gesture.moveBy(const Offset(1, 0));
await tester.pump();
await tester.pumpAndSettle();
expect(
controller.selection,
const TextSelection.collapsed(offset: 62, affinity: TextAffinity.downstream),
);
await gesture.moveBy(const Offset(1, 0));
await tester.pump();
await tester.pumpAndSettle();
expect(
controller.selection,
const TextSelection.collapsed(offset: 66, affinity: TextAffinity.upstream),
......
......@@ -34,7 +34,9 @@ class FakeEditableTextState with TextSelectionDelegate {
void hideToolbar([bool hideHandles = true]) { }
@override
void userUpdateTextEditingValue(TextEditingValue value, SelectionChangedCause cause) { }
void userUpdateTextEditingValue(TextEditingValue value, SelectionChangedCause cause) {
textEditingValue = value;
}
@override
void bringIntoView(TextPosition position) { }
......
......@@ -650,7 +650,7 @@ void main() {
// false.
state.userUpdateTextEditingValue(
state.textEditingValue.copyWith(selection: const TextSelection.collapsed(offset: 90)),
null,
SelectionChangedCause.tap,
);
await tester.pumpAndSettle();
expect(isCaretOnScreen(tester), isTrue);
......@@ -674,7 +674,7 @@ void main() {
state.userUpdateTextEditingValue(
state.textEditingValue.copyWith(selection: const TextSelection.collapsed(offset: 100)),
null,
SelectionChangedCause.tap,
);
await tester.pumpAndSettle();
expect(isCaretOnScreen(tester), isTrue);
......
......@@ -4938,21 +4938,19 @@ void main() {
testWidgets('keyboard text selection works (RawKeyEvent)', (WidgetTester tester) async {
debugKeyEventSimulatorTransitModeOverride = KeyDataTransitMode.rawKeyData;
addTearDown(() { debugKeyEventSimulatorTransitModeOverride = null; });
await testTextEditing(tester, targetPlatform: defaultTargetPlatform);
debugKeyEventSimulatorTransitModeOverride = null;
// On web, using keyboard for selection is handled by the browser.
}, variant: TargetPlatformVariant.all(), skip: kIsWeb); // [intended]
testWidgets('keyboard text selection works (ui.KeyData then RawKeyEvent)', (WidgetTester tester) async {
debugKeyEventSimulatorTransitModeOverride = KeyDataTransitMode.keyDataThenRawKeyData;
addTearDown(() { debugKeyEventSimulatorTransitModeOverride = null; });
await testTextEditing(tester, targetPlatform: defaultTargetPlatform);
debugKeyEventSimulatorTransitModeOverride = null;
// On web, using keyboard for selection is handled by the browser.
}, variant: TargetPlatformVariant.all(), skip: kIsWeb); // [intended]
......@@ -6047,7 +6045,6 @@ void main() {
'TextInput.setStyle',
'TextInput.setEditingState',
'TextInput.setEditingState',
'TextInput.show',
'TextInput.setCaretRect',
];
expect(
......@@ -6091,16 +6088,14 @@ void main() {
'TextInput.setStyle',
'TextInput.setEditingState',
'TextInput.setEditingState',
'TextInput.show',
'TextInput.setCaretRect',
'TextInput.show',
];
expect(tester.testTextInput.log.length, logOrder.length);
int index = 0;
for (final MethodCall m in tester.testTextInput.log) {
expect(m.method, logOrder[index]);
index++;
}
expect(
tester.testTextInput.log.map((MethodCall methodCall) => methodCall.method),
logOrder,
);
expect(tester.testTextInput.editingState!['text'], 'flutter is the best!');
});
......@@ -6141,7 +6136,6 @@ void main() {
'TextInput.setStyle',
'TextInput.setEditingState',
'TextInput.setEditingState',
'TextInput.show',
'TextInput.setCaretRect',
'TextInput.setEditingState',
];
......@@ -6215,7 +6209,7 @@ void main() {
selection: controller.selection,
));
expect(log.length, 0);
expect(log, isEmpty);
// setEditingState is called when remote value modified by the formatter.
state.updateEditingValue(TextEditingValue(
......@@ -7460,6 +7454,115 @@ void main() {
});
});
group('onChanged callbacks are edge-triggered', () {
SelectionChangedCallback? onSelectionChanged;
ValueChanged<String>? onChanged;
final TextEditingController controller = TextEditingController();
final Widget editableText = EditableText(
showSelectionHandles: false,
controller: controller,
focusNode: FocusNode(),
cursorColor: Colors.red,
backgroundCursorColor: Colors.blue,
style: Typography.material2018(platform: TargetPlatform.android).black.subtitle1!.copyWith(fontFamily: 'Roboto'),
keyboardType: TextInputType.text,
selectionControls: materialTextSelectionControls,
onSelectionChanged: (TextSelection selection, SelectionChangedCause? cause) => onSelectionChanged?.call(selection, cause),
onChanged: (String value) => onChanged?.call(value),
);
tearDown(() {
onSelectionChanged = null;
onChanged = null;
});
testWidgets('onSelectionChanged', (WidgetTester tester) async {
TextSelection? selection;
SelectionChangedCause? cause;
onSelectionChanged = (TextSelection newSelection, SelectionChangedCause? newCause) {
selection = newSelection;
cause = newCause;
};
controller.value = const TextEditingValue(text: 'text', selection: TextSelection(baseOffset: 1, extentOffset: 2));
await tester.pumpWidget(MaterialApp(
home: editableText,
));
final EditableTextState state = tester.state(find.byWidget(editableText));
await tester.showKeyboard(find.byWidget(editableText));
await tester.pump();
// No user input.
expect(selection, isNull);
expect(cause, isNull);
// Selection didn't change but the cause did (keyboard).
state.updateEditingValue(
const TextEditingValue(text: 'test text', selection: TextSelection(baseOffset: 1, extentOffset: 2)),
);
expect(selection, const TextSelection(baseOffset: 1, extentOffset: 2));
expect(cause, SelectionChangedCause.keyboard);
// Selection and cause both changed
await tester.enterText(find.byWidget(editableText), 'test text');
expect(selection, const TextSelection.collapsed(offset: 9));
expect(cause, SelectionChangedCause.keyboard);
selection = null;
cause = null;
// Nothing changes.
state.userUpdateTextEditingValue(
const TextEditingValue(text: 'test text', selection: TextSelection.collapsed(offset: 9)),
SelectionChangedCause.keyboard
);
expect(selection, isNull);
expect(cause, isNull);
// The cause changes.
state.userUpdateTextEditingValue(
const TextEditingValue(text: 'test text', selection: TextSelection.collapsed(offset: 9)),
SelectionChangedCause.toolBar,
);
expect(selection, const TextSelection.collapsed(offset: 9));
expect(cause, SelectionChangedCause.toolBar);
});
testWidgets('onChanged', (WidgetTester tester) async {
String? newText;
onChanged = (String text) => newText = text;
controller.value = const TextEditingValue(text: 'text', selection: TextSelection(baseOffset: 1, extentOffset: 2));
await tester.pumpWidget(MaterialApp(
home: editableText,
));
final EditableTextState state = tester.state(find.byWidget(editableText));
await tester.showKeyboard(find.byWidget(editableText));
await tester.pump();
// No user input.
expect(newText, isNull);
state.updateEditingValue(
const TextEditingValue(text: 'text', selection: TextSelection(baseOffset: 1, extentOffset: 3)),
);
// Selection & cause changed but the text didn't;
expect(newText, isNull);
state.updateEditingValue(
const TextEditingValue(text: 'test text', selection: TextSelection(baseOffset: 1, extentOffset: 3)),
);
// Now the text is changed.
expect(newText, 'test text');
});
});
group('callback errors', () {
const String errorText = 'Test EditableText callback error';
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment