Unverified Commit b3d12ebd authored by LongCatIsLooong's avatar LongCatIsLooong Committed by GitHub

Revert "[EditableText] call `onSelectionChanged` only when there're actual...

Revert "[EditableText] call `onSelectionChanged` only when there're actual selection/cause changes (#87971)" (#88183)
parent 72a1e65a
...@@ -593,6 +593,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin, ...@@ -593,6 +593,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
bool _wasSelectingVerticallyWithKeyboard = false; bool _wasSelectingVerticallyWithKeyboard = false;
void _setTextEditingValue(TextEditingValue newValue, SelectionChangedCause cause) { void _setTextEditingValue(TextEditingValue newValue, SelectionChangedCause cause) {
textSelectionDelegate.textEditingValue = newValue;
textSelectionDelegate.userUpdateTextEditingValue(newValue, cause); textSelectionDelegate.userUpdateTextEditingValue(newValue, cause);
} }
...@@ -612,11 +613,11 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin, ...@@ -612,11 +613,11 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
extentOffset: math.min(nextSelection.extentOffset, textLength), extentOffset: math.min(nextSelection.extentOffset, textLength),
); );
} }
_handleSelectionChange(nextSelection, cause);
_setTextEditingValue( _setTextEditingValue(
textSelectionDelegate.textEditingValue.copyWith(selection: nextSelection), textSelectionDelegate.textEditingValue.copyWith(selection: nextSelection),
cause, cause,
); );
_handleSelectionChange(nextSelection, cause);
} }
void _handleSelectionChange( void _handleSelectionChange(
......
...@@ -1226,7 +1226,7 @@ class TextSelectionGestureDetectorBuilder { ...@@ -1226,7 +1226,7 @@ class TextSelectionGestureDetectorBuilder {
@protected @protected
void onDoubleTapDown(TapDownDetails details) { void onDoubleTapDown(TapDownDetails details) {
if (delegate.selectionEnabled) { if (delegate.selectionEnabled) {
renderEditable.selectWord(cause: SelectionChangedCause.doubleTap); renderEditable.selectWord(cause: SelectionChangedCause.tap);
if (shouldShowSelectionToolbar) if (shouldShowSelectionToolbar)
editableText.showToolbar(); editableText.showToolbar();
} }
......
...@@ -2381,7 +2381,6 @@ void main() { ...@@ -2381,7 +2381,6 @@ void main() {
await gesture.moveBy(const Offset(600, 0)); await gesture.moveBy(const Offset(600, 0));
// To the edge of the screen basically. // To the edge of the screen basically.
await tester.pump(); await tester.pump();
await tester.pumpAndSettle();
expect( expect(
controller.selection, controller.selection,
const TextSelection.collapsed(offset: 54, affinity: TextAffinity.upstream), const TextSelection.collapsed(offset: 54, affinity: TextAffinity.upstream),
...@@ -2389,14 +2388,12 @@ void main() { ...@@ -2389,14 +2388,12 @@ void main() {
// Keep moving out. // Keep moving out.
await gesture.moveBy(const Offset(1, 0)); await gesture.moveBy(const Offset(1, 0));
await tester.pump(); await tester.pump();
await tester.pumpAndSettle();
expect( expect(
controller.selection, controller.selection,
const TextSelection.collapsed(offset: 61, affinity: TextAffinity.upstream), const TextSelection.collapsed(offset: 61, affinity: TextAffinity.upstream),
); );
await gesture.moveBy(const Offset(1, 0)); await gesture.moveBy(const Offset(1, 0));
await tester.pump(); await tester.pump();
await tester.pumpAndSettle();
expect( expect(
controller.selection, controller.selection,
const TextSelection.collapsed(offset: 66, affinity: TextAffinity.upstream), const TextSelection.collapsed(offset: 66, affinity: TextAffinity.upstream),
......
...@@ -3871,7 +3871,7 @@ void main() { ...@@ -3871,7 +3871,7 @@ void main() {
editableTextState.textEditingValue.copyWith( editableTextState.textEditingValue.copyWith(
selection: TextSelection.collapsed(offset: longText.length), selection: TextSelection.collapsed(offset: longText.length),
), ),
SelectionChangedCause.tap, null,
); );
await tester.pump(); // TODO(ianh): Figure out why this extra pump is needed. await tester.pump(); // TODO(ianh): Figure out why this extra pump is needed.
...@@ -3908,7 +3908,7 @@ void main() { ...@@ -3908,7 +3908,7 @@ void main() {
editableTextState.textEditingValue.copyWith( editableTextState.textEditingValue.copyWith(
selection: const TextSelection.collapsed(offset: tallText.length), selection: const TextSelection.collapsed(offset: tallText.length),
), ),
SelectionChangedCause.tap, null,
); );
await tester.pump(); await tester.pump();
await skipPastScrollingAnimation(tester); await skipPastScrollingAnimation(tester);
...@@ -7709,7 +7709,6 @@ void main() { ...@@ -7709,7 +7709,6 @@ void main() {
await gesture.moveBy(const Offset(600, 0)); await gesture.moveBy(const Offset(600, 0));
// To the edge of the screen basically. // To the edge of the screen basically.
await tester.pump(); await tester.pump();
await tester.pumpAndSettle();
expect( expect(
controller.selection, controller.selection,
const TextSelection.collapsed(offset: 56, affinity: TextAffinity.downstream), const TextSelection.collapsed(offset: 56, affinity: TextAffinity.downstream),
...@@ -7717,14 +7716,12 @@ void main() { ...@@ -7717,14 +7716,12 @@ void main() {
// Keep moving out. // Keep moving out.
await gesture.moveBy(const Offset(1, 0)); await gesture.moveBy(const Offset(1, 0));
await tester.pump(); await tester.pump();
await tester.pumpAndSettle();
expect( expect(
controller.selection, controller.selection,
const TextSelection.collapsed(offset: 62, affinity: TextAffinity.downstream), const TextSelection.collapsed(offset: 62, affinity: TextAffinity.downstream),
); );
await gesture.moveBy(const Offset(1, 0)); await gesture.moveBy(const Offset(1, 0));
await tester.pump(); await tester.pump();
await tester.pumpAndSettle();
expect( expect(
controller.selection, controller.selection,
const TextSelection.collapsed(offset: 66, affinity: TextAffinity.upstream), const TextSelection.collapsed(offset: 66, affinity: TextAffinity.upstream),
......
...@@ -34,9 +34,7 @@ class FakeEditableTextState with TextSelectionDelegate { ...@@ -34,9 +34,7 @@ class FakeEditableTextState with TextSelectionDelegate {
void hideToolbar([bool hideHandles = true]) { } void hideToolbar([bool hideHandles = true]) { }
@override @override
void userUpdateTextEditingValue(TextEditingValue value, SelectionChangedCause cause) { void userUpdateTextEditingValue(TextEditingValue value, SelectionChangedCause cause) { }
textEditingValue = value;
}
@override @override
void bringIntoView(TextPosition position) { } void bringIntoView(TextPosition position) { }
......
...@@ -650,7 +650,7 @@ void main() { ...@@ -650,7 +650,7 @@ void main() {
// false. // false.
state.userUpdateTextEditingValue( state.userUpdateTextEditingValue(
state.textEditingValue.copyWith(selection: const TextSelection.collapsed(offset: 90)), state.textEditingValue.copyWith(selection: const TextSelection.collapsed(offset: 90)),
SelectionChangedCause.tap, null,
); );
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(isCaretOnScreen(tester), isTrue); expect(isCaretOnScreen(tester), isTrue);
...@@ -674,7 +674,7 @@ void main() { ...@@ -674,7 +674,7 @@ void main() {
state.userUpdateTextEditingValue( state.userUpdateTextEditingValue(
state.textEditingValue.copyWith(selection: const TextSelection.collapsed(offset: 100)), state.textEditingValue.copyWith(selection: const TextSelection.collapsed(offset: 100)),
SelectionChangedCause.tap, null,
); );
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(isCaretOnScreen(tester), isTrue); expect(isCaretOnScreen(tester), isTrue);
......
...@@ -4938,19 +4938,21 @@ void main() { ...@@ -4938,19 +4938,21 @@ void main() {
testWidgets('keyboard text selection works (RawKeyEvent)', (WidgetTester tester) async { testWidgets('keyboard text selection works (RawKeyEvent)', (WidgetTester tester) async {
debugKeyEventSimulatorTransitModeOverride = KeyDataTransitMode.rawKeyData; debugKeyEventSimulatorTransitModeOverride = KeyDataTransitMode.rawKeyData;
addTearDown(() { debugKeyEventSimulatorTransitModeOverride = null; });
await testTextEditing(tester, targetPlatform: defaultTargetPlatform); await testTextEditing(tester, targetPlatform: defaultTargetPlatform);
debugKeyEventSimulatorTransitModeOverride = null; debugKeyEventSimulatorTransitModeOverride = null;
// On web, using keyboard for selection is handled by the browser. // On web, using keyboard for selection is handled by the browser.
}, variant: TargetPlatformVariant.all(), skip: kIsWeb); // [intended] }, variant: TargetPlatformVariant.all(), skip: kIsWeb); // [intended]
testWidgets('keyboard text selection works (ui.KeyData then RawKeyEvent)', (WidgetTester tester) async { testWidgets('keyboard text selection works (ui.KeyData then RawKeyEvent)', (WidgetTester tester) async {
debugKeyEventSimulatorTransitModeOverride = KeyDataTransitMode.keyDataThenRawKeyData; debugKeyEventSimulatorTransitModeOverride = KeyDataTransitMode.keyDataThenRawKeyData;
addTearDown(() { debugKeyEventSimulatorTransitModeOverride = null; });
await testTextEditing(tester, targetPlatform: defaultTargetPlatform); await testTextEditing(tester, targetPlatform: defaultTargetPlatform);
debugKeyEventSimulatorTransitModeOverride = null; debugKeyEventSimulatorTransitModeOverride = null;
// On web, using keyboard for selection is handled by the browser. // On web, using keyboard for selection is handled by the browser.
}, variant: TargetPlatformVariant.all(), skip: kIsWeb); // [intended] }, variant: TargetPlatformVariant.all(), skip: kIsWeb); // [intended]
...@@ -6045,6 +6047,7 @@ void main() { ...@@ -6045,6 +6047,7 @@ void main() {
'TextInput.setStyle', 'TextInput.setStyle',
'TextInput.setEditingState', 'TextInput.setEditingState',
'TextInput.setEditingState', 'TextInput.setEditingState',
'TextInput.show',
'TextInput.setCaretRect', 'TextInput.setCaretRect',
]; ];
expect( expect(
...@@ -6088,14 +6091,16 @@ void main() { ...@@ -6088,14 +6091,16 @@ void main() {
'TextInput.setStyle', 'TextInput.setStyle',
'TextInput.setEditingState', 'TextInput.setEditingState',
'TextInput.setEditingState', 'TextInput.setEditingState',
'TextInput.show',
'TextInput.setCaretRect', 'TextInput.setCaretRect',
'TextInput.show',
]; ];
expect(tester.testTextInput.log.length, logOrder.length);
expect( int index = 0;
tester.testTextInput.log.map((MethodCall methodCall) => methodCall.method), for (final MethodCall m in tester.testTextInput.log) {
logOrder, expect(m.method, logOrder[index]);
); index++;
}
expect(tester.testTextInput.editingState!['text'], 'flutter is the best!'); expect(tester.testTextInput.editingState!['text'], 'flutter is the best!');
}); });
...@@ -6136,6 +6141,7 @@ void main() { ...@@ -6136,6 +6141,7 @@ void main() {
'TextInput.setStyle', 'TextInput.setStyle',
'TextInput.setEditingState', 'TextInput.setEditingState',
'TextInput.setEditingState', 'TextInput.setEditingState',
'TextInput.show',
'TextInput.setCaretRect', 'TextInput.setCaretRect',
'TextInput.setEditingState', 'TextInput.setEditingState',
]; ];
...@@ -6209,7 +6215,7 @@ void main() { ...@@ -6209,7 +6215,7 @@ void main() {
selection: controller.selection, selection: controller.selection,
)); ));
expect(log, isEmpty); expect(log.length, 0);
// setEditingState is called when remote value modified by the formatter. // setEditingState is called when remote value modified by the formatter.
state.updateEditingValue(TextEditingValue( state.updateEditingValue(TextEditingValue(
...@@ -7454,115 +7460,6 @@ void main() { ...@@ -7454,115 +7460,6 @@ 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', () { group('callback errors', () {
const String errorText = 'Test EditableText callback error'; 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