diff --git a/.changeset/human-sign-in-error-copy.md b/.changeset/human-sign-in-error-copy.md new file mode 100644 index 00000000..7d27cc54 --- /dev/null +++ b/.changeset/human-sign-in-error-copy.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Sign-in errors are written in plain English instead of showing the raw failure code. + +**Affects:** End users + +**End users:** a rejected code used to report "Invalid OTP", an unexplained acronym that ran straight into the recovery link beside it — "Invalid OTP Send a new code". It now reads "That code didn't work." followed by the link, and the two other common failures are similarly rewritten: an aged-out code says "That code has expired.", and one rejected after repeated wrong attempts says "Too many tries — that code is no longer usable.". Any failure outside those three is still shown as-is rather than hidden behind a generic apology, so an unexpected problem can still be reported accurately. diff --git a/.changeset/unify-link-affordances.md b/.changeset/unify-link-affordances.md new file mode 100644 index 00000000..b9966d3a --- /dev/null +++ b/.changeset/unify-link-affordances.md @@ -0,0 +1,11 @@ +--- +'ePDS': patch +--- + +Clickable text on the sign-in screens now looks the same wherever it appears. + +**Affects:** End users, Client app developers + +**End users:** the same action no longer renders differently depending on where it sits. "Resend code", "Use different email" and "Recover with backup email" are now all plain text that darkens when you point at it, while actions embedded in a sentence — "Send a new code" in an error message, and the Terms of Use and Privacy Policy links — stay underlined so they remain visible against the text around them. Pointing at an underlined action no longer makes its underline vanish, and every one of these actions now shows a focus outline when reached with the keyboard. + +**Client app developers:** `--muted-foreground` now also sets the colour of the `.btn-secondary` actions on the sign-in page, which previously hardcoded `#6b6b6b`. If you override that custom property in your `branding.css`, it will now recolour "Resend code" and "Use different email" alongside "Recover with backup email" and the other muted text. Choose a value that clears 4.5:1 contrast against your card background. `--recovery-link-display` is unchanged. diff --git a/e2e/step-definitions/auth.steps.ts b/e2e/step-definitions/auth.steps.ts index 95722734..71abc40f 100644 --- a/e2e/step-definitions/auth.steps.ts +++ b/e2e/step-definitions/auth.steps.ts @@ -611,7 +611,7 @@ Then( ) Then( - 'the verification form shows an {string} error', + /^the verification form shows (?:an|the) "([^"]*)" error$/, async function (this: EpdsWorld, expected: string) { const page = getPage(this) await expect(page.locator('#error-msg')).toBeVisible({ timeout: 10_000 }) diff --git a/features/passwordless-authentication.feature b/features/passwordless-authentication.feature index fd1c9139..09858b62 100644 --- a/features/passwordless-authentication.feature +++ b/features/passwordless-authentication.feature @@ -250,7 +250,7 @@ Feature: Passwordless authentication via email OTP # better-auth verification row only. We deliberately leave the # auth_flow row + cookie alive to mirror reality at the 10-minute mark. # After the OTP has been aged past expiry, submitting it must fail with - # the helpful "OTP expired" message; resending must produce a fresh + # the "That code has expired." message; resending must produce a fresh # code that completes the flow normally. # @email @otp-expiry @@ -263,7 +263,7 @@ Feature: Passwordless authentication via email OTP And the login page shows an OTP verification form When more than 10 minutes pass before the user enters the OTP And the user enters the OTP code - Then the verification form shows an "OTP expired" error + Then the verification form shows the "That code has expired." error And the OTP entry boxes are visible and enabled When the user enters two digits from the old OTP And the user requests a new OTP via the resend button diff --git a/packages/auth-service/src/__tests__/login-page.test.ts b/packages/auth-service/src/__tests__/login-page.test.ts index 158f9068..01a8784c 100644 --- a/packages/auth-service/src/__tests__/login-page.test.ts +++ b/packages/auth-service/src/__tests__/login-page.test.ts @@ -524,6 +524,43 @@ function renderDefault(overrides: Partial = {}): string { }) } +describe('renderLoginPage sign-in error copy', () => { + it.each([ + ['Invalid OTP', "That code didn't work."], + ['OTP expired', 'That code has expired.'], + ['Too many attempts', 'Too many tries — that code is no longer usable.'], + ])('rewrites better-auth %s as end-user copy', (raw, display) => { + const html = renderDefault() + expect(html).toContain(`case '${raw}':`) + expect(html).toContain(display) + }) + + it('passes an unrecognised error through verbatim', () => { + // A failure that names itself can be diagnosed from a screenshot; + // one collapsed into a generic apology cannot. + const html = renderDefault() + expect(html).toMatch(/default:\s*return raw;/) + }) + + it('keys the expired branch off the raw reason, not the display copy', () => { + // otpErrorText() owns the wording. If the branch matched the + // rewritten string instead, editing that copy could silently + // reroute which recovery action the user is offered. + const html = renderDefault() + expect(html).toContain('test(result.rawError || result.error)') + expect(html).toContain('rawError: raw') + expect(html).not.toMatch( + /isExpired = \/expir\|too long\/i\.test\(result\.error\)/, + ) + }) + + it('separates the message from its inline action', () => { + // Without this the banner reads "Invalid OTP Send a new code". + const html = renderDefault() + expect(html).toMatch(/\/\[\.!\?\]\$\/\.test\(msg\) \? ' ' : '\. '/) + }) +}) + describe('renderLoginPage OTP verify-form double-submit latch (regression)', () => { it('declares the verifying flag at IIFE scope so input/paste/submit handlers share it', () => { const html = renderDefault() @@ -888,3 +925,84 @@ describe('renderLoginPage OTP grid charset filter', () => { ) }) }) + +describe('renderLoginPage link-affordance convention', () => { + // The sign-in page styles four kinds of clickable text. Before this + // suite they diverged: .recovery-link was underlined while the + // .btn-secondary buttons beside it were not, and .flash-action + // REMOVED its underline on hover while .recovery-link kept it. + // + // The convention pinned here: + // STANDALONE actions (.btn-secondary, .recovery-link) sit in their + // own row, so position and spacing already read as actionable — + // no underline. + // IN-SENTENCE actions (.flash-action, .terms-link) are surrounded + // by prose and inherit its colour, so the underline is their only + // affordance — always underlined. + // Hover darkens to #1A130F everywhere and never toggles the + // underline, in either direction. + // + // Assertions match whole declaration blocks so a rule that merely + // mentions the property elsewhere cannot satisfy them. + + function ruleFor(html: string, selector: string): string { + const idx = html.indexOf(`\n ${selector} {`) + expect(idx, `no rule found for "${selector}"`).toBeGreaterThan(0) + const open = html.indexOf('{', idx) + const close = html.indexOf('}', open) + expect(close).toBeGreaterThan(open) + return html.slice(open + 1, close) + } + + const STANDALONE = ['.btn-secondary', '.recovery-link'] + const IN_SENTENCE = ['.flash-action', '.terms-link'] + + it.each(STANDALONE)( + 'renders %s without an underline (standalone action)', + (selector) => { + const rule = ruleFor(renderDefault(), selector) + expect(rule).toContain('text-decoration: none') + expect(rule).not.toContain('text-decoration: underline') + }, + ) + + it.each(IN_SENTENCE)( + 'renders %s underlined (in-sentence action)', + (selector) => { + const rule = ruleFor(renderDefault(), selector) + expect(rule).toContain('text-decoration: underline') + }, + ) + + it.each([...STANDALONE, ...IN_SENTENCE])( + 'darkens %s on hover without touching its underline', + (selector) => { + const rule = ruleFor(renderDefault(), `${selector}:hover`) + expect(rule).toContain('color: #1A130F') + // A disappearing (or appearing) underline under the cursor is + // disorienting — hover must never change text-decoration. + expect(rule).not.toContain('text-decoration') + }, + ) + + it.each([...STANDALONE, ...IN_SENTENCE])( + 'gives %s a visible focus ring for keyboard users', + (selector) => { + const rule = ruleFor(renderDefault(), `${selector}:focus-visible`) + expect(rule).toContain('outline: 2px solid var(--focus-border') + expect(rule).toContain('outline-offset: 2px') + }, + ) + + it('drives both standalone actions from the overridable --muted-foreground token', () => { + // Trusted clients retheme via branding.css. If one standalone + // action hardcoded its colour and the other read the token, an + // override would split the cluster apart again. + const html = renderDefault() + for (const selector of STANDALONE) { + expect(ruleFor(html, selector)).toContain( + 'color: var(--muted-foreground)', + ) + } + }) +}) diff --git a/packages/auth-service/src/routes/account-login.ts b/packages/auth-service/src/routes/account-login.ts index a9769eec..786b795f 100644 --- a/packages/auth-service/src/routes/account-login.ts +++ b/packages/auth-service/src/routes/account-login.ts @@ -251,7 +251,11 @@ const CSS = ` .btn-primary { width: 100%; padding: 12px; background: #0f1828; color: white; border: none; border-radius: 8px; font-size: 16px; font-weight: 500; cursor: pointer; } .btn-primary:hover { background: #1a2a40; } .btn-primary:focus-visible { outline: 2px solid #0f1828; outline-offset: 2px; } - .btn-secondary { display: inline-block; color: #0f1828; background: none; border: none; font-size: 14px; cursor: pointer; text-decoration: underline; border-radius: 4px; } + /* Standalone action in its own row — see the link-affordance convention + documented in login-page.ts. No underline, darkens on hover. "Resend + code" here and on the sign-in page must not render differently. */ + .btn-secondary { display: inline-block; color: #0f1828; background: none; border: none; font-size: 14px; cursor: pointer; text-decoration: none; border-radius: 4px; } + .btn-secondary:hover { color: #000; } .btn-secondary:focus-visible { outline: 2px solid #0f1828; outline-offset: 2px; } .error { color: #dc3545; background: #fdf0f0; padding: 12px; border-radius: 8px; margin: 12px 0; } ` diff --git a/packages/auth-service/src/routes/login-page.ts b/packages/auth-service/src/routes/login-page.ts index b5c1c4ee..5bfb41ba 100644 --- a/packages/auth-service/src/routes/login-page.ts +++ b/packages/auth-service/src/routes/login-page.ts @@ -633,7 +633,14 @@ export function renderLoginPage(opts: { .btn-primary:hover { opacity: 0.9; } .btn-primary:focus-visible { outline: 2px solid var(--focus-border, #2563eb); outline-offset: 2px; } .btn-primary:disabled { opacity: 0.7; cursor: not-allowed; } - .btn-secondary { display: inline-block; color: #6b6b6b; background: none; border: none; font-size: 14px; font-weight: 500; cursor: pointer; padding: 4px 0; border-radius: 4px; } + /* Link-affordance convention (see also .flash-action / .terms-link): + STANDALONE actions sit in their own row, where position and spacing + already read as actionable, so they carry no underline. IN-SENTENCE + actions are surrounded by prose and need an underline to be + identifiable at all. Both darken to #1A130F on hover; no rule ever + toggles an underline on or off, because an affordance that appears + or vanishes under the cursor is disorienting. */ + .btn-secondary { display: inline-block; color: var(--muted-foreground); background: none; border: none; font-size: 14px; font-weight: 500; cursor: pointer; padding: 4px 0; border-radius: 4px; text-decoration: none; } .btn-secondary:hover { color: #1A130F; } .btn-secondary:focus-visible { outline: 2px solid var(--focus-border, #2563eb); outline-offset: 2px; } .btn-social { display: flex; align-items: center; justify-content: center; gap: 8px; width: 100%; padding: 13px 20px; border: 1px solid var(--btn-secondary-border); border-radius: 9999px; font-size: 15px; font-weight: 500; cursor: pointer; text-decoration: none; background: white; color: #333; margin-bottom: 8px; transition: background 0.15s; } @@ -648,22 +655,33 @@ export function renderLoginPage(opts: { /* Inline action button rendered next to an OTP-expired error so the user doesn't have to hunt for the separate Resend button. Styled as a link rather than a button to make it visually - continuous with the message text. */ + continuous with the message text. IN-SENTENCE action: it inherits + the surrounding error colour, so the underline is its only marker + of being clickable and must survive hover. */ .flash-action { background: none; border: none; padding: 0; font: inherit; color: inherit; text-decoration: underline; cursor: pointer; } - .flash-action:hover { text-decoration: none; } + .flash-action:hover { color: #1A130F; } + .flash-action:focus-visible { outline: 2px solid var(--focus-border, #2563eb); outline-offset: 2px; border-radius: 4px; } .step-otp { display: none; } .step-otp.active { display: block; } .step-email.hidden { display: none; } .terms { margin-top: 24px; color: var(--muted-foreground); font-size: 13px; font-weight: 400; line-height: 1.5; text-align: center; } + /* IN-SENTENCE action: sits inside the terms sentence and inherits its + colour, so the underline is its only marker and must survive hover. */ .terms-link { color: inherit; text-decoration: underline; cursor: pointer; } + .terms-link:hover { color: #1A130F; } + .terms-link:focus-visible { outline: 2px solid var(--focus-border, #2563eb); outline-offset: 2px; border-radius: 4px; } .powered-by { display: flex; align-items: center; justify-content: center; gap: 8px; margin-top: 16px; color: var(--muted-foreground); font-size: 13px; text-decoration: none; cursor: pointer; } .powered-by:hover, .powered-by:focus, .powered-by:visited { color: var(--muted-foreground); text-decoration: none; } .powered-by .certified-mark { height: 14px; width: auto; display: block; } /* Recovery-via-backup-email link. Shown by default; trusted clients hide it by setting --recovery-link-display: none in their injected - branding.css. */ - .recovery-link { display: var(--recovery-link-display, block); margin-top: 16px; color: var(--muted-foreground); font-size: 13px; text-decoration: underline; text-align: center; } + branding.css. STANDALONE action: it shares the action cluster + under Verify with the .btn-secondary buttons, and the anchor-vs- + button split behind the old underline was never legible to a + user — those buttons are deliberately styled to look like links. */ + .recovery-link { display: var(--recovery-link-display, block); margin-top: 16px; color: var(--muted-foreground); font-size: 13px; text-decoration: none; text-align: center; } .recovery-link:hover { color: #1A130F; } + .recovery-link:focus-visible { outline: 2px solid var(--focus-border, #2563eb); outline-offset: 2px; border-radius: 4px; } ${renderOptionalStyleTag(opts.customCss)} @@ -1095,6 +1113,29 @@ export function renderLoginPage(opts: { * handler runs the supplied callback. When actionLabel is * absent, behaves like showError. */ + /** + * Rewrite better-auth's verification errors as end-user copy. + * + * better-auth returns developer-facing strings — "Invalid OTP" is + * an unexplained acronym with no article, and none of them say + * what to do next. The rendered text is the one thing a user + * actually reads when sign-in fails, so it is worth owning. + * + * Anything unrecognised passes through verbatim rather than + * collapsing into a generic apology: an unexpected failure that + * still names itself can be diagnosed from a screenshot, and one + * that says "Something went wrong" cannot. + */ + function otpErrorText(raw) { + switch (raw) { + case 'Invalid OTP': return "That code didn't work."; + case 'OTP expired': return 'That code has expired.'; + case 'Too many attempts': + return 'Too many tries — that code is no longer usable.'; + default: return raw; + } + } + function showErrorWithAction(msg, actionLabel, onClick) { if (!actionLabel || typeof onClick !== 'function') { showError(msg); @@ -1102,7 +1143,13 @@ export function renderLoginPage(opts: { } setFlash('error', function(frag) { appendMessage(frag, msg); - frag.appendChild(document.createTextNode(' ')); + // Separate the sentence from the action. Mapped copy already + // ends in a full stop, but a passed-through better-auth string + // may not, so supply one rather than letting the two run + // together as "Invalid OTP Send a new code". + frag.appendChild( + document.createTextNode(/[.!?]$/.test(msg) ? ' ' : '. '), + ); var btn = document.createElement('button'); btn.type = 'button'; btn.className = 'flash-action'; @@ -1241,7 +1288,12 @@ export function renderLoginPage(opts: { }); if (!res.ok) { var data = await res.json().catch(function() { return {}; }); - return { error: data.message || data.error || 'Invalid code' }; + var raw = data.message || data.error || 'Invalid code'; + // Keep the raw reason alongside the display text: callers + // branch on it (see isExpired below), and branching on the + // rewritten copy would couple control flow to wording, so + // an innocuous copy edit could silently change behaviour. + return { error: otpErrorText(raw), rawError: raw }; } // Success: redirect to /auth/complete to complete the AT Protocol flow window.location.href = '/auth/complete'; @@ -1321,7 +1373,10 @@ export function renderLoginPage(opts: { // match catches the better-auth wording ("Invalid or // expired code") and the auth-service wording ("OTP // expired") plus generic "expir"/"too long" variants. - var isExpired = /expir|too long/i.test(result.error); + // Test the raw better-auth reason, not the rewritten copy: + // otpErrorText() owns the wording, and matching against it + // would mean a copy edit could silently reroute the branch. + var isExpired = /expir|too long/i.test(result.rawError || result.error); if (isExpired) { // Only offer "Send a new code" when the PAR is still // alive. If it isn't, a fresh OTP would issue but diff --git a/packages/auth-service/src/routes/recovery.ts b/packages/auth-service/src/routes/recovery.ts index 6d3e235b..70504f41 100644 --- a/packages/auth-service/src/routes/recovery.ts +++ b/packages/auth-service/src/routes/recovery.ts @@ -514,7 +514,11 @@ const CSS = ` .btn-primary { width: 100%; padding: 12px; background: #0f1828; color: white; border: none; border-radius: 8px; font-size: 16px; font-weight: 500; cursor: pointer; } .btn-primary:hover { background: #1a2a40; } .btn-primary:focus-visible { outline: 2px solid #0f1828; outline-offset: 2px; } - .btn-secondary { display: inline-block; margin-top: 12px; color: #0f1828; background: none; border: none; font-size: 14px; cursor: pointer; text-decoration: underline; border-radius: 4px; } + /* Standalone action in its own row — see the link-affordance convention + documented in login-page.ts. No underline, darkens on hover. "Resend + code" here and on the sign-in page must not render differently. */ + .btn-secondary { display: inline-block; margin-top: 12px; color: #0f1828; background: none; border: none; font-size: 14px; cursor: pointer; text-decoration: none; border-radius: 4px; } + .btn-secondary:hover { color: #000; } .btn-secondary:focus-visible { outline: 2px solid #0f1828; outline-offset: 2px; } .error { color: #dc3545; background: #fdf0f0; padding: 12px; border-radius: 8px; margin: 12px 0; } ` diff --git a/packages/demo/src/lib/theme.ts b/packages/demo/src/lib/theme.ts index fd8bf339..1d1f2edb 100644 --- a/packages/demo/src/lib/theme.ts +++ b/packages/demo/src/lib/theme.ts @@ -128,7 +128,11 @@ function buildInjectedCss( `.otp-box:focus { border-color: ${page.focusBorder} !important; }`, `.btn-primary { background: ${page.primary}; color: ${page.primaryText}; }`, `.btn-primary:hover { background: ${page.primaryHover}; }`, - `.btn-secondary { color: ${page.textMuted}; }`, + // Standalone actions under Verify (.btn-secondary buttons and the + // .recovery-link anchor) are one visual class, so they must theme + // identically — see the link-affordance convention in login-page.ts. + `.btn-secondary, .recovery-link { color: ${page.textMuted}; }`, + `.btn-secondary:hover, .recovery-link:hover { color: ${page.text}; }`, `.btn-social { background: ${page.inputBg}; border-color: ${page.inputBorder}; color: ${page.text}; }`, `.btn-atproto { background: ${page.inputBg} !important; border-color: ${page.inputBorder} !important; color: ${page.text} !important; }`, `.btn-social:hover { background: ${page.inputBorder}; }`, @@ -136,8 +140,6 @@ function buildInjectedCss( `.divider::before, .divider::after { background: ${page.inputBorder}; }`, `.error { background: ${page.errorBg}; color: ${page.errorText}; }`, `.flash-msg.error { background: ${page.errorBg}; color: ${page.errorText}; }`, - `.recovery-link { color: ${page.textHint}; }`, - `.recovery-link:hover { color: ${page.textMuted}; }`, `.handle-row { border-color: ${page.inputBorder}; }`, `.handle-suffix { color: ${page.textHint}; background: ${page.inputBg}; border-color: ${page.inputBorder}; }`, '.status.available { color: #4ade80; }',