Phone interactions: long-press menu, touch listeners, sidebar across routes, composer retention (E1, E6, E7, E10, D3, J3, J4, I6, B28) - #1900
Merged
SawyerHood merged 13 commits intoAug 19, 2026
Conversation
SawyerHood
changed the title
This was referenced Aug 19, 2026
SawyerHood
force-pushed
the
bb/mobile-perf/mobile-interactions
branch
2 times, most recently
from
August 19, 2026 07:50
9f112ed to
8977bee
Compare
SawyerHood
marked this pull request as ready for review
August 19, 2026 08:04
SawyerHood
changed the title
SawyerHood
force-pushed
the
bb/mobile-perf/mobile-interactions
branch
2 times, most recently
from
August 19, 2026 15:59
b34cbdf to
ad7193e
Compare
The pull-request check pill and the Info tab's GitHub logo fetched light and dark favicon PNGs from github.githubassets.com: two cross-origin image requests per pull request surface, on every cold start, arriving late over the connect tunnel and shifting layout on phones. Draw the check glyph from the bundled GitHub mark plus a status dot in theme tokens (success/destructive/attention), and render the logo with the same bundled icon. Both follow the active theme through currentColor instead of swapping two images with dark: utilities. Co-Authored-By: Claude <noreply@anthropic.com>
Turn-summary details, file previews (thread storage/host, project, environment) and diff patches used React Query's default five-minute gcTime, so a phone browsing several threads kept every visited thread's heavy payloads resident. Timeline windows are deliberately left alone because delta refetch depends on the cached window surviving a leave. Turn-summary details and the file preview queries now carry HEAVY_PAYLOAD_QUERY_POLICY (gcTime 60 s after the last observer). Diff patches are observer-less (read with getQueryData), so a plain gcTime would count from the write and could drop a patch the open panel still shows. writeDiffPatchEntry now builds the query without a gc timer and useEnvironmentDiffPatches holds a per-environment reader lease; the last release evicts the environment's patches 60 s later, and a remount inside that window cancels the eviction. Covered by a fake-timer test. Co-Authored-By: Claude <noreply@anthropic.com>
useFixedPanelTabsStorageMaintenance re-scanned localStorage and schema-parsed every persisted fixed-panel blob on every thread navigation, in the same task as the route change. It now schedules one prune per page load from idle time (requestIdleCallback with a timeout, setTimeout fallback), and the prune reads lastUsedAt from the parsed JSON first so expired blobs are dropped without the zod parse. useFixedPanelTabsState and useUpdateFixedPanelTabsState wrote through the storage atom even when reconciliation returned the current state, which serialized and re-wrote localStorage on every mount. Both now read the atom from the store and skip the write when nothing changed. Tests cover the no-rewrite path and the prune decisions. Co-Authored-By: Claude <noreply@anthropic.com>
Two paths left a NON-passive window touchmove listener installed on compact viewports, which makes iOS Safari and Chrome Android dispatch the first move of every scroll gesture synchronously through the main thread before compositor scrolling can start: - dnd-kit's TouchSensor.setup() keeps a permanent no-op listener alive for as long as any DndContext using it is mounted. The compact sidebar mounts several DndContexts at boot inside its closed drawer, so the listener existed on every page. useSidebarReorderDnd now uses SidebarTouchSensor, a subclass whose setup installs the same listener only while the compact drawer is showing (touch reorder keeps working there) and always on wide layouts. It tracks the drawer through a tiny external store written by SidebarProvider, so no memoized row subscribes to the sidebar context. SecondaryPanelTabStrip wires its TouchSensor only while its panel is open with two or more tabs. - SidebarInset's swipe-open registered a non-passive touchmove for every touch that started anywhere in the content area. Only touches in the left edge zone (24-72 px) keep the non-passive path; deeper touches keep the swipe recognizer on a passive listener and skip preventDefault. The pointer path is unchanged. Tests assert the sensor's install/remove behaviour and the passive vs non-passive registration per touch start position. Co-Authored-By: Claude <noreply@anthropic.com>
…ones Thread and project rows wrapped every row in a Radix ContextMenu whose 700 ms touch long-press opened a modal Radix menu: aria-hidden on #root, RemoveScroll's non-passive touchmove, and body pointer-events:none with the full timeline mounted behind the drawer. On phones the "..." button is hidden, so this was the only path to row actions. On compact viewports the rows now render inside CompactLongPressMenu: a small long-press detector (same 700 ms delay, 10 px slop, right-click support, WebkitTouchCallout:none) that opens the existing responsive drawer with the same DropdownMenu items the "..." button uses. Nothing is mounted for the menu until the first open, and the click that follows a long press is swallowed so the row does not also navigate. Wide viewports keep the desktop context menu. Co-Authored-By: Claude <noreply@anthropic.com>
AppLayoutSidebar swapped the whole AppSidebar tree (its own <Sidebar> panel, ProjectList, every realized row, DndContexts, windowing observers) for SettingsSidebar/ToolsSidebar by route mode. On compact viewports the return trip remounted ProjectList inside the closed drawer, in the same task as the destination page render, paying the mount cost the persistent drawer was introduced to avoid. On compact viewports AppLayoutSidebar now renders one persistent <Sidebar> panel and hosts bodies inside it: the AppSidebar body stays mounted and is hidden while a Settings/Tools body shows. AppSidebar, SettingsSidebar/ToolsSidebar and SectionSidebar gained a mobileHosted mode that renders the body without its own shell; the hidden app body leaves the thread-search shortcut unhandled. Wide viewports keep one shell per mode. The 220 ms hold of the visible body during a deferred close is preserved. The test asserts a single panel element and a single AppSidebar mount across app -> settings -> tools -> app. Co-Authored-By: Claude <noreply@anthropic.com>
ThreadDetailPromptArea swapped FollowUpPromptBox out of the tree while a permission request or user question was pending, replacing it with a banner grid. Every approval therefore tore down and rebuilt the TipTap editor, the composer's pickers and its plugin host, and the remount landed in the same task as the approval's timeline update; on phones this made each approve tap noticeably heavy. FollowUpPromptBox now takes `pendingInteraction`: the composer editor shell stays mounted and is hidden (draft, history and DOM identity survive), the interaction renders as the last stack item, and the footer pickers are treated as read-only so their keyboard chords cannot open a popover anchored to a hidden trigger. The prompt area passes the interaction plus the reduced pending stack (child banners, plan mode and goal cards) and keeps the blocked "pending-interaction" submit mode. Tests assert editor DOM identity across the pending state and the existing stack ordering. Co-Authored-By: Claude <noreply@anthropic.com>
RootComposeView showed only "Loading…" until the sidebar bootstrap resolved, so on a cold phone start the composer (the page's whole purpose) appeared one full round trip late even though nothing in it needed the project list to paint. The composer now renders immediately. NewThreadComposer derives `sidebarNavigationSettled` once, marks the project picker as loading until then (ProjectSelector shows a "Loading projects…" label and stays non-interactive instead of the misleading "Work in a project" empty state), and gates the projectId-keyed queries (threads for environment reuse, prompt history) on the settled project id so a cold start does not fetch and cache data for a candidate project that may fall back to Personal. The unused render-prop flag is dropped. Tests cover the loading picker and disabled queries while pending, and the enabled state once settled. Co-Authored-By: Claude <noreply@anthropic.com>
The retention lease in environment-diff-patch-cache-owner.ts imports environmentDiffPatchQueryKeyPrefix; the cache-owner boundary test enumerates each owner's query-key imports and must list it. Co-Authored-By: Claude <noreply@anthropic.com>
SecondaryPanelTabStrip gated dnd-kit's TouchSensor by passing null into useSensors when the panel was closed. dnd-kit's DndContext keys its sensor setup effect on the sensor classes, and React skips an effect whose dependency array only changed size (it also logs a dev warning), so opening the panel never ran TouchSensor.setup (no iOS touchmove guard for touch reorder) and a panel that mounted open never removed the scroll-blocking listener on close. Keep the sensor slot and swap in an InertTouchSensor whose setup installs nothing; the class change is a real dependency change, so the listener installs on open and is torn down on close. Covered by a render test that fails on the previous wiring. Co-Authored-By: Claude <noreply@anthropic.com>
ProjectActionsContextMenu wraps a whole project section, thread rows included, so a long press (or right-click) on a thread row bubbled to the section's CompactLongPressMenu as well: both timers fired and the thread drawer and the project drawer opened together. The innermost menu now claims the native pointerdown, and the contextmenu path honours defaultPrevented the way Radix's ContextMenuTrigger does. A contextmenu fired from an active touch press (Chrome Android) also arms the post-press click suppression so the lift does not navigate. Co-Authored-By: Claude <noreply@anthropic.com>
- AppSidebar: while hosted-and-hidden behind a Settings/Tools body on phones, leave thread.previous/next and the thread jump shortcuts unhandled too, matching wide viewports where those routes replace the sidebar, instead of clicking rows the user cannot see. - useFixedPanelTabsStorageMaintenance no longer takes an ignored panelStateId; callers updated. The once-per-page-load, off-mount-task scheduling is now covered by a test (which also uses the previously unused test reset). Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
SawyerHood
force-pushed
the
bb/mobile-perf/mobile-interactions
branch
from
August 19, 2026 16:13
ad7193e to
f288726
Compare
SawyerHood
added a commit
that referenced
this pull request
Aug 19, 2026
…d; row containment (C1, C3, C4) (#1899) ## What was wrong - The in-progress assistant message re-ran the whole react-markdown pipeline on every text delta (`MarkdownPreview` memoizes on `content ===`), 2-5 full parses per second of a growing document, 10-30 ms each on a phone (C3). - Mermaid blocks called `mermaid.render` on mount and on every streaming delta, with no viewport gate and no reuse of a previous SVG (C4). - Every loaded top-level timeline row stayed in layout/paint; each style/layout pass on a phone (keyboard, orientation, streaming growth) walked all mounted rows (C1 step 1). ## What changed - `streaming-markdown-split.ts`: `splitStreamingMarkdown(text)` returns a settled prefix and a live tail. The boundary is the last blank line that is outside an open fenced block / `$$` math block / list continuation and is followed by a complete line, so the boundary only moves forward as text streams. `ConversationMessageContent` renders the streaming assistant row as two memoized `MarkdownPreview` documents (settled + tail) and falls back to one document when no boundary exists or when the row is complete. Seam classes restore the `last:mb-0` / `first:mt-0` margins at the split so completion does not shift layout. `ThreadTimelineRows` computes the streaming row (`findStreamingAssistantMessageId`: trailing leaf row while the runtime runs, descending pending turns and pending delegations) and provides it through a context; delegation output uses the split while pending. - `markdown-mermaid-render-cache.ts` + `MarkdownMermaidDiagram`: one shared IntersectionObserver gates the first render (256px root margin); source changes after the first render are debounced 300 ms trailing and keep the previous SVG visible; rendered SVGs are stored in a 32-entry LRU keyed by (source, theme, palette epoch); a cached diagram paints synchronously on mount. - `timeline-row-containment.ts` + `TimelineRowsList`: top-level row wrappers get `max-md:[content-visibility:auto] max-md:[contain-intrinsic-block-size:auto_1.25rem]`; conversation rows add an inline length-based `contain-intrinsic-block-size` estimate (bucketed). Nested lists keep plain wrappers. Compact-only because paint containment clips the assistant table breakout on wide layouts. ## How you verified - New tests: `streaming-markdown-split.test.ts` (fences incl. marker matching, `$$` blocks and inline `$$x$$`, loose lists / indented continuation, headings, monotonic boundary while streaming); `ConversationMessageContent.streaming.test.tsx` (mocks react-markdown to count parses: only the tail re-parses per delta, boundary moves forward, completion collapses to one document, open fence stays in the tail); `ThreadTimelineRows.streaming.test.ts` (streaming row selection incl. pending turn children, pending delegation childRows, completed turn -> null); `markdown-mermaid-diagram.render.test.tsx` (no render before viewport entry, one shared observer, 300 ms debounce keeps previous SVG, remount served from cache without calling mermaid, LRU eviction); `ThreadTimelineRows.containment.test.tsx` (top-level wrappers get the class + conversation rows carry the estimate, nested wrappers do not). - `pnpm exec turbo run typecheck --filter=@bb/app`: pass. `pnpm exec turbo run lint --filter=@bb/app`: 0 errors, 147 warnings (same count as baseline; touched files carry the same 12 pre-existing warnings as before). `pnpm exec turbo run test --filter=@bb/app -- src/components/thread/timeline src/components/ui/markdown-* src/components/thread/embedded-chat/EmbeddedThreadChat.test.tsx src/views/thread-detail`: 48 files, 411 tests pass. ## Update (2026-08-19) `content-visibility: auto` is now armed only where CSS scroll anchoring exists (`CSS.supports("overflow-anchor", "none")`: Chromium, Firefox). WebKit has no scroll anchoring, so a replayed size that differs from the real row size moved the visible content and read as a flash-and-scroll while a thread settled on iOS. iOS keeps the inert intrinsic-size estimate; Chromium/Firefox keep the layout/paint savings. Test added for the WebKit path. Fixes: part of the mobile / iOS Safari performance program (verified sweep report in the bb thread; no single issue). ## Stack context Layer 20 of 22 in the `bb/mobile-perf/*` stack (bottom → top: quick wins first, big rocks last). - Prerequisite (layer below): `bb/mobile-perf/rerender-amplifiers` (#1898). - Next layer: `bb/mobile-perf/mobile-interactions` (#1900). - Audit findings addressed: see title IDs. Review: approved-with-fixes; 1 review fix commit(s). - Reviewer notes / follow-ups: C1 still needs an iOS Simulator pass (Linux review box; caniuse confirms iOS Safari through 26.3 has no scroll anchoring). With the fix, rows are laid out once before opting in, so | C3 (design trade-off, not fixed): plugin message-directive components and stateful markdown widgets (code-block wrap toggle) that sit in the live tail remount when the boundary mov | C4: cached SVGs reuse the first renderer's element ids, so two simultaneously mounted copies of the same diagram (e.g. split view of the same thread) share ids; visually identical | C1: paint containment can clip the assistant table breakout by ~2px inside expanded turn bodies on compact (nested list is pl-3/pr-2 asymmetric). Negligible. - Wire/contract: None. No server/daemon wire, route, command or protocol-version change. New required prop `streaming: boolean` on the assistant variant of ConversationMessageContent (app-internal component contract; stories/tests updated). > AGENT GENERATED: by Claude Code (claude-mangosteen-eap) --------- Co-authored-by: Claude <noreply@anthropic.com>
SawyerHood
added a commit
that referenced
this pull request
Aug 19, 2026
…w (G2, G3) (#1901) ## What was wrong Diff-tab text cards fetched the full old and new file for every modified card as soon as it came within 200 px of the viewport, only so pierre could show expand-context buttons; the enriched diff was then tokenized and rendered a second time. The file preview rendered whole files un-virtualized with no size cap, remounted pierre when the highlighted AST arrived (throwing away DOM and scroll position), and re-encoded every text file to a base64 `data:` URL that only ever fed a cache key. ## What changed - Diff cards render from the patch alone. An app-owned "Expand context" row requests the contents on demand (with retry); fine-pointer devices still request them during idle time so desktop keeps zero-click expansion. Image/SVG cards keep fetching on viewport entry. Added/deleted files never offer the row. - The file preview code view owns its scroll container and registers it as pierre's virtualizer root, so only rows near the viewport render. Deep links scroll the virtualized viewport toward the target until the row exists. - Files over 5,000 lines or 512 KB render a prefix with "Load full file"; a line link past the prefix loads the whole file; loading keeps the scroll offset. - Pierre mounts once per file; plain->highlighted no longer remounts. - Workspace previews build a `data:` URL only for image/video; text previews carry the `/diff/file` route URL. - New stories for the capped view, a deep link past the cap, and a content-sized scroller. ## How you verified - New tests: `DiffFileCard.contextExpansion.test.tsx` (coarse pointer: no fetch on reveal, fetch on click, affordance retires; fine pointer: idle auto-fetch; error -> Retry; added file never fetches — 3 of 4 fail before), `FilePreview.test.tsx` (single mount on highlight, 5,000-line cap + Load full file + `:head` cacheKey, 512 KB cap, line link past the cap shows whole file, target-line scroll on the virtualized viewport), `environment-queries.test.tsx` (text preview has no data URL; image does). - `pnpm exec turbo run typecheck --filter=@bb/app`, `pnpm exec turbo run lint --filter=@bb/app` (pre-existing warnings only), `pnpm exec turbo run test --filter=@bb/app -- src/components/secondary-panel src/components/git-diff src/hooks/queries src/lib/file-preview.test.ts` (39 files, 256 tests pass). - Visual: ladle static build of FilePreview stories served locally and driven with headless Chromium — virtualized row counts, deep-link scroll to line 6,500, capped notice, Load full file (scroll offset preserved), wrap mode scrolling, and the content-sized (skill detail) container all behaved as intended. Fixes: part of the mobile / iOS Safari performance program (verified sweep report in the bb thread; no single issue). ## Stack context Layer 22 of 22 in the `bb/mobile-perf/*` stack (bottom → top: quick wins first, big rocks last). - Prerequisite (layer below): `bb/mobile-perf/mobile-interactions` (#1900). - Audit findings addressed: see title IDs. Review: approved-with-fixes; 1 review fix commit(s). - Deliberately not done here: G2 optional `maxBytes` param on /environments/:id/diff/file: not added. The route was not touched (the fix is on-demand fetching in the client), and a real cap - Reviewer notes / follow-ups: Non-blocking, pre-existing: `GitDiffCard` (timeline/thread surface) never passes `patchText` to `useGitDiffCardBody`, so its text cards never get context expansion (before this bra | Non-blocking: on fine pointers, when a text card's patch identity changes while contents are already loaded, the fetch effect briefly starts and cancels one fetch before the idle r | Not done by design (per implementer): the `maxBytes` route param from G2 was skipped because it needs a daemon change; no wire changes on this branch, so no HOST_DAEMON_PROTOCOL_VE | Small UX change on desktop worth knowing: code previews now own their scroller (`usesFullHeightLayout`), so the file header stays fixed while code scrolls, matching iframe/CSV prev - Wire/contract: None. No server-contract, route, SDK, or host-daemon protocol changes; HOST_DAEMON_PROTOCOL_VERSION untouched. > AGENT GENERATED: by Claude Code (claude-mangosteen-eap) --------- Co-authored-by: Claude <noreply@anthropic.com>
SawyerHood
added a commit
that referenced
this pull request
Aug 20, 2026
## What was wrong PR #1900 replaced the GitHub favicon PNGs (fetched from github.githubassets.com) with the bundled hugeicons `Github` outline glyph plus a separate status dot. That outline fills the whole 16px box with a stroked octocat and has a different silhouette, so the PR check icon in tab strips, the sidebar, and the thread metadata panel looked much larger than the favicon it replaced. ## What changed - New `apps/app/src/components/pull-request/GithubFaviconIcon.tsx` inlines the exact path data from GitHub's `favicon.svg`, `favicon-success.svg`, `favicon-failure.svg`, and `favicon-pending.svg` (the mark with its cut-out plus the check / X / circle glyph). The mark fills with `currentColor`; the glyph uses `fill-success` / `fill-destructive` / `fill-attention`. The light and dark favicons share the same paths, so one SVG serves both themes and still makes no cross-origin request. - `PullRequestStatusPill.tsx` (`PullRequestGithubCheckIcon`) and `ThreadMetadataContent.tsx` render it in place of the hugeicons glyph. - No wire, CLI, or plugin API changes. ## How you verified - Rasterized the new SVGs at 16px next to the real favicon PNGs downscaled to 16px; the pairs are indistinguishable for success, failure, and pending. - `pnpm exec turbo run typecheck --filter=@bb/app`: pass. - eslint + prettier on the touched files: clean. - vitest `src/components/pull-request src/components/secondary-panel src/components/thread-list`: 31 files, 229 tests pass. Fixes: visual regression from #1900 (no issue filed). > AGENT GENERATED: by Claude Opus 5 Co-authored-by: Claude <noreply@anthropic.com>
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
1 participant
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.
What was wrong
On phones (iOS Safari, compact viewports) several app-side patterns made every page and every thread heavier than it needed to be:
What changed
CompactLongPressMenu(long-press/right-click detector, nothing mounted until first open) opens the existing responsive drawer with the same DropdownMenu items; desktop keeps the context menu.SidebarTouchSensorinstalls dnd-kit's listener only while the compact drawer shows (external store written by SidebarProvider, no context subscription); tab strip wires TouchSensor only when open with 2+ tabs.<Sidebar>panel hosts the AppSidebar body (hidden while Settings/Tools body shows);mobileHostedmode on AppSidebar/SectionSidebar; 220 ms close hold preserved.pendingInteractionprop on FollowUpPromptBox keeps the editor shell mounted and hidden, interaction as last stack item, footer pickers read-only.focusEndForTap); Stop/voice/drafted submit work from the row; Fix lost first tap on mobile follow-up submit #1771 handoff untouched.HEAVY_PAYLOAD_QUERY_POLICY(60 s gc) on turn-summary details and file previews; diff patches use a reader lease with a 60 s post-unmount eviction (observer-less entries no longer gc while shown).How you verified
pnpm exec turbo run typecheck --filter=@bb/app: pass.pnpm exec turbo run lint --filter=@bb/app: 0 errors (147 pre-existing warnings, unchanged count).pnpm exec turbo run test --filter=@bb/app --force: 364 files, 2906 passed, 3 skipped (one registry boundary test fixed in the last commit and re-run green).Fixes: mobile perf sweep findings E1, E6, E7, E10, D3, D5 (reverted, see below), J3, J4, I6, B28.
Update (2026-08-19)
The deferred compact follow-up editor (D5: static stand-in row + pointerdown/click focus handoff) was removed from this PR after a device test on iPhone showed the caret rendering above the composer after the first tap and a ~11 px footer shift when the real editor replaced the stand-in. The compact composer keeps the previously verified always-mounted behavior (#1263, #1381, #1771). D5 stays open as a follow-up once it can be verified on iOS.
Fixes: part of the mobile / iOS Safari performance program (verified sweep report in the bb thread; no single issue).
Stack context
Layer 21 of 22 in the
bb/mobile-perf/*stack (bottom → top: quick wins first, big rocks last).bb/mobile-perf/markdown-streaming-and-containment(Split streaming markdown into settled prefix + live tail; gate mermaid; row containment (C1, C3, C4) #1899).bb/mobile-perf/diff-and-file-preview(Fetch diff-card context on demand; virtualize and cap the file preview (G2, G3) #1901).