From 5857f40e2d3fe46519b15989f68af5ab10818423 Mon Sep 17 00:00:00 2001 From: Artjoms Stukans Date: Fri, 25 Sep 2026 17:29:11 +0200 Subject: [PATCH 1/3] Show a sign-in code as two steps, each with its own action The prompted sign-in panel was a link, a code and a countdown in a column, and read as unfinished. It is now two numbered steps in the order they are used: 1. Open the sign-in page - the link, with an icon button that copies the code and then opens the page in a new tab. Copying comes first because a browser refuses a clipboard write once the new tab has focus. 2. Enter this code - shown large and spaced, with a copy button. A status line says what was just copied, or how long the code has left. The link and its button are offered only for an https address. --- spire-ui/docs/WIDGETS.md | 1 + .../src/components/HarnessSignInCode.test.tsx | 52 ++++++++++ spire-ui/src/components/HarnessSignInCode.tsx | 96 +++++++++++++++++++ .../HarnessSubscriptionSignIn.test.tsx | 2 +- .../components/HarnessSubscriptionSignIn.tsx | 22 ++--- spire-ui/src/index.css | 21 ++++ 6 files changed, 182 insertions(+), 12 deletions(-) create mode 100644 spire-ui/src/components/HarnessSignInCode.test.tsx create mode 100644 spire-ui/src/components/HarnessSignInCode.tsx diff --git a/spire-ui/docs/WIDGETS.md b/spire-ui/docs/WIDGETS.md index 18ead739..2d073797 100644 --- a/spire-ui/docs/WIDGETS.md +++ b/spire-ui/docs/WIDGETS.md @@ -24,6 +24,7 @@ as well as its appearance: labels, empty states, pending-save lock-out and succe | Person identity | `ActorPicker`, `actorLabel`; people are stable provider IDs with display metadata, distinct from credential accounts | [ActorPicker](../src/components/ActorPicker.tsx) | | Select menu | Shared `Select` provides keyboard navigation, type-ahead and a portalled listbox. Native selects also use `field`; explain missing prerequisites and empty options | [Select](../src/components/Select.tsx), [SettingsContextProviders](../src/components/SettingsContextProviders.tsx) | | Copyable values | `CopyField`, `CopyableValue`; avoid full URLs as button labels | [CopyField](../src/components/CopyField.tsx), [RepositoryDetail](../src/components/repositories/RepositoryDetail.tsx) | +| A code typed on another device | `HarnessSignInCode`: numbered steps in the order they are used, each with its one icon action — copy the code; copy the code and open the https page, copying first because a browser refuses a clipboard write once the new tab has focus. The status line says what was copied or how long is left | [HarnessSignInCode](../src/components/HarnessSignInCode.tsx) | | Confirmation | `ConfirmDialog`; retain pending state and explicit action names | [ConfirmDialog](../src/components/ConfirmDialog.tsx) | | Navigation | A visible icon with `ic`, size 16, alongside the page name — asserted for every rail entry, because Approvals shipped without one and read as unfinished | [App](../src/App.tsx) | | **Create, edit or inspect one row** | `SidePanel`; a screen lists at full width, a panel beside it changes. Its form is a `fieldset` so `disabled` locks every control during a save. `tabs` splits a panel that does more than one job; `wide` suits a panel that carries a table | [SidePanel](../src/components/SidePanel.tsx), [RepositoryRegistryPage](../src/components/repositories/RepositoryRegistryPage.tsx) | diff --git a/spire-ui/src/components/HarnessSignInCode.test.tsx b/spire-ui/src/components/HarnessSignInCode.test.tsx new file mode 100644 index 00000000..bb6c6b69 --- /dev/null +++ b/spire-ui/src/components/HarnessSignInCode.test.tsx @@ -0,0 +1,52 @@ +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { afterEach, beforeEach, expect, it, vi } from 'vitest'; +import HarnessSignInCode from './HarnessSignInCode'; + +// Placeholder link and code: this is about the buttons, not the vendor's real values. +const LINK = 'https://auth.example.test/device'; +const CODE = 'TEST-12345'; + +let order: string[]; + +beforeEach(() => { + order = []; + Object.assign(navigator, { clipboard: { writeText: vi.fn(async (text: string) => { order.push(`copy:${text}`); }) } }); + vi.spyOn(window, 'open').mockImplementation((url?: string | URL) => { order.push(`open:${String(url)}`); return null; }); +}); +afterEach(cleanup); + +it('copies the code from the button beside it', () => { + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Copy the code' })); + + expect(order).toEqual([`copy:${CODE}`]); + expect(screen.getByRole('status')).toHaveTextContent('Code copied.'); +}); + +// The page that opens asks for the code, so it is copied too — and FIRST: once the new tab has focus, +// a browser refuses a clipboard write. +it('copies the code and then opens the sign-in page in a new tab', () => { + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Copy the code and open the sign-in page' })); + + expect(order).toEqual([`copy:${CODE}`, `open:${LINK}`]); + expect(window.open).toHaveBeenCalledWith(LINK, '_blank', 'noopener,noreferrer'); + expect(screen.getByRole('status')).toHaveTextContent('Paste it on the page that just opened'); +}); + +// The worker accepts only the vendor's own host; this is the second lock, where a click would follow it. +it('offers no link and no open button for an address that is not https', () => { + render(); + + expect(screen.queryByRole('link')).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Copy the code and open the sign-in page' })).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Copy the code' })).toBeInTheDocument(); +}); + +it('says when the code has run out', () => { + render(); + + expect(screen.getByRole('status')).toHaveTextContent('The code has expired.'); +}); diff --git a/spire-ui/src/components/HarnessSignInCode.tsx b/spire-ui/src/components/HarnessSignInCode.tsx new file mode 100644 index 00000000..9e712b1d --- /dev/null +++ b/spire-ui/src/components/HarnessSignInCode.tsx @@ -0,0 +1,96 @@ +import { useEffect, useRef, useState } from 'react'; +import { Check, Clock, Copy, ExternalLink } from 'lucide-react'; +import Tooltip from './Tooltip'; + +interface Props { + verificationUri: string; + userCode: string; + /** "13m 30s", or null once the code has run out. Recomputed by the parent on every poll. */ + remaining: string | null; +} + +/** Which button last copied, so its icon can confirm and the status line can say what happened. */ +type Copied = 'code' | 'open' | null; + +/** + * What the operator does with a device code: open one page, type one code (M3.5 part F). + * + *

