diff --git a/CHANGELOG.md b/CHANGELOG.md index b221be27..f0fe541b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -126,6 +126,25 @@ auto-generated per-PR notes; this file is the curated, human-readable history. product's own body size instead of to user-agent typography. ### Fixed +- The shared anchored-dialog focus trap (`openAnchoredDialog`, #439) now owns + **every** Tab/Shift+Tab transition instead of only wrapping at the first and + last declared focusable element. Native sequential-focus policy is not + portable: WebKit/Safari's default "Tab highlights every item on a webpage" + preference is off, so browser-delegated middle-of-list traversal could skip + checkboxes and buttons entirely — in the multi-select and time-range + popovers this meant keyboard focus could never reach Clear/Cancel/Apply, or + escape the modal trap altogether. The eligible set is still recomputed on + every press (disabled/hidden/removed controls drop out immediately, restored + ones rejoin without reopening the dialog), and a focused element no longer in + that set is treated as unfocused (Tab goes to the first eligible element, + Shift+Tab to the last). Verified in Chromium, Firefox, and WebKit. The + primitive also gained `handle.reclaimFocus()`: disabling the + currently-focused control (the multi-select's busy state disables every row + but Cancel while a Filter source is loading) natively blurs it out of the + dialog entirely, past the trap's dialog-scoped reach — a plain Tab press + right after would never re-enter the modal. Multi-select now calls it the + moment busy state lands, recovering focus onto Cancel instead of leaving it + stranded on the page behind the modal. - The Dashboard surface (#425) is brought onto the token system it was written before. Its **title** was `13px/600` — half a pixel above `--text-body`, a step the One-Pixel Floor forbids because nobody can see it — and now steps up by diff --git a/src/ui/multi-select-field.ts b/src/ui/multi-select-field.ts index 686deba5..0bd50841 100644 --- a/src/ui/multi-select-field.ts +++ b/src/ui/multi-select-field.ts @@ -439,8 +439,16 @@ export function buildMultiSelectField(opts: MultiSelectFieldOpts): MultiSelectFi for (const row of rows) row.cb.disabled = busy; clearBtn.disabled = busy; applyBtn.disabled = busy; - if (busy) liveEl.textContent = 'Loading options…'; - else applyFilter(); // restores the normal "N of M options" live text + if (busy) { + liveEl.textContent = 'Loading options…'; + // #439: disabling the currently-focused control (Search/an option/ + // Clear/Apply — every non-Cancel row above) natively blurs it OUT of + // the dialog, past the shared Tab trap's reach (dialog-scoped, so it + // only sees events targeting its own subtree). Reclaim focus onto + // Cancel — the one row this loop never disables — so it never lands + // on the page behind the modal. + handle.reclaimFocus(); + } else applyFilter(); // restores the normal "N of M options" live text } // The generic dialog chrome (#335): overlay + backdrop-close, the ARIA diff --git a/src/ui/popover.ts b/src/ui/popover.ts index 82e3b007..e501ad31 100644 --- a/src/ui/popover.ts +++ b/src/ui/popover.ts @@ -77,6 +77,16 @@ export interface AnchoredDialogHandle { * `skipFocus`. Idempotent — every dismissal path funnels here, and a second * call is a harmless no-op that never re-fires `onClose`. */ close(opts?: { skipFocus?: boolean }): void; + /** #439: call this immediately after mutating disabled/hidden state on the + * dialog's own content (e.g. a busy-state toggle) — the dialog-scoped Tab + * trap only runs on an event whose target is inside `dialog`, but DISABLING + * the currently-focused control natively blurs it out to `` (outside + * the trap's reach) rather than to another element, so an ordinary Tab + * press right after can leave the modal entirely undetected. Recomputes the + * eligible set and, if the active element is no longer in it (evicted, or + * never was — outside the dialog), moves focus to the first eligible + * element; a no-op when the active element is still eligible. */ + reclaimFocus(): void; } export function openAnchoredDialog(opts: AnchoredDialogOptions): AnchoredDialogHandle { @@ -102,20 +112,40 @@ export function openAnchoredDialog(opts: AnchoredDialogOptions): AnchoredDialogH return [...dialog.querySelectorAll('input, button')] .filter((el) => !el.closest('[hidden]') && !(el as HTMLInputElement | HTMLButtonElement).disabled); } + // #439: the primitive owns EVERY Tab/Shift+Tab transition, not just + // boundary wrapping — native sequential-focus policy is not portable + // (WebKit/Safari's default "Tab highlights every item" preference is OFF, + // so browser-delegated middle-of-list traversal can skip buttons and + // checkboxes entirely, stranding keyboard users before Apply). Current + // index in the freshly-recomputed set decides the next stop; an + // out-of-set active element (focus outside the dialog, or on a node the + // last recompute dropped via disable/hide/removal) is treated as if + // nothing were focused, per the issue's boundary rule. const onTabTrap = (e: KeyboardEvent): void => { if (e.key !== 'Tab') return; const items = focusableEls(); if (items.length === 0) return; // nothing to trap — let the browser handle it - const first = items[0]; - const last = items[items.length - 1]; - const activeEl = d.activeElement as HTMLElement | null; - if (e.shiftKey) { - if (!activeEl || activeEl === first || !dialog.contains(activeEl)) { e.preventDefault(); last.focus(); } - } else if (!activeEl || activeEl === last || !dialog.contains(activeEl)) { - e.preventDefault(); first.focus(); - } + const active = d.activeElement as HTMLElement | null; + const current = active ? items.indexOf(active) : -1; + const delta = e.shiftKey ? -1 : 1; + const next = current < 0 ? (e.shiftKey ? items.length - 1 : 0) : (current + delta + items.length) % items.length; + e.preventDefault(); + items[next].focus(); }; + // #439: the consumer-facing escape hatch — see the handle's own doc comment + // for why disabling the focused control natively evicts it past the + // dialog-scoped trap's reach. `items.includes(active)` (not an + // `dialog.contains` check alone) is what catches this: a just-disabled + // element stays `dialog.contains`-true but drops out of `focusableEls()`. + function reclaimFocus(): void { + const items = focusableEls(); + if (items.length === 0) return; + const active = d.activeElement as HTMLElement | null; + if (active && items.includes(active)) return; // still eligible — no-op + items[0].focus(); + } + // The single teardown funnel. Idempotent: the `open` guard means teardown + // `onClose` run exactly once no matter how many dismissal paths reach it. function close(closeOpts: { skipFocus?: boolean } = {}): void { @@ -160,5 +190,5 @@ export function openAnchoredDialog(opts: AnchoredDialogOptions): AnchoredDialogH const focusTarget = opts.initialFocus?.(dialog); if (focusTarget) focusTarget.focus(); - return { dialog, isOpen: () => open, close }; + return { dialog, isOpen: () => open, close, reclaimFocus }; } diff --git a/tests/e2e/multi-select.html b/tests/e2e/multi-select.html index 63bb8d7d..0fd3bf46 100644 --- a/tests/e2e/multi-select.html +++ b/tests/e2e/multi-select.html @@ -20,6 +20,7 @@ onApply: () => {}, onFallbackCommit: () => {}, }); document.querySelector('#root').append(field.el); + window.__field = field; // #439: lets specs drive updateStatus() to exercise the busy/focus-eviction path window.__ready = true; diff --git a/tests/e2e/multi-select.spec.js b/tests/e2e/multi-select.spec.js index 61960afd..1163cc15 100644 --- a/tests/e2e/multi-select.spec.js +++ b/tests/e2e/multi-select.spec.js @@ -1,5 +1,102 @@ import { test, expect } from '@playwright/test'; +test.describe('Multi-select keyboard traversal (#439)', () => { + test.beforeEach(async ({ page }) => { + await page.goto('/tests/e2e/multi-select.html'); + await page.waitForFunction(() => window.__ready === true); + }); + + // The shared anchored-dialog focus trap must own every Tab/Shift+Tab + // transition rather than delegate middle-of-list traversal to the browser + // (WebKit's default "Tab highlights every item" preference is OFF, so + // native traversal there can skip checkboxes/buttons entirely). This proves + // the real multi-select consumer sequence in every engine, not just the + // primitive in isolation (see popover.test.ts for the unit-level coverage). + test('Tab traverses the real control sequence in DOM order, reaches Apply, and wraps in both directions', async ({ page }) => { + await page.getByRole('button', { name: 'City filter, 0 selected' }).click(); + + const dialog = page.getByRole('dialog', { name: 'City options' }); + await expect(dialog).toBeVisible(); + + const search = page.getByPlaceholder('Search City options'); + const selectVisible = page.locator('.ms-select-all-cb'); + const optionCb = page.locator('.ms-option input[type="checkbox"]').first(); + const clear = page.getByRole('button', { name: 'Clear', exact: true }); + const cancel = page.getByRole('button', { name: 'Cancel', exact: true }); + const apply = page.getByRole('button', { name: 'Apply', exact: true }); + + // Initial focus lands on Search. + await expect(search).toBeFocused(); + + // Ordinary Tab visits every declared control, in order, ending on Apply. + for (const next of [selectVisible, optionCb, clear, cancel, apply]) { + await page.keyboard.press('Tab'); + await expect(next).toBeFocused(); + const inDialog = await dialog.evaluate((d) => d.contains(document.activeElement)); + expect(inDialog).toBe(true); + } + + // Apply reached via ordinary Tab visibly matches :focus-visible. + expect(await apply.evaluate((el) => el.matches(':focus-visible'))).toBe(true); + const applyOutline = await apply.evaluate((el) => getComputedStyle(el).outlineStyle); + expect(applyOutline).not.toBe('none'); + + // One more Tab wraps forward to Search; Shift+Tab wraps back to Apply. + await page.keyboard.press('Tab'); + await expect(search).toBeFocused(); + await page.keyboard.press('Shift+Tab'); + await expect(apply).toBeFocused(); + }); + + // Entering the busy state disables every row except Cancel. Disabling the + // CURRENTLY FOCUSED row natively blurs it out of the dialog entirely (to + // ) rather than to another element — the shared Tab trap is + // dialog-scoped, so an ordinary Tab press right after would never reach it, + // and focus could escape the modal. `reclaimFocus()` closes that gap by + // moving focus onto Cancel (the one row the busy toggle never disables) the + // moment the eviction happens. This must hold for every row that can be + // legitimately focused before the loading state lands. + test('a control disabled by the busy state is reclaimed onto Cancel, and traversal stays trapped', async ({ page }) => { + await page.getByRole('button', { name: 'City filter, 0 selected' }).click(); + const dialog = page.getByRole('dialog', { name: 'City options' }); + await expect(dialog).toBeVisible(); + + const search = page.getByPlaceholder('Search City options'); + const selectVisible = page.locator('.ms-select-all-cb'); + const optionCb = page.locator('.ms-option input[type="checkbox"]').first(); + const clear = page.getByRole('button', { name: 'Clear', exact: true }); + const cancel = page.getByRole('button', { name: 'Cancel', exact: true }); + const apply = page.getByRole('button', { name: 'Apply', exact: true }); + + for (const focusTarget of [search, selectVisible, optionCb, clear, apply]) { + // Reset to an interactive state before each focus + eviction round. + await page.evaluate(() => window.__field.updateStatus({ status: 'ready' })); + await focusTarget.focus(); + await expect(focusTarget).toBeFocused(); + + await page.evaluate(() => window.__field.updateStatus({ status: 'loading' })); + // Reclaimed onto Cancel — never left on the page behind the modal. + await expect(cancel).toBeFocused(); + const inDialog = await dialog.evaluate((d) => d.contains(document.activeElement)); + expect(inDialog).toBe(true); + } + + // While busy, every OTHER row is disabled (excluded from the eligible + // set), so Tab/Shift+Tab from Cancel — the only eligible row — cannot + // leave the dialog: the single-element case keeps it in place. + await page.keyboard.press('Tab'); + await expect(cancel).toBeFocused(); + await page.keyboard.press('Shift+Tab'); + await expect(cancel).toBeFocused(); + + // Re-enabling restores ordinary traversal through the freshly recomputed + // eligible set: from Cancel, the next declared row is Apply again. + await page.evaluate(() => window.__field.updateStatus({ status: 'ready' })); + await page.keyboard.press('Tab'); + await expect(apply).toBeFocused(); + }); +}); + test.describe('Multi-select Apply action states (#386)', () => { test.beforeEach(async ({ page }) => { await page.goto('/tests/e2e/multi-select.html'); @@ -27,10 +124,14 @@ test.describe('Multi-select Apply action states (#386)', () => { // Enter focus through keyboard navigation so :focus-visible is the // state under test; programmatic focus intentionally does not promise - // that modality in browsers. + // that modality in browsers. The shared focus trap (#439) now owns + // every transition deterministically, so a bounded loop still applies + // only as a defensive bound — it must terminate well before 10 presses. for (let i = 0; i < 10 && !(await apply.evaluate((el) => el === document.activeElement)); i++) { await page.keyboard.press('Tab'); } + await expect(apply).toBeFocused(); + expect(await apply.evaluate((el) => el.matches(':focus-visible'))).toBe(true); expect((await styles(apply)).outline).not.toBe('none'); const box = await apply.boundingBox(); diff --git a/tests/e2e/time-range.spec.js b/tests/e2e/time-range.spec.js index 8b53a2e7..f61c2bfc 100644 --- a/tests/e2e/time-range.spec.js +++ b/tests/e2e/time-range.spec.js @@ -172,6 +172,43 @@ test.describe('Dashboard compound time-range control', () => { await expect(page.locator('.trf-popover')).toHaveCount(0); }); + // #439: the shared anchored-dialog focus trap must own every Tab transition + // for THIS consumer too, including through a dynamically rendered set (the + // right column's constants list) whose membership changes while the dialog + // stays open — proving the trap recomputes on every press rather than + // caching the set it saw when the popover first mounted. + test('Tab reaches Apply through the dynamically rendered constants column, and a narrower re-render is honored by the very next Tab', async ({ page }) => { + await open(page); + // A genuine focus (not the initial programmatic one) activates To, + // rendering its constants as dynamic buttons in the right column. + await toBox(page).focus(); + await expect(page.locator('.trf-right-header')).toHaveText('To · constants'); + expect(await page.locator('.trf-const').count()).toBeGreaterThan(1); + + const dialog = page.getByRole('dialog', { name: 'Time range' }); + const toCaret = page.locator('.trf-caret[aria-label="Show constants for To"]'); + await page.keyboard.press('Tab'); // To input -> To's caret + await expect(toCaret).toBeFocused(); + await page.keyboard.press('Tab'); // caret -> first constant button + await expect(page.locator('.trf-const').first()).toBeFocused(); + expect(await dialog.evaluate((d) => d.contains(document.activeElement))).toBe(true); + + // Narrow the right column to a single constant — the trap must use the + // FRESHLY rendered set on the next press, not the one captured before. + // 'now' is the one constant whose value/label contains "now" (no "-Nx + // ago" token does), and it stays a valid, in-range bound so Apply stays + // enabled and reachable afterward. + await toBox(page).fill('now'); + await expect(page.locator('.trf-const')).toHaveCount(1); + await page.keyboard.press('Tab'); // To input -> To's caret + await page.keyboard.press('Tab'); // caret -> the ONE remaining constant + await expect(page.locator('.trf-const')).toBeFocused(); + await page.keyboard.press('Tab'); // -> Cancel + await expect(page.locator('.trf-btn:not(.trf-btn-primary)')).toBeFocused(); + await page.keyboard.press('Tab'); // -> Apply + await expect(applyBtn(page)).toBeFocused(); + }); + test('renders the control and popover in both light and dark themes', async ({ page }) => { const bg = async () => page.locator('.trf-popover').evaluate((n) => getComputedStyle(n).backgroundColor); diff --git a/tests/unit/multi-select-field.test.ts b/tests/unit/multi-select-field.test.ts index 37f75a0e..642f14e0 100644 --- a/tests/unit/multi-select-field.test.ts +++ b/tests/unit/multi-select-field.test.ts @@ -616,15 +616,22 @@ describe('buildMultiSelectField — Tab focus trap inside the dialog (#189 F3)', expect(document.activeElement).toBe(applyBtn()); }); - it('Tab/Shift-Tab from an element in the middle of the dialog does not trap (default behavior)', () => { + it('Tab/Shift-Tab from an element in the middle of the dialog moves deterministically (#439)', () => { + // The primitive owns every transition now, not just the boundary wrap — + // native browser traversal is not portable (WebKit/Safari can skip + // buttons/checkboxes with the default "Tab highlights every item" + // preference off). Declared order: search, select-visible, option + // checkboxes (a/b/c), Clear, Cancel, Apply. const handle = buildMultiSelectField(baseOpts()); document.body.appendChild(handle.el); click(triggerEl(handle.el)); selectAllCb().focus(); const forward = tab(popover()!); + expect(forward).toBe(false); // preventDefault-ed + expect(document.activeElement).toBe(optionCbs()[0]); // next: first option row const backward = tab(popover()!, true); - expect(forward).toBe(true); // not preventDefault-ed — the browser's own Tab order applies - expect(backward).toBe(true); + expect(backward).toBe(false); // preventDefault-ed + expect(document.activeElement).toBe(selectAllCb()); // back to select-visible }); }); @@ -647,6 +654,21 @@ describe('buildMultiSelectField — loading affordance while the popover is open expect(handle.isOpen()).toBe(true); }); + it('reclaims focus onto Cancel when entering busy state evicts the focused control (#439)', () => { + // Disabling the currently-focused control natively blurs it OUT of the + // dialog, past the shared Tab trap's dialog-scoped reach — verified for + // each row the busy toggle disables (every row except Cancel itself). + const handle = buildMultiSelectField(baseOpts({ value: ['a'], active: true })); + document.body.appendChild(handle.el); + click(triggerEl(handle.el)); + for (const focusTarget of [searchInput, selectAllCb, () => optionCbs()[0], clearBtn, applyBtn]) { + handle.updateStatus({ status: 'ready' }); // reset to interactive before each focus + focusTarget().focus(); + handle.updateStatus({ status: 'loading' }); + expect(document.activeElement).toBe(cancelBtn()); + } + }); + it('restores the checklist body and the normal live-region count once status returns to ready', () => { const handle = buildMultiSelectField(baseOpts({ value: ['a'], active: true })); document.body.appendChild(handle.el); diff --git a/tests/unit/popover.test.ts b/tests/unit/popover.test.ts index f226fb5a..ba068676 100644 --- a/tests/unit/popover.test.ts +++ b/tests/unit/popover.test.ts @@ -179,28 +179,12 @@ describe('openAnchoredDialog — dismissal paths', () => { }); }); -describe('openAnchoredDialog — Tab focus trap', () => { +describe('openAnchoredDialog — Tab focus trap (#439: primitive owns every transition)', () => { const tab = (target: EventTarget, shiftKey = false): boolean => key(target, 'Tab', shiftKey); - it('Tab from the last focusable wraps to the first', () => { - const { open, input, button } = setup(); - open(); - button.focus(); // last focusable - tab(dialogEl()!); - expect(document.activeElement).toBe(input); // first - }); - - it('Shift+Tab from the first focusable wraps to the last', () => { - const { open, input, button } = setup(); - open(); - input.focus(); // first focusable - tab(dialogEl()!, true); - expect(document.activeElement).toBe(button); // last - }); - - it('recomputes the focusable set on every press: disabling the last row moves the wrap target', () => { - // Three rows; disable the last so the trap must recompute rather than reuse - // a cached list. + // A three-row dialog (a, b, c) is enough to exercise first/middle/last for + // both directions without depending on `setup()`'s two-row fixture. + function setupThree(): { trigger: HTMLButtonElement; a: HTMLInputElement; b: HTMLButtonElement; c: HTMLButtonElement } { const trigger = h('button', {}) as HTMLButtonElement; document.body.appendChild(trigger); const a = h('input', { class: 'r-a' }) as HTMLInputElement; @@ -208,56 +192,239 @@ describe('openAnchoredDialog — Tab focus trap', () => { const c = h('button', { class: 'r-c' }) as HTMLButtonElement; const content = h('div', { style: { display: 'contents' } }, a, b, c); openAnchoredDialog({ document, trigger, ariaLabel: 'x', content, dialogClassName: 'pv-popover' }); - c.disabled = true; // now the last focusable is b - b.focus(); - tab(dialogEl()!); // from the (new) last → wraps to first (a) - expect(document.activeElement).toBe(a); + return { trigger, a, b, c }; + } + + describe('forward traversal', () => { + it('Tab from the first element focuses the second', () => { + const { a, b } = setupThree(); + a.focus(); + tab(dialogEl()!); + expect(document.activeElement).toBe(b); + }); + + it('Tab from a middle element focuses the next element', () => { + const { b, c } = setupThree(); + b.focus(); + tab(dialogEl()!); + expect(document.activeElement).toBe(c); + }); + + it('Tab from the last element wraps to the first', () => { + const { a, c } = setupThree(); + c.focus(); + tab(dialogEl()!); + expect(document.activeElement).toBe(a); + }); + + it('preventDefault()s every non-empty forward traversal', () => { + const { a } = setupThree(); + a.focus(); + const handled = tab(dialogEl()!); + expect(handled).toBe(false); // preventDefault-ed + }); }); - it('a hidden row is excluded from the trap', () => { - const trigger = h('button', {}) as HTMLButtonElement; - document.body.appendChild(trigger); - const a = h('input', { class: 'r-a' }) as HTMLInputElement; - const wrap = h('div', { hidden: true }, h('button', { class: 'r-b' })); - const c = h('button', { class: 'r-c' }) as HTMLButtonElement; - const content = h('div', { style: { display: 'contents' } }, a, wrap, c); - openAnchoredDialog({ document, trigger, ariaLabel: 'x', content, dialogClassName: 'pv-popover' }); - c.focus(); // last visible focusable - tab(dialogEl()!); - expect(document.activeElement).toBe(a); + describe('backward traversal', () => { + it('Shift+Tab from the last element focuses the previous element', () => { + const { b, c } = setupThree(); + c.focus(); + tab(dialogEl()!, true); + expect(document.activeElement).toBe(b); + }); + + it('Shift+Tab from a middle element focuses the previous element', () => { + const { a, b } = setupThree(); + b.focus(); + tab(dialogEl()!, true); + expect(document.activeElement).toBe(a); + }); + + it('Shift+Tab from the first element wraps to the last', () => { + const { a, c } = setupThree(); + a.focus(); + tab(dialogEl()!, true); + expect(document.activeElement).toBe(c); + }); + + it('preventDefault()s every non-empty backward traversal', () => { + const { c } = setupThree(); + c.focus(); + const handled = tab(dialogEl()!, true); + expect(handled).toBe(false); // preventDefault-ed + }); }); - it('Tab from a middle element does not trap (browser default order applies)', () => { + describe('dynamic eligibility', () => { + it('disabling a middle element is reflected on the very next press', () => { + const { a, b, c } = setupThree(); + b.disabled = true; // eligible set is now [a, c] + a.focus(); + tab(dialogEl()!); + expect(document.activeElement).toBe(c); + }); + + it('disabling the last element moves the wrap target', () => { + const { a, b, c } = setupThree(); + c.disabled = true; // eligible set is now [a, b] + b.focus(); + tab(dialogEl()!); + expect(document.activeElement).toBe(a); + }); + + it('an element inside a hidden ancestor is excluded', () => { + const trigger = h('button', {}) as HTMLButtonElement; + document.body.appendChild(trigger); + const a = h('input', { class: 'r-a' }) as HTMLInputElement; + const wrap = h('div', { hidden: true }, h('button', { class: 'r-b' })); + const c = h('button', { class: 'r-c' }) as HTMLButtonElement; + const content = h('div', { style: { display: 'contents' } }, a, wrap, c); + openAnchoredDialog({ document, trigger, ariaLabel: 'x', content, dialogClassName: 'pv-popover' }); + a.focus(); + tab(dialogEl()!); + expect(document.activeElement).toBe(c); + }); + + it('re-enabling a disabled element makes it eligible again without reopening the dialog', () => { + const { a, b, c } = setupThree(); + b.disabled = true; + a.focus(); + tab(dialogEl()!); // skips disabled b → c + expect(document.activeElement).toBe(c); + b.disabled = false; + tab(dialogEl()!); // c → wraps to a + expect(document.activeElement).toBe(a); + tab(dialogEl()!); // a → b, now eligible again + expect(document.activeElement).toBe(b); + }); + + it('a focused element disabled before the next Tab is treated as out of the set: Tab chooses the first', () => { + const { a, b } = setupThree(); + b.focus(); + b.disabled = true; + tab(dialogEl()!); + expect(document.activeElement).toBe(a); + }); + + it('a focused element disabled before the next Shift+Tab is treated as out of the set: Shift+Tab chooses the last', () => { + const { b, c } = setupThree(); + b.focus(); + b.disabled = true; + tab(dialogEl()!, true); + expect(document.activeElement).toBe(c); + }); + + it('a focused element removed before the next Tab is treated as out of the set: Tab chooses the first', () => { + const { a, b } = setupThree(); + b.focus(); + b.remove(); + tab(dialogEl()!); + expect(document.activeElement).toBe(a); + }); + }); + + describe('edge cases', () => { + it('a single eligible element retains focus on Tab', () => { + const trigger = h('button', {}) as HTMLButtonElement; + document.body.appendChild(trigger); + const only = h('button', { class: 'only' }) as HTMLButtonElement; + const content = h('div', { style: { display: 'contents' } }, only); + openAnchoredDialog({ document, trigger, ariaLabel: 'x', content, dialogClassName: 'pv-popover' }); + only.focus(); + tab(dialogEl()!); + expect(document.activeElement).toBe(only); + }); + + it('a single eligible element retains focus on Shift+Tab', () => { + const trigger = h('button', {}) as HTMLButtonElement; + document.body.appendChild(trigger); + const only = h('button', { class: 'only' }) as HTMLButtonElement; + const content = h('div', { style: { display: 'contents' } }, only); + openAnchoredDialog({ document, trigger, ariaLabel: 'x', content, dialogClassName: 'pv-popover' }); + only.focus(); + tab(dialogEl()!, true); + expect(document.activeElement).toBe(only); + }); + + it('Tab with no focusable content is a no-op (empty-set guard)', () => { + const trigger = h('button', {}) as HTMLButtonElement; + document.body.appendChild(trigger); + const content = h('div', {}, h('span', {}, 'no focusables here')); + openAnchoredDialog({ document, trigger, ariaLabel: 'x', content, dialogClassName: 'pv-popover' }); + const handled = tab(dialogEl()!); + expect(handled).toBe(true); // untrapped — browser handles it + }); + + it('a non-Tab key inside the dialog is ignored by the trap', () => { + const { open, button } = setup(); + open(); + button.focus(); + const handled = key(dialogEl()!, 'ArrowDown'); + expect(handled).toBe(true); // not preventDefault-ed + expect(document.activeElement).toBe(button); // focus unchanged + }); + + it('the listener is scoped to the live dialog and removed on close', () => { + const { open, input } = setup(); + const handle = open(); + handle.close(); + // Re-append the (now-detached) dialog-scoped input to the body and Tab + // on it directly — with the listener removed, nothing intercepts it, so + // the key event is left unhandled and focus is untouched. + document.body.appendChild(input); + input.focus(); + const handled = tab(input); + expect(handled).toBe(true); // no trap left listening + expect(document.activeElement).toBe(input); // no trap side effect moved focus + }); + }); +}); + +describe('openAnchoredDialog — reclaimFocus (#439)', () => { + // A caller that disables the currently-focused control (e.g. a busy-state + // toggle) natively evicts focus past the dialog-scoped Tab trap's reach — + // the trap only runs on an event whose target is inside `dialog`, and a + // disabled element can no longer be that target. `reclaimFocus()` is the + // escape hatch: called right after the mutation, it recovers focus onto + // the first still-eligible element. + + it('is a no-op when the active element is still eligible', () => { + const { open, input, button } = setup(); + const handle = open(); + button.focus(); + handle.reclaimFocus(); + expect(document.activeElement).toBe(button); + }); + + it('moves focus to the first eligible element when the active element was just disabled', () => { const trigger = h('button', {}) as HTMLButtonElement; document.body.appendChild(trigger); const a = h('input', { class: 'r-a' }) as HTMLInputElement; - const b = h('input', { class: 'r-b' }) as HTMLInputElement; + const b = h('button', { class: 'r-b' }) as HTMLButtonElement; const c = h('button', { class: 'r-c' }) as HTMLButtonElement; const content = h('div', { style: { display: 'contents' } }, a, b, c); - openAnchoredDialog({ document, trigger, ariaLabel: 'x', content, dialogClassName: 'pv-popover' }); - b.focus(); // middle - const forward = tab(dialogEl()!); - const backward = tab(dialogEl()!, true); - expect(forward).toBe(true); // not preventDefault-ed - expect(backward).toBe(true); + const handle = openAnchoredDialog({ document, trigger, ariaLabel: 'x', content, dialogClassName: 'pv-popover' }); + b.focus(); + b.disabled = true; // the mutation a busy-state toggle performs + handle.reclaimFocus(); + expect(document.activeElement).toBe(a); // first still-eligible row }); - it('a non-Tab key inside the dialog is ignored by the trap', () => { - const { open, button } = setup(); - open(); - button.focus(); - const handled = key(dialogEl()!, 'ArrowDown'); - expect(handled).toBe(true); // not preventDefault-ed - expect(document.activeElement).toBe(button); // focus unchanged + it('recovers focus that a caller mutation already moved outside the dialog entirely', () => { + const { trigger, open, input } = setup(); + const handle = open(); + trigger.focus(); // simulates the native evict-to-outside-the-dialog case + handle.reclaimFocus(); + expect(document.activeElement).toBe(input); // first eligible row inside the dialog }); - it('Tab with no focusable content is a no-op (empty-set guard)', () => { + it('is a no-op (safe) when nothing in the dialog is eligible', () => { const trigger = h('button', {}) as HTMLButtonElement; document.body.appendChild(trigger); - const content = h('div', {}, h('span', {}, 'no focusables here')); - openAnchoredDialog({ document, trigger, ariaLabel: 'x', content, dialogClassName: 'pv-popover' }); - const handled = tab(dialogEl()!); - expect(handled).toBe(true); // untrapped — browser handles it + const only = h('button', { class: 'only', disabled: true }) as HTMLButtonElement; + const content = h('div', { style: { display: 'contents' } }, only); + const handle = openAnchoredDialog({ document, trigger, ariaLabel: 'x', content, dialogClassName: 'pv-popover' }); + expect(() => handle.reclaimFocus()).not.toThrow(); }); });