From d161b5987d7aa8f1978f45e81531a5fec2524c63 Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Wed, 23 Sep 2026 08:45:00 +0200 Subject: [PATCH] Dismiss an Android bottom sheet with Back instead of the screen (#5245) * Dismiss an Android bottom sheet with Back instead of the screen Bottom sheets render through a portal inside the app's own view tree rather than a native modal window, so Android offered Back to the navigator underneath them. On the model selector that meant one Back press left the chat and the app, with the sheet still open. `IsolatedBottomSheetModal` now claims `hardwareBackPress` for as long as the sheet is on screen and dismisses itself instead. Listeners are offered the press newest-first, so a stacked sheet answers before the one below it, and Back falls back through to the navigator once no sheet is left. * Move the Back press onto the sheet visibility tracker The first version held the press in `IsolatedBottomSheetModal` with its own `isOnScreen` state and a merged ref, duplicating the phase the visibility tracker already keeps, and it could only be tested through JSDOM with `vi.mock` and `vi.hoisted`, which docs/testing.md rules out. The tracker already owns presentation state and the sheet controller, so it takes a `BackPressSource` port and holds the press for exactly the window it is in the `presented` phase. `IsolatedBottomSheetModal` goes back to what it was on main. Every sheet call site reaches the tracker through `useIsolatedBottomSheetVisibility`, so coverage is unchanged. The real source registers nothing off Android, where there is no such press and `react-native-web`'s `BackHandler` logs an error on subscribe. The test is now a plain unit test against a fake adapter, with no mocks, no JSDOM, and no component mounting. --------- Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com> --- .../isolated-bottom-sheet-modal/back-press.ts | 16 ++++ .../ui/isolated-bottom-sheet-modal/index.tsx | 7 +- .../visibility-tracker.test.ts | 84 +++++++++++++++++++ .../visibility-tracker.ts | 56 +++++++++++-- 4 files changed, 155 insertions(+), 8 deletions(-) create mode 100644 packages/app/src/components/ui/isolated-bottom-sheet-modal/back-press.ts diff --git a/packages/app/src/components/ui/isolated-bottom-sheet-modal/back-press.ts b/packages/app/src/components/ui/isolated-bottom-sheet-modal/back-press.ts new file mode 100644 index 0000000000..d8f58356d6 --- /dev/null +++ b/packages/app/src/components/ui/isolated-bottom-sheet-modal/back-press.ts @@ -0,0 +1,16 @@ +import { BackHandler, Platform } from "react-native"; +import type { BackPressSource } from "./visibility-tracker"; + +/** + * Android's hardware Back press. + * + * iOS and web have no such press, and `react-native-web`'s `BackHandler` logs an error as soon as + * anything subscribes, so nothing is registered off Android. + */ +export const systemBackPress: BackPressSource = { + subscribe(onBackPress) { + if (Platform.OS !== "android") return () => {}; + const subscription = BackHandler.addEventListener("hardwareBackPress", onBackPress); + return () => subscription.remove(); + }, +}; diff --git a/packages/app/src/components/ui/isolated-bottom-sheet-modal/index.tsx b/packages/app/src/components/ui/isolated-bottom-sheet-modal/index.tsx index 18ecff9127..f7869823b8 100644 --- a/packages/app/src/components/ui/isolated-bottom-sheet-modal/index.tsx +++ b/packages/app/src/components/ui/isolated-bottom-sheet-modal/index.tsx @@ -5,6 +5,7 @@ import { import React from "react"; import { forwardRef, useCallback, useEffect, useMemo, useRef } from "react"; import type { ElementRef, ReactNode } from "react"; +import { systemBackPress } from "./back-press"; import { type BottomSheetController, createBottomSheetVisibilityTracker, @@ -85,7 +86,11 @@ export function useIsolatedBottomSheetVisibility({ onCloseRef.current = onClose; const tracker = useMemo( - () => createBottomSheetVisibilityTracker({ onClose: () => onCloseRef.current() }), + () => + createBottomSheetVisibilityTracker({ + onClose: () => onCloseRef.current(), + backPress: systemBackPress, + }), [], ); diff --git a/packages/app/src/components/ui/isolated-bottom-sheet-modal/visibility-tracker.test.ts b/packages/app/src/components/ui/isolated-bottom-sheet-modal/visibility-tracker.test.ts index d70dffdaa1..89ecce2b1e 100644 --- a/packages/app/src/components/ui/isolated-bottom-sheet-modal/visibility-tracker.test.ts +++ b/packages/app/src/components/ui/isolated-bottom-sheet-modal/visibility-tracker.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { + type BackPressSource, type BottomSheetController, createBottomSheetVisibilityTracker, } from "./visibility-tracker"; @@ -20,21 +21,55 @@ class FakeBottomSheet implements BottomSheetController { } } +/** + * Stands in for Android's `BackHandler`, which offers the press to listeners newest-first and + * stops at the first one that claims it. + */ +class FakeBackPress implements BackPressSource { + private listeners: (() => boolean)[] = []; + + subscribe(onBackPress: () => boolean): () => void { + this.listeners.push(onBackPress); + return () => { + const index = this.listeners.indexOf(onBackPress); + if (index >= 0) this.listeners.splice(index, 1); + }; + } + + /** True when a sheet claimed the press, false when it falls through to the navigator. */ + press(): boolean { + for (let index = this.listeners.length - 1; index >= 0; index--) { + if (this.listeners[index]?.()) return true; + } + return false; + } +} + function setup() { const sheet = new FakeBottomSheet(); + const backPress = new FakeBackPress(); let closeCount = 0; const tracker = createBottomSheetVisibilityTracker({ onClose: () => { closeCount += 1; }, + backPress, }); return { sheet, + backPress, tracker, closeCount: () => closeCount, }; } +/** Opens a sheet and runs it through the presentation the real sheet reports back. */ +function openSheet(context: ReturnType) { + context.tracker.attachController(context.sheet); + context.tracker.syncDesired({ visible: true }); + context.tracker.handleSheetIndexChange(0); +} + describe("bottom sheet visibility tracker", () => { it("presents once when the sheet becomes visible and dismisses when it goes back to hidden", () => { const { sheet, tracker } = setup(); @@ -160,4 +195,53 @@ describe("bottom sheet visibility tracker", () => { expect(sheet.events).toEqual([{ type: "present" }, { type: "present" }]); }); + + it("dismisses the sheet on a Back press instead of letting it reach the navigator", () => { + const context = setup(); + openSheet(context); + + expect(context.backPress.press()).toBe(true); + expect(context.sheet.events).toEqual([{ type: "present" }, { type: "dismiss" }]); + }); + + it("leaves Back to the navigator before the sheet is on screen and after it closes", () => { + const context = setup(); + context.tracker.attachController(context.sheet); + context.tracker.syncDesired({ visible: false }); + + expect(context.backPress.press()).toBe(false); + + openSheet(context); + context.tracker.handleSheetIndexChange(-1); + context.tracker.handleSheetDismiss(); + + expect(context.backPress.press()).toBe(false); + }); + + it("gives the Back press to the sheet presented last", () => { + const backPress = new FakeBackPress(); + const sheets = ["below", "above"].map((name) => { + const sheet = new FakeBottomSheet(); + const tracker = createBottomSheetVisibilityTracker({ onClose: () => {}, backPress }); + tracker.attachController(sheet); + tracker.syncDesired({ visible: true }); + tracker.handleSheetIndexChange(0); + return { name, sheet }; + }); + + backPress.press(); + + expect(sheets.map((entry) => [entry.name, entry.sheet.events.at(-1)?.type])).toEqual([ + ["below", "present"], + ["above", "dismiss"], + ]); + }); + + it("stops claiming Back once the sheet leaves the tree", () => { + const context = setup(); + openSheet(context); + context.tracker.attachController(null); + + expect(context.backPress.press()).toBe(false); + }); }); diff --git a/packages/app/src/components/ui/isolated-bottom-sheet-modal/visibility-tracker.ts b/packages/app/src/components/ui/isolated-bottom-sheet-modal/visibility-tracker.ts index 048d85f361..54ee6f58cd 100644 --- a/packages/app/src/components/ui/isolated-bottom-sheet-modal/visibility-tracker.ts +++ b/packages/app/src/components/ui/isolated-bottom-sheet-modal/visibility-tracker.ts @@ -8,6 +8,22 @@ export interface BottomSheetVisibilityInput { isEnabled?: boolean; } +/** + * The system Back press, as the tracker needs it. + * + * A sheet is presented through `@gorhom/portal`, which renders it into the app's own view tree + * rather than a native modal window, so Android delivers Back to the navigator underneath it + * unless something on top claims the press first. + * + * `subscribe` registers a listener and returns its removal. The listener returns `true` when it + * claims the press. Listeners are offered the press newest-first, so a tracker holds its + * subscription only while its sheet is on screen, and a stacked sheet answers before the one + * below it. + */ +export interface BackPressSource { + subscribe(onBackPress: () => boolean): () => void; +} + export interface BottomSheetVisibilityTracker { attachController(controller: BottomSheetController | null): void; syncDesired(input: BottomSheetVisibilityInput): void; @@ -19,23 +35,46 @@ type BottomSheetPhase = "closed" | "presenting" | "presented" | "dismissing"; export function createBottomSheetVisibilityTracker(opts: { onClose: () => void; + backPress?: BackPressSource; }): BottomSheetVisibilityTracker { let controller: BottomSheetController | null = null; let visible = false; let isEnabled: boolean | undefined; let phase: BottomSheetPhase = "closed"; let hasNotifiedClose = false; + let releaseBackPress: (() => void) | null = null; + + function setPhase(next: BottomSheetPhase): void { + phase = next; + syncBackPress(); + } + + /** Holds the Back press for exactly the window the sheet is on screen, and no longer. */ + function syncBackPress(): void { + const wantsBackPress = phase === "presented" && controller !== null; + if (wantsBackPress === (releaseBackPress !== null)) return; + if (!wantsBackPress) { + releaseBackPress?.(); + releaseBackPress = null; + return; + } + releaseBackPress = + opts.backPress?.subscribe(() => { + dismiss(); + return true; + }) ?? null; + } function present(): void { if (!controller || phase !== "closed") return; - phase = "presenting"; hasNotifiedClose = false; + setPhase("presenting"); controller.present(); } function dismiss(): void { if (!controller || phase === "closed" || phase === "dismissing") return; - phase = "dismissing"; + setPhase("dismissing"); controller.dismiss(); } @@ -48,6 +87,9 @@ export function createBottomSheetVisibilityTracker(opts: { return { attachController(next) { controller = next; + // A sheet that left the tree cannot answer, so it lets go of the press without changing + // phase: the parent has not acknowledged the dismissal yet. + syncBackPress(); if (next && visible && isEnabled !== false) { present(); } @@ -61,7 +103,7 @@ export function createBottomSheetVisibilityTracker(opts: { return; } if (phase === "dismissing") { - phase = "closed"; + setPhase("closed"); hasNotifiedClose = false; return; } @@ -70,21 +112,21 @@ export function createBottomSheetVisibilityTracker(opts: { handleSheetIndexChange(index) { if (index !== -1) { if (phase === "presenting" || phase === "dismissing") { - phase = "presented"; + setPhase("presented"); } return; } if (phase === "presenting" || phase === "presented") { - phase = "dismissing"; + setPhase("dismissing"); } }, handleSheetDismiss() { if (visible) { - phase = "dismissing"; + setPhase("dismissing"); notifyClose(); return; } - phase = "closed"; + setPhase("closed"); hasNotifiedClose = false; }, };