fix(ui/navigation): pop the sheet entry when a drag dismisses it - #1278
Merged
Conversation
UnstyledBottomSheet has no dismissal callback, so nothing tied "the sheet settled back at Hidden" to popping its nav entry — that wiring was lost when the scene moved off Material3's ModalBottomSheet (6e49a91), whose onDismissRequest did the pop. A drag-to-dismiss therefore left the (now invisible) sheet entry on the backstack: currentRouteKey still resolved to the sheet, so chrome that hides for sheets — the v2 tab bar — stayed hidden, and the full-size alpha-0 scrim kept swallowing touches until the next tap hit it and popped the entry. Hence "the tab bar doesn't come back until you interact with the screen" after dismissing region selection. Observe the sheet's state rather than the gesture, so every dismissal route is covered: once the sheet has actually been presented, settling back at Hidden pops the entry. All paths funnel through an idempotent finishDismiss() that re-arms per sheetGeneration, so the scrim-tap and pendingSheetDismiss (dismiss-then-replace) paths can't double-pop while a same-route replace still works.
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.
The bug
Drag a sheet that shows over the v2 tab bar closed (region selection from the wallet tab) and the tab bar doesn't come back — it stays hidden until you tap the screen, and that first tap is swallowed rather than doing what you aimed it at.
Root cause
UnstyledBottomSheet(compose-unstyled) has no dismissal callback. That wiring existed when the scene used Material3'sModalBottomSheet— itsonDismissRequestpopped the nav entry once a user drag settled the sheet to Hidden — and nothing replaced it when the scene was rebuilt on the unstyled sheet in 6e49a91.So a drag-dismiss hides the sheet without telling anyone:
AppRoute.Main.Sheet(...)entry stays on the backstack, sonavigator.currentRouteKeystill resolves to the sheet →topTab == null→AppNavigationBarkeeps the tab bar hidden;noRippleClickable { dismiss(true) }, which — seeingcurrentDetent == Hidden— pops immediately.That is exactly the reported "doesn't show until you interact with the screen".
The fix
Watch the sheet's state instead of the gesture, so every dismissal route is covered rather than just the drag:
targetDetent != Hidden— targeted, not settled, so an interrupted open animation still counts);isIdle && currentDetent == Hidden— a conjunction, because a partial drag flips the target to Hidden and back as the finger crosses the threshold);Both explicit dismiss paths (
dismiss()'s animate-then-pop and thependingSheetDismissreplace) now route through the same idempotentfinishDismiss(), which the settle observer also calls — so the entry pops exactly once per presentation and can't take a second backstack entry with it. The guard resets onsheetGeneration, so a same-route dismiss-then-replace still re-arms.Tests
New
ModalBottomSheetSceneTest(Robolectric + compose test rule) — the drag case fails on the pre-fix code (expected:<1> but was:<0>) and passes after:dragging the sheet closed pops its entryan open sheet does not pop its entry— control, guards the eager/double-pop directionVerified:
:ui:navigation:testDebugUnitTest— 31 tests, 0 failures (existingCodeNavigator*andInnerFlowNavigatorsuites included):apps:flipcash:core:testDebugUnitTest+:apps:flipcash:app:testDebugUnitTest— green