mirror of
https://github.com/bilawalsidhu/gods-eye-view.git
synced 2026-11-06 11:24:00 +00:00
Focus the Map Source tray when it is actually visible, not 240ms later (#54)
`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) <[email protected]> Claude-Session: https://claude.ai/code/session_01Q6VRsPswjNtZfv4jzgkCUf
This commit is contained in:
1 parent
759652207f
commit
56d52e284f
2 files changed
+52
-9
No files matched your search
@@ -190,9 +190,32 @@ try {
|
|||||||
JSON.stringify(esriTileFailureFallback),
|
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.focus('#control-panel-toggle');
|
||||||
await page.keyboard.press('Enter');
|
await page.keyboard.press('Enter');
|
||||||
await new Promise((resolve) => setTimeout(resolve, 300));
|
await waitTray('true', 'photoreal');
|
||||||
const keyboardOpen = await page.evaluate(() => ({
|
const keyboardOpen = await page.evaluate(() => ({
|
||||||
expanded: document.getElementById('control-panel-toggle').getAttribute('aria-expanded'),
|
expanded: document.getElementById('control-panel-toggle').getAttribute('aria-expanded'),
|
||||||
activeStack: document.activeElement?.dataset?.stackId || null,
|
activeStack: document.activeElement?.dataset?.stackId || null,
|
||||||
@@ -204,7 +227,7 @@ try {
|
|||||||
);
|
);
|
||||||
|
|
||||||
await page.keyboard.press('Escape');
|
await page.keyboard.press('Escape');
|
||||||
await new Promise((resolve) => setTimeout(resolve, 40));
|
await waitTray('false', 'toggle');
|
||||||
const keyboardClose = await page.evaluate(() => ({
|
const keyboardClose = await page.evaluate(() => ({
|
||||||
expanded: document.getElementById('control-panel-toggle').getAttribute('aria-expanded'),
|
expanded: document.getElementById('control-panel-toggle').getAttribute('aria-expanded'),
|
||||||
activeId: document.activeElement?.id || null,
|
activeId: document.activeElement?.id || null,
|
||||||
@@ -216,7 +239,7 @@ try {
|
|||||||
);
|
);
|
||||||
|
|
||||||
await page.keyboard.press('Space');
|
await page.keyboard.press('Space');
|
||||||
await new Promise((resolve) => setTimeout(resolve, 300));
|
await waitTray('true', 'photoreal');
|
||||||
const spaceOpen = await page.evaluate(() => ({
|
const spaceOpen = await page.evaluate(() => ({
|
||||||
expanded: document.getElementById('control-panel-toggle').getAttribute('aria-expanded'),
|
expanded: document.getElementById('control-panel-toggle').getAttribute('aria-expanded'),
|
||||||
activeStack: document.activeElement?.dataset?.stackId || null,
|
activeStack: document.activeElement?.dataset?.stackId || null,
|
||||||
@@ -231,9 +254,11 @@ try {
|
|||||||
await page.keyboard.down('Enter');
|
await page.keyboard.down('Enter');
|
||||||
await new Promise((resolve) => setTimeout(resolve, 320));
|
await new Promise((resolve) => setTimeout(resolve, 320));
|
||||||
await page.keyboard.up('Enter');
|
await page.keyboard.up('Enter');
|
||||||
|
await waitTray('true', 'photoreal'); // let the hold's scheduled focus hand-off land
|
||||||
await page.keyboard.press('Escape');
|
await page.keyboard.press('Escape');
|
||||||
|
await waitTray('false', 'toggle');
|
||||||
await page.keyboard.press('Enter');
|
await page.keyboard.press('Enter');
|
||||||
await new Promise((resolve) => setTimeout(resolve, 300));
|
await waitTray('true', 'photoreal');
|
||||||
const longHoldRecovery = await page.evaluate(() => ({
|
const longHoldRecovery = await page.evaluate(() => ({
|
||||||
expanded: document.getElementById('control-panel-toggle').getAttribute('aria-expanded'),
|
expanded: document.getElementById('control-panel-toggle').getAttribute('aria-expanded'),
|
||||||
activeStack: document.activeElement?.dataset?.stackId || null,
|
activeStack: document.activeElement?.dataset?.stackId || null,
|
||||||
|
|||||||
@@ -4195,16 +4195,34 @@ export class StyleManager {
|
|||||||
});
|
});
|
||||||
|
|
||||||
const focusMapSource = () => {
|
const focusMapSource = () => {
|
||||||
if (panelId !== 'control-panel') return;
|
if (panelId !== 'control-panel') return false;
|
||||||
panelEl.querySelector('.map-stack-chip.active, .map-stack-chip')?.focus?.({ preventScroll: true });
|
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 = () => {
|
const scheduleMapSourceFocus = () => {
|
||||||
clearTimeout(disclosureFocusTimer);
|
clearTimeout(disclosureFocusTimer);
|
||||||
disclosureFocusTimer = window.setTimeout(() => {
|
let attempts = 0;
|
||||||
|
const attemptFocus = () => {
|
||||||
disclosureFocusTimer = null;
|
disclosureFocusTimer = null;
|
||||||
if (!panelEl.classList.contains('collapsed')) focusMapSource();
|
if (panelEl.classList.contains('collapsed')) return;
|
||||||
}, 240);
|
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 } = {}) => {
|
const toggleDisclosure = ({ focusSource = false } = {}) => {
|
||||||
|
|||||||
Reference in new issue
Block a user