Improve auto-show/hide toolbar

[eranl]

1. Auto-show toolbar on any selection change/cursor move (if setting is on)
2. Only auto-hide toolbar while typing a word (if setting is on)

Should any of this be optional?

Mostly fixes #2281.

[Helium314]

Do the isResumed changes have any noticeable effects, other than for your use in LatinIME? It's used in Suggest, and I don't want to introduce any further bugs there.
Looks like "@return whether we started composing this word by resuming suggestion on an existing string" is incorrect with these changes.

[Helium314]

The difference of 1 is how you detect typing? If so, you should also consider that keys can input multiple characters, and this is not only for customized layouts, but normal for some languages.

[eranl]

Can you please give me an example to try?

[Helium314]

For example you have क्ष on the hindi compact layout, or a bunch of letters using combining marks on mansi north, e.g. е̄.

[eranl]

Indeed failed. Interestingly, these multi-character keys result in two onUpdateSelection calls: one with composingSpanEnd == - 1, and another with newSelStart == oldSelStart, at least on my Android 16 device. So I fixed this by adding another condition.

Do you think this is robust enough? I guess this is another reason to make 1 optional as well.

[Helium314]

Interestingly, these multi-character keys result in two onUpdateSelection calls

This is because multiple chars results in InputLogic.onTextInput, which does first finishComposingText, and then commitText, while adding a single codepoint does setComposingText.
Thanks for noticing, I don't really like this inconsitency... noted for the planned RichInputConnection overhaul, if I ever find time for that.

Do you think this is robust enough?

Not sure yet. I'll go over it again, because I had some idea of not using onUpdateSelection at all, but need to find it in my heap of maybe-try-this notes.

[Helium314]

Should any of this be optional?

I suspect 2. might be unwanted for some users, so better make it optional. 1. should be fine I think.

[maruuk]

I think 1 should also be optional. If I type an incorrect word and autocorrect doesn't work, I click on it to select the correct one from the suggestion bar. After this change, I'll have to expand the suggestion bar, which will slow down my typing.

Btw, since you are improving the auto-show/hide toolbar, could you please take a look at these issues? #993 #1855

[eranl]

I think 1 should also be optional.

@Helium314, your call.

#993 #1855

Both of these seem to be fixed by this PR.

[eranl]

Do the isResumed changes have any noticeable effects, other than for your use in LatinIME?

Yes, and I now think this should either be a separate PR, or I should add a separate flag.

Specifically:

• setting isResumed to false after processing an event, since otherwise it remained true after a cursor move. This has the effect of enabling autocorrect when typing after moving the cursor into an existing word.
• setting isResumed to false after a normal backspace event, since it is otherwise set to true. This has the effect of enabling autocorrect when backspacing after moving the cursor into an existing word.

My understanding of isResumed has been as an indication that we have a new composed word due to a cursor move etc. Is my understanding wrong? Should I add a separate flag? I think it would nice if autocorrect and toolbar autohide behaved consistently, so that's an argument against a separate flag. Do my changes make sense? Should I submit them as a separate PR?

Originally, I was also setting isResumed to false after reverting an auto-correction. That was unnecessary. Reverted.

I guess I should have documented all of this in the PR description.

further

This was uncalled for.

[Helium314]

Thanks for your explanation with onResumed. Need to check more closely, but on first glance both effects seem to be improvements to me.

This was uncalled for.

Sorry, indeed it was.
I meant to imply that Suggest is a bit fragile. It's very easy to introduce unwanted and unintended changes there.

[Helium314]

I found a few problems:

• when moving the cursor across a period (space swipe), the suggestion strip still disappears
  • when swiping quickly it works, probably because the movement is 2 characters
• when cursor is behind a period, selection change does not cause the toolbar to show up
• when switching between words and adding / deleting letters in the words, deletion sometimes deletes the space after the word instead of the letter before the cursor

One possible alternative to the resumed change would be to use INPUT_STYLE_RECORRECTION in InputLogic.restartSuggestions (line 1851). This needs a small adjustment in SuggestionStripLayoutHelper.shouldOmitTypedWord that should be fine.

The current toolbar showing on selection change might not be a good / robust approach. One alternative (not sure if better) could be to show if InputLogic.onUpdateSelection returns true, but then you'd need to find a way to also show on horizontal space swipe.
(Actually with the current implementation horizontal space swipe the return whether the cursor has moved as a result of user interaction doc of InputLogic.onUpdateSelection is incorrect...)

[maruuk]

Both of these seem to be fixed by this PR.

I checked and it doesn't work for me.