Two numbered steps rather than a link and a code in a column, because that is the order they are + * used in, and each carries the one action it needs. The open action copies the code first: the page + * it opens asks for exactly that, and a browser refuses a clipboard write once focus has moved to the + * new tab, so the order is not cosmetic. + * + *

The link is offered only when it is https. The worker already accepts only the vendor's own host, + * so this is the second lock, placed where a click would follow it. + */ +export default function HarnessSignInCode({ verificationUri, userCode, remaining }: Props) { + const [copied, setCopied] = useState(null); + const timer = useRef | undefined>(undefined); + useEffect(() => () => clearTimeout(timer.current), []); + const safeUri = isHttps(verificationUri) ? verificationUri : null; + + function confirm(which: Copied) { + setCopied(which); + clearTimeout(timer.current); + timer.current = setTimeout(() => setCopied(null), 2000); + } + + function copyCode() { + void navigator.clipboard?.writeText(userCode); + confirm('code'); + } + + function copyAndOpen() { + if (!safeUri) return; + void navigator.clipboard?.writeText(userCode); + window.open(safeUri, '_blank', 'noopener,noreferrer'); + confirm('open'); + } + + return

+
    +
  1. + Open the sign-in page +
    + {safeUri + ? {safeUri} + : {verificationUri}} + {safeUri && + + } +
    +
    2. +
  2. + Sign in with the account the factory will use, then enter this code +
    + {userCode} + + + +
    +
    3. +
+

+

