[SuperEditor][Web] Fix selection change with CMD + RIGHT (Resolves #2304) #2327
Conversation
|
Playing around a bit with the css flutter generates, it seems that the html input does end the line before "steps" (red is the html input text):
|
|
This probably shouldn't be too surprising that this happens given that Flutter uses a different text layout implementation than the browser. Can you file an issue with Flutter? Also, did you test jumping by word to see if that's broken in a similar way? |
| @@ -635,6 +635,20 @@ ExecutionInstruction doNothingWithLeftRightArrowKeysAtMiddleOfTextOnWeb({ | |||
| return ExecutionInstruction.continueExecution; | |||
| } | |||
|
|
|||
| if ((CurrentPlatform.isApple && HardwareKeyboard.instance.isMetaPressed) || | |||
There was a problem hiding this comment.
Rather than have this blocking handler know about certain key combo exceptions, can you just move or replicate the relevant key combo above this one in the priority list, along with a comment about why?
There was a problem hiding this comment.
Do you mean to have a moveToStartOrEndOfLineWithArrowKeysOnWeb ?
There was a problem hiding this comment.
I'm not sure about the specifics, because I don't know which handlers this one is currently blocking, but the name you mentioned seems to make sense. If we had such a handler higher up in priority, instead of blocking logic, would it still fix the issue?
There was a problem hiding this comment.
Added a new handler for that.
I'll try to make a repo.
Jumping by word looks fine. |
super_editor/lib/src/default_editor/document_hardware_keyboard/document_keyboard_actions.dart
Outdated
Show resolved
Hide resolved
| if (!arrowKeys.contains(keyEvent.logicalKey)) { | ||
| return ExecutionInstruction.continueExecution; | ||
| } | ||
|
|
There was a problem hiding this comment.
Should we check for any other control keys, like Option? A quick check of cmd+option+arrow shows that something else happens in my browser. It doesn't move my cursor. Instead, it seems to switch browser tabs...
There was a problem hiding this comment.
I tried in chrome and pressing cmd + option + arrow doesn't hit this handler. The browser switches tabs as expected.
Maybe the browser is handling the shortcut before us and prevent the event?
|
Updated the PR to prevent moving between nodes with CMD + LEFT/RIGHT arrow as discussed in the original ticket. |
| @@ -1616,6 +1616,58 @@ This is a paragraph | |||
| ), | |||
| ); | |||
| }); | |||
|
|
|||
| testWidgetsOnApple('with CMD + LEFT ARROW at the beginning of a paragraph', (tester) async { | |||
There was a problem hiding this comment.
These tests verify that CMD + LEFT ARROW do nothing "at the beginning of the paragraph" and the right arrow does nothing "at the end of the paragraph".
I'm confused by this.
CMD + LEFT/RIGHT ARROW should do nothing on Linux ever. Not just at the beginning and end of lines.
It's on a Mac that they should do nothing at the beginning/end of lines.
Also, isn't this ticket about web? If so, shouldn't the new tests simulate web, and reference web?
There was a problem hiding this comment.
CMD + LEFT/RIGHT ARROW should do nothing on Linux ever. Not just at the beginning and end of lines.
But we are not running on Linux, we are running on Apple Devices. I create a testWidgetsOnMacDesktopAndWeb that might be more appropriate here.
Also, isn't this ticket about web? If so, shouldn't the new tests simulate web, and reference web?
Yes, but in this comment Brian agreed that we should keep this behavior consistent for web and desktop.
There was a problem hiding this comment.
Aren't these tests declared inside of a "Linux" group?
There was a problem hiding this comment.
I didn't notice that. Moved to "Mac and iOS >" group.
[SuperEditor][Web] Fix selection change with CMD + RIGHT. Resolves #2304
Steps to reproduce:
Expected: Cursor is placed at the end of the line, Actual: Cursor is placed at the beginning of the word "steps"
This wrong offset is being reported by the selection change delta. On web, Flutter relies on html inputs to handle text input. When the user presses CMD + RIGHT, the browser moves the selection to where it thinks the end of the line is, and the Flutter code generates text deltas for us.
I haven't dug deep into this, but this seems our text layout and the browser text layout are different. For example, in the repro text, the browser thinks the line ends before "steps".
We started handling deltas for moving the caret with the arrow keys in #1269. I think I failed to document why we need to let the IME generate non-text deltas for this. However, I tested the issues reported in the original ticket (#1224) and I wasn't able to reproduce any of them.
This PR changes
doNothingWithLeftRightArrowKeysAtMiddleOfTextOnWebto avoid sending the key event back to the OS if CMD is pressed.The ideal solution would be to investigate why our line breaks are different from the browser line breaks, but this will require a deeper investigation.
As a reference, in our "Regular Flutter TextField Demo", if we limit the width of the Flutter TextField to 400px, we can also see the caret moving to the wrong position.