Skip to content

Commit 38a34cc

Browse files
committed
test(qa): pin focus trap + restore, and de-flake the delete fixture (om-lv4q)
Adversarial review, F3+F4, plus coverage for F1/F2. F3: awaitRowCount(2) before reading rowsBefore. The count ran after awaiting only `thead th`, which renders before GET /api/supplies resolves — a fresh instance of the race tracked in om-yd1h. Wait for the settled count first so the read cannot see 0. F4: trashFor throws on a lookup miss. findIndex returns -1, and Playwright's .nth(-1) selects the LAST row rather than erroring, so a miss would silently click the wrong row's trash — invisible today only because SUPPLY_GONE happens to be last. Fail loud. New assertions: Tab / Tab / Shift+Tab from open all keep focus on a button inside the dialog (the trap), and closing returns focus to the trash button with title="Delete row" (the restore). The "Cancel is default-focused" check moved ahead of the Tab presses, since Tab is exactly what moves focus off Cancel.
1 parent 79cd8a6 commit 38a34cc

1 file changed

Lines changed: 36 additions & 10 deletions

File tree

qa/do-tab.e2e.mjs

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -567,15 +567,16 @@ try {
567567
const supplyRows = () => page.locator(supplyRowSel)
568568
const supplyItems = async () => (await api('/supplies')).map((s) => s.item)
569569
// The grid renders values into <input>s, so a row is found by input VALUE, not text.
570-
const trashFor = async (item) =>
571-
supplyRows()
572-
.nth(
573-
await supplyRows().evaluateAll(
574-
(rows, it) => rows.findIndex((r) => [...r.querySelectorAll('input')].some((i) => i.value === it)),
575-
item,
576-
),
577-
)
578-
.locator('button[title="Delete row"]')
570+
const trashFor = async (item) => {
571+
const idx = await supplyRows().evaluateAll(
572+
(rows, it) => rows.findIndex((r) => [...r.querySelectorAll('input')].some((i) => i.value === it)),
573+
item,
574+
)
575+
// findIndex → -1 on a miss, and Playwright's .nth(-1) selects the LAST row instead
576+
// of erroring — so a lookup miss would silently click the wrong row's trash. Fail loud.
577+
if (idx < 0) throw new Error(`trashFor: no supply row with an input value === ${JSON.stringify(item)}`)
578+
return supplyRows().nth(idx).locator('button[title="Delete row"]')
579+
}
579580
const confirmDlg = page.getByRole('dialog')
580581
const cancelBtn = () => confirmDlg.getByRole('button', { name: 'Cancel', exact: true })
581582
const awaitRowCount = (n) =>
@@ -587,6 +588,11 @@ try {
587588
)
588589
.catch(() => {}) // let the ok() below report the miss, with numbers
589590

591+
// Wait for the async GET /api/supplies to populate the grid BEFORE counting — the
592+
// `thead th` we waited on renders before load() resolves, so a bare count here would
593+
// race the fetch and could read 0 (the flake class tracked in om-yd1h). We seeded
594+
// exactly two rows into this otherwise-untouched grid, so 2 is the settled count.
595+
await awaitRowCount(2)
590596
const rowsBefore = await supplyRows().count()
591597
ok('fixture: both QA supply rows are in the grid',
592598
rowsBefore === 2 && (await supplyItems()).includes(SUPPLY_GONE), `${rowsBefore} row(s)`)
@@ -600,9 +606,25 @@ try {
600606
dialogText.includes(SUPPLY_GONE) && dialogText.includes('$13.37') && !dialogText.includes(SUPPLY_KEPT),
601607
JSON.stringify(dialogText))
602608

603-
// --- Cancel KEEPS the row ---
609+
// Default focus is on Cancel — asserted BEFORE any Tab moves it (a stray Enter, the
610+
// key already under your finger in a grid you commit cells with, then cancels).
604611
ok('Cancel is the default-focused control (so a stray Enter cancels, not deletes)',
605612
await cancelBtn().evaluate((el) => el === document.activeElement))
613+
614+
// --- Focus is trapped: aria-modal is a promise the keyboard must keep ---
615+
// Tab from Cancel (Cancel → Delete → wrap back to Cancel). Without the trap, the
616+
// second Tab would walk out into the live grid — onto another row's trash button or
617+
// a hidden input whose blur fires a write behind the "modal".
618+
const inDialog = () =>
619+
confirmDlg.evaluate((el) => el.contains(document.activeElement) && document.activeElement.tagName === 'BUTTON')
620+
await page.keyboard.press('Tab')
621+
ok('Tab keeps focus inside the dialog (1st)', await inDialog())
622+
await page.keyboard.press('Tab')
623+
ok('Tab wraps within the dialog rather than escaping to the grid (2nd)', await inDialog())
624+
await page.keyboard.press('Shift+Tab')
625+
ok('Shift+Tab also stays inside the dialog', await inDialog())
626+
627+
// --- Cancel KEEPS the row ---
606628
await cancelBtn().click()
607629
await confirmDlg.waitFor({ state: 'detached', timeout: 5000 })
608630
ok('Cancel keeps the row (grid)', (await supplyRows().count()) === rowsBefore)
@@ -615,6 +637,10 @@ try {
615637
await confirmDlg.waitFor({ state: 'detached', timeout: 5000 })
616638
ok('Escape keeps the row',
617639
(await supplyRows().count()) === rowsBefore && (await supplyItems()).includes(SUPPLY_GONE))
640+
// Closing returns focus to the trash button that opened it — not <body> — so the
641+
// keyboard user resumes where they were.
642+
ok('closing restores focus to the trash button that opened the dialog',
643+
await page.evaluate(() => document.activeElement?.getAttribute('title') === 'Delete row'))
618644

619645
// --- Confirm REMOVES it — from the grid AND from the database ---
620646
await (await trashFor(SUPPLY_GONE)).click()

0 commit comments

Comments
 (0)