+
; +} + +/** What just happened, or else how long the code has left. */ +function statusLine(copied: Copied, remaining: string | null): string { + if (copied === 'open') return 'Code copied. Paste it on the page that just opened.'; + if (copied === 'code') return 'Code copied.'; + if (!remaining) return 'The code has expired.'; + return `The code expires in ${remaining}. This panel closes by itself once you approve.`; +} + +function isHttps(uri: string): boolean { + try { return new URL(uri).protocol === 'https:'; } + catch { return false; } +} diff --git a/spire-ui/src/components/HarnessSubscriptionSignIn.test.tsx b/spire-ui/src/components/HarnessSubscriptionSignIn.test.tsx index 63a5e52b..be8647b6 100644 --- a/spire-ui/src/components/HarnessSubscriptionSignIn.test.tsx +++ b/spire-ui/src/components/HarnessSubscriptionSignIn.test.tsx @@ -33,7 +33,7 @@ it('shows the link and the code the operator must use', async () => { // A new tab, and no referrer or opener handed to whatever is on the other side. expect(link).toHaveAttribute('rel', expect.stringContaining('noopener')); expect(screen.getByLabelText('One-time code')).toHaveTextContent('ABCD-12345'); - expect(screen.getByText(/left$/)).toBeInTheDocument(); + expect(screen.getByText(/^The code expires in \d+m \d{2}s\./)).toBeInTheDocument(); await waitFor(() => expect(api.startHarnessSignIn).toHaveBeenCalledWith('TEST-factory-seat', 'codex')); }); diff --git a/spire-ui/src/components/HarnessSubscriptionSignIn.tsx b/spire-ui/src/components/HarnessSubscriptionSignIn.tsx index 94bcc2a7..8453d143 100644 --- a/spire-ui/src/components/HarnessSubscriptionSignIn.tsx +++ b/spire-ui/src/components/HarnessSubscriptionSignIn.tsx @@ -1,5 +1,6 @@ import { useEffect, useRef, useState } from 'react'; import { cancelHarnessSignIn, fetchHarnessSignIn, fetchHarnessSignInProgress, startHarnessSignIn, type HarnessSignInView } from '../api'; +import HarnessSignInCode from './HarnessSignInCode'; import SettingField from './SettingField'; /** How often the screen asks. The operator is walking to another device; this is not a race. */ @@ -20,11 +21,12 @@ const REASONS: Record = { const sentence = (reason: string | null) => (reason && REASONS[reason]) || 'The sign-in did not finish, and nothing was stored.'; -function remaining(expiresAt: string | null): string { - if (!expiresAt) return ''; +/** "13m 30s" until the code runs out, then null. */ +function remaining(expiresAt: string | null): string | null { + if (!expiresAt) return null; const seconds = Math.floor((new Date(expiresAt).getTime() - Date.now()) / 1000); - if (seconds <= 0) return 'expired'; - return `${Math.floor(seconds / 60)}m ${String(seconds % 60).padStart(2, '0')}s left`; + if (seconds <= 0) return null; + return `${Math.floor(seconds / 60)}m ${String(seconds % 60).padStart(2, '0')}s`; } interface Props { @@ -49,7 +51,8 @@ export default function HarnessSubscriptionSignIn({ harness, done }: Props) { const [label, setLabel] = useState(''); const [signIn, setSignIn] = useState(null); const [error, setError] = useState(''), [busy, setBusy] = useState(false); - const [tick, setTick] = useState(0); + // Only re-renders the countdown between polls; the value itself is never read. + const [, setTick] = useState(0); const active = useRef(true); /** * Which sign-in the screen is on. Every start, cancel and unmount moves it on, and a poll that @@ -123,12 +126,9 @@ export default function HarnessSubscriptionSignIn({ harness, done }: Props) { if (watching && signIn) { return <> {signIn.state === 'PENDING' &&

Starting the sign-in. This takes a few seconds.

} - {signIn.state === 'PROMPTED' && signIn.verificationUri && signIn.userCode && <> -

Open this link on any device, sign in, and type the code.

-

{signIn.verificationUri}

-

{signIn.userCode}

-

{remaining(signIn.expiresAt)}

- } + {signIn.state === 'PROMPTED' && signIn.verificationUri && signIn.userCode && + } {error &&

{error}

}
diff --git a/spire-ui/src/index.css b/spire-ui/src/index.css index ae1f7f3d..a2668624 100644 --- a/spire-ui/src/index.css +++ b/spire-ui/src/index.css @@ -1680,6 +1680,27 @@ .field-pr { flex: 1 1 100%; min-width: 0; } } +/* ---- signing a harness in with a subscription: the device code (M3.5 part F) ---- */ +@layer components { + .signin-code { display: flex; flex-direction: column; gap: 14px; } + /* Numbered because they are used in this order; the number is real order, never decoration. */ + .signin-steps { list-style: none; counter-reset: signin; margin: 0; padding: 0; display: flex; flex-direction: column; gap: 14px; } + .signin-steps li { counter-increment: signin; display: grid; grid-template-columns: 22px 1fr; column-gap: 10px; row-gap: 7px; } + .signin-steps li::before { + content: counter(signin); grid-row: 1 / span 2; + width: 22px; height: 22px; border-radius: 50%; display: inline-flex; align-items: center; justify-content: center; + font-size: 11.5px; font-weight: 600; color: var(--iris); background: var(--iris-soft); + } + .signin-step-title { font-size: 12.5px; font-weight: 550; color: var(--text); line-height: 22px; } + .signin-steps .reveal-value { padding: 6px 6px 6px 12px; } + .signin-link { overflow: hidden; text-overflow: ellipsis; white-space: nowrap; color: var(--iris); text-decoration: none; } + .signin-link:hover { text-decoration: underline; } + /* The one thing typed by hand on another device: large, spaced, and easy to read aloud. */ + .signin-steps .reveal-value .signin-user-code { font-size: 22px; font-weight: 600; letter-spacing: 0.12em; } + .signin-expiry { display: flex; align-items: center; gap: 7px; margin: 0; font-size: 12px; color: var(--text-2); } + .signin-expiry svg { flex: none; color: var(--text-3); } +} + /* ---- proving an SCM account by signing in (FR-11) ---- */ @layer components { /* One platform's sign-in application: a summary row that opens into its form. */ From 51e2bf80f536a7cd8e9286b54241e86a2d8029a1 Mon Sep 17 00:00:00 2001 From: Artjoms Stukans Date: Fri, 25 Sep 2026 17:43:14 +0200 Subject: [PATCH 2/3] Report a sign-in code copy only once the browser confirms it Review of PR #177: both buttons said "Code copied" before the clipboard write settled, so a refused or missing clipboard sent the operator to paste whatever they had copied before. Copy now reports the browser's answer: "copied" on success, and "could not be copied - type it from here" when the write is refused or the API is missing. The open button still starts the copy first and opens the page in the same click - a window opened after waiting is blocked as a pop-up - and reports the copy when it lands. --- .../src/components/HarnessSignInCode.test.tsx | 69 ++++++++++++++++--- spire-ui/src/components/HarnessSignInCode.tsx | 64 +++++++++++------ spire-ui/src/index.css | 1 + 3 files changed, 101 insertions(+), 33 deletions(-) diff --git a/spire-ui/src/components/HarnessSignInCode.test.tsx b/spire-ui/src/components/HarnessSignInCode.test.tsx index bb6c6b69..069d8edb 100644 --- a/spire-ui/src/components/HarnessSignInCode.test.tsx +++ b/spire-ui/src/components/HarnessSignInCode.test.tsx @@ -1,4 +1,4 @@ -import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'; import { afterEach, beforeEach, expect, it, vi } from 'vitest'; import HarnessSignInCode from './HarnessSignInCode'; @@ -7,35 +7,82 @@ const LINK = 'https://auth.example.test/device'; const CODE = 'TEST-12345'; let order: string[]; +/** Settles the pending clipboard write, so a test decides when — and whether — the browser confirms. */ +let settle: (ok: boolean) => void; + +function clipboardThatWaits() { + Object.assign(navigator, { + clipboard: { + writeText: vi.fn((text: string) => { + order.push(`copy:${text}`); + return new Promise((resolve, reject) => { settle = ok => (ok ? resolve() : reject(new Error('TEST denied'))); }); + }), + }, + }); +} beforeEach(() => { order = []; - Object.assign(navigator, { clipboard: { writeText: vi.fn(async (text: string) => { order.push(`copy:${text}`); }) } }); + clipboardThatWaits(); vi.spyOn(window, 'open').mockImplementation((url?: string | URL) => { order.push(`open:${String(url)}`); return null; }); }); afterEach(cleanup); -it('copies the code from the button beside it', () => { - render(); +const view = (remaining: string | null = '13m 30s') => + render(); - fireEvent.click(screen.getByRole('button', { name: 'Copy the code' })); +it('says the code was copied only once the browser confirms it', async () => { + view(); + fireEvent.click(screen.getByRole('button', { name: 'Copy the code' })); expect(order).toEqual([`copy:${CODE}`]); + expect(screen.getByRole('status')).not.toHaveTextContent('copied'); + + await act(async () => settle(true)); expect(screen.getByRole('status')).toHaveTextContent('Code copied.'); }); -// The page that opens asks for the code, so it is copied too — and FIRST: once the new tab has focus, -// a browser refuses a clipboard write. -it('copies the code and then opens the sign-in page in a new tab', () => { - render(); +// Saying "copied" when it was not would send the operator to paste whatever was on the clipboard before. +it('says so when the browser refuses the copy', async () => { + view(); - fireEvent.click(screen.getByRole('button', { name: 'Copy the code and open the sign-in page' })); + fireEvent.click(screen.getByRole('button', { name: 'Copy the code' })); + await act(async () => settle(false)); + + expect(screen.getByRole('status')).toHaveTextContent('The code could not be copied. Type it from here.'); +}); + +it('says so when there is no clipboard to copy to', async () => { + Object.assign(navigator, { clipboard: undefined }); + view(); + + await act(async () => { fireEvent.click(screen.getByRole('button', { name: 'Copy the code' })); }); + expect(screen.getByRole('status')).toHaveTextContent('could not be copied'); +}); + +// The copy is STARTED before the page opens (a browser refuses it once the new tab has focus), and the +// page opens in the same click (a window opened after waiting is blocked as a pop-up). +it('starts the copy, opens the page in the same click, and reports the copy when it lands', async () => { + view(); + + fireEvent.click(screen.getByRole('button', { name: 'Copy the code and open the sign-in page' })); expect(order).toEqual([`copy:${CODE}`, `open:${LINK}`]); expect(window.open).toHaveBeenCalledWith(LINK, '_blank', 'noopener,noreferrer'); + + await act(async () => settle(true)); expect(screen.getByRole('status')).toHaveTextContent('Paste it on the page that just opened'); }); +it('says the page opened without the code when that copy fails', async () => { + view(); + + fireEvent.click(screen.getByRole('button', { name: 'Copy the code and open the sign-in page' })); + await act(async () => settle(false)); + + expect(screen.getByRole('status')).toHaveTextContent('The page opened, but the code could not be copied'); +}); + // The worker accepts only the vendor's own host; this is the second lock, where a click would follow it. it('offers no link and no open button for an address that is not https', () => { render(); @@ -46,7 +93,7 @@ it('offers no link and no open button for an address that is not https', () => { }); it('says when the code has run out', () => { - render(); + view(null); expect(screen.getByRole('status')).toHaveTextContent('The code has expired.'); }); diff --git a/spire-ui/src/components/HarnessSignInCode.tsx b/spire-ui/src/components/HarnessSignInCode.tsx index 9e712b1d..8d5a773c 100644 --- a/spire-ui/src/components/HarnessSignInCode.tsx +++ b/spire-ui/src/components/HarnessSignInCode.tsx @@ -9,44 +9,51 @@ interface Props { remaining: string | null; } -/** Which button last copied, so its icon can confirm and the status line can say what happened. */ -type Copied = 'code' | 'open' | null; +/** What the last click achieved. A copy is reported only once the browser has confirmed it. */ +type Outcome = 'copied' | 'copy-failed' | 'opened-copied' | 'opened-copy-failed' | null; /** * What the operator does with a device code: open one page, type one code (M3.5 part F). * *

Two numbered steps rather than a link and a code in a column, because that is the order they are - * used in, and each carries the one action it needs. The open action copies the code first: the page - * it opens asks for exactly that, and a browser refuses a clipboard write once focus has moved to the - * new tab, so the order is not cosmetic. + * used in, and each carries the one action it needs. + * + *

The open action starts the copy, then opens the page in the same click. The copy is STARTED first + * because a browser refuses a clipboard write once focus has moved to the new tab; the page is opened + * without waiting for it because a window opened after an await is no longer part of the click, and a + * browser blocks it as a pop-up. So the result of the copy arrives after the page is open, and is + * reported then — "copied" only when the browser said so, never assumed (review of PR #177). * *

The link is offered only when it is https. The worker already accepts only the vendor's own host, * so this is the second lock, placed where a click would follow it. */ export default function HarnessSignInCode({ verificationUri, userCode, remaining }: Props) { - const [copied, setCopied] = useState(null); + const [outcome, setOutcome] = useState(null); const timer = useRef | undefined>(undefined); - useEffect(() => () => clearTimeout(timer.current), []); + const mounted = useRef(true); + useEffect(() => () => { mounted.current = false; clearTimeout(timer.current); }, []); const safeUri = isHttps(verificationUri) ? verificationUri : null; - function confirm(which: Copied) { - setCopied(which); + function report(next: Outcome) { + if (!mounted.current) return; + setOutcome(next); clearTimeout(timer.current); - timer.current = setTimeout(() => setCopied(null), 2000); + // A failure stays until the next click: the operator has to act on it. A success fades. + if (next === 'copied' || next === 'opened-copied') timer.current = setTimeout(() => setOutcome(null), 2000); } function copyCode() { - void navigator.clipboard?.writeText(userCode); - confirm('code'); + void copy(userCode).then(done => report(done ? 'copied' : 'copy-failed')); } function copyAndOpen() { if (!safeUri) return; - void navigator.clipboard?.writeText(userCode); + const copying = copy(userCode); window.open(safeUri, '_blank', 'noopener,noreferrer'); - confirm('open'); + void copying.then(done => report(done ? 'opened-copied' : 'opened-copy-failed')); } + const failed = outcome === 'copy-failed' || outcome === 'opened-copy-failed'; return

  1. @@ -58,7 +65,7 @@ export default function HarnessSignInCode({ verificationUri, userCode, remaining {safeUri && }
@@ -69,23 +76,36 @@ export default function HarnessSignInCode({ verificationUri, userCode, remaining {userCode}
-

+

; } -/** What just happened, or else how long the code has left. */ -function statusLine(copied: Copied, remaining: string | null): string { - if (copied === 'open') return 'Code copied. Paste it on the page that just opened.'; - if (copied === 'code') return 'Code copied.'; +/** True only when the browser confirmed the write. A missing API or a refusal is a failure, said as one. */ +function copy(text: string): Promise { + const clipboard = typeof navigator === 'undefined' ? undefined : navigator.clipboard; + if (!clipboard?.writeText) return Promise.resolve(false); + try { + return clipboard.writeText(text).then(() => true, () => false); + } catch { + return Promise.resolve(false); + } +} + +/** What the last click achieved, or else how long the code has left. */ +function statusLine(outcome: Outcome, remaining: string | null): string { + if (outcome === 'opened-copied') return 'Code copied. Paste it on the page that just opened.'; + if (outcome === 'opened-copy-failed') return 'The page opened, but the code could not be copied. Type it from here.'; + if (outcome === 'copied') return 'Code copied.'; + if (outcome === 'copy-failed') return 'The code could not be copied. Type it from here.'; if (!remaining) return 'The code has expired.'; return `The code expires in ${remaining}. This panel closes by itself once you approve.`; } diff --git a/spire-ui/src/index.css b/spire-ui/src/index.css index a2668624..c4b81585 100644 --- a/spire-ui/src/index.css +++ b/spire-ui/src/index.css @@ -1699,6 +1699,7 @@ .signin-steps .reveal-value .signin-user-code { font-size: 22px; font-weight: 600; letter-spacing: 0.12em; } .signin-expiry { display: flex; align-items: center; gap: 7px; margin: 0; font-size: 12px; color: var(--text-2); } .signin-expiry svg { flex: none; color: var(--text-3); } + .signin-copy-failed, .signin-copy-failed svg { color: var(--warn); } } /* ---- proving an SCM account by signing in (FR-11) ---- */ From dd1a10b636555cfd0fc17b3624c33a9a6b1dbb1f Mon Sep 17 00:00:00 2001 From: Artjoms Stukans Date: Fri, 25 Sep 2026 17:46:18 +0200 Subject: [PATCH 3/3] Keep sign-in copy feedback working under StrictMode and past expiry Second review of PR #177: - The mounted flag was cleared by the first unmount of React's development StrictMode cycle and never set again, so every copy result was dropped in the dev build. It is now set on every mount. - A copy failure never clears, and it was checked before expiry, so "type it from here" stayed on screen for a code that had run out. Expiry now wins. --- .../src/components/HarnessSignInCode.test.tsx | 23 +++++++++++++++++++ spire-ui/src/components/HarnessSignInCode.tsx | 15 ++++++++---- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/spire-ui/src/components/HarnessSignInCode.test.tsx b/spire-ui/src/components/HarnessSignInCode.test.tsx index 069d8edb..e34a02f1 100644 --- a/spire-ui/src/components/HarnessSignInCode.test.tsx +++ b/spire-ui/src/components/HarnessSignInCode.test.tsx @@ -1,3 +1,4 @@ +import { StrictMode } from 'react'; import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'; import { afterEach, beforeEach, expect, it, vi } from 'vitest'; import HarnessSignInCode from './HarnessSignInCode'; @@ -92,6 +93,28 @@ it('offers no link and no open button for an address that is not https', () => { expect(screen.getByRole('button', { name: 'Copy the code' })).toBeInTheDocument(); }); +// The dev build runs under StrictMode, which mounts, unmounts and mounts again. The result of a copy +// must still reach the screen afterwards (review of PR #177). +it('still reports a copy after StrictMode mounts the panel twice', async () => { + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Copy the code' })); + await act(async () => settle(true)); + + expect(screen.getByRole('status')).toHaveTextContent('Code copied.'); +}); + +// Asking the operator to type a code that has already run out is worse than saying it ran out. +it('says the code expired even after a copy failed', async () => { + const { rerender } = view(); + fireEvent.click(screen.getByRole('button', { name: 'Copy the code' })); + await act(async () => settle(false)); + + rerender(); + + expect(screen.getByRole('status')).toHaveTextContent('The code has expired.'); +}); + it('says when the code has run out', () => { view(null); diff --git a/spire-ui/src/components/HarnessSignInCode.tsx b/spire-ui/src/components/HarnessSignInCode.tsx index 8d5a773c..d74600df 100644 --- a/spire-ui/src/components/HarnessSignInCode.tsx +++ b/spire-ui/src/components/HarnessSignInCode.tsx @@ -31,7 +31,13 @@ export default function HarnessSignInCode({ verificationUri, userCode, remaining const [outcome, setOutcome] = useState(null); const timer = useRef | undefined>(undefined); const mounted = useRef(true); - useEffect(() => () => { mounted.current = false; clearTimeout(timer.current); }, []); + // Set on every mount, not only at creation: React's development StrictMode mounts, unmounts and + // mounts again, and a flag cleared by the first unmount would silence every copy result after it + // (review of PR #177). + useEffect(() => { + mounted.current = true; + return () => { mounted.current = false; clearTimeout(timer.current); }; + }, []); const safeUri = isHttps(verificationUri) ? verificationUri : null; function report(next: Outcome) { @@ -53,7 +59,7 @@ export default function HarnessSignInCode({ verificationUri, userCode, remaining void copying.then(done => report(done ? 'opened-copied' : 'opened-copy-failed')); } - const failed = outcome === 'copy-failed' || outcome === 'opened-copy-failed'; + const failed = remaining !== null && (outcome === 'copy-failed' || outcome === 'opened-copy-failed'); return
  1. @@ -100,13 +106,14 @@ function copy(text: string): Promise { } } -/** What the last click achieved, or else how long the code has left. */ +/** How long the code has left, unless the last click has something to say about a code still usable. */ function statusLine(outcome: Outcome, remaining: string | null): string { + // Expiry wins: a copy failure asks the operator to type the code, which is pointless once it is gone. + if (!remaining) return 'The code has expired.'; if (outcome === 'opened-copied') return 'Code copied. Paste it on the page that just opened.'; if (outcome === 'opened-copy-failed') return 'The page opened, but the code could not be copied. Type it from here.'; if (outcome === 'copied') return 'Code copied.'; if (outcome === 'copy-failed') return 'The code could not be copied. Type it from here.'; - if (!remaining) return 'The code has expired.'; return `The code expires in ${remaining}. This panel closes by itself once you approve.`; }