Skip to content

Commit c7688dd

Browse files
Fix inconsistent app icon active states in chat-first rail (#4834)
* Give the chat-first rail one owner for active icon state Search Chats rendered outside the nav tablist with hover-only styling and had no ChatFirstPrimaryTab member, so it could never show an active state. The apps rail derived its inactive treatment from activeAppId alone, which could not distinguish an unresolved surface from a nav surface owning the rail, so every app icon kept its in-color active treatment whenever Search Chats, Scheduled, Integrations, or New chat took the main area. Both surfaces now resolve active state through a shared active-surface module, so exactly one rail entry reads as active. * Make the desktop rail's active nav surface a tested pure resolver Scheduled tasks and the chats view are rail surfaces without an appId, so they have to name a primary tab or the rail cannot tell them from an unresolved surface. That derivation was an inline useMemo with no coverage; extracting it lets a test fail if a future surface stops naming its tab. --------- Co-authored-by: Builder.io <builder-bot@builder.io>
1 parent c192f8c commit c7688dd

14 files changed

Lines changed: 550 additions & 84 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@agent-native/core": patch
3+
"@agent-native/dispatch": patch
4+
---
5+
6+
Give the chat-first rail one owner for active state so exactly one entry ever
7+
reads as active. `activeAppId` alone could not distinguish "no surface resolved
8+
yet" from "a nav surface is active with no app selected", so every app icon kept
9+
its in-color active treatment whenever Search Chats, Scheduled, Integrations, or
10+
New chat owned the main area. Search Chats was worse: it rendered outside the
11+
tablist with hover-only styling and no `ChatFirstPrimaryTab` member, so it could
12+
never show an active state at all.
13+
14+
`ChatFirstPrimaryTab` now includes `search`, `ChatFirstAppsRail` accepts
15+
`activeTab`, and both the rail and the primary navigation derive their
16+
active/inactive presentation from the shared `chatFirstActiveSurface`,
17+
`chatFirstAppIconState`, and `chatFirstNavTabActive` helpers.

packages/code-agents-ui/src/CodeAgentsApp.spec.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
getCodeAgentWorktreeRecoveryState,
1111
groupCodeAgentModelOptions,
1212
normalizeModelSelection,
13+
resolveCodeAgentsPrimaryTab,
1314
resolveNewSessionExtensionComposerState,
1415
shouldShowCodeAgentCredentialCallout,
1516
shouldCloseWatchedChatFirstSession,
@@ -679,3 +680,43 @@ describe("chat-first session watch bounds", () => {
679680
).toBe(false);
680681
});
681682
});
683+
684+
describe("resolveCodeAgentsPrimaryTab", () => {
685+
it("activates Search while its panel owns the main area", () => {
686+
expect(
687+
resolveCodeAgentsPrimaryTab({
688+
chatFirstMainKind: "code",
689+
searchPanelOpen: true,
690+
hostActiveTab: "new-chat",
691+
}),
692+
).toBe("search");
693+
});
694+
695+
it("keeps the host tab when the search panel is closed", () => {
696+
expect(
697+
resolveCodeAgentsPrimaryTab({
698+
chatFirstMainKind: "code",
699+
searchPanelOpen: false,
700+
hostActiveTab: "scheduled",
701+
}),
702+
).toBe("scheduled");
703+
});
704+
705+
it("releases Search when another surface takes the main area", () => {
706+
expect(
707+
resolveCodeAgentsPrimaryTab({
708+
chatFirstMainKind: "agent",
709+
searchPanelOpen: true,
710+
}),
711+
).toBeUndefined();
712+
});
713+
714+
it("resolves no tab when nothing owns the rail", () => {
715+
expect(
716+
resolveCodeAgentsPrimaryTab({
717+
chatFirstMainKind: "code",
718+
searchPanelOpen: false,
719+
}),
720+
).toBeUndefined();
721+
});
722+
});

