Let apps open and close the keyboard on demand, and let selection exist when keyboard is closed (Resolves #875) #876
Conversation
…d is down, but selection is explicitly removed when toolbar is closed.
|
@Jethro87 can you give this PR a try for your keyboard use-case? I made a bit of a mess of the IME code. But before I spend time cleaning it up, I want to make sure we solved the interactions that you need. To see a demo of the behavior that you're interested in, check the example app for Here's a video: Screen.Recording.2022-12-04.at.11.39.38.PM.mov |
|
@matthew-carroll The demo interactions work as expected. I integrated the "above keyboard" mobile toolbar into the "expanded" toolbar to test if adding attributions to the selected text works, and it worked well. Whenever I type anything, however, I get this exception: |
|
@Jethro87 do you wanna try this again and see if that bug is resolved? If it is, do you want to try integrating this on a branch of your app and see how it goes? There will be some further massaging of the API and implementation, but if you try to integrate the current implementation, it might demonstrate the parts that work well, and the parts that don't. |
|
@matthew-carroll The typing bug has been resolved, but I found a couple more bugs. I will list them below. In the meantime, I'll integrate this into Clearful and let you know how it goes. The following bugs occur with this base document: They both occur when grabbing the single selection handle (Android) and moving it anywhere in the document. Bug 1: Move the selection handle anywhere in the document without typing firstBug 2: Type anything in the document, then move the selection handle anywhere else in the document |
|
@Jethro87 I fixed at least one composing region issue. Can you try again? |
|
@matthew-carroll Got another assertion error. This happened again after typing in a doc, then moving the selection handle around. |
|
@Jethro87 can you let me know what you did, exactly? My initial attempt to type and move the caret isn't crashing anything on my end. |
|
@matthew-carroll I will attempt to create a simpler example that can be reproduced. I've done much of the integration into my app and it's there that I've been testing this. FWIW, this doesn't seem to be happening at all on iOS. |
|
@matthew-carroll I found a bug I can reproduce on iOS and Android. It's detailed below. Also, I have a question. Currently, tapping the editor causes the keyboard to rise. This is expected when a document is opened and it there is no selection / the IME is not attached. I don't necessarily expect this when the editor has selection / the IME is attached and the keyboard is lowered with the panel visible. In this state, tapping the editor anywhere will bring the keyboard back up. If a user has a document that they want to format, for example, and the formatting options are in the behind-keyboard-panel, they would have to double tap to select a word, lower the keyboard, apply the format, tap another word (which raises the keyboard), lower the keyboard, apply the format, etc, etc. In both your demo and my app there is a button on top of the keyboard/panel that allows the user to dismiss the panel and bring the keyboard back up. Now that the editor can continue to have selection when the keyboard is down, I'm wondering if the responsibility to raise the keyboard should be on the developer, rather than super_editor automatically raising the keyboard on tap. Something like this:
What do you think? -- Here's how to reproduce the bug
The following happens to me reliably on iOS and Android: |
|
@Jethro87 I tried those steps on an emulator and a Pixel 3a, but I wasn't able to cause a crash when moving the handles with the keyboard collapsed. I'll take a look at the keyboard expansion issue once we work out the bugs with the current implementation. |
|
@Jethro87 do you have more specific repro steps that we can try? |
|
@matthew-carroll I've been testing this quite a bit in Clearful the past few days. Things are going well overall, and I haven't been able to reproduce those selection bugs that were happening. Not sure what happened there. I've found two issues that have been introduced by having to set 1. When the editor contains a document with many nodes and the keyboard is up, I cannot scroll to the bottom of the document.RPReplay_Final1670962858.mov2. Auto-scrolling no longer works properly when tapping a document without selection.RPReplay_Final1670962902.mov |
|
Re: can't scroll to end... Apply @override
Widget build(BuildContext context) {
return Scaffold(
resizeToAvoidBottomInset: false,
body: Stack(
children: [
Positioned.fill(
child: Padding(
padding: MediaQuery.of(context).viewInsets,
child: SuperEditor(
editor: _editor,
composer: _composer,
),
),
),
Positioned(
left: 0,
right: 0,
bottom: 0,
child: BehindKeyboardPanel(
onOpenKeyboard: _openKeyboard,
onCloseKeyboard: _closeKeyboard,
onEndEditing: _endEditing,
),
),
],
),
);
} |
|
Re: no auto-scrolling... The above solution, which changes the height of |
…uperEditor doesn't flow behind keyboard panel.
|
@matthew-carroll That fixed both issues. Thanks! |
|
Great. Is the only remaining issue the fact that re-selecting a selected document automatically raises the keyboard? Or are there still other use-case issues? |
|
@matthew-carroll I haven't found any other issues. Things look good, other than re-selecting a selected document automatically raises the keyboard. |
…'t re-open keyboard
|
@Jethro87 let me know if the latest change successfully avoids any keyboard opening when you don't want it to. If we've solved all the use-cases, then it will be time to add tests to lock down the behaviors, followed by a general IME refactoring to create pieces that make more sense for behavior changes like this. |
|
@matthew-carroll Mostly, it looks good, though I noticed that double tapping automatically dismisses the keyboard / shows the panel. We want the user to dismiss the keyboard / show the panel explicitly. Once the panel is shown, they also have to close it explicitly. Other than this, I think the functionality we want is all there. |
|
@Jethro87 let me know if that fixes the double and triple tap keyboard issues. |
…le of further API simplification
…e vanilla Flutter IME artifacts.
…tead of implementing directly on SuperEditorImeInteractor
…ata structure called SuperEditorImePolicies
…xtRange to DocumentRange
|
@Jethro87 I've pushed a candidate for the final API. Here's an explanation. Previously, I tried moving a bunch of IME stuff into the The current approach is to create as many small widgets as possible, which take up narrow responsibilities. At a high level, you control IME behavior with return SuperEditor(
// Other properties...
softwareKeyboardController: _keyboardController,
imePolicies: SuperEditorImePolicies(
openKeyboardOnSelectionChange: false,
clearSelectionWhenImeDisconnects: false,
),
)Use the This PR converts the
Please let me know if this API looks good to you. |
|
@matthew-carroll I've updated my repo with the proposed API and am testing it. It looks good so far. I did notice one bug that happens whenever I pop the screen I'm on. This happens in the |
|
@Jethro87 can you let me know exactly what I need to do to trigger that error? |
|
@matthew-carroll Sorry. I have a couple of I did some more digging, and I can remove the error if I wrap the Usually, before I exit the route, I will have called But I also tested swiping left-to-right to pop the route while the keyboard was still up (meaning The error in question is the assertion at line 124 of At this point, I'm not sure if this is indeed a bug or if one should simply default to using the delegate check. |
|
@Jethro87 I adjusted the |
|
@matthew-carroll That worked 👍 |
|
@Jethro87 do you feel good about us merging this in at this point? |
|
@matthew-carroll Yes. I'm good with merging. Thanks. |
|
Note for the future, this PR might bring back an issue that we worked around in #791 I tried to reproduce that issue on this branch and I couldn't reproduce it. The change I made in this PR was to bring back an |
…st when keyboard is closed (Resolves superlistapp#875) (superlistapp#876) * Includes general refactoring and packaging of Super Editor IME behavior
This PR lets apps open and close the IME through a
SuperEditorDocumentComposer. It also lets the document selection exist even after the keyboard has closed.