diff --git a/apps/hosted/web/src/implementation/pages/app-accounts.tsx b/apps/hosted/web/src/implementation/pages/app-accounts.tsx index 3cd58bb9b3..c0d2f35c69 100644 --- a/apps/hosted/web/src/implementation/pages/app-accounts.tsx +++ b/apps/hosted/web/src/implementation/pages/app-accounts.tsx @@ -156,9 +156,11 @@ export function AppAccounts({ ? { pending, choose: (slot, value) => void chooser.choose(slot, value) } : undefined } - removeAccountAction={(slot, account, label) => { + removeAccountAction={(slot, account, label, bound) => { if (!canUse || profile === undefined) return null; const saved = accounts.find((item) => item.id === account); + // An unselected account offers only its own actions; with none, it has no menu. + if (!bound && saved === undefined) return null; const requirement = app.requirements.accounts[slot]; return ( void unused.check(removed)} > {saved && ( <> - setRenaming(saved.id)}>Rename + setRenaming(saved.id)}> + Edit details + {editable && requirement !== undefined && ( reconnect(slot, saved)}> {requirement.definition.auth[saved.method]?.type === "oauth2" diff --git a/apps/local/web/src/implementation/pages/app-accounts.tsx b/apps/local/web/src/implementation/pages/app-accounts.tsx index ef2cf177bd..5977142201 100644 --- a/apps/local/web/src/implementation/pages/app-accounts.tsx +++ b/apps/local/web/src/implementation/pages/app-accounts.tsx @@ -158,9 +158,11 @@ export function AppAccounts({ pending: chooser.pending, choose: (slot, value) => void chooser.choose(slot, value), }} - removeAccountAction={(slot, account, label) => { + removeAccountAction={(slot, account, label, bound) => { if (profile === undefined) return null; const saved = accounts.find((item) => item.id === account); + // An unselected account offers only its own actions; with none, it has no menu. + if (!bound && saved === undefined) return null; const requirement = app.requirements.accounts[slot]; return ( void unused.check(removed)} > {saved && ( <> - setRenaming(saved.id)}>Rename + setRenaming(saved.id)}> + Edit details + {requirement !== undefined && ( replace(slot, saved.id)}> {requirement.definition.auth[saved.method]?.type === "oauth2" diff --git a/e2e/tests/local-account-naming.spec.ts b/e2e/tests/local-account-naming.spec.ts index 1dac309c81..0ab0a7f288 100644 --- a/e2e/tests/local-account-naming.spec.ts +++ b/e2e/tests/local-account-naming.spec.ts @@ -232,6 +232,83 @@ export default defineApp({ accounts: { service } }, async () => ({ tools: router expect(yield* savedLabel(fromDialog.id)).toBe("Dialog key"); yield* browser.checkpoint("Named account selected for the app"); + // Every account row keeps its menu in view, not only the selected one or on hover. + expect( + yield* browser.use("Unselected accounts show their menu without hover", (page) => + page.mouse + .move(0, 0) + .then(() => + page + .getByRole("button", { name: "Manage Supplied name", exact: true }) + .evaluate((element) => getComputedStyle(element).opacity), + ), + ), + ).toBe("1"); + expect( + yield* browser.use("An unselected account's menu cannot remove it", (page) => + page + .getByRole("button", { name: "Manage Supplied name", exact: true }) + .click() + .then(() => + page.getByRole("menuitem", { name: "Edit details", exact: true }).waitFor(), + ) + // Let the menu finish opening so the checkpoint shows it, not its fade-in. + .then(() => + page + .getByRole("menu") + .evaluate((menu) => + Promise.all(menu.getAnimations().map((animation) => animation.finished)), + ), + ) + .then(() => page.getByRole("menuitem").allTextContents()), + ), + ).toEqual(["Edit details", "Update credentials"]); + yield* browser.checkpoint("Unselected account menu"); + yield* browser.use("Close the unselected account's menu", (page) => + page.keyboard.press("Escape"), + ); + + // An unselected account's menu opens the same dialogs as a selected one. + const edit = yield* browser.use("Edit an unselected account's details", (page) => + page + .getByRole("button", { name: "Manage Supplied name", exact: true }) + .click() + .then(() => page.getByRole("menuitem", { name: "Edit details", exact: true }).click()) + .then(() => page.getByRole("dialog", { name: "Edit account", exact: true })) + .then((dialog) => dialog.waitFor({ state: "visible" }).then(() => dialog)), + ); + yield* browser.checkpoint("Edit details from an unselected account"); + yield* browser.use("Cancel editing the details", () => + edit + .getByRole("button", { name: "Cancel", exact: true }) + .click() + .then(() => edit.waitFor({ state: "hidden" })), + ); + const unselectedCredentials = yield* browser.use( + "Update an unselected account's credentials", + (page) => + page + .getByRole("button", { name: "Manage Supplied name", exact: true }) + .click() + .then(() => + page.getByRole("menuitem", { name: "Update credentials", exact: true }).click(), + ) + .then(() => page.getByRole("dialog", { name: "Update credentials", exact: true })) + .then((dialog) => + dialog + .getByLabel("Token", { exact: true }) + .waitFor({ state: "visible" }) + .then(() => dialog), + ), + ); + yield* browser.checkpoint("Update credentials from an unselected account"); + yield* browser.use("Close the credentials dialog", () => + unselectedCredentials + .getByRole("button", { name: "Close", exact: true }) + .click() + .then(() => unselectedCredentials.waitFor({ state: "hidden" })), + ); + // Credentials are replaced from the app that selects the account, through its menu. yield* browser.use("Update the named account's credentials", (page) => page diff --git a/packages/ui/src/implementation/dashboard/app-accounts.tsx b/packages/ui/src/implementation/dashboard/app-accounts.tsx index cb843db938..228f48c159 100644 --- a/packages/ui/src/implementation/dashboard/app-accounts.tsx +++ b/packages/ui/src/implementation/dashboard/app-accounts.tsx @@ -78,6 +78,8 @@ type AccountBindingProps = { readonly slot: string; readonly account: AccountId; readonly label: string; + /** Whether this profile selects the account; only a selected account can be removed from it. */ + readonly bound: boolean; readonly update: Atom.AtomResultFn< Omit, Profile, @@ -122,12 +124,9 @@ function useRemoveAccountBinding({ }; } -const bindingActionClass = - "shrink-0 text-muted-foreground [@media(hover:hover)]:opacity-0 group-hover/account:opacity-100 group-focus-within/account:opacity-100 focus-visible:opacity-100 data-loading:opacity-100"; - /** - * A selected account's actions in this app: the host's items, then removal from the profile. - * Removal keeps the reusable account and every other provider selection. + * An account's actions in this app: the host's items, then removal from the profile when the + * profile selects it. Removal keeps the reusable account and every other provider selection. */ export function AccountBindingMenu({ children, @@ -143,7 +142,7 @@ export function AccountBindingMenu({ variant="ghost" size="icon-xs" aria-label={`Manage ${props.label}`} - className={`${bindingActionClass} hover:text-foreground data-[state=open]:opacity-100`} + className="shrink-0 text-muted-foreground hover:text-foreground" loading={removal.pending} > @@ -151,14 +150,16 @@ export function AccountBindingMenu({ {children} - {children && } - void removal.remove()} - > - Remove - + {children && props.bound && } + {props.bound && ( + void removal.remove()} + > + Remove + + )} {removal.error && ( @@ -362,7 +363,12 @@ export function AppAccounts({ readonly chooseAction?: ReactNode; readonly reconnectAction?: (account: AccountSummary) => ReactNode; readonly accountActions?: (slot: string, requirement: AccountRequirement) => ReactNode; - readonly removeAccountAction?: (slot: string, account: AccountId, label: string) => ReactNode; + readonly removeAccountAction?: ( + slot: string, + account: AccountId, + label: string, + bound: boolean, + ) => ReactNode; readonly onCreateProfile?: (() => void) | undefined; /** Check one account again; its result reaches `accounts` through the host's queries. */ readonly revalidate?: ((account: AccountId) => Promise) | undefined; @@ -501,8 +507,7 @@ export function AppAccounts({ )} {status} {/* Unchecking only unselects; removal is its own action and may offer deletion. */} - {bound && - removeAccountAction?.(slot, id, label ?? "Account disconnected")} + {removeAccountAction?.(slot, id, label ?? "Account disconnected", bound)} ); })}