packages/code-agents-ui/src/CodeAgentsApp.tsx

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,21 @@ export function shouldCloseWatchedChatFirstSession(input: {
331331
return !input.watchedRunPresent;
332332
}
333333

334+
/**
335+
* Search owns the rail only while its panel owns the main area, so the tab
336+
* highlight can never disagree with what is actually on screen.
337+
*/
338+
export function resolveCodeAgentsPrimaryTab(input: {
339+
chatFirstMainKind: "agent" | "code";
340+
searchPanelOpen: boolean;
341+
hostActiveTab?: ChatFirstPrimaryTab;
342+
}): ChatFirstPrimaryTab | undefined {
343+
if (input.chatFirstMainKind === "code" && input.searchPanelOpen) {
344+
return "search";
345+
}
346+
return input.hostActiveTab;
347+
}
348+
334349
export interface CodeAgentsAppProps {
335350
apps: AppConfig[];
336351
host: CodeAgentsHost;
@@ -2752,10 +2767,18 @@ export default function CodeAgentsApp({
27522767
[host.transferRun, requestPortalTransfer],
27532768
);
27542769

2770+
const activePrimaryTab = resolveCodeAgentsPrimaryTab({
2771+
chatFirstMainKind,
2772+
searchPanelOpen,
2773+
...(chatFirstNavigation?.activeTab
2774+
? { hostActiveTab: chatFirstNavigation.activeTab }
2775+
: {}),
2776+
});
2777+
const searchPanelActive = activePrimaryTab === "search";
27552778
const showingSelectedRunDetail =
27562779
!workbenchOpen &&
27572780
!mobilePanelOpen &&
2758-
!searchPanelOpen &&
2781+
!searchPanelActive &&
27592782
Boolean(selectedRun);
27602783

27612784
return (
@@ -2782,7 +2805,7 @@ export default function CodeAgentsApp({
27822805
onOpenIntegrations={() => chatFirstNavigation?.onOpenIntegrations()}
27832806
onOpenScheduled={() => chatFirstNavigation?.onOpenScheduled()}
27842807
onSearch={openSearchPanel}
2785-
activeTab={chatFirstNavigation?.activeTab}
2808+
activeTab={activePrimaryTab}
27862809
collapsed={railCollapsed}
27872810
stickyNewChat
27882811
/>
@@ -2929,7 +2952,7 @@ export default function CodeAgentsApp({
29292952
onCopyLink={copyMobileLink}
29302953
onOpenSettings={onOpenSettings}
29312954
/>
2932-
) : searchPanelOpen ? (
2955+
) : searchPanelActive ? (
29332956
<SearchChatsPanel
29342957
query={searchQuery}
29352958
results={searchResults}
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { describe, expect, it } from "vitest";
2+
3+
import {
4+
chatFirstActiveSurface,
5+
chatFirstAppIconState,
6+
chatFirstNavTabActive,
7+
} from "./active-surface.js";
8+
9+
describe("chatFirstActiveSurface", () => {
10+
it("keeps an unresolved surface distinct from an active nav surface", () => {
11+
expect(chatFirstActiveSurface({})).toBeUndefined();
12+
expect(chatFirstActiveSurface({ activeTab: "new-chat" })).toEqual({
13+
kind: "nav",
14+
tab: "new-chat",
15+
});
16+
});
17+
18+
it("reports the selected app when no nav surface is named", () => {
19+
expect(chatFirstActiveSurface({ activeAppId: "mail" })).toEqual({
20+
kind: "app",
21+
appId: "mail",
22+
});
23+
});
24+
25+
it("lets the named nav surface win over a stale app selection", () => {
26+
expect(
27+
chatFirstActiveSurface({ activeAppId: "dispatch", activeTab: "search" }),
28+
).toEqual({ kind: "nav", tab: "search" });
29+
});
30+
});
31+
32+
describe("chatFirstAppIconState", () => {
33+
it("marks nothing active or inactive before a surface resolves", () => {
34+
expect(chatFirstAppIconState(undefined, "mail")).toEqual({
35+
isActive: false,
36+
isInactive: false,
37+
});
38+
});
39+
40+
it("activates only the selected app", () => {
41+
const surface = chatFirstActiveSurface({ activeAppId: "mail" });
42+
43+
expect(chatFirstAppIconState(surface, "mail")).toEqual({
44+
isActive: true,
45+
isInactive: false,
46+
});
47+
expect(chatFirstAppIconState(surface, "calendar")).toEqual({
48+
isActive: false,
49+
isInactive: true,
50+
});
51+
});
52+
53+
it("deactivates every app while a nav surface owns the rail", () => {
54+
for (const tab of [
55+
"new-chat",
56+
"integrations",
57+
"scheduled",
58+
"search",
59+
] as const) {
60+
const surface = chatFirstActiveSurface({ activeTab: tab });
61+
for (const appId of ["mail", "calendar", "design", "clips"]) {
62+
expect(chatFirstAppIconState(surface, appId)).toEqual({
63+
isActive: false,
64+
isInactive: true,
65+
});
66+
}
67+
}
68+
});
69+
});
70+
71+
describe("chatFirstNavTabActive", () => {
72+
it("activates only the named nav tab", () => {
73+
const surface = chatFirstActiveSurface({ activeTab: "search" });
74+
75+
expect(chatFirstNavTabActive(surface, "search")).toBe(true);
76+
for (const tab of ["new-chat", "integrations", "scheduled"] as const) {
77+
expect(chatFirstNavTabActive(surface, tab)).toBe(false);
78+
}
79+
});
80+
81+
it("activates no nav tab while an app owns the rail", () => {
82+
const surface = chatFirstActiveSurface({ activeAppId: "mail" });
83+
84+
for (const tab of [
85+
"new-chat",
86+
"integrations",
87+
"scheduled",
88+
"search",
89+
] as const) {
90+
expect(chatFirstNavTabActive(surface, tab)).toBe(false);
91+
}
92+
});
93+
});
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import type { ChatFirstAppIconRenderOptions } from "./types.js";
2+
3+
export type ChatFirstPrimaryTab =
4+
| "new-chat"
5+
| "integrations"
6+
| "scheduled"
7+
| "search";
8+
9+
/**
10+
* The one rail entry that owns the active presentation. `undefined` means the
11+
* host has not resolved a surface yet, which must stay distinguishable from a
12+
* nav surface owning the rail while no app is selected.
13+
*/
14+
export type ChatFirstActiveSurface =
15+
| { kind: "app"; appId: string }
16+
| { kind: "nav"; tab: ChatFirstPrimaryTab };
17+
18+
export function chatFirstActiveSurface({
19+
activeAppId,
20+
activeTab,
21+
}: {
22+
activeAppId?: string;
23+
activeTab?: ChatFirstPrimaryTab;
24+
}): ChatFirstActiveSurface | undefined {
25+
if (activeTab !== undefined) return { kind: "nav", tab: activeTab };
26+
if (activeAppId !== undefined) return { kind: "app", appId: activeAppId };
27+
return undefined;
28+
}
29+
30+
export function chatFirstAppIconState(
31+
surface: ChatFirstActiveSurface | undefined,
32+
appId: string,
33+
): ChatFirstAppIconRenderOptions {
34+
if (!surface) return { isActive: false, isInactive: false };
35+
const isActive = surface.kind === "app" && surface.appId === appId;
36+
return { isActive, isInactive: !isActive };
37+
}
38+
39+
export function chatFirstNavTabActive(
40+
surface: ChatFirstActiveSurface | undefined,
41+
tab: ChatFirstPrimaryTab,
42+
): boolean {
43+
return surface?.kind === "nav" && surface.tab === tab;
44+
}

packages/core/src/client/chat-first/apps-rail.spec.tsx

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ describe("ChatFirstAppsRail", () => {
109109
).toBe("true");
110110
});
111111

112-
it("keeps app icons in color when no app is selected", () => {
112+
it("keeps app icons in color before any surface resolves", () => {
113113
act(() => {
114114
root.render(
115115
<ChatFirstAppsRail
@@ -134,6 +134,100 @@ describe("ChatFirstAppsRail", () => {
134134
).toBe("false");
135135
});
136136

137+
it("grays every app icon while a nav surface owns the rail", () => {
138+
act(() => {
139+
root.render(
140+
<ChatFirstAppsRail
141+
apps={[
142+
{ id: "content", name: "Content" },
143+
{ id: "analytics", name: "Analytics" },
144+
]}
145+
activeTab="search"
146+
collapsed
147+
onOpenApp={vi.fn()}
148+
renderIcon={(app, options) => (
149+
<span data-icon-inactive={options.isInactive}>{app.name}</span>
150+
)}
151+
/>,
152+
);
153+
});
154+
155+
const icons = [
156+
...container.querySelectorAll<HTMLElement>("[data-chat-first-app-icon]"),
157+
];
158+
expect(icons).toHaveLength(2);
159+
for (const icon of icons) {
160+
expect(icon.className).toContain("grayscale");
161+
expect(
162+
icon
163+
.querySelector("[data-icon-inactive]")
164+
?.getAttribute("data-icon-inactive"),
165+
).toBe("true");
166+
expect(
167+
icon.closest("[data-chat-first-app]")?.className.split(" "),
168+
).not.toContain("bg-sidebar-accent");
169+
}
170+
});
171+
172+
it("grays every expanded app row while a nav surface owns the rail", () => {
173+
act(() => {
174+
root.render(
175+
<ChatFirstAppsRail
176+
apps={[
177+
{ id: "content", name: "Content" },
178+
{ id: "analytics", name: "Analytics" },
179+
]}
180+
activeTab="new-chat"
181+
onOpenApp={vi.fn()}
182+
renderIcon={(app, options) => (
183+
<span data-icon-inactive={options.isInactive}>{app.name}</span>
184+
)}
185+
/>,
186+
);
187+
});
188+
189+
const rows = [
190+
...container.querySelectorAll<HTMLElement>("[data-chat-first-app]"),
191+
];
192+
expect(rows).toHaveLength(2);
193+
for (const row of rows) {
194+
expect(row.className.split(" ")).not.toContain("bg-sidebar-accent");
195+
expect(
196+
row.querySelector("[data-chat-first-app-icon]")?.className,
197+
).toContain("grayscale");
198+
}
199+
});
200+
201+
it("keeps the selected app active when a nav tab is not resolved", () => {
202+
act(() => {
203+
root.render(
204+
<ChatFirstAppsRail
205+
apps={[
206+
{ id: "content", name: "Content" },
207+
{ id: "analytics", name: "Analytics" },
208+
]}
209+
activeAppId="analytics"
210+
collapsed
211+
onOpenApp={vi.fn()}
212+
renderIcon={(app, options) => (
213+
<span data-icon-inactive={options.isInactive}>{app.name}</span>
214+
)}
215+
/>,
216+
);
217+
});
218+
219+
expect(
220+
container.querySelector<HTMLElement>(
221+
'[data-app-id="analytics"] [data-chat-first-app-icon]',
222+
)?.className,
223+
).not.toContain("grayscale");
224+
expect(
225+
container.querySelector<HTMLElement>(
226+
'[data-app-id="content"] [data-chat-first-app-icon]',
227+
)?.className,
228+
).toContain("grayscale");
229+
});
230+
137231
it("shows a collapsed app name immediately on hover", async () => {
138232
act(() => {
139233
root.render(

0 commit comments

Comments
 (0)