jb/paseo
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(app): dismiss the composer track panel when a row is chosen

Browse Source 
Track pills became menu surfaces when the trackers moved out of stacked
cards, but ComposerTrackRow kept the raw Pressable it had as an inline
card row. Nothing in that path reaches selectItem, so the popover or
sheet stayed parked over the transcript after opening a subagent, and on
iOS the action fired while UIKit was still tearing the sheet down. Rows
now hand the press to the engine and take closeOnSelect, as MenuItem
does; bulk archive opts out because its progress lands in the panel.

The running mark is the shared StatusRing rather than a still dot the
pill drew for itself, and the remaining dots read their colour from the
one bucket-to-colour map. The slot is sized for the ring in every state
so the label does not step sideways when the last child finishes.

Mounting the real menu engine in a test needed harness work: stubs for
two packages that ship untranspiled Flow, reanimated left out of Vite's
prebundle so its .web files win, and the gaps filled in the unistyles
test theme.
This commit is contained in:
Mohamed Boudra
2026-08-17 23:43:01 +02:00
parent 1cc6712e51
commit c859b59c99
13 changed files with 396 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/app/e2e/browser/provider-subagents.real.spec.ts
+3
View File
@@ -89,6 +89,9 @@ test.describe("real provider subagent timelines", () => {
}
await rows.first().click();
// Choosing a row is a menu selection: the track panel goes with it.
await expect(page.getByTestId("subagents-track-header-panel")).toBeHidden();
const panel = page.getByTestId("provider-subagent-panel");
await expect(panel).toBeVisible({ timeout: 30_000 });
if (scenario.expectedSubtitle) {
packages/app/src/composer/tracks-panel.browser.test.tsx
+183
View File
@@ -0,0 +1,183 @@
import React, { act, type ReactNode } from "react";
import { createRoot, type Root } from "react-dom/client";
import { Pressable, Text } from "react-native";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { ComposerTrackPill, ComposerTrackRow } from "./tracks";
// App sources compile against the classic JSX runtime, which expects React on the global.
beforeEach(() => vi.stubGlobal("React", React));
/**
* The real menu engine, in a real browser, because both things under test only exist there: the
* panel's dismissal runs through `selectItem`, and the running mark is a Web Animations rotation
* that jsdom cannot report.
*/
interface Mounted {
root: Root;
container: HTMLDivElement;
}
const mounted: Mounted[] = [];
function mount(node: ReactNode): HTMLDivElement {
const container = document.createElement("div");
document.body.appendChild(container);
const root = createRoot(container);
act(() => root.render(node));
mounted.push({ root, container });
return container;
}
afterEach(() => {
for (const entry of mounted.splice(0)) {
act(() => entry.root.unmount());
entry.container.remove();
}
});
function click(element: Element): void {
act(() => {
(element as HTMLElement).click();
});
}
function pill(): HTMLElement {
const trigger = document.querySelector('[data-testid="pill"]');
if (!(trigger instanceof HTMLElement)) {
throw new Error("track pill did not render");
}
return trigger;
}
function openPanel(): void {
click(pill());
}
function row(testID: string): Element | null {
return document.querySelector(`[data-testid="${testID}"]`);
}
function isPanelOpen(): boolean {
return pill().getAttribute("aria-expanded") === "true";
}
describe("composer track panel", () => {
it("dismisses itself when a row is chosen", () => {
const onPress = vi.fn();
mount(
<ComposerTrackPill testID="pill" label="3 subagents" panelTitle="Subagents">
<ComposerTrackRow accessibilityLabel="Subagent one" testID="row" onPress={onPress}>
<Text>Subagent one</Text>
</ComposerTrackRow>
</ComposerTrackPill>,
);
openPanel();
const target = row("row");
expect(target).not.toBeNull();
click(target as Element);
expect(onPress).toHaveBeenCalledTimes(1);
expect(isPanelOpen()).toBe(false);
expect(row("row")).toBeNull();
});
it("stays open for a row whose result lands in the panel", () => {
const onPress = vi.fn();
mount(
<ComposerTrackPill testID="pill" label="3 subagents" panelTitle="Subagents">
<ComposerTrackRow
accessibilityLabel="Archive finished"
testID="row"
closeOnSelect={false}
onPress={onPress}
>
<Text>Archive finished</Text>
</ComposerTrackRow>
</ComposerTrackPill>,
);
openPanel();
click(row("row") as Element);
expect(onPress).toHaveBeenCalledTimes(1);
expect(isPanelOpen()).toBe(true);
expect(row("row")).not.toBeNull();
});
it("leaves the panel alone when a row's own action button is pressed", () => {
const onPress = vi.fn();
const onAction = vi.fn();
mount(
<ComposerTrackPill testID="pill" label="3 subagents" panelTitle="Subagents">
<ComposerTrackRow accessibilityLabel="Subagent one" testID="row" onPress={onPress}>
<Text>Subagent one</Text>
<Pressable accessibilityRole="button" testID="row-archive" onPress={onAction}>
<Text>Archive</Text>
</Pressable>
</ComposerTrackRow>
</ComposerTrackPill>,
);
openPanel();
click(row("row-archive") as Element);
expect(onAction).toHaveBeenCalledTimes(1);
expect(onPress).not.toHaveBeenCalled();
expect(isPanelOpen()).toBe(true);
});
});
describe("composer track pill status mark", () => {
function mountMark(statusBucket: "running" | "failed"): HTMLElement {
const container = mount(
<ComposerTrackPill
testID="pill"
label="3 subagents"
panelTitle="Subagents"
statusBucket={statusBucket}
>
<Text>rows</Text>
</ComposerTrackPill>,
);
const mark = container.querySelector('[data-testid="pill"] > div');
if (!(mark instanceof HTMLElement)) {
throw new Error("pill did not render a status mark");
}
return mark;
}
it("spins the shared ring while a child is running", () => {
const mark = mountMark("running");
const animated = [...mark.querySelectorAll("*")].filter(
(element) => element.getAnimations().length > 0,
);
expect(animated).toHaveLength(1);
// The rotation is on the carrier; the quarter arc it turns is the coloured top border inside.
const arc = animated[0]?.firstElementChild as HTMLElement;
const arcStyle = getComputedStyle(arc);
expect(arcStyle.borderTopColor).toBe("rgb(38, 138, 224)");
expect(arcStyle.borderLeftColor).toBe("rgba(0, 0, 0, 0)");
});
it("draws a still dot for every other state", () => {
const mark = mountMark("failed");
const animated = [...mark.querySelectorAll("*")].filter(
(element) => element.getAnimations().length > 0,
);
expect(animated).toHaveLength(0);
const dot = mark.firstElementChild as HTMLElement;
expect(getComputedStyle(dot).backgroundColor).toBe("rgb(241, 46, 47)");
});
it("reserves one slot whatever the state, so the label never steps sideways", () => {
const running = mountMark("running").getBoundingClientRect().width;
const failed = mountMark("failed").getBoundingClientRect().width;
expect(running).toBe(failed);
});
});
packages/app/src/composer/tracks.test.tsx
+3
View File
@@ -12,6 +12,9 @@ vi.mock("react-native", () => ({
},
Pressable: "button",
Text: "span",
// The status ring reaches for plain React Native styles rather than Unistyles ones — see the
// note on `rotatorStyles` in status-ring/frame.tsx.
StyleSheet: { create: <T,>(styles: T): T => styles },
View: ({
children,
style,
packages/app/src/composer/tracks.tsx
+99 -72
View File
@@ -8,8 +8,11 @@ import {
useMenuContext,
type MenuTriggerState,
} from "@/components/ui/menu";
import { StatusRing } from "@/components/status-ring";
import { STATUS_RING_FRAME_SIZE } from "@/components/status-ring/geometry";
import { MAX_CONTENT_WIDTH } from "@/constants/layout";
import { isWeb } from "@/constants/platform";
import { getStatusDotColor } from "@/utils/status-dot-color";
import { STATUS_INDICATOR_FILLED_DOT_SIZE } from "@/utils/status-indicator-geometry";
import type { SidebarStateBucket } from "@/utils/sidebar-agent-state";
import { COMPOSER_PILL_CLEARANCE, composerPillStyles } from "./pill-styles";
@@ -43,7 +46,7 @@ export interface ComposerTrackPillProps {
panelTitle: string;
testID: string;
accessibilityLabel?: string;
/** Drawn as a leading dot. `null` leaves the pill text-only. */
/** Drawn as a leading mark — a dot, or the running ring. `null` leaves the pill text-only. */
statusBucket?: SidebarStateBucket | null;
/** Panel body. Rendered into a popover on wide screens and a sheet on compact ones. */
children: ReactNode;
@@ -135,7 +138,7 @@ function ComposerTrackPillTrigger({
{...ariaExpandedProps}
style={pillStyle}
>
<ComposerTrackDot bucket={statusBucket} />
<ComposerTrackMark bucket={statusBucket} />
<Text style={labelStyle} numberOfLines={1}>
{label}
</Text>
@@ -148,6 +151,12 @@ export interface ComposerTrackRowProps {
children: ReactNode | ((state: { active: boolean }) => ReactNode);
/** Rows that open something are pressable and fill on press or hover. A read-only row is not. */
onPress?: () => void;
/**
* Dismiss the panel when this row is chosen. Same name, same default as `MenuItem`, because it
* is the same decision: a row that navigates away is done with the panel, a row whose result
* lands inside the panel is not.
*/
closeOnSelect?: boolean;
disabled?: boolean;
accessibilityLabel?: string;
testID?: string;
@@ -160,17 +169,27 @@ export interface ComposerTrackRowProps {
*
* Hover lives on the outer plain View and press on the inner Pressable, per docs/hover.md: rows
* carry action buttons, and a Pressable tracking its own hover fights every Pressable inside it.
*
* A press is a menu selection, not a bare callback: the panel is a menu surface, so dismissal —
* and on iOS the wait for UIKit to finish tearing the sheet down before the action runs — belongs
* to the engine that opened it. The row only decides whether choosing it ends the panel.
*/
export function ComposerTrackRow({
children,
onPress,
closeOnSelect = true,
disabled = false,
accessibilityLabel,
testID,
}: ComposerTrackRowProps): ReactElement {
const { selectItem } = useMenuContext("ComposerTrackRow");
const [hovered, setHovered] = useState(false);
const handlePointerEnter = useCallback(() => setHovered(true), []);
const handlePointerLeave = useCallback(() => setHovered(false), []);
const handleSelect = useCallback(
() => selectItem(onPress, closeOnSelect),
[closeOnSelect, onPress, selectItem],
);
const renderRow = useCallback(
(active: boolean) => (
@@ -196,7 +215,7 @@ export function ComposerTrackRow({
accessibilityLabel={accessibilityLabel}
testID={testID}
disabled={disabled}
onPress={onPress}
onPress={handleSelect}
>
{renderPressed}
</Pressable>
@@ -204,23 +223,27 @@ export function ComposerTrackRow({
);
}
function ComposerTrackDot({ bucket }: { bucket: SidebarStateBucket | null }): ReactElement | null {
const colorStyle = bucket ? dotColorStyle(bucket) : null;
const dotStyle = useMemo(() => [styles.dot, colorStyle], [colorStyle]);
if (!colorStyle) {
/**
* The pill's leading state mark. Running is the ring every other running indicator in the app
* uses; the rest are the dot it grows from.
*/
function ComposerTrackMark({ bucket }: { bucket: SidebarStateBucket | null }): ReactElement | null {
if (!bucket) {
return null;
}
return <View style={dotStyle} />;
return (
<View style={styles.mark}>
{bucket === "running" ? <StatusRing /> : <View style={dotColorStyle(bucket)} />}
</View>
);
}
function dotColorStyle(bucket: SidebarStateBucket) {
function dotColorStyle(bucket: Exclude<SidebarStateBucket, "running">) {
switch (bucket) {
case "needs_input":
return styles.dotNeedsInput;
case "failed":
return styles.dotFailed;
case "running":
return styles.dotRunning;
case "attention":
return styles.dotAttention;
case "done":
@@ -228,67 +251,71 @@ function dotColorStyle(bucket: SidebarStateBucket) {
}
}
const styles = StyleSheet.create((theme) => ({
bar: {
position: "absolute",
left: 0,
right: 0,
bottom: 0,
alignItems: "center",
paddingHorizontal: theme.spacing[4],
paddingBottom: {
xs: COMPOSER_PILL_CLEARANCE.compact,
md: COMPOSER_PILL_CLEARANCE.wide,
},
},
track: {
width: "100%",
maxWidth: MAX_CONTENT_WIDTH,
flexDirection: "row",
alignItems: "center",
gap: theme.spacing[1],
},
// The rail every panel row sits on: inset from the panel edge so the fill is a rounded block
// inside it, and tall enough that revealing an action button cannot resize the row.
row: {
flexDirection: "row",
alignItems: "center",
gap: theme.spacing[2],
minHeight: 32,
marginHorizontal: theme.spacing[1],
paddingHorizontal: theme.spacing[2],
paddingVertical: theme.spacing[1],
borderRadius: theme.borderRadius.md,
},
rowActive: {
flexDirection: "row",
alignItems: "center",
gap: theme.spacing[2],
minHeight: 32,
marginHorizontal: theme.spacing[1],
paddingHorizontal: theme.spacing[2],
paddingVertical: theme.spacing[1],
borderRadius: theme.borderRadius.md,
backgroundColor: theme.colors.surface2,
},
dot: {
const styles = StyleSheet.create((theme) => {
// Colours come from the one bucket-to-colour map so the pill cannot drift from the status dots
// everywhere else, and are baked into each variant so the style prop stays a stable object.
const statusDot = (bucket: Exclude<SidebarStateBucket, "running">) => ({
width: STATUS_INDICATOR_FILLED_DOT_SIZE,
height: STATUS_INDICATOR_FILLED_DOT_SIZE,
borderRadius: theme.borderRadius.full,
},
dotNeedsInput: {
backgroundColor: theme.colors.statusDotWarning,
},
dotFailed: {
backgroundColor: theme.colors.statusDotDanger,
},
dotRunning: {
backgroundColor: theme.colors.statusDotRunning,
},
dotAttention: {
backgroundColor: theme.colors.statusDotSuccess,
},
dotDone: {
backgroundColor: theme.colors.border,
},
}));
backgroundColor: getStatusDotColor({ theme, bucket, showDoneAsInactive: true }) ?? undefined,
});
return {
bar: {
position: "absolute",
left: 0,
right: 0,
bottom: 0,
alignItems: "center",
paddingHorizontal: theme.spacing[4],
paddingBottom: {
xs: COMPOSER_PILL_CLEARANCE.compact,
md: COMPOSER_PILL_CLEARANCE.wide,
},
},
track: {
width: "100%",
maxWidth: MAX_CONTENT_WIDTH,
flexDirection: "row",
alignItems: "center",
gap: theme.spacing[1],
},
// The rail every panel row sits on: inset from the panel edge so the fill is a rounded block
// inside it, and tall enough that revealing an action button cannot resize the row.
row: {
flexDirection: "row",
alignItems: "center",
gap: theme.spacing[2],
minHeight: 32,
marginHorizontal: theme.spacing[1],
paddingHorizontal: theme.spacing[2],
paddingVertical: theme.spacing[1],
borderRadius: theme.borderRadius.md,
},
rowActive: {
flexDirection: "row",
alignItems: "center",
gap: theme.spacing[2],
minHeight: 32,
marginHorizontal: theme.spacing[1],
paddingHorizontal: theme.spacing[2],
paddingVertical: theme.spacing[1],
borderRadius: theme.borderRadius.md,
backgroundColor: theme.colors.surface2,
},
// Sized for the widest mark that can land in it, which is the ring rather than the dot. The
// slot is the same width whatever the state, so the label does not step sideways when the last
// child stops running.
mark: {
width: STATUS_RING_FRAME_SIZE,
height: STATUS_RING_FRAME_SIZE,
alignItems: "center",
justifyContent: "center",
},
dotNeedsInput: statusDot("needs_input"),
dotFailed: statusDot("failed"),
dotAttention: statusDot("attention"),
dotDone: statusDot("done"),
};
});
packages/app/src/subagents/track.tsx
+3
View File
@@ -155,6 +155,9 @@ function ArchiveFinishedRow({
accessibilityLabel={t("subagents.archiveFinishedAction")}
testID="subagents-track-archive-finished"
disabled={disabled}
// Progress and the retry count land on this row, so the panel is where the result of
// pressing it shows up. Dismissing would hide the thing the press produces.
closeOnSelect={false}
onPress={onPress}
>
{renderRow}
packages/app/test-stubs/gorhom-bottom-sheet.ts
+13
View File
@@ -0,0 +1,13 @@
// The package ships untranspiled Flow, which the unit project cannot parse. The sheet is a native
// presentation; tests that reach a menu do so through the popover, so rendering nothing is enough
// to keep the module graph importable.
const Stub = () => null;
const PassThrough = ({ children }: { children?: unknown }) => children;
export default Stub;
export const BottomSheetModal = Stub;
export const BottomSheetModalProvider = PassThrough;
export const BottomSheetBackdrop = Stub;
export const BottomSheetScrollView = PassThrough;
export const BottomSheetView = PassThrough;
export const BottomSheetTextInput = Stub;
packages/app/test-stubs/lucide-react-native.ts
+1
View File
@@ -22,6 +22,7 @@ export const Blocks = StubIcon;
export const Bot = StubIcon;
export const Brain = StubIcon;
export const Check = StubIcon;
export const CheckCircle = StubIcon;
export const CheckCircle2 = StubIcon;
export const ChevronDown = StubIcon;
export const ChevronLeft = StubIcon;
packages/app/test-stubs/react-native-fabric-shim.ts
+8
View File
@@ -0,0 +1,8 @@
// Reanimated's native code path imports `react-native/Libraries/Renderer/shims/ReactFabric`.
// The `react-native` alias below points at a react-native-web file, so the subpath cannot
// resolve; on web reanimated takes `findHostInstance.web.js` and never reaches this.
export function findHostInstance_DEPRECATED(): null {
return null;
}
export default { findHostInstance_DEPRECATED };
packages/app/test-stubs/react-native-safe-area-context.ts
+12
View File
@@ -0,0 +1,12 @@
// The package ships untranspiled Flow, which the unit project cannot parse. Insets are a device
// fact no test asserts on, so they are zero.
const ZERO_INSETS = { top: 0, right: 0, bottom: 0, left: 0 } as const;
export const useSafeAreaInsets = () => ZERO_INSETS;
export const useSafeAreaFrame = () => ({ x: 0, y: 0, width: 0, height: 0 });
export const SafeAreaProvider = ({ children }: { children?: unknown }) => children;
export const SafeAreaView = ({ children }: { children?: unknown }) => children;
export const initialWindowMetrics = {
insets: ZERO_INSETS,
frame: { x: 0, y: 0, width: 0, height: 0 },
};
packages/app/test-stubs/react-native-unistyles.ts
+27 -1
View File
@@ -7,6 +7,11 @@ const testTheme = {
statusDanger: "#b91c1c",
statusWarning: "#d97706",
statusMerged: "#7c3aed",
// The light band's values, so a test can name the colour it expects.
statusDotSuccess: "#299f51",
statusDotDanger: "#f12e2f",
statusDotWarning: "#b37824",
statusDotRunning: "#268ae0",
accent: "#2563eb",
accentForeground: "#ffffff",
destructive: "#dc2626",
@@ -19,6 +24,7 @@ const testTheme = {
palette: {
amber: { 500: "#f59e0b" },
blue: { 300: "#93c5fd" },
green: { 500: "#22c55e" },
red: { 300: "#fca5a5" },
white: "#ffffff",
},
@@ -45,8 +51,28 @@ const testTheme = {
xl: 12,
full: 9999,
},
iconSize: { sm: 16 },
iconSize: { sm: 16, md: 20 },
opacity: { 50: 0.5 },
shadow: {
sm: {
shadowColor: "rgba(0, 0, 0, 0.02)",
shadowOffset: { width: 0, height: 2 },
shadowRadius: 8,
elevation: 2,
},
md: {
shadowColor: "rgba(0, 0, 0, 0.04)",
shadowOffset: { width: 0, height: 4 },
shadowRadius: 16,
elevation: 4,
},
lg: {
shadowColor: "rgba(0, 0, 0, 0.08)",
shadowOffset: { width: 0, height: 8 },
shadowRadius: 24,
elevation: 8,
},
},
};
type StyleFactory<T> = (theme: typeof testTheme) => T;
packages/app/test-stubs/reanimated-validate-worklets-version.ts
+3
View File
@@ -0,0 +1,3 @@
// Reanimated imports this CJS script with a default import. Unbundled, the browser cannot interop
// it, and the check it performs is a native-build concern with nothing to say in a browser test.
export default function validateWorkletsVersion(): void {}
packages/app/vitest.config.ts
+35
View File
@@ -60,6 +60,20 @@ export default defineConfig({
},
},
},
// Reanimated ships one file per platform and picks between them by extension
// (`findHostInstance.web.js`). Vite's dependency optimizer does not apply `resolve.extensions`,
// so it scans the native files and dies on imports react-native-web has no answer for.
// Unbundled, the same imports go through the resolver below and land on the web files.
optimizeDeps: {
exclude: ["react-native-reanimated"],
},
// The globals a React Native bundler defines, which esbuild is no longer there to supply for
// the package excluded above.
define: {
"process.env.JEST_WORKER_ID": "undefined",
__DEV__: "false",
global: "globalThis",
},
resolve: {
extensions: [
".web.mjs",
@@ -86,6 +100,13 @@ export default defineConfig({
replacement: path.resolve(__dirname, "../relay/src/index.ts"),
},
{ find: "@", replacement: path.resolve(__dirname, "src") },
// Must precede the `react-native` alias: a string `find` matches by prefix, so this subpath
// would otherwise resolve inside a react-native-web *file* and break the dependency scan.
// Reanimated only imports it on the native path, which no test takes.
{
find: /^react-native\/Libraries\/Renderer\/shims\/ReactFabric$/,
replacement: path.resolve(__dirname, "test-stubs/react-native-fabric-shim.ts"),
},
// Point to the ESM build so Vite can transform its imports and apply the
// react alias below (the CJS build uses require('react') which bypasses
// Vite alias resolution).
@@ -114,6 +135,20 @@ export default defineConfig({
find: /^react-native-svg$/,
replacement: path.resolve(__dirname, "test-stubs/react-native-svg.ts"),
},
// Both ship untranspiled Flow and fail to parse on import, which takes out any test that
// mounts a menu surface.
{
find: /^react-native-safe-area-context$/,
replacement: path.resolve(__dirname, "test-stubs/react-native-safe-area-context.ts"),
},
{
find: /^@gorhom\/bottom-sheet$/,
replacement: path.resolve(__dirname, "test-stubs/gorhom-bottom-sheet.ts"),
},
{
find: /^react-native-reanimated\/scripts\/validate-worklets-version$/,
replacement: path.resolve(__dirname, "test-stubs/reanimated-validate-worklets-version.ts"),
},
{
find: /^expo-linking$/,
replacement: path.resolve(__dirname, "test-stubs/expo-linking.ts"),