diff --git a/apps/web/src/components/sidebar/SidebarUpdatePill.browser.test.tsx b/apps/web/src/components/sidebar/SidebarUpdatePill.browser.test.tsx index 0f751bc52aef6..d908ee2b385a5 100644 --- a/apps/web/src/components/sidebar/SidebarUpdatePill.browser.test.tsx +++ b/apps/web/src/components/sidebar/SidebarUpdatePill.browser.test.tsx @@ -11,8 +11,15 @@ const mocks = vi.hoisted(() => { const listeners = new Set<() => void>(); return { update: null as DesktopUpdateState | null, - hasUpdates: false, - rows: [] as ReadonlyArray, + isElectron: true, + everything: { + hasUpdates: false, + summary: "Update everything", + version: null, + providerCount: 0, + rows: [] as ReadonlyArray, + restarts: [], + }, subscribe: (listener: () => void) => { listeners.add(listener); return () => listeners.delete(listener); @@ -30,20 +37,18 @@ vi.mock("../../state/desktopUpdate", async (importOriginal) => ({ () => mocks.update, ), })); -// The pill only renders in the desktop app. vi.mock("../../env", async (importOriginal) => ({ ...(await importOriginal()), - isElectron: true, + get isElectron() { + return mocks.isElectron; + }, })); -vi.mock("./useUpdateEverything", () => ({ - useUpdateEverything: () => ({ - hasUpdates: mocks.hasUpdates, - summary: "Update everything", - version: null, - providerCount: 0, - rows: mocks.rows, - restarts: [], - }), +vi.mock("./useUpdateEverything", async () => ({ + useUpdateEverything: (await import("react")).useSyncExternalStore.bind( + null, + mocks.subscribe, + () => mocks.everything, + ), })); function nightlyUpdate(releaseNotes: DesktopUpdateState["releaseNotes"]): DesktopUpdateState { @@ -76,11 +81,12 @@ function setup( { hasUpdates = false, rows = [], - }: { hasUpdates?: boolean; rows?: ReadonlyArray } = {}, + isElectron = true, + }: { hasUpdates?: boolean; rows?: ReadonlyArray; isElectron?: boolean } = {}, ) { mocks.update = update; - mocks.hasUpdates = hasUpdates; - mocks.rows = rows; + mocks.isElectron = isElectron; + mocks.everything = { ...mocks.everything, hasUpdates, rows }; } function publish(update: DesktopUpdateState | null) { @@ -90,6 +96,14 @@ function publish(update: DesktopUpdateState | null) { }); } +const LAPTOP_CODEX: UpdateEverythingRow = { + environmentId: null, + label: "Laptop", + app: null, + skipped: null, + providers: ["Codex"], +}; + const releaseNotes = () => document.querySelector('[aria-label="Update release notes"]'); describe("SidebarUpdatePill release notes", () => { @@ -127,7 +141,7 @@ describe("SidebarUpdatePill release notes", () => { describe("SidebarUpdatePill update everything", () => { it("does not refocus the update button after a pointer cancel", async () => { - setup(null, { hasUpdates: true }); + setup(null, { hasUpdates: true, rows: [LAPTOP_CODEX] }); await using screen = await render(); const button = screen.getByRole("button", { name: "Update everything" }); await userEvent.click(button); @@ -140,7 +154,7 @@ describe("SidebarUpdatePill update everything", () => { }); it("returns focus to the update button after Escape", async () => { - setup(null, { hasUpdates: true }); + setup(null, { hasUpdates: true, rows: [LAPTOP_CODEX] }); await using screen = await render(); const button = screen.getByRole("button", { name: "Update everything" }); await userEvent.click(button); @@ -167,4 +181,23 @@ describe("SidebarUpdatePill update everything", () => { await expect.element(row.getByRole("cell", { name: label, exact: true })).toBeVisible(); await expect.element(row.getByRole("cell", { name: "Claude", exact: true })).toBeVisible(); }); + + it("keeps an open dialog on web when the updates finish and the pill hides", async () => { + setup(null, { hasUpdates: true, rows: [LAPTOP_CODEX], isElectron: false }); + await using screen = await render(); + await userEvent.click(screen.getByRole("button", { name: "Update everything" })); + await expect.element(page.getByRole("button", { name: "Update", exact: true })).toBeVisible(); + + await act(() => { + mocks.everything = { ...mocks.everything, hasUpdates: false, rows: [] }; + mocks.notify(); + }); + await expect + .element(screen.getByRole("button", { name: "Update everything" })) + .not.toBeInTheDocument(); + const dialog = page.getByRole("alertdialog"); + await expect.element(dialog.getByText("Everything is up to date")).toBeVisible(); + await userEvent.click(dialog.getByRole("button", { name: "Close" })); + await vi.waitFor(() => expect(document.querySelector('[role="alertdialog"]')).toBeNull()); + }); }); diff --git a/apps/web/src/components/sidebar/SidebarUpdatePill.tsx b/apps/web/src/components/sidebar/SidebarUpdatePill.tsx index 5df8007ef4677..6e024265e2e27 100644 --- a/apps/web/src/components/sidebar/SidebarUpdatePill.tsx +++ b/apps/web/src/components/sidebar/SidebarUpdatePill.tsx @@ -4,7 +4,7 @@ import { managedUpdatesDenial, } from "@t3tools/contracts"; import { TriangleAlertIcon } from "lucide-react"; -import { type ComponentProps, useId, useRef, useState } from "react"; +import { type ComponentProps, type ReactNode, useId, useRef, useState } from "react"; import { flushSync } from "react-dom"; import { settlePromise, squashAtomCommandFailure } from "@t3tools/client-runtime/state/runtime"; import { isElectron } from "../../env"; @@ -401,8 +401,6 @@ function SidebarUpdateControl() { ); }; - if (!isElectron && !runsEverything && !isRunningEverything) return null; - const statusIcon = ( void runEverything()} /> ); + // Every path renders the same [pill, dialog] shape, so an open dialog outlives a hidden pill. + const withDialog = (pill: ReactNode) => ( + <> + {pill} + {confirmDialog} + + ); + if (!isElectron && !runsEverything && !isRunningEverything) return withDialog(null); if (run !== null) { const local: UpdateRunLocalState = @@ -432,7 +438,7 @@ function SidebarUpdateControl() { : state?.status === "downloading" ? { status: "downloading", percent: state.downloadPercent } : { status: "waiting" }; - return ( + return withDialog( - {confirmDialog} - + , ); } @@ -507,7 +512,7 @@ function SidebarUpdateControl() { ); - return ( + return withDialog( ) : null} - {confirmDialog} - + , ); } diff --git a/apps/web/src/components/sidebar/UpdateEverythingDialog.tsx b/apps/web/src/components/sidebar/UpdateEverythingDialog.tsx index 586438bb4420e..be61cad85c191 100644 --- a/apps/web/src/components/sidebar/UpdateEverythingDialog.tsx +++ b/apps/web/src/components/sidebar/UpdateEverythingDialog.tsx @@ -127,26 +127,43 @@ function UpdateEverythingDialogContent({ const restarting = new Set(restarts.map((machine) => machine.environmentId)); const updating = rows.filter((row) => row.app !== null || row.providers.length > 0); const skipped = rows.filter((row) => row.skipped !== null); + // Updates can finish elsewhere while this is open. A track switch with no rows still saves the track. + const empty = title === undefined && updating.length === 0; return ( <> - {title ?? `Update ${updating.length} ${updating.length === 1 ? "machine" : "machines"}?`} + {empty + ? checking + ? "Checking for updates…" + : skipped.length > 0 + ? "Nothing to update right now" + : "Everything is up to date" + : (title ?? + `Update ${updating.length} ${updating.length === 1 ? "machine" : "machines"}?`)} - {version !== null ? ( + {empty ? ( + checking || skipped.length > 0 ? null : ( + "Nothing is left to update." + ) + ) : ( <> - T3 Code {version} + {version !== null ? ( + <> + T3 Code {version} + + ) : null} + {version !== null && providerCount > 0 ? " · " : null} + {providerCount > 0 + ? `${providerCount} provider ${providerCount === 1 ? "update" : "updates"}` + : null} + {checking + ? `${version !== null || providerCount > 0 ? " · " : ""}Checking for more…` + : null} - ) : null} - {version !== null && providerCount > 0 ? " · " : null} - {providerCount > 0 - ? `${providerCount} provider ${providerCount === 1 ? "update" : "updates"}` - : null} - {checking - ? `${version !== null || providerCount > 0 ? " · " : ""}Checking for more…` - : null} + )} @@ -156,55 +173,57 @@ function UpdateEverythingDialogContent({ {note}

))} - {/* Machine names truncate first. Provider lists wrap between names, never inside one. */} -
+ {empty ? null : ( + // Machine names truncate first. Provider lists wrap between names, never inside one.
- Machine - T3 Code - Providers - -
- {updating.map((row) => (
- - {row.label} - - - - {row.app !== null ? APP_LABELS[row.app] : null} - {row.app === null && row.skipped !== null ? ( - Skipped - ) : null} - {row.app === "this-app" ? ( - restarts last - ) : null} - - - {row.providers.map((provider, index) => ( - - {provider} - {index < row.providers.length - 1 ? ", " : null} - - ))} - - - {row.environmentId !== null && restarting.has(row.environmentId) ? ( - - ) : null} - + Machine + T3 Code + Providers +
- ))} -
+ {updating.map((row) => ( +
+ + {row.label} + + + + {row.app !== null ? APP_LABELS[row.app] : null} + {row.app === null && row.skipped !== null ? ( + Skipped + ) : null} + {row.app === "this-app" ? ( + restarts last + ) : null} + + + {row.providers.map((provider, index) => ( + + {provider} + {index < row.providers.length - 1 ? ", " : null} + + ))} + + + {row.environmentId !== null && restarting.has(row.environmentId) ? ( + + ) : null} + +
+ ))} + + )} {skipped.length > 0 ? ( @@ -242,11 +261,13 @@ function UpdateEverythingDialogContent({

) : null} }> - Cancel + {empty ? "Close" : "Cancel"} - + {empty ? null : ( + + )} );