From 56d52e284f3fba88d572262eb0db863073782c53 Mon Sep 17 00:00:00 2001 From: Tom-Neverwinter <3943634+Tom-Neverwinter@users.noreply.github.com> Date: Wed, 26 Aug 2026 14:22:46 -0500 Subject: [PATCH] Focus the Map Source tray when it is actually visible, not 240ms later (#54) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `qa:map-source-tray` failed on a clean checkout at "Enter opens the tray and hands focus to a Map Source tile". The gate was right: this is a real keyboard-accessibility bug, not a flaky assertion. Opening the tray schedules the focus hand-off to a Map Source tile 240ms later (scheduleMapSourceFocus). The tray itself fades in behind a 180ms `visibility` transition on .dock-popover-content, and a chip inside it cannot take focus until that transition lands. Those two timers race. When the machine is slow enough that the fade has not finished at the 240ms mark, `chip.focus()` is called on a `visibility: hidden` element — which is a SILENT no-op — and nothing retries. The tray sits open with keyboard focus stranded on the disclosure, unreachable. Instrumenting HTMLElement.prototype.focus during a failing run showed exactly that: focus() called once on the photoreal chip, the chip connected and not disabled, computed visibility `hidden`, activeElement unchanged. Fix the hand-off rather than the delay: - focusMapSource now reports whether focus actually landed (activeElement === chip) instead of assuming .focus() worked. - scheduleMapSourceFocus retries on a 30ms cadence until it lands, bounded to ~720ms past the first attempt so a permanently hidden tray cannot spin. Also stop the QA gate racing the same timers. Its keyboard checks slept a fixed 300ms and asserted focus had already moved; they now wait on controller truth (aria-expanded plus where focus landed), the way the Bing-switch check already did. That replaces four fixed sleeps, not just the long-Enter-hold one the issue named. The wait only settles state — the check() below each one is still the assertion, so a swallowed timeout surfaces as that check failing with real values rather than an opaque puppeteer error. Verified against a preview build: `qa:map-source-tray` goes from 3 failures to 20/20 passing, twice in a row. Unit suite unchanged (2674 passing). The #44 half of the original work is intentionally dropped: upstream's track-regression.mjs already fixed the ground-3d sampleHeight floor, and more thoroughly — it pins no absolute call count at all and adds a guard against a timed-out driver falsely looking flat. Closes #54. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Q6VRsPswjNtZfv4jzgkCUf --- scripts/qa-map-source-tray.mjs | 33 +++++++++++++++++++++++++++++---- src/ui.js | 28 +++++++++++++++++++++++----- 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/scripts/qa-map-source-tray.mjs b/scripts/qa-map-source-tray.mjs index 0071475a3..9826b952e 100644 --- a/scripts/qa-map-source-tray.mjs +++ b/scripts/qa-map-source-tray.mjs @@ -190,9 +190,32 @@ try { JSON.stringify(esriTileFailureFallback), ); + // The tray's state changes are timer-driven: opening schedules the focus + // hand-off to a Map Source tile 240ms later (scheduleMapSourceFocus in + // ui.js). A fixed sleep races that timer rather than observing it settle — + // 300ms left only 60ms of margin, which is what made these checks flake on + // a clean checkout (#54). Wait on controller truth instead, the way the + // Bing-switch check further down already does. + // The wait only settles state; the check() below it is still the assertion, + // so a swallowed timeout surfaces as that check failing with real values + // rather than as an opaque puppeteer error. + const waitTray = (wantExpanded, wantFocus) => page.waitForFunction( + (expanded, focus) => { + const toggle = document.getElementById('control-panel-toggle'); + if (toggle?.getAttribute('aria-expanded') !== expanded) return false; + if (!focus) return true; + const active = document.activeElement; + return focus === 'toggle' + ? active?.id === 'control-panel-toggle' + : active?.dataset?.stackId === focus; + }, + { timeout: 2000 }, + wantExpanded, wantFocus, + ).catch(() => {}); + await page.focus('#control-panel-toggle'); await page.keyboard.press('Enter'); - await new Promise((resolve) => setTimeout(resolve, 300)); + await waitTray('true', 'photoreal'); const keyboardOpen = await page.evaluate(() => ({ expanded: document.getElementById('control-panel-toggle').getAttribute('aria-expanded'), activeStack: document.activeElement?.dataset?.stackId || null, @@ -204,7 +227,7 @@ try { ); await page.keyboard.press('Escape'); - await new Promise((resolve) => setTimeout(resolve, 40)); + await waitTray('false', 'toggle'); const keyboardClose = await page.evaluate(() => ({ expanded: document.getElementById('control-panel-toggle').getAttribute('aria-expanded'), activeId: document.activeElement?.id || null, @@ -216,7 +239,7 @@ try { ); await page.keyboard.press('Space'); - await new Promise((resolve) => setTimeout(resolve, 300)); + await waitTray('true', 'photoreal'); const spaceOpen = await page.evaluate(() => ({ expanded: document.getElementById('control-panel-toggle').getAttribute('aria-expanded'), activeStack: document.activeElement?.dataset?.stackId || null, @@ -231,9 +254,11 @@ try { await page.keyboard.down('Enter'); await new Promise((resolve) => setTimeout(resolve, 320)); await page.keyboard.up('Enter'); + await waitTray('true', 'photoreal'); // let the hold's scheduled focus hand-off land await page.keyboard.press('Escape'); + await waitTray('false', 'toggle'); await page.keyboard.press('Enter'); - await new Promise((resolve) => setTimeout(resolve, 300)); + await waitTray('true', 'photoreal'); const longHoldRecovery = await page.evaluate(() => ({ expanded: document.getElementById('control-panel-toggle').getAttribute('aria-expanded'), activeStack: document.activeElement?.dataset?.stackId || null, diff --git a/src/ui.js b/src/ui.js index 668e752d4..ab9d7bd4c 100644 --- a/src/ui.js +++ b/src/ui.js @@ -4195,16 +4195,34 @@ export class StyleManager { }); const focusMapSource = () => { - if (panelId !== 'control-panel') return; - panelEl.querySelector('.map-stack-chip.active, .map-stack-chip')?.focus?.({ preventScroll: true }); + if (panelId !== 'control-panel') return false; + const chip = panelEl.querySelector('.map-stack-chip.active, .map-stack-chip'); + if (!chip?.focus) return false; + chip.focus({ preventScroll: true }); + // .focus() on a still-hidden element is a SILENT no-op, so the caller + // has to check whether focus actually landed rather than assume it did. + return document.activeElement === chip; }; + // The tray opens behind a 180ms `visibility` transition (.dock-popover-content + // in style.css), and a chip inside it cannot take focus until that lands. + // A single fixed delay therefore races the transition: when the machine is + // slow enough that the fade has not finished by the time the timer fires, + // focus() silently does nothing and the keyboard user is stranded on the + // disclosure with an open tray they cannot reach (#54). Retry on a short + // cadence until focus actually lands, bounded so a permanently hidden tray + // cannot spin. const scheduleMapSourceFocus = () => { clearTimeout(disclosureFocusTimer); - disclosureFocusTimer = window.setTimeout(() => { + let attempts = 0; + const attemptFocus = () => { disclosureFocusTimer = null; - if (!panelEl.classList.contains('collapsed')) focusMapSource(); - }, 240); + if (panelEl.classList.contains('collapsed')) return; + if (focusMapSource()) return; + if (++attempts > 24) return; // ~720ms past the first try, then give up + disclosureFocusTimer = window.setTimeout(attemptFocus, 30); + }; + disclosureFocusTimer = window.setTimeout(attemptFocus, 240); }; const toggleDisclosure = ({ focusSource = false } = {}) => {