From 240bb54c29b476e6827b300716ed4f98ea3cc0e1 Mon Sep 17 00:00:00 2001 From: heaven Date: Wed, 20 May 2026 00:59:17 +0300 Subject: [PATCH] refactor(stream-header): reset live drag on gesture teardown, drop dead pinned-local fallback, narrow commit() to peek|closed, add exhaustive transition guards and align stale comments --- .../components/mobile-tabs-pager/style.css.ts | 7 +- .../stream-header/StreamHeader.css.ts | 21 +++--- .../components/stream-header/StreamHeader.tsx | 13 ++-- src/app/components/stream-header/geometry.ts | 11 ++-- .../stream-header/useCurtainBodyGesture.ts | 50 +++++++++++--- .../stream-header/useCurtainHandleGesture.ts | 61 +++++++++++++---- .../stream-header/useCurtainState.ts | 65 +++++++++---------- src/app/pages/client/bots/Bots.tsx | 9 ++- 8 files changed, 153 insertions(+), 84 deletions(-) diff --git a/src/app/components/mobile-tabs-pager/style.css.ts b/src/app/components/mobile-tabs-pager/style.css.ts index 905522c1..09f5acd2 100644 --- a/src/app/components/mobile-tabs-pager/style.css.ts +++ b/src/app/components/mobile-tabs-pager/style.css.ts @@ -143,8 +143,11 @@ export const strip = style({ // // No paddingTop here: the per-pane StreamHeader still renders its // own tabs row (kept for the curtain's TABS_ROW_PX snap math, just -// painted invisible via visibility:hidden), and PageNav's inner -// column reserves the status-bar safe-area inset via its own +// painted invisible via `opacity: 0` — load-bearing because +// `visibility: hidden` would remove the row from hit-testing and +// the per-pane Segments need to capture taps at rest, see +// `StreamHeader.tsx` tabsRow rationale), and PageNav's inner column +// reserves the status-bar safe-area inset via its own // `paddingTop: var(--vojo-safe-top)`. The static header overlay at // the pager root simply paints OVER the same screen zone, so the // underlying geometry stays identical to non-pager mode. diff --git a/src/app/components/stream-header/StreamHeader.css.ts b/src/app/components/stream-header/StreamHeader.css.ts index efb85b6d..3e57a902 100644 --- a/src/app/components/stream-header/StreamHeader.css.ts +++ b/src/app/components/stream-header/StreamHeader.css.ts @@ -182,20 +182,17 @@ export const handleBar = style({ // Wrapper around `bottomPinned` inside the curtain. Anchored to the // curtain's flex-bottom by virtue of being the last child. The TSX -// applies a `transform: translateY(keyboardH)` to this element when -// the on-screen keyboard rises (via `VisualViewport.height` shrink) -// so the row stays at its ORIGINAL viewport-bottom position — under -// the keyboard, clipped by the curtain's `overflow: hidden`. Without -// this compensation, `interactive-widget=resizes-content` (global -// meta — load-bearing for the room composer) shrinks the layout -// viewport, dragging every `bottom: 0` element up over the inline -// form. The DirectSelfRow ending up immediately above the keyboard -// blocks the user's view of the form they're typing into. +// collapses this slot to `{ height: 0, overflow: hidden }` when the +// on-screen keyboard rises (via `VisualViewport.height` shrink) so +// the row neither paints nor claims flex space above the keyboard. +// Without this compensation, `interactive-widget=resizes-content` +// (global viewport meta — load-bearing for the room composer) +// shrinks the layout viewport, dragging every `bottom: 0` element +// up over the inline form. The DirectSelfRow ending up immediately +// above the keyboard would block the user's view of the form they're +// typing into. export const bottomPinnedSlot = style({ flexShrink: 0, - // Compositor hint — the transform is applied/cleared on every - // VisualViewport resize while a keyboard is open. - willChange: 'transform', }); // Segment button (Direct / Channels / Bots). diff --git a/src/app/components/stream-header/StreamHeader.tsx b/src/app/components/stream-header/StreamHeader.tsx index 0da5db6e..670e08ca 100644 --- a/src/app/components/stream-header/StreamHeader.tsx +++ b/src/app/components/stream-header/StreamHeader.tsx @@ -51,12 +51,13 @@ type StreamHeaderProps = { bottomPinned?: ReactNode; // Stable identifier used to persist the curtain's pinned overlay // across listing-pane remounts (the user taps into a Room and back, - // which unmounts the listing pane). When provided, pin state is - // stored in `curtainPinnedByTabAtom[pinKey]`; without it, pin lives - // in a local useState that resets on unmount. Listing surfaces - // wired into the mobile pager (Direct / Channels / Bots) all pass - // a key; other consumers can omit it. - pinKey?: string; + // which unmounts the listing pane). Pin state is stored in + // `curtainPinnedByTabAtom[pinKey]` so it outlives any individual + // StreamHeader instance. Each listing tab (Direct/Channels/Bots) + // passes its own key; the Channels landing CTA and workspace + // listing share `"channels"` so pin survives the toggle between + // empty state and a chosen workspace. + pinKey: string; }; export function StreamHeader({ scrollRef, children, bottomPinned, pinKey }: StreamHeaderProps) { diff --git a/src/app/components/stream-header/geometry.ts b/src/app/components/stream-header/geometry.ts index d7bcc48f..30088219 100644 --- a/src/app/components/stream-header/geometry.ts +++ b/src/app/components/stream-header/geometry.ts @@ -106,10 +106,13 @@ export const PIN_TRAVEL_PX = TABS_ROW_PX; // displacement is reached with a longer finger pull because the body // path is rubber-banded (×0.65). // -// Unpin is the one exception that keeps a hard ±PIN_TRAVEL_PX clamp: -// the handle-only contract makes it a deliberate full-travel pull, -// so we don't want the finger overshooting past closed into peek -// territory mid-gesture. +// Unpin's clamp is asymmetric — `pinned-free` lower-bounds the live +// delta at 0 (no destination above pinned) but leaves the upper +// direction unclamped so the same gesture can carry the curtain +// through closed into peek territory in one motion. The handle-only +// contract on unpin means the body never resolves to `pinned-free`, +// so the no-upper-clamp tolerance only applies on the dedicated +// drag-handle. export const PIN_COMMIT_THRESHOLD = 0.95; // Drag-handle hit-zone at the top of the curtain. NATIVE-ONLY: the diff --git a/src/app/components/stream-header/useCurtainBodyGesture.ts b/src/app/components/stream-header/useCurtainBodyGesture.ts index 94a44dfa..c9b8319f 100644 --- a/src/app/components/stream-header/useCurtainBodyGesture.ts +++ b/src/app/components/stream-header/useCurtainBodyGesture.ts @@ -10,7 +10,11 @@ import { RUBBER_BAND, } from './geometry'; import { CurtainSnap, isFormSnap } from './useCurtainState'; -import { CurtainTransition, resolveCurtainTransition } from './useCurtainHandleGesture'; +import { + assertNeverCurtainTransition, + CurtainTransition, + resolveCurtainTransition, +} from './useCurtainHandleGesture'; type Args = { // The curtain element. Touch listeners bind here so anywhere on the @@ -54,9 +58,10 @@ type Args = { // Live drag delta sink — feeds the curtain's `top` via React state, // no direct DOM writes. setLiveDrag: (px: number, dragging: boolean) => void; - // Snap commit (peek / close-peek / form-close). pin/unpin flips + // Snap commit (peek / close-peek / form-close). Narrowed to the two + // non-form destinations the hook ever reaches. pin/unpin flips // `pinned` instead. - commit: (next: CurtainSnap) => void; + commit: (next: 'peek' | 'closed') => void; // Suppress gesture binding entirely. Same conditions as the handle // hook — see StreamHeader's `gestureDisabled`. disabled?: boolean; @@ -261,13 +266,21 @@ export function useCurtainBodyGesture({ lastDelta = Math.min(0, delta * RUBBER_BAND); atCommit = -lastDelta >= ACTIVE_CLOSE_THRESHOLD_PX; break; - // `pinned-free` is intentionally absent — the pinned-bail - // at touchstart prevents the body hook from ever resolving - // to it. If a future change exposes pinned-free on the - // body, add the dispatch alongside this default so the - // linter keeps the switch exhaustive. - default: + case 'pinned-free': + // Unreachable on the body — the pinned bail at touchstart + // prevents the hook from ever resolving this transition. + // Kept here so the `never` default below stays exhaustive + // and a future opening of pinned-free on the body would + // need to wire the dispatch explicitly. break; + case null: + // Unreachable: `engaged` is set only after `transition` is + // resolved non-null in the dead-zone block above. + break; + default: { + assertNeverCurtainTransition(transition); + break; + } } setLiveDrag(lastDelta, true); emitHandle(true, atCommit); @@ -308,9 +321,18 @@ export function useCurtainBodyGesture({ setLiveDrag(0, false); } break; - default: + case 'pinned-free': + case null: + // Both unreachable per the touchmove switch above; the + // setLiveDrag fallback preserves spring-back behaviour if a + // future change exposes either path here. setLiveDrag(0, false); break; + default: { + assertNeverCurtainTransition(transition); + setLiveDrag(0, false); + break; + } } startX = null; startY = null; @@ -341,6 +363,14 @@ export function useCurtainBodyGesture({ curtain.removeEventListener('touchmove', onTouchMove); curtain.removeEventListener('touchend', onTouchEnd); curtain.removeEventListener('touchcancel', onTouchCancel); + // Same teardown contract as the handle hook — see its cleanup for + // the rationale. If `disabled` flips true while a body drag is in + // flight, the touchend never reaches us and the curtain would stay + // frozen at the finger position until the next touch. + if (engaged) { + setLiveDrag(0, false); + emitHandle(false, false); + } }; // setLiveDrag is stable; the ref args are stable. `snap`, `pinned`, // `setPinned` and `commit` are ref-mirrored. Only `disabled` needs diff --git a/src/app/components/stream-header/useCurtainHandleGesture.ts b/src/app/components/stream-header/useCurtainHandleGesture.ts index 4f0bfa5d..aa99be04 100644 --- a/src/app/components/stream-header/useCurtainHandleGesture.ts +++ b/src/app/components/stream-header/useCurtainHandleGesture.ts @@ -33,9 +33,10 @@ type Args = { // curtain's `top` re-render — no direct DOM writes. setLiveDrag: (px: number, dragging: boolean) => void; // Snap commit. Called on release for peek / close-peek / form-close - // (the pin / unpin paths flip `pinned` instead). Also resets + // (the pin / unpin paths flip `pinned` instead). Narrowed to the + // two non-form destinations the hook ever reaches. Also resets // liveDragPx + isDragging atomically inside the parent state. - commit: (next: CurtainSnap) => void; + commit: (next: 'peek' | 'closed') => void; // Suppress gesture binding entirely. Used to gate motion when a // bottom sheet is open or when this pane is inactive inside the // swipe pager. @@ -79,6 +80,14 @@ type Args = { // touchstart while pinned so unpin remains a deliberate handle pull. export type CurtainTransition = 'closed-free' | 'pinned-free' | 'close-peek' | 'form-close'; +// Exhaustive-check helper. Used in the `default` branch of every +// switch over `CurtainTransition | null` so that adding a fifth +// variant to the union fails typecheck at every dispatch site +// rather than silently no-op'ing through default. The argument is +// prefixed with `_` so eslint's `argsIgnorePattern: '^_'` keeps the +// rule happy without us tagging it `// eslint-disable`. +export const assertNeverCurtainTransition = (_value: never): void => {}; + // Decide which transition the gesture arms based on the snap state // at direction-resolution time and the finger direction. `null` means // the (snap, pinned, direction) triple has no valid motion and the @@ -128,11 +137,12 @@ export function resolveCurtainTransition( // hijacked under their finger. The body is also fully inert while // pinned, so unpin (and unpin → peek overshoot) stays a deliberate // handle pull. -// History note: an earlier `useCurtainGesture` bound the peek / -// form-close paths to the list scroll viewport directly. That coupling -// produced repeating «drag-up at scrollTop=0 hijacks for pin» / «drag- -// down at scrollTop=0 hijacks for peek» bugs and was removed when -// pin / unpin moved here. +// +// Design rationale: gestures used to bind to the chat list's scroll +// viewport directly, which produced repeating «drag-at-scrollTop=0 +// hijacks for pin/peek» bugs. Moving every transition onto a +// dedicated handle (plus an opt-in body surface that bails on +// scrollable lists) removes the scroll/gesture race entirely. // // Per-transition dynamics — all track the finger 1:1, but the clamp // shapes differ to keep on-screen motion sensible while preserving @@ -325,10 +335,19 @@ export function useCurtainHandleGesture({ lastDelta = Math.min(0, delta); atCommit = -lastDelta >= ACTIVE_CLOSE_THRESHOLD_PX; break; - default: - // Unreachable — transition is non-null past the dead-zone - // resolution above and is never cleared mid-gesture. + case null: + // Unreachable: `engaged` is set only after `transition` is + // resolved non-null in the dead-zone block above; reaching + // this case would imply the gesture engaged without a + // transition, which the control flow above forbids. break; + default: { + // Exhaustive guard. The `never` cast turns a future addition + // to `CurtainTransition` into a compile error here — adding + // a fifth member without wiring its dispatch fails typecheck. + assertNeverCurtainTransition(transition); + break; + } } setLiveDrag(lastDelta, true); emitHandle(true, atCommit); @@ -398,9 +417,19 @@ export function useCurtainHandleGesture({ setLiveDrag(0, false); } break; - default: + case null: + // Unreachable: `engaged` is set only after `transition` is + // resolved non-null. Mirrors the touchmove switch. setLiveDrag(0, false); break; + default: { + // Exhaustive guard — see the touchmove switch for the same + // pattern. setLiveDrag fallback preserves spring-back if a + // future transition lands here unhandled at runtime. + assertNeverCurtainTransition(transition); + setLiveDrag(0, false); + break; + } } startX = null; startY = null; @@ -432,6 +461,16 @@ export function useCurtainHandleGesture({ handle.removeEventListener('touchmove', onTouchMove); handle.removeEventListener('touchend', onTouchEnd); handle.removeEventListener('touchcancel', onTouchCancel); + // If `disabled` flips true while a drag is in flight, the touchend + // we'd normally rely on for snap-back never reaches us (the listener + // is gone). Without an explicit reset the curtain stays frozen at + // the finger position with `transition: none` and the grabber pill + // stuck Primary-blue until the user starts a new touch — visible as + // a half-open curtain after, say, a sheet opens mid-drag. + if (engaged) { + setLiveDrag(0, false); + emitHandle(false, false); + } }; // setLiveDrag is a stable useCallback; handleRef is stable. `snap`, // `pinned`, `setPinned` and `commit` are mirrored via the refs diff --git a/src/app/components/stream-header/useCurtainState.ts b/src/app/components/stream-header/useCurtainState.ts index cabd03cd..8e9b8711 100644 --- a/src/app/components/stream-header/useCurtainState.ts +++ b/src/app/components/stream-header/useCurtainState.ts @@ -35,9 +35,7 @@ export type CurtainState = { // the consumer-supplied `pinKey` so the lock survives the route- // driven listing-pane unmount when the user taps into a Room and // back. Each tab keeps its own pin (Direct/Channels/Bots are - // independent). If no `pinKey` is provided, the pin lives in a - // local useState that resets on unmount — fine for non-listing - // surfaces where pinning isn't expected anyway. + // independent). pinned: boolean; // Setter for the pinned overlay. Called by the gesture hook on // commit (drag-up-from-closed past threshold sets true; drag-down- @@ -69,7 +67,10 @@ export type CurtainState = { close: () => void; // Commit a snap stop directly. Used by the touch gesture on release. // Also resets `liveDragPx` and `isDragging` in one batched update. - commit: (next: CurtainSnap) => void; + // Narrowed to the two non-form destinations the gesture hooks ever + // reach — peek-reveal and close. Form snaps are entered through + // `open()` which sets `activeForm` synchronously alongside the snap. + commit: (next: 'peek' | 'closed') => void; // Setter for the live drag delta — called from the touch gesture on // every touchmove. Updates are batched by React inside event handlers. setLiveDrag: (px: number, dragging: boolean) => void; @@ -101,35 +102,30 @@ export function snapTopPx(snap: CurtainSnap, formH: number | null): number { } } -export function useCurtainState(pinKey?: string): CurtainState { +export function useCurtainState(pinKey: string): CurtainState { const [snap, setSnap] = useState('closed'); const [activeForm, setActiveForm] = useState(null); const [formHeightPx, setFormHeightPx] = useState(null); const [liveDragPx, setLiveDragPx] = useState(0); const [isDragging, setIsDragging] = useState(false); - // Pin storage split: atom-backed when `pinKey` is supplied (survives - // listing-pane remount on Room navigate-back), local useState - // fallback when no key is supplied (web/non-listing mounts where - // pinning isn't expected). + // Per-tab pin lives in `curtainPinnedByTabAtom` so the lock survives + // the route-driven listing-pane unmount that happens when the user + // taps into a Room and back. The atom outlives any individual + // StreamHeader instance. const [pinnedMap, setPinnedMap] = useAtom(curtainPinnedByTabAtom); - const [pinnedLocal, setPinnedLocal] = useState(false); - const pinned = pinKey ? !!pinnedMap[pinKey] : pinnedLocal; + const pinned = !!pinnedMap[pinKey]; const formMeasureRef = useRef(null); const setPinned = useCallback( (next: boolean) => { - if (pinKey) { - setPinnedMap((prev) => { - // Compare-and-skip so we don't allocate a fresh object (and - // re-render every other subscriber of the atom) when nothing - // actually changes. - if (!!prev[pinKey] === next) return prev; - return { ...prev, [pinKey]: next }; - }); - } else { - setPinnedLocal(next); - } + setPinnedMap((prev) => { + // Compare-and-skip so we don't allocate a fresh object (and + // re-render every other subscriber of the atom) when nothing + // actually changes. + if (!!prev[pinKey] === next) return prev; + return { ...prev, [pinKey]: next }; + }); // Drop any in-flight live drag on commit so the curtain renders // at the new pinned-derived top without a residual finger offset. setLiveDragPx(0); @@ -145,12 +141,12 @@ export function useCurtainState(pinKey?: string): CurtainState { setLiveDragPx(0); setIsDragging(false); // Safety net: clear pin so the form is visible. In practice the - // static pager header's icons (the only call site of open()) are - // covered by the curtain when pinned, so the user can't trigger - // this directly — but a future programmatic open() or a per-pane - // tabsRow that escapes the pager-mode visibility:hidden gate - // would otherwise mount the form behind the still-pinned curtain - // at curtainTop=0 and the user would see an invisible form. + // visible openers (static pager header icons, in-pane chips on + // non-pager surfaces) are all covered by the curtain when pinned, + // so the user can't trigger this directly — but a future + // programmatic open() would otherwise mount the form behind the + // still-pinned curtain at curtainTop=0 and present an invisible + // form. setPinned(false); }, [setPinned] @@ -162,17 +158,14 @@ export function useCurtainState(pinKey?: string): CurtainState { setIsDragging(false); }, []); - const commit = useCallback((next: CurtainSnap) => { + const commit = useCallback((next: 'peek' | 'closed') => { setSnap(next); setLiveDragPx(0); setIsDragging(false); - if (isFormSnap(next)) { - setActiveForm(next === 'form-search' ? 'search' : 'chat'); - } - // Note: when committing to a non-form snap (peek*/closed) we do - // NOT clear `activeForm` here — it stays set so the closing - // transition has form content beneath. `acknowledgeClosed` clears - // it once the curtain settles at `closed`. + // `activeForm` is intentionally NOT cleared here — it stays set + // so the closing transition has form content beneath the curtain + // as it slides up. `acknowledgeClosed` clears it once the snap + // settles at `closed`. }, []); const setLiveDrag = useCallback((px: number, dragging: boolean) => { diff --git a/src/app/pages/client/bots/Bots.tsx b/src/app/pages/client/bots/Bots.tsx index 6576ce0e..8ab34de0 100644 --- a/src/app/pages/client/bots/Bots.tsx +++ b/src/app/pages/client/bots/Bots.tsx @@ -21,9 +21,12 @@ function BotRow({ preset }: { preset: BotPreset }) { export function Bots() { const bots = useBotPresets(); - // `scrollRef` is passed to the header so the touch gesture (native - // only) can recognise list scrollTop=0 and engage the curtain peek. - // Icons + click flows work on every platform regardless. + // `scrollRef` is forwarded so the curtain body gesture can check + // whether the list is scrollable and bail to native scroll on long + // lists. Short / empty lists let the curtain body itself drive the + // gesture. The dedicated 32 px drag-handle on the curtain works + // regardless of this ref. Native-only — desktop / Electron have + // no curtain gestures. const scrollRef = useRef(null); // Skip PageNav surface in pager mode — see Direct.tsx for the // rationale; the static header behind the strip owns the visible