diff --git a/apps/web/src/pages/insights-page.tsx b/apps/web/src/pages/insights-page.tsx index d42b793a5..e1469c038 100644 --- a/apps/web/src/pages/insights-page.tsx +++ b/apps/web/src/pages/insights-page.tsx @@ -204,6 +204,7 @@ export function InsightsRunsHistory({ loading, nextCursor, onOpenRun, + embedded = false, }: { readonly runs: readonly InsightsRun[]; readonly loading: boolean; @@ -212,14 +213,23 @@ export function InsightsRunsHistory({ * so the view says so instead of silently truncating at 100. */ readonly nextCursor: string | null; readonly onOpenRun: (id: string) => void; + /** Rendered inside the Settings dialog: crumbs carry no `href` (a link + * would navigate the host app out from under the dialog) and the host + * shell's sidebar toggle is hidden. */ + readonly embedded?: boolean; }) { const purpose = purposeRunsForInsights(runs); const groups = groupRunsByDefinition(purpose); return (
@@ -323,19 +333,37 @@ function RunFailureDetail({ export function InsightsRunDetail({ run, tenantId, + embedded = false, + onBack, }: { readonly run: InsightsRun | null; readonly tenantId: string | null; + /** Same embedded contract as `InsightsRunsHistory`: no crumb links, no + * host sidebar toggle — plus a way back, since the host URL stays put. */ + readonly embedded?: boolean; + readonly onBack?: () => void; }) { const failed = run !== null && insightsRunStatus(run) === "failed"; return (
+ Back to runs + + ) : undefined + } />
@@ -371,6 +399,7 @@ export function InsightsPage({ path, runs, tenantId = null, + embedded = false, }: { readonly path: string; readonly runs: APIQuery<{ @@ -378,14 +407,19 @@ export function InsightsPage({ nextCursor: string | null; }>; readonly tenantId?: string | null; + /** Rendered inside the Settings dialog: a run opens inline in local state + * instead of navigating the host app to a top-level route (which would + * unmount the dialog). */ + readonly embedded?: boolean; }) { const navigate = useNavigate(); const { mode, runId } = parseInsightsPath(path); + const [embeddedRunId, setEmbeddedRunId] = useState(null); if (runs.kind === "unauthenticated") { return (
- + @@ -401,12 +435,29 @@ export function InsightsPage({ return ; } + if (embedded && embeddedRunId !== null) { + const run = runsData.find((r) => r.id === embeddedRunId) ?? null; + return ( + setEmbeddedRunId(null)} + /> + ); + } + return ( navigate(`${INSIGHTS_RUNS_PATH}/${encodeURIComponent(id)}`)} + onOpenRun={ + embedded + ? (id) => setEmbeddedRunId(id) + : (id) => navigate(`${INSIGHTS_RUNS_PATH}/${encodeURIComponent(id)}`) + } + embedded={embedded} /> ); } @@ -478,7 +529,13 @@ function InsightsLandingRedirect({ benchTenantId }: { readonly benchTenantId: st return ; } -export function InsightsRoute({ path }: { readonly path?: string }) { +export function InsightsRoute({ + path, + embedded = false, +}: { + readonly path?: string; + readonly embedded?: boolean; +}) { const { selectedTenantId: benchTenantId } = useBench(); // A run opened from a workbench's Insights carries `from=`, so it reads // that workbench's tenant rather than the bench. @@ -515,7 +572,14 @@ export function InsightsRoute({ path }: { readonly path?: string }) { nextCursor: string | null; }> = selectedTenantId === null ? { kind: "ready", data: { data: [], nextCursor: null } } : runs; - return ; + return ( + + ); } /** Resolves the workbench-scoped route's own workbench list. */ diff --git a/apps/web/src/settings-groups.tsx b/apps/web/src/settings-groups.tsx index b68cf924d..ed5480689 100644 --- a/apps/web/src/settings-groups.tsx +++ b/apps/web/src/settings-groups.tsx @@ -6,6 +6,7 @@ import { insertEveryoneSections, resolveSettingsSectionGroups } from "@/settings import type { SettingsSectionGroup, TenancyAccess } from "@/settings"; import { InsightsRoute } from "./pages/insights-page"; +import { SETTINGS_INSIGHTS_PATH } from "./settings-insights-path"; export function resolveAppSettingsSectionGroups( access: TenancyAccess, @@ -16,7 +17,7 @@ export function resolveAppSettingsSectionGroups( id: "insights", title: "Insights", icon: ChartBar, - render: () => , + render: () => , }, ]); } diff --git a/apps/web/src/settings-insights-path.test.ts b/apps/web/src/settings-insights-path.test.ts new file mode 100644 index 000000000..3955bf02c --- /dev/null +++ b/apps/web/src/settings-insights-path.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, test } from "bun:test"; + +import { parseInsightsPath } from "./insights-path"; +import { SETTINGS_INSIGHTS_PATH } from "./settings-insights-path"; +import type { TenancyAccess } from "./settings/access"; + +describe("settings-hosted insights path", () => { + test("renders the runs history inline instead of landing-redirecting away", () => { + expect(parseInsightsPath(SETTINGS_INSIGHTS_PATH).mode).toBe("runs"); + }); + + test("the settings URL itself is an insights landing page, so the dialog must never hand it to the route", () => { + expect(parseInsightsPath("/settings/insights").mode).toBe("landing"); + }); + + test("the embedded insights section gets an explicit non-landing path, never the host URL fallback", async () => { + // The section's module chain reads `window.location` at import time, and + // bun's runner has no DOM — stub just enough of it to evaluate, then + // inspect the returned element's props instead of mounting. + Object.assign(globalThis, { + window: { + location: { pathname: "/settings/insights" }, + history: { replaceState: () => {} }, + addEventListener: () => {}, + removeEventListener: () => {}, + }, + }); + const { resolveAppSettingsSectionGroups } = await import("./settings-groups"); + const access: TenancyAccess = { + people: "allowed", + roles: "allowed", + grants: "allowed", + credentials: "allowed", + }; + const sections = resolveAppSettingsSectionGroups(access).flatMap((group) => group.sections); + const insights = sections.find((section) => section.id === "insights"); + if (insights === undefined) throw new Error("insights section missing from settings groups"); + const { path, embedded } = insights.render({ tenantId: null, principalId: null }).props as { + readonly path?: unknown; + readonly embedded?: unknown; + }; + if (typeof path !== "string") { + throw new Error("insights section must hand the embedded route an explicit path"); + } + expect(parseInsightsPath(path).mode).not.toBe("landing"); + // The embedded route must suppress its full-page chrome (crumb links, + // run-row top-level navigation, sidebar toggle), or clicking any of them + // navigates the host app out from under the dialog. + expect(embedded).toBe(true); + }); +}); diff --git a/apps/web/src/settings-insights-path.ts b/apps/web/src/settings-insights-path.ts new file mode 100644 index 000000000..976bd4750 --- /dev/null +++ b/apps/web/src/settings-insights-path.ts @@ -0,0 +1,9 @@ +// The path the Settings dialog hands its embedded Insights route. The route +// falls back to the browser URL when it gets no path, and the dialog's own +// `/settings/insights` parses as an Insights landing page — landing renders a +// redirect to a top-level route, which unmounts the dialog. Hand it the runs +// history instead: that mode renders inline and never navigates away. + +import { INSIGHTS_RUNS_PATH } from "./path-ids"; + +export const SETTINGS_INSIGHTS_PATH = INSIGHTS_RUNS_PATH; diff --git a/apps/web/src/shell/stage-top-bar.tsx b/apps/web/src/shell/stage-top-bar.tsx index 754ece23c..a08525a31 100644 --- a/apps/web/src/shell/stage-top-bar.tsx +++ b/apps/web/src/shell/stage-top-bar.tsx @@ -20,6 +20,7 @@ export function StageTopBar({ chip, filter, actions, + hideSidebarToggle = false, }: { /** The page's title trail: parents first, the page itself last. */ readonly crumbs: readonly StageCrumb[]; @@ -33,11 +34,14 @@ export function StageTopBar({ readonly filter?: StageSearchProps; /** The primary-action slot: the buttons and inputs this page owns. */ readonly actions?: ReactNode; + /** Views embedded in a dialog (Settings' Insights section) own no sidebar, + * so the host shell's toggle would flip the page behind the dialog. */ + readonly hideSidebarToggle?: boolean; }) { const hasSubtitle = subtitle !== undefined && subtitle !== null; return (
- + {hideSidebarToggle ? null : }
diff --git a/e2e/browser/settings-dialog.test.ts b/e2e/browser/settings-dialog.test.ts new file mode 100644 index 000000000..3d4f37b17 --- /dev/null +++ b/e2e/browser/settings-dialog.test.ts @@ -0,0 +1,64 @@ +// The Settings dialog keeps its Insights section inline: clicking the +// section leaves the dialog mounted with the runs panel (no top-level +// navigation unmounting the dialog), and Close dismisses it. +import { expect, test } from "bun:test"; +import { bootBrowserApp, browserGate } from "../lib/browser"; +import { clickText, runFirstRunFlow, STEP_TIMEOUT, waitForText } from "../lib/first-run"; + +const describeBrowser = browserGate(import.meta.path); + +// Reads the dialog's embedded chrome: any crumb link out to /insights or a +// host sidebar toggle would navigate (or flip) the page behind the dialog. +const EMBEDDED_CHROME_SCRIPT = `(() => { + const dialog = document.querySelector(".settings-dialog"); + if (dialog === null) return null; + return { + crumbLink: dialog.querySelector('a[href^="/insights"]')?.getAttribute("href") ?? null, + sidebarToggle: dialog.querySelector(".stage-sidebar-toggle") !== null, + }; +})()`; + +describeBrowser("settings dialog", () => { + const app = bootBrowserApp(); + + test("the Insights section renders runs inline and Close dismisses", async () => { + const { page, errors } = await app().newPage(); + try { + await runFirstRunFlow(page, app().origin); + + // Open Settings over the workbench page; a cold load falls back to a + // backdrop page and mounts the dialog on top. + await page.goto(`${app().origin}/settings`, { waitUntil: "networkidle0" }); + await page.waitForSelector(".settings-dialog", { timeout: STEP_TIMEOUT }); + await waitForText(page, "Settings"); + + // Click the Insights section: the section button must not close the + // dialog (the regression), so the dialog stays mounted at /settings + // with the runs panel rendered inline. + await clickText(page, ".settings-nav button", "Insights"); + await page.waitForFunction( + `location.pathname.startsWith("/settings") && document.querySelector(".settings-dialog") !== null && document.body.innerText.includes("Run history")`, + { timeout: STEP_TIMEOUT }, + ); + + const chrome = (await page.evaluate(EMBEDDED_CHROME_SCRIPT)) as { + crumbLink: string | null; + sidebarToggle: boolean; + } | null; + expect(chrome).toEqual({ crumbLink: null, sidebarToggle: false }); + + // Close dismisses the dialog back to the page behind it. + await page.click('.settings-dialog [aria-label="Close"]'); + await page.waitForFunction(`document.querySelector(".settings-dialog") === null`, { + timeout: STEP_TIMEOUT, + }); + expect(new URL(page.url()).pathname.startsWith("/settings")).toBe(false); + } catch (cause) { + process.stderr.write( + `settings-dialog failed at ${page.url()}\nconsole: ${errors.join(" | ")}\n`, + ); + throw cause; + } + await page.close(); + }, 240_000); +});