fix(tipcard): take the keyboard down before handling a deeplink - #1323
Merged
Conversation
Tapping a tip link while resuming from a chat presented the tip card over a
live keyboard. The chat's message input still holds focus across the
background, so the window restores the IME as the link is routed, and the card
resolves over a network round-trip with the keyboard up the whole time.
Dismiss the keyboard in App once a deeplink dispatches to a real action, before
anything is routed or presented. This covers every branch there, not just
PresentTipCard: they all reroute what's on screen, including /tip/{self}, which
AppRouter diverts to a Menu-sheet Navigate. The one path that doesn't navigate
(an email code delivered in place via EmailCodeChannel) auto-verifies on
arrival, so taking the keyboard down there is right too.
BillOverlay catches an IME the platform restores after we've routed. It fires
only on a false -> true transition: the overlay is hosted per navigation entry,
so a screen opened while a bill is up composes a fresh copy with the bill
already present, and firing there would stomp TipCardDecorator's LaunchChat,
which opens the chat with the keyboard up on purpose.
KeyboardController.dismiss() clears editor focus before hiding. hide() alone
leaves the field focused, so the platform brings the keyboard straight back.
Two behaviours carry the fix and neither had a test. KeyboardControllerTest pins the hide/dismiss split: `hide` leaves the editor focused, `dismiss` doesn't. Focus is the mechanism — the window restores the IME for whatever still holds it on the way back from the background — so the tree needs a non-editor focusable beside the field. `clearFocus` hands focus to the next candidate rather than leaving none, and a tree whose only focusable is the editor takes it straight back. OnBillPresentedTest covers the transition guard, which is the part most likely to be wrong: the overlay is hosted per navigation entry, so a bill already up at first composition must not count as a presentation, or the fresh chat entry would take down the keyboard that the post-tip hand-off opened on purpose. The guard moves out of BillOverlay into an internal OnBillPresented composable to make it reachable from a test.
dismiss() existed so a caller could opt into clearing editor focus before hiding the IME. Across the codebase no caller wants the other behaviour: hideIfVisible accounts for 29 sites and every one hides, waits out the IME animation, then pops, dismisses a sheet, or advances a flow step; the two direct hide() callers pop or dismiss on tap. Both show() callers request focus explicitly first, so nothing pairs a hide with a later show that depends on the field still holding focus. That makes the safe behaviour the one worth defaulting to, and a retainFocus parameter a flag with no false branch. Folding it into hide() also carries the fix to the 29 hideIfVisible sites, which had the same latent problem as the tip card: IME down, field still focused, keyboard restored on the way back from the background. Tapping the message list to put the keyboard away now deselects the composer too, matching the rest of the app's leave-the-field behaviour.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tapping a tip link while resuming from a chat presents the tip card over a live keyboard. The chat's message input still holds focus across the background, so the window restores the IME as the link is routed — and the card resolves over a network round-trip, so the keyboard is up for that whole window too.
Changes
App.kt— dismiss the keyboard once a deeplink dispatches to a real action, before anything is routed or presented.Scoped to any non-
Noneaction rather than justPresentTipCard: every branch there reroutes what's on screen, including/tip/{self}, whichAppRouterdiverts to a Menu-sheetNavigateand which had the same problem. The one path that doesn't navigate — an email code delivered in place viaEmailCodeChannel— auto-verifies on arrival, so taking the keyboard down there is right too.Cold start is untouched:
onPendingActiononly fires fromMainRooton theLoadingroute, where no keyboard can be up.BillOverlay.kt— catches an IME the platform restores after we've routed.Fires only on a
false -> truetransition. The overlay is hosted per navigation entry byNavBillOverlayEntryDecorator, so a screen opened while a bill is up composes a fresh copy with the bill already present. Unguarded, that would stompTipCardDecorator'sLaunchChat, which opens the chat withopenKeyboard = trueon purpose.KeyboardController.hide()— now clears editor focus before hiding the IME. Hiding alone leaves the field focused, so the platform brings the keyboard straight back; clearing focus removes the target it would be restored onto.This started as a separate
dismiss()and folded back in, because no caller wants the other behaviour.hideIfVisibleis 29 of them, and every one hides, waits out the IME animation, then pops, dismisses a sheet, or advances a flow step. The two directhide()callers pop or dismiss on tap. Bothshow()callers request focus explicitly first, so nothing pairs a hide with a later show that depends on retained focus — which leftretainFocusas a flag with no false branch.So the
hideIfVisiblesites get the fix too. They had the same latent problem as the tip card: IME down, field still focused, keyboard restored on the way back from the background.Tests
KeyboardControllerTestpins the focus half ofhide.hideIfVisibleinherits it by delegation but isn't covered: it gates onvisible, read from window insets that Robolectric never reports for the IME.OnBillPresentedTestcovers the transition guard, which moves out ofBillOverlayinto an internalOnBillPresentedcomposable to be reachable from a test — an already-presented bill at first composition is a no-op, a bill that appears while watching is not.