diff --git a/devlog/_plan/260905_admin_token_local_ux/assets/admin-token-dialog-before-after.png b/devlog/_plan/260905_admin_token_local_ux/assets/admin-token-dialog-before-after.png new file mode 100644 index 0000000000..8a81f43c8d Binary files /dev/null and b/devlog/_plan/260905_admin_token_local_ux/assets/admin-token-dialog-before-after.png differ diff --git a/gui/src/admin-token-dialog.ts b/gui/src/admin-token-dialog.ts index af3cdc80e0..f02fed97fb 100644 --- a/gui/src/admin-token-dialog.ts +++ b/gui/src/admin-token-dialog.ts @@ -94,6 +94,18 @@ export function promptForAdminToken( form.append(heading, accountField, tokenField, validationError, actions); dialog.append(form); + /* + * #3483: the notice must carry no text while it is hidden. + * + * The element is mounted up front so `role="alert"` has a stable target, and the CSS + * now scopes `.notice`'s `display` to `:not([hidden])`. Clearing the text alongside the + * `hidden` flag keeps the two halves of "there is no error" from drifting apart. + */ + const setValidationError = (text: string | null): void => { + validationError.textContent = text ?? ""; + validationError.hidden = text === null; + }; + const finish = (value: string | null): void => { if (settled) return; settled = true; @@ -113,7 +125,7 @@ export function promptForAdminToken( } password.disabled = true; submit.disabled = true; - validationError.hidden = true; + setValidationError(null); void verifyToken(token).then((result) => { if (settled) return; @@ -124,18 +136,16 @@ export function promptForAdminToken( password.value = ""; password.disabled = false; submit.disabled = false; - validationError.textContent = result === "rejected" + setValidationError(result === "rejected" ? messages["auth.adminTokenRejected"] - : messages["auth.adminTokenUnavailable"]; - validationError.hidden = false; + : messages["auth.adminTokenUnavailable"]); password.focus(); }).catch(() => { if (settled) return; password.value = ""; password.disabled = false; submit.disabled = false; - validationError.textContent = messages["auth.adminTokenUnavailable"]; - validationError.hidden = false; + setValidationError(messages["auth.adminTokenUnavailable"]); password.focus(); }); }); diff --git a/gui/src/styles.css b/gui/src/styles.css index 027b055c3a..d95ca0c48e 100644 --- a/gui/src/styles.css +++ b/gui/src/styles.css @@ -1304,7 +1304,17 @@ select.input { appearance: none; } .empty svg { width: 30px; height: 30px; color: var(--faint); margin-bottom: 12px; } .empty .title { color: var(--text); font-weight: var(--weight-semibold); margin-bottom: 6px; } -.notice { font-size: var(--text-control); line-height: var(--leading-body); padding: 9px 12px; border-radius: var(--radius-sm); margin-bottom: 14px; display: flex; align-items: center; gap: 8px; max-width: var(--prose-measure); } +/* + `:not([hidden])`, not a bare `display: flex` — same cascade trap the combos panels hit. + + The admin-token dialog mounts its error notice up front and hides it with the `hidden` + attribute until a token is actually rejected. `[hidden] { display: none }` is a + USER-AGENT rule, so an author `display: flex` here beat it and painted an empty red + bordered box the moment the dialog opened (#3483). Specificity never got a vote; origin + decided it. Scoping the display to `:not([hidden])` lets the UA rule win again. +*/ +.notice:not([hidden]) { display: flex; } +.notice { font-size: var(--text-control); line-height: var(--leading-body); padding: 9px 12px; border-radius: var(--radius-sm); margin-bottom: 14px; align-items: center; gap: 8px; max-width: var(--prose-measure); } .notice svg { width: 15px; height: 15px; flex-shrink: 0; } /* Portaled status toast — out of document flow so Providers (etc.) do not reflow. */ @@ -1950,12 +1960,13 @@ dialog.modal-overlay::backdrop { background: light-dark(#fffbeb, color-mix(in oklab, var(--amber) 20%, var(--surface))); color: light-dark(#92400e, #fef3c7); border: 1px solid color-mix(in srgb, var(--amber) 32%, transparent); - display: flex; align-items: center; gap: 6px; margin-bottom: 12px; max-width: var(--prose-measure); } +/* Same UA-vs-author `[hidden]` rule as `.notice` above: `.notice-warn` is used on its own. */ +.notice-warn:not([hidden]) { display: flex; } .notice-warn svg { width: 14px; height: 14px; flex-shrink: 0; color: var(--amber); } .startup-runtime-notice__text, @@ -1998,8 +2009,10 @@ dialog.modal-overlay::backdrop { box-sizing: border-box; } /* Message above, command+copy below. */ -.notice.notice-warn.startup-runtime-notice { +.notice.notice-warn.startup-runtime-notice:not([hidden]) { display: flex; +} +.notice.notice-warn.startup-runtime-notice { flex-direction: column; align-items: stretch; gap: 8px; diff --git a/gui/tests/admin-token-dialog.test.ts b/gui/tests/admin-token-dialog.test.ts index 09f237e412..767d348d35 100644 --- a/gui/tests/admin-token-dialog.test.ts +++ b/gui/tests/admin-token-dialog.test.ts @@ -126,3 +126,56 @@ test("uses the active UI locale instead of re-detecting browser storage", async dialog.dispatchEvent(new testWindow.Event("cancel", { cancelable: true })); expect(await pending).toBeNull(); }); + +/* + * #3483 — the dialog opened with an empty red bordered notice already painted. + * + * The DOM half: while there is no error the alert must be hidden AND carry no text, so + * "hidden" and "empty" cannot drift apart. + */ +test("the validation alert is hidden and empty until a token is actually rejected", async () => { + const pending = promptForAdminToken(async () => "rejected"); + const dialog = document.querySelector("#opencodex-admin-token-dialog")!; + const form = dialog.querySelector("form")!; + const alert = dialog.querySelector('[role="alert"]')!; + + expect(alert.hidden).toBe(true); + expect(alert.textContent).toBe(""); + + const password = form.elements.namedItem("password") as HTMLInputElement; + password.value = "wrong-token"; + form.dispatchEvent(new testWindow.Event("submit", { bubbles: true, cancelable: true })); + await Promise.resolve(); + await Promise.resolve(); + + expect(alert.hidden).toBe(false); + expect(alert.textContent).toContain("rejected"); + + dialog.dispatchEvent(new testWindow.Event("cancel", { cancelable: true })); + expect(await pending).toBeNull(); +}); + +/* + * The CSS half, and the one that actually reproduces the report. + * + * happy-dom applies no author stylesheet and does no layout, so `alert.hidden === true` + * passes even while a real browser paints the box: `[hidden] { display: none }` is a + * USER-AGENT rule and a bare `.notice { display: flex }` outranks it by origin. The + * stylesheet is the only place this contract can be checked — same oracle the combos + * workspace uses after the identical bug (gui/tests/combos-detail-tabs-dom.test.tsx). + */ +test("notice display rules are scoped so a hidden notice cannot paint", async () => { + const css = await Bun.file(new URL("../src/styles.css", import.meta.url)).text(); + + expect(css).toContain(".notice:not([hidden])"); + expect(css).toContain(".notice-warn:not([hidden])"); + expect(css).toContain(".notice.notice-warn.startup-runtime-notice:not([hidden])"); + + // No notice rule may set `display` without the :not([hidden]) guard. + for (const block of css.matchAll(/(^|\})\s*([^{}]*\.notice[^{}]*)\{([^}]*)\}/g)) { + const selector = block[2]!.trim(); + const body = block[3]!; + if (!/(^|[\s,])display\s*:/.test(body)) continue; + expect(selector).toContain(":not([hidden])"); + } +});