Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SuperTextField] Fix new line duplication (Resolves #1468) #1469

Merged
merged 11 commits into from
Oct 17, 2023
Merged

[SuperTextField] Fix new line duplication (Resolves #1468) #1469

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
fd43623
[SuperTextField] Fix new line duplication (Resolves #1468)
angelosilvestre Sep 26, 2023
e680a7c
Update comment
angelosilvestre Sep 26, 2023
8f5b94c
Check for ime connections
angelosilvestre Sep 28, 2023
3f357a3
Update comments and naming
angelosilvestre Sep 30, 2023
d049437
Generate insertion deltas using test robots
angelosilvestre Sep 30, 2023
9b8aea5
Remove new line handlers for IME mode
angelosilvestre Sep 30, 2023
6b400d2
Use adaptive robot methods
angelosilvestre Oct 3, 2023
81b29e3
Always use deltas to insert new lines
angelosilvestre Oct 4, 2023
96c5c7c
Bump flutter_test_robots dependency
angelosilvestre Oct 4, 2023
77754b0
PR updates
angelosilvestre Oct 4, 2023
ac6ea26
Remove import
angelosilvestre Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions super_editor/lib/src/super_textfield/desktop/desktop_textfield.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1308,7 +1308,11 @@ class _SuperTextFieldImeInteractorState extends State<SuperTextFieldImeInteracto
void _onPerformAction(TextInputAction action) {
switch (action) {
case TextInputAction.newline:
_textController.insertNewline();
// Do nothing for IME newline actions.
//
// Mac: Key presses flow, unhandled, to the OS and turn into IME selectors. We handle newlines there.
// Windows/Linux: Key presses flow, unhandled, to the OS and turn into text deltas. We handle newlines there.
// Android/iOS: This text field implementation is only for desktop, mobile is handled elsewhere.
break;
case TextInputAction.done:
widget.focusNode.unfocus();
Expand Down Expand Up @@ -1761,7 +1765,6 @@ const defaultTextFieldImeKeyboardHandlers = <TextFieldKeyboardHandler>[
// handlers, passing the key combo to the OS on Mac. Place all custom Mac key
// combos above this handler.
DefaultSuperTextFieldKeyboardHandlers.sendKeyEventToMacOs,
DefaultSuperTextFieldKeyboardHandlers.insertNewlineWhenEnterIsPressed,
DefaultSuperTextFieldKeyboardHandlers.moveCaretToStartOrEnd,
DefaultSuperTextFieldKeyboardHandlers.moveUpDownLeftAndRightWithArrowKeys,
DefaultSuperTextFieldKeyboardHandlers.moveToLineStartWithHome,
Expand Down
2 changes: 1 addition & 1 deletion super_editor/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ dependencies:
# Dependencies for testing tools that we ship with super_editor
flutter_test:
sdk: flutter
flutter_test_robots: 0.0.21
flutter_test_robots: 0.0.22

dependency_overrides:
# # Override to local mono-repo path so devs can test this repo
Expand Down
1 change: 1 addition & 0 deletions super_editor/test/super_editor/supereditor_test_tools.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'dart:math';
import 'dart:ui' as ui;

import 'package:flutter/foundation.dart';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need this import?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. Removed.

import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:mockito/mockito.dart';
Expand Down
93 changes: 15 additions & 78 deletions super_editor/test/super_textfield/super_textfield_ime_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ void main() {
});

group('inserts line', () {
testWidgetsOnWindowsAndLinux('when ENTER is pressed in middle of text', (tester) async {
testWidgetsOnDesktop('when ENTER is pressed in middle of text', (tester) async {
await _pumpSuperTextField(
tester,
AttributedTextEditingController(
Expand All @@ -240,31 +240,13 @@ void main() {
);
await tester.placeCaretInSuperTextField(8);

await tester.pressEnter();
await tester.pressEnterAdaptive(getter: imeClientGetter);

expect(SuperTextFieldInspector.findText().text, "this is \nsome text");
expect(SuperTextFieldInspector.findSelection(), const TextSelection.collapsed(offset: 9));
});

testWidgetsOnMac('when ENTER is pressed in middle of text (on MAC)', (tester) async {
await _pumpSuperTextField(
tester,
AttributedTextEditingController(
text: AttributedText('this is some text'),
),
);
await tester.placeCaretInSuperTextField(8);

// Simulate the user pressing ENTER.
// On macOS, pressing ENTER generates a new line input action.
await tester.testTextInput.receiveAction(TextInputAction.newline);
await tester.pump();

expect(SuperTextFieldInspector.findText().text, "this is \nsome text");
expect(SuperTextFieldInspector.findSelection(), const TextSelection.collapsed(offset: 9));
});

testWidgetsOnWindowsAndLinux('when ENTER is pressed at beginning of text', (tester) async {
testWidgetsOnDesktop('when ENTER is pressed at beginning of text', (tester) async {
await _pumpSuperTextField(
tester,
AttributedTextEditingController(
Expand All @@ -273,46 +255,13 @@ void main() {
);
await tester.placeCaretInSuperTextField(0);

await tester.pressEnter();
await tester.pressEnterAdaptive(getter: imeClientGetter);

expect(SuperTextFieldInspector.findText().text, "\nthis is some text");
expect(SuperTextFieldInspector.findSelection(), const TextSelection.collapsed(offset: 1));
});

testWidgetsOnMac('when ENTER is pressed at beginning of text (on MAC)', (tester) async {
await _pumpSuperTextField(
tester,
AttributedTextEditingController(
text: AttributedText('this is some text'),
),
);
await tester.placeCaretInSuperTextField(0);

// Simulate the user pressing ENTER.
// On macOS, pressing ENTER generates a new line input action.
await tester.testTextInput.receiveAction(TextInputAction.newline);
await tester.pump();

expect(SuperTextFieldInspector.findText().text, "\nthis is some text");
expect(SuperTextFieldInspector.findSelection(), const TextSelection.collapsed(offset: 1));
});

testWidgetsOnWindowsAndLinux('when ENTER is pressed at end of text', (tester) async {
await _pumpSuperTextField(
tester,
AttributedTextEditingController(
text: AttributedText('this is some text'),
),
);
await tester.placeCaretInSuperTextField(17);

await tester.pressEnter();

expect(SuperTextFieldInspector.findText().text, "this is some text\n");
expect(SuperTextFieldInspector.findSelection(), const TextSelection.collapsed(offset: 18));
});

testWidgetsOnMac('when ENTER is pressed at end of text (on MAC)', (tester) async {
testWidgetsOnDesktop('when ENTER is pressed at end of text', (tester) async {
await _pumpSuperTextField(
tester,
AttributedTextEditingController(
Expand All @@ -321,16 +270,13 @@ void main() {
);
await tester.placeCaretInSuperTextField(17);

// Simulate the user pressing ENTER.
// On macOS, pressing ENTER generates a new line input action.
await tester.testTextInput.receiveAction(TextInputAction.newline);
await tester.pump();
await tester.pressEnterAdaptive(getter: imeClientGetter);

expect(SuperTextFieldInspector.findText().text, "this is some text\n");
expect(SuperTextFieldInspector.findSelection(), const TextSelection.collapsed(offset: 18));
});

// TODO: Make this a Windows + Linux test when Flutter supports numpad enter on windows
// TODO: Merge this with the testWidgetsOnMac below when Flutter supports numpad enter on windows
testWidgetsOnLinux('when NUMPAD ENTER is pressed in middle of text', (tester) async {
await _pumpSuperTextField(
tester,
Expand All @@ -340,7 +286,7 @@ void main() {
);
await tester.placeCaretInSuperTextField(8);

await tester.pressNumpadEnter();
await tester.pressNumpadEnterAdaptive(getter: imeClientGetter);

expect(SuperTextFieldInspector.findText().text, "this is \nsome text");
expect(SuperTextFieldInspector.findSelection(), const TextSelection.collapsed(offset: 9));
Expand All @@ -355,16 +301,13 @@ void main() {
);
await tester.placeCaretInSuperTextField(8);

// Simulate the user pressing ENTER.
// On macOS, pressing ENTER generates a new line input action.
await tester.testTextInput.receiveAction(TextInputAction.newline);
await tester.pump();
await tester.pressNumpadEnterAdaptive(getter: imeClientGetter);

expect(SuperTextFieldInspector.findText().text, "this is \nsome text");
expect(SuperTextFieldInspector.findSelection(), const TextSelection.collapsed(offset: 9));
});

// TODO: Make this a Windows + Linux test when Flutter supports numpad enter on windows
// TODO: Merge this with the testWidgetsOnMac below when Flutter supports numpad enter on windows
testWidgetsOnLinux('when NUMPAD ENTER is pressed at beginning of text', (tester) async {
await _pumpSuperTextField(
tester,
Expand All @@ -374,7 +317,7 @@ void main() {
);
await tester.placeCaretInSuperTextField(0);

await tester.pressNumpadEnter();
await tester.pressNumpadEnterAdaptive(getter: imeClientGetter);

expect(SuperTextFieldInspector.findText().text, "\nthis is some text");
expect(SuperTextFieldInspector.findSelection(), const TextSelection.collapsed(offset: 1));
Expand All @@ -389,16 +332,13 @@ void main() {
);
await tester.placeCaretInSuperTextField(0);

// Simulate the user pressing ENTER.
// On macOS, pressing ENTER generates a new line input action.
await tester.testTextInput.receiveAction(TextInputAction.newline);
await tester.pump();
await tester.pressNumpadEnterAdaptive(getter: imeClientGetter);

expect(SuperTextFieldInspector.findText().text, "\nthis is some text");
expect(SuperTextFieldInspector.findSelection(), const TextSelection.collapsed(offset: 1));
});

// TODO: Make this a Windows + Linux test when Flutter supports numpad enter on windows
// TODO: Merge this with the testWidgetsOnMac below when Flutter supports numpad enter on windows
testWidgetsOnLinux('when NUMPAD ENTER is pressed at end of text', (tester) async {
await _pumpSuperTextField(
tester,
Expand All @@ -408,7 +348,7 @@ void main() {
);
await tester.placeCaretInSuperTextField(17);

await tester.pressNumpadEnter();
await tester.pressNumpadEnterAdaptive(getter: imeClientGetter);

expect(SuperTextFieldInspector.findText().text, "this is some text\n");
expect(SuperTextFieldInspector.findSelection(), const TextSelection.collapsed(offset: 18));
Expand All @@ -423,10 +363,7 @@ void main() {
);
await tester.placeCaretInSuperTextField(17);

// Simulate the user pressing ENTER.
// On macOS, pressing ENTER generates a new line input action.
await tester.testTextInput.receiveAction(TextInputAction.newline);
await tester.pump();
await tester.pressNumpadEnterAdaptive(getter: imeClientGetter);

expect(SuperTextFieldInspector.findText().text, "this is some text\n");
expect(SuperTextFieldInspector.findSelection(), const TextSelection.collapsed(offset: 18));
Expand Down
Loading