Skip to content

Commit b0ccdf3

Browse files
fix(web): repair the Browser surface card and collapse the profile menu row
Widening `onAddBrowser` to `(profileId?: string)` broke the empty-state Browser card: it is passed straight to a DOM click handler, so React handed the MouseEvent in as the profile id and the open silently failed schema validation. Fixed structurally rather than at the call site — `onAddBrowser` goes back to taking no arguments, and choosing a profile is a separate `onAddBrowserInProfile(profileId)`. The type now rejects wiring the profile variant to a DOM handler, so this cannot recur; adding it surfaced all four call sites immediately. The "+" menu is one row again. The submenu trigger is itself clickable and opens the default profile, with hover or arrow revealing the rest, so the common case stays a single click. The menu is controlled so that action can dismiss it, which a submenu trigger does not do on its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0e6a571 commit b0ccdf3

5 files changed

Lines changed: 61 additions & 19 deletions

File tree

apps/web/src/components/ChatView.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6642,7 +6642,8 @@ function ChatViewContent(props: ChatViewProps) {
66426642
onCloseSurfacesToRight={closeRightPanelSurfacesToRight}
66436643
onCloseAllSurfaces={closeAllRightPanelSurfaces}
66446644
onCopyFilePath={copyRightPanelFilePath}
6645-
onAddBrowser={createBrowserSurface}
6645+
onAddBrowser={() => createBrowserSurface()}
6646+
onAddBrowserInProfile={createBrowserSurface}
66466647
onAddTerminal={addTerminalSurface}
66476648
onAddDiff={addDiffSurface}
66486649
onAddFiles={addFilesSurface}
@@ -6681,7 +6682,8 @@ function ChatViewContent(props: ChatViewProps) {
66816682
onCloseSurfacesToRight={closeRightPanelSurfacesToRight}
66826683
onCloseAllSurfaces={closeAllRightPanelSurfaces}
66836684
onCopyFilePath={copyRightPanelFilePath}
6684-
onAddBrowser={createBrowserSurface}
6685+
onAddBrowser={() => createBrowserSurface()}
6686+
onAddBrowserInProfile={createBrowserSurface}
66856687
onAddTerminal={addTerminalSurface}
66866688
onAddDiff={addDiffSurface}
66876689
onAddFiles={addFilesSurface}

apps/web/src/components/RightPanelTabs.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ function renderTabs(first: DesktopPreviewFavicon | null, second?: DesktopPreview
7373
onCloseAllSurfaces={() => undefined}
7474
onCopyFilePath={() => undefined}
7575
onAddBrowser={() => undefined}
76+
onAddBrowserInProfile={() => undefined}
7677
onAddTerminal={() => undefined}
7778
onAddPullRequest={() => undefined}
7879
onAddDiff={() => undefined}

apps/web/src/components/RightPanelTabs.tsx

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,13 @@ interface RightPanelTabsProps {
6868
onCloseSurfacesToRight: (surface: RightPanelSurface) => void;
6969
onCloseAllSurfaces: () => void;
7070
onCopyFilePath: (relativePath: string) => void;
71-
onAddBrowser: (profileId?: string) => void;
71+
onAddBrowser: () => void;
72+
/**
73+
* Separate from `onAddBrowser` on purpose: that one is passed directly as a
74+
* DOM click handler, and a `(profileId?: string)` signature would silently
75+
* accept the MouseEvent as a profile id.
76+
*/
77+
onAddBrowserInProfile: (profileId: string) => void;
7278
onAddTerminal: () => void;
7379
onAddDiff: () => void;
7480
onAddFiles: () => void;
@@ -526,6 +532,9 @@ function SurfaceIcon({
526532
export function RightPanelTabs(props: RightPanelTabsProps) {
527533
const ownsDesktopTitleBar = isElectron && props.mode === "inline";
528534
const browserProfiles = useBrowserDefaults().profiles;
535+
// Controlled so the submenu trigger's own action can dismiss the menu; a
536+
// submenu trigger does not close it the way a plain item does.
537+
const [addSurfaceMenuOpen, setAddSurfaceMenuOpen] = useState(false);
529538
const { resolvedTheme } = useTheme();
530539
const tabListRef = useRef<HTMLDivElement>(null);
531540

@@ -691,7 +700,7 @@ export function RightPanelTabs(props: RightPanelTabsProps) {
691700
);
692701
})}
693702
{props.surfaces.length > 0 ? (
694-
<Menu>
703+
<Menu open={addSurfaceMenuOpen} onOpenChange={setAddSurfaceMenuOpen}>
695704
<MenuTrigger
696705
render={
697706
<Button
@@ -705,34 +714,45 @@ export function RightPanelTabs(props: RightPanelTabsProps) {
705714
<Plus className="size-3.5" />
706715
</MenuTrigger>
707716
<MenuPopup align="start" side="bottom" sideOffset={6} className="min-w-44">
708-
<SurfaceMenuItem
709-
available={props.browserAvailable}
710-
disabledReason={SURFACE_DISABLED_REASONS.browser}
711-
onClick={() => props.onAddBrowser()}
712-
>
713-
<Globe2 />
714-
Browser
715-
</SurfaceMenuItem>
716717
{props.browserAvailable ? (
717718
<MenuSub>
718719
{/*
719-
A tab's profile is fixed at open — Electron only honours
720-
a partition before the guest attaches — so the choice
721-
belongs here rather than on an already-open tab.
720+
Clicking the trigger opens the default profile, so the
721+
common case stays one click and there is no second row;
722+
hover or arrow reveals the rest. The choice lives at
723+
open time because a tab's profile is fixed then —
724+
Electron only honours a partition before attach.
722725
*/}
723-
<MenuSubTrigger>
726+
<MenuSubTrigger
727+
onClick={() => {
728+
setAddSurfaceMenuOpen(false);
729+
props.onAddBrowser();
730+
}}
731+
>
724732
<Globe2 />
725-
Browser in profile
733+
Browser
726734
</MenuSubTrigger>
727735
<MenuSubPopup className="min-w-40">
728736
{browserProfiles.map((profile) => (
729-
<MenuItem key={profile.id} onClick={() => props.onAddBrowser(profile.id)}>
737+
<MenuItem
738+
key={profile.id}
739+
onClick={() => props.onAddBrowserInProfile(profile.id)}
740+
>
730741
{profile.name}
731742
</MenuItem>
732743
))}
733744
</MenuSubPopup>
734745
</MenuSub>
735-
) : null}
746+
) : (
747+
<SurfaceMenuItem
748+
available={false}
749+
disabledReason={SURFACE_DISABLED_REASONS.browser}
750+
onClick={props.onAddBrowser}
751+
>
752+
<Globe2 />
753+
Browser
754+
</SurfaceMenuItem>
755+
)}
736756
<SurfaceMenuItem
737757
available={props.terminalAvailable}
738758
disabledReason={SURFACE_DISABLED_REASONS.terminal}

apps/web/src/components/preview/addBrowserSurface.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,24 @@ beforeEach(() => {
3737
});
3838

3939
describe("addBrowserSurface", () => {
40+
it("opens under the requested profile", async () => {
41+
const openPreview = vi.fn(async (_input: PreviewOpenInput) =>
42+
AsyncResult.success(snapshot("tab-1")),
43+
);
44+
45+
await addBrowserSurface({
46+
threadRef,
47+
openPreview: ({ input }) => openPreview(input),
48+
profileId: "profile-work",
49+
});
50+
51+
expect(openPreview).toHaveBeenCalledWith({
52+
threadId: "thread-1",
53+
viewport: FILL_PREVIEW_VIEWPORT,
54+
profileId: "profile-work",
55+
});
56+
});
57+
4058
it("creates another preview session when a browser tab is already active", async () => {
4159
const first = snapshot("tab-1");
4260
const second = snapshot("tab-2");

apps/web/src/routes/_chat.pull-requests.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1571,6 +1571,7 @@ function PullRequestsRouteView() {
15711571
onCloseAllSurfaces={closeAllSurfaces}
15721572
onCopyFilePath={() => undefined}
15731573
onAddBrowser={() => undefined}
1574+
onAddBrowserInProfile={() => undefined}
15741575
onAddTerminal={() => undefined}
15751576
onAddDiff={() => undefined}
15761577
onAddFiles={() => undefined}

0 commit comments

Comments
 (0)