fix(chat): render the ephemeral create options inline at phone width - #7327
Conversation
The 'New ephemeral chat' entry in the + New caret dropdown was an unconditional Radix flyout submenu. At phone width a flyout pins to the trigger's side and only shifts vertically, so it opened past the left viewport edge and the Incognito/Temporary rows were unreadable. Apply the same treatment the folder picker and crew submenu received in kirodotdev#6909: at phone width, list the two ephemeral rows inline under a caption label; wider viewports keep the flyout unchanged. Rows are hoisted into a shared fragment (folderRows pattern) so the two branches cannot drift. Two mobile-viewport tests added alongside the existing four desktop tests.
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed |
Design Review (Fable 5, fork) — ✅ PASSDesign-level review of The change faithfully replicates the #6909 pattern already used twice in this same menu (folder picker at line 5254, crew submenu at 5306), shares the rows between branches to prevent drift, and the Design-Verdict: PASS A real phone-width clipping bug fixed with the exact pattern its sibling submenus already use; shared rows prevent branch drift. [DESIGN-REVIEWED] 96929c2 |
UX Review (Fable 5, fork) — ✅ PASSUX-level review of The network fetch isn't permitted in this sandbox, so I'll review the added screenshots from the diff and description as the instructions direct. I have everything needed: the change applies, verbatim, the same UX-Verdict: PASS Fixes an unusable off-screen flyout by reusing the exact inline-caption pattern this menu already teaches; nothing new for a user to learn. [UX-REVIEWED] 96929c2 |
First Principles Review (Fable 5, fork) — ✅ PASSPremise-level review of Verification is complete. The review: First-Principles-Verdict: PASS A measured phone-width defect (flyout at x = −181) gets the exact treatment #6909 already codified for its sibling submenus; nothing rides along. What this change shipsIntent: make the ephemeral-chat create options readable and tappable on a phone — a FIX.
Checks run: no shared responsive-submenu wrapper exists to reuse ( Watch
[FIRST-PRINCIPLES-REVIEWED] 96929c2 |
GPT 5.6 Review (fork) — ✅ no blocking findingsReviewed Review detailsNo findings. |
bolichen97
left a comment
There was a problem hiding this comment.
Tier 1 auto-approve: fix (4 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fix(chat) renders ephemeral create options inline at phone width, clear root cause (menu clipped at narrow width). CodeQL is not applicable on this fork PR (default-setup emits no check-run); SAST coverage is Semgrep only, latest run success with 0 annotations.
bolichen97
left a comment
There was a problem hiding this comment.
Tier 1 auto-approve: fix (4 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fix with a clear root cause — the ephemeral create options overflowed their popover at phone width, and the fix renders them inline instead. CodeQL is not applicable on this fork PR (default-setup emits no check-run); SAST coverage is Semgrep only, latest run success with 0 annotations.
Problem / Motivation
At phone width, the New ephemeral chat entry in the sidebar's + New caret menu (added in #6857) opens its options as a Radix flyout submenu. A flyout pins to its trigger's side and only shifts vertically on collision, and at phone width there is no horizontal room beside the parent menu — so it opens mostly past the left viewport edge. Measured at 390×844: the flyout renders at x = −181 with a 264px width; the Incognito/Temporary labels are unreadable, only the tail of each gloss shows.
Why it matters
On a phone the feature is effectively unusable: the two options can't be read, and tapping blind is the only way to use them. #6909 already established that flyouts don't work at phone width and converted the folder picker and crew submenu to inline lists — this entry merged in parallel and missed the treatment, so it's the one remaining submenu in this menu that still clips.
What changed (motivation → approach → change)
Goal: make the ephemeral options readable and tappable at phone width without touching the desktop experience. Approach: apply exactly the pattern #6909 codified for the sibling submenus in this same menu — at
isMobile, render the rows inline under aDropdownMenuLabelcaption; wider viewports keep the flyout.Change, in
website/src/pages/ChatSidebar.tsx(one hunk):ephemeralRowsfragment (mirroring the folder picker'sfolderRows), so the mobile and desktop branches render the same rows and cannot drift.isMobile(already in scope — it gates the folder picker a few lines below) selects between the inline caption + rows and the existingDropdownMenuSubflyout. The flyout branch's markup, handlers, testids, and disabled states are unchanged; the caption copies the folder caption's styling and reuses the existingpages.chatSidebar.new_ephemeral_chatkey. No new i18n keys, no changes to the shared dropdown component.Tests
website/src/test/ChatSidebar.ephemeralCreate.test.tsxgrows from 4 to 6 tests. TheuseIsMobilehook is mocked with the same mutable-box approachChatSidebar.narrowDurationPickers.test.tsx(#6909's test) uses, defaulting to desktop so the 4 existing flyout tests are unaffected. New: (a) at mobile the caret menu shows the ephemeral caption with both rows inline and no flyout trigger; (b) at mobile, activating a row still issues the create with the correctmemory_mode(and the run mode stays non-orchestrator).Full frontend suite on the branch: 26,889 passed, 0 real failures (the 17 Electron reds are the known missing-nested-dev-deps set, reproduced on the base).
tsc -b, eslint, jscpd clean.Manual verification
Ran the branch under a dev server at a 390×844 viewport and exercised the real menu; the screenshots below are captures from that session (scripted Playwright). Desktop flyout re-verified unchanged by the pre-existing tests.
Screenshots / video
Before — the flyout opens off the left edge (parent menu at x≈78, flyout at x = −181; only gloss tails visible):
After — caption + both options inline, fully readable:
Pattern harvest
Pattern: a
DropdownMenuSubflyout added to the sidebar create menu without theisMobileinline branch — at phone width a Radix flyout pins to the trigger's side and clips off-viewport. This is a recurring class, not a one-off: #6909 fixed the folder and crew submenus, and this entry (developed in parallel) recurred it.Rule candidate: ast-grep/semgrep — flag
<DropdownMenuSub>added in a component that importsuseIsMobilewhen the JSX is not guarded by anisMobileconditional. Durable alternative: extract the #6909 branch into a sharedResponsiveMenuSubwrapper (flyout on desktop, caption + inline rows on mobile) so new submenus get the correct behavior by construction.Related Issues
Fixes #7326
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)