Integrate IME for mobile doc editing #374
Conversation
…er to debug IME issues
…so fixed some replace and insertion issues by applying the replace and insertion ranges from their deltas.
|
@miguelcmedeiros @Jethro87 - Can each of you try out this PR for mobile document editing? I'd like to figure out the minimal set of changes/fixes from this point to where each of you can ship your desired features. |
|
@matthew-carroll I've run On the iOS simulator, selecting 'Toggle Software Keyboard' results in this output: I get the same output when I type any letter using my physical keyboard in the iOS simulator: Can you point me in the right direction here? |
|
In
click to expand |
Btw, this is working fine on my end (except for the caret placement after the replacement -- I guess that's related to the first issue). I'm using Another note, I believe the Edit: Everything else seems to work great for iOS. |
|
@Jethro87 are you running the example app, or trying to use it in your own app? If you're using the example app, did you open the mobile-specific examples? |
|
@osaxma for the newline, are you just pressing the action button on the keyboard to get the newline, or are you doing something else? |
|
I am using the action button on the virtual keyboard. It looks like the error happens because
Here's the log: |
@matthew-carroll I'm running the example app. I was originally running in Flutter master, then switched to stable with the same issue. |
|
@Jethro87 what happens when you tap around the document? Does the caret and/or handles appear? Does the keyboard come up when you place the caret? Also, I'm a bit confused by the output you shared above. It looks like you're trying to use the keyboard input, not the IME input. You're sure that you opened one of the mobile demos? There shouldn't even be a keyboard listener in those... Is it possible that you're using the latest version of super_editor on |
That's it, @matthew-carroll, thanks. I initially was going to run this in my own app, as I had things set up from the last time. After switching to running the example, I neglected to check out the proper branch. Working now. Will test it thoroughly tomorrow. |
|
The following two points look to be the same as above #374 (comment)
The following points are for iOS.
|
|
I also noticed that when we change node type using the keyboard toolbar, that it leaves the caret in the same coordinates with the same size. See the following video: |
…based on the updated node's content.
|
@Jethro87 @osaxma @miguelcmedeiros - the caret timing issue should be resolved with the latest push |
I can confirm :) |
|
The following points are for Android.
|
|
@Jethro87 can you describe the difficulty with selecting an empty paragraph? I know there was a caret timing issue, and there was an issue with inserting new characters, both of which should now be fixed. But you also mentioned that you had a hard time selecting the empty paragraph. I just tried tapping anywhere horizontally within an empty paragraph and it's bringing the keyboard up for me without issue. |
|
@matthew-carroll I found a few more issues.
super_editor_double_space_period.mov
super_editor_autocorrect_bug.mov
super_editor_block_formatting_selection.mov
super_editor_inline_formatting_not_showing.mov
super_editor_delete_selection_bug.mov
|
There was a problem hiding this comment.
I managed to integrate these changes into our project and most of it works well, with one exception.
Not sure if I'm misusing the API, but I don't seem to get the clipper passed to createOverlayControlsClipper to refresh, so that it can accommodate for showing/hiding the toolbar or changing its height.
| } | ||
|
|
||
| // Place the caret at the beginning of the second node. | ||
| // Place the caret at the beginning of the new node. | ||
| print("Placing caret in new text node: $newNodeId"); |
| return ListenableBuilder( | ||
| listenable: widget.editingController, | ||
| builder: (context) { | ||
| print("Clipper method: ${widget.createOverlayControlsClipper}"); |
@matthew-carroll |
|
@miguelcmedeiros the builder provides access to a Passing a If the demo configuration isn't working, please provide repro steps so that I can see what's not working on my end. |
|
@Jethro87 Is the auto-capitalization behavior a ship blocker for mobile editing? I think we're going to have to introduce our own rules for that kind of behavior because we have to prepend a bogus character, like "~", before any text in a node. Without the bogus character, the keyboard won't report a deletion, which prevents us from deleting a paragraph and moving the caret up to the one above. But the inclusion of the bogus character makes the IME believe you're always editing the 2nd+ character. Off the top of my head, I don't know what a quick solution would be to this problem. |
|
@miguelcmedeiros for the clipper, are you saying that you don't know how to re-calculate the clipper when your toolbar changes? If so, try defining your own Something like: class MyCustomClipper extends CustomClipper<Rect> {
MyCustomClipper({
required Listenable toolbarChangeNotifier,
}) : super(reclip: toolbarChangeNotifier);
}Then pass a builder that instantiates that clipper: SuperEditor(
//...
createOverlayControlsClipper: (_) => MyCustomClipper(toolbarChangeNotifier: myToolbarWatcher),
); |
@miguelcmedeiros - I removed the print statements |
|
@matthew-carroll, thanks for the tip with |
|
@matthew-carroll, while testing using the latest changes we noticed that on Android pressing and holding the backspace key when the selection is at the end of a paragraph, it "releases" the key after deleting one character. |
|
Thanks, @matthew-carroll, the other issues look good. Auto-capitalization of the first word in a new node, as it is now, isn't a great experience. Not only does the user have to explicitly capitalize the first word (a non-standard behavior), but adding a new node causes the keyboard to "flash", as per this video: super_editor_first_letter_capitalize.movCan you set the keyboard to (sentence) capitalize when the carat is next to the bogus character? If so, this might be a good enough workaround, and I wonder if this would get rid of the "flash" when a user adds a new node. |
|
@Jethro87 - I was able to get the desired capitalization behavior by replacing the invisible bogus characters, like "~" and "^" at the front of the text, with the substring ". ". This makes the IME think that the text at the beginning of an empty paragraph is actually the start of a new sentence, so you get caps. I recommend doing a full regression test. I can't write automated tests around this behavior because it depends upon unpredictable delta reporting from the OS. @miguelcmedeiros - I don't think I'm gonna be able to solve that deletion problem. The issue is that whenever we send a composing When you hold down the delete button, Android sends us a non-zero composing region. We don't care about composing regions, so we always send I tried to always send the OS whatever composing region it sends us, but that resulted in occasional You can see the Flutter code that forces this restart, here: and Please let me know if there are any more ship blocking issues, or if we can merge this in. |
@matthew-carroll, this doesn't have to block this PR, but it has to be solved before we release Android apps with SuperEditor. I'll create a ticket for this issue, so that we can tackle it separately. |
|
@matthew-carroll The capitalization behavior is quite good, nice work. I did other testing and didn't find any issues, other than "Cut/Copy/Paste" still not working, but I think (?) this is intentional. |
|
@Jethro87 is there anything blocking you from implementing cut/copy/paste yourself? I put that dummy popover toolbar in there just to prove that it can be placed and moved correctly. But given that Flutter still doesn't have a legitimate copy buffer, I think each app is gonna want to decide how to interpret that data. For example, when you copy the content, what format should we use? Markdown, HTML, something else? |
|
@matthew-carroll Right, that makes sense. This isn't a problem I've given thought to but I understand the issue. Will think more on it. |
|
Ok, I'm gonna merge this in. Fingers crossed. |
Integrates IME input with document editing on Android and iOS.
The example app includes a demo for iOS and Android. Each one is styled appropriately, but regardless of which demo you run, the IME connection that it's using is based on the underlying platform. Therefore, even if the Android example is run on iOS, it's still using the standard iOS IME deltas.
Known issues: