Skip to content

Commit eab3a62

Browse files
Explorer: modal focus management and non-duplicated drag redraw
The two follow-ups worth taking from the review of the previous change. Both were listed in that commit as deliberately deferred. Modal focus management (WCAG 2.4.3, 4.1.2) The overlays had no dialog semantics and no focus handling: opening one left focus behind on the page, closing it dropped the user at the top of the document, and Tab walked straight out into the page behind the dialog. Added role="dialog" + aria-modal + an accessible name, focus into the dialog on open, restore to the trigger on close, and a Tab trap in both directions. aria-modal hides the background from assistive tech but does nothing about the tab order, which is why the trap is also needed. One non-obvious interaction: the previous change hid the closed overlays with `transition: opacity, visibility 0.25s`. Transitioning visibility symmetrically keeps the overlay non-visible for the whole 250 ms after opening, and a visibility:hidden element cannot take focus -- the focus() call was silently rejected, with focus() invoked but no focusin event. Visibility now flips immediately on open and is delayed only on close, so the fade-out still reads correctly. Drag redraw no longer runs twice per frame onSliderChange both schedules a coalesced redraw and keeps the animation loop alive, and the loop already redraws both canvases every frame -- so update() from the coalesced callback drew them again. Measured over a sustained drag: 1.68 canvas redraws per animation frame, i.e. ~40% wasted work on the one interaction path that the worker, mean-only and crossfade work never touched. The loop owns the canvases while it is running; the coalesced callback still performs the non-canvas work, since dropping it would freeze the readouts and the screen-reader summary mid-drag. 1.68 -> 1.08 redraws per frame, and 47 -> 65 frames completed in the same 2 s window. Not taken, with reasons, so this does not get re-litigated: - #scatter-canvas still has no text equivalent. It is the primary surface, but a 149-row table helps nobody; exposing it well means deciding what to surface (Pareto frontier summary? keyboard navigation between points?) and is a design task, not a small fix. - _reduceMotion is still sampled once at module load. Live-updating it is ~5 lines, but the only way to hit the gap is flipping the OS setting mid-session. Verification: lint 0, test-js 13/13, e2e 105 on macOS and 109 on Linux in the matching Playwright image, both Lighthouse profiles green with zero failing accessibility audits. The new modal-focus and drag-redraw tests were each confirmed to fail against the defect they pin.
1 parent 4fa4e07 commit eab3a62

5 files changed

Lines changed: 223 additions & 7 deletions

File tree

docs/index.html

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,9 @@ <h1>BOxCrete</h1>
126126

127127
<!-- About Modal Overlay -->
128128
<div class="about-overlay" id="about-overlay">
129-
<div class="about-modal">
129+
<div class="about-modal" role="dialog" aria-modal="true" aria-labelledby="about-modal-title">
130130
<button class="about-close" aria-label="Close" data-modal-close>&times;</button>
131-
<h2>About This Explorer</h2>
131+
<h2 id="about-modal-title">About This Explorer</h2>
132132
<p>This tool predicts concrete compressive strength using a Gaussian Process model trained on laboratory mortar and concrete specimens. Predictions reflect controlled lab conditions and <strong>may differ from field performance</strong> due to batching variability, curing environment, and aggregate properties.</p>
133133
<h3>How to Use</h3>
134134
<ul>
@@ -147,7 +147,7 @@ <h3>Papers</h3>
147147

148148
<!-- Video Modal Overlay -->
149149
<div class="video-overlay" id="video-overlay">
150-
<div class="video-modal">
150+
<div class="video-modal" role="dialog" aria-modal="true" aria-label="BOxCrete video">
151151
<button class="video-close" aria-label="Close" data-modal-close>&times;</button>
152152
<div class="video-frame-wrap">
153153
<iframe id="video-iframe"
@@ -454,23 +454,66 @@ <h3>References</h3>
454454
// Generic modal: handles close button ([data-modal-close]), backdrop click,
455455
// and Escape key. Optional onOpen/onClose hooks let callers run side effects
456456
// (e.g. lazy-loading the video iframe).
457+
// Focusable descendants, in DOM order. Recomputed per use because the
458+
// video modal's iframe is populated lazily on open.
459+
const FOCUSABLE =
460+
'a[href], button:not([disabled]), iframe, input:not([disabled]), ' +
461+
'select:not([disabled]), textarea:not([disabled]), [tabindex]:not([tabindex="-1"])';
462+
function focusablesIn(root) {
463+
return Array.from(root.querySelectorAll(FOCUSABLE))
464+
.filter((el) => el.offsetWidth > 0 || el.offsetHeight > 0 || el === document.activeElement);
465+
}
466+
457467
function setupModal(overlay, { onOpen, onClose } = {}) {
468+
const dialog = overlay.querySelector('[role="dialog"]') || overlay;
469+
// Element to hand focus back to on close. Without this, dismissing a
470+
// dialog drops the keyboard user back at the top of the document.
471+
let lastFocused = null;
472+
458473
const close = () => {
459474
overlay.classList.remove('visible');
460475
onClose?.();
476+
if (lastFocused && document.contains(lastFocused)) lastFocused.focus();
477+
lastFocused = null;
461478
};
462479
const open = () => {
480+
lastFocused = document.activeElement;
463481
onOpen?.();
464482
overlay.classList.add('visible');
483+
// Focus must land inside the dialog, or the user is told a dialog
484+
// opened while their focus is still behind it. The close button is the
485+
// conventional target and is always present.
486+
//
487+
// Works synchronously because .visible flips visibility with no
488+
// transition delay; a visibility:hidden element cannot take focus.
489+
const first = dialog.querySelector('[data-modal-close]') || focusablesIn(dialog)[0];
490+
first?.focus();
465491
};
492+
466493
for (const btn of overlay.querySelectorAll('[data-modal-close]')) {
467494
btn.addEventListener('click', close);
468495
}
469496
overlay.addEventListener('click', (e) => {
470497
if (e.target === overlay) close();
471498
});
472499
document.addEventListener('keydown', (e) => {
473-
if (e.key === 'Escape' && overlay.classList.contains('visible')) close();
500+
if (!overlay.classList.contains('visible')) return;
501+
if (e.key === 'Escape') { close(); return; }
502+
if (e.key !== 'Tab') return;
503+
// Trap: aria-modal hides the background from assistive tech, but it
504+
// does not stop Tab walking out into the page behind the dialog.
505+
const items = focusablesIn(dialog);
506+
if (items.length === 0) return;
507+
const first = items[0];
508+
const last = items[items.length - 1];
509+
const active = document.activeElement;
510+
if (e.shiftKey && (active === first || !dialog.contains(active))) {
511+
e.preventDefault();
512+
last.focus();
513+
} else if (!e.shiftKey && (active === last || !dialog.contains(active))) {
514+
e.preventDefault();
515+
first.focus();
516+
}
474517
});
475518
return { open, close };
476519
}

docs/style.css

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,12 +246,19 @@ body {
246246
the app's own in the heading outline. `visibility` still animates, so the
247247
fade is preserved. */
248248
visibility: hidden;
249-
transition: opacity 0.25s ease, visibility 0.25s ease;
249+
/* Delay the flip to hidden until the fade-out finishes, but flip to visible
250+
IMMEDIATELY on open (see .visible). Transitioning visibility symmetrically
251+
keeps the overlay non-visible for the whole 250 ms, and a
252+
visibility:hidden element cannot take focus -- the focus() call on open is
253+
silently rejected. */
254+
transition: opacity 0.25s ease, visibility 0s linear 0.25s;
250255
}
251256
.about-overlay.visible {
252257
opacity: 1;
253258
pointer-events: all;
254259
visibility: visible;
260+
/* No delay in this direction: focusable as soon as the class lands. */
261+
transition: opacity 0.25s ease, visibility 0s linear 0s;
255262
}
256263
.about-modal {
257264
background: var(--glass-bg);
@@ -346,12 +353,19 @@ body {
346353
the app's own in the heading outline. `visibility` still animates, so the
347354
fade is preserved. */
348355
visibility: hidden;
349-
transition: opacity 0.25s ease, visibility 0.25s ease;
356+
/* Delay the flip to hidden until the fade-out finishes, but flip to visible
357+
IMMEDIATELY on open (see .visible). Transitioning visibility symmetrically
358+
keeps the overlay non-visible for the whole 250 ms, and a
359+
visibility:hidden element cannot take focus -- the focus() call on open is
360+
silently rejected. */
361+
transition: opacity 0.25s ease, visibility 0s linear 0.25s;
350362
}
351363
.video-overlay.visible {
352364
opacity: 1;
353365
pointer-events: all;
354366
visibility: visible;
367+
/* No delay in this direction: focusable as soon as the class lands. */
368+
transition: opacity 0.25s ease, visibility 0s linear 0s;
355369
}
356370
.video-modal {
357371
background: var(--glass-bg);

docs/ui.mjs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -667,7 +667,21 @@ function requestRedraw() {
667667
if (_pendingRedraw !== null) return;
668668
_pendingRedraw = requestAnimationFrame(() => {
669669
_pendingRedraw = null;
670-
update();
670+
// When the animation loop is running it already redraws both canvases
671+
// every frame, so calling update() here draws them a second time.
672+
// Measured during a sustained drag: 1.68 canvas redraws per animation
673+
// frame, i.e. ~40% of the work on the one hot path that none of the
674+
// earlier optimisations touched.
675+
//
676+
// The loop only owns the CANVASES, so the rest of update() still has to
677+
// run -- dropping it would freeze the readouts and the screen-reader
678+
// summary mid-drag.
679+
if (animLoopId !== null) {
680+
updateReadouts();
681+
scheduleCurveSummary();
682+
} else {
683+
update();
684+
}
671685
updateMixInsight();
672686
checkExtrapolationWarning();
673687
});

test/e2e/drag-redraw.spec.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import { test, expect } from "@playwright/test";
2+
3+
/**
4+
* Drag redraw efficiency.
5+
*
6+
* `onSliderChange` schedules a coalesced redraw AND keeps the animation loop
7+
* alive. The loop already redraws both canvases every frame, so update() from
8+
* the coalesced callback drew them a second time — measured at 1.68 canvas
9+
* redraws per animation frame during a sustained drag, i.e. ~40% wasted work
10+
* on the one interaction path that the earlier transition/worker optimisations
11+
* never touched.
12+
*
13+
* The loop owns the canvases while it runs; the coalesced callback still has
14+
* to do the non-canvas work, so this also pins that the readouts keep updating.
15+
*/
16+
test("drag does not redraw the canvases twice per frame", async ({ page }, testInfo) => {
17+
test.skip(testInfo.project.name !== "desktop", "one project is enough");
18+
19+
await page.goto("/?test=1");
20+
await page.waitForFunction(() => (window as any).__test?.modelReady === true, null, {
21+
timeout: 30000,
22+
});
23+
await page.waitForTimeout(800);
24+
25+
const r = await page.evaluate(async () => {
26+
// setupHiDPICanvas assigns canvas.width on every draw, so counting
27+
// assignments counts redraws directly.
28+
const counts: Record<string, number> = { curve: 0, scatter: 0 };
29+
for (const [key, id] of [
30+
["curve", "curve-canvas"],
31+
["scatter", "scatter-canvas"],
32+
] as const) {
33+
const c = document.getElementById(id) as HTMLCanvasElement;
34+
let real = c.width;
35+
Object.defineProperty(c, "width", {
36+
get: () => real,
37+
set: (v) => { counts[key]++; real = v; },
38+
configurable: true,
39+
});
40+
}
41+
42+
let frames = 0;
43+
let running = true;
44+
const tick = () => { if (running) { frames++; requestAnimationFrame(tick); } };
45+
requestAnimationFrame(tick);
46+
47+
// Drive input faster than 60 Hz, as a touch drag or high-polling mouse does.
48+
const s = document.querySelector("#sliders input[type=range]") as HTMLInputElement;
49+
const min = parseFloat(s.min);
50+
const max = parseFloat(s.max);
51+
const t0 = performance.now();
52+
let events = 0;
53+
while (performance.now() - t0 < 1500) {
54+
s.value = String(min + (max - min) * ((events % 20) / 20));
55+
s.dispatchEvent(new Event("input", { bubbles: true }));
56+
events++;
57+
await new Promise((res) => setTimeout(res, 8));
58+
}
59+
running = false;
60+
await new Promise((res) => setTimeout(res, 300));
61+
62+
return { ...counts, frames, readout: document.getElementById("gwp-value")?.textContent };
63+
});
64+
65+
expect(r.frames, "no animation frames observed").toBeGreaterThan(10);
66+
67+
const perFrame = r.curve / r.frames;
68+
expect(
69+
perFrame,
70+
`${r.curve} curve redraws over ${r.frames} frames = ${perFrame.toFixed(2)}/frame ` +
71+
"(was 1.68 when the loop and the coalesced callback both drew)",
72+
).toBeLessThan(1.35);
73+
74+
// The coalesced callback must still do the non-canvas work.
75+
expect(r.readout, "readouts stopped updating during the drag").toBeTruthy();
76+
expect(r.readout).not.toBe("–");
77+
});

test/e2e/modal-focus.spec.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import { test, expect } from "@playwright/test";
2+
3+
/**
4+
* Modal focus management (WCAG 2.4.3 Focus Order, 4.1.2 Name/Role/Value).
5+
*
6+
* The overlays previously had no dialog semantics and no focus handling at
7+
* all: opening one left focus behind on the page, closing it dropped the user
8+
* at the top of the document, and Tab walked straight out into the page
9+
* behind the dialog.
10+
*/
11+
test.describe("modal focus management", () => {
12+
test.beforeEach(async ({ page }, testInfo) => {
13+
test.skip(testInfo.project.name !== "desktop", "About link is desktop-visible");
14+
await page.goto("/");
15+
await page.locator("#sliders input[type=range]").first().waitFor({ timeout: 15000 });
16+
});
17+
18+
test("the dialog is announced as a dialog and is labelled", async ({ page }) => {
19+
const dialog = page.locator("#about-overlay [role=dialog]");
20+
await expect(dialog).toHaveAttribute("aria-modal", "true");
21+
const labelledby = await dialog.getAttribute("aria-labelledby");
22+
expect(labelledby, "dialog needs an accessible name").toBeTruthy();
23+
await expect(page.locator(`#${labelledby}`)).toHaveText(/About/i);
24+
});
25+
26+
test("opening moves focus into the dialog", async ({ page }) => {
27+
await page.locator("#about-link").click();
28+
await expect(page.locator("#about-overlay")).toHaveClass(/visible/);
29+
const inside = await page.evaluate(() => {
30+
const d = document.querySelector("#about-overlay [role=dialog]")!;
31+
return d.contains(document.activeElement);
32+
});
33+
expect(inside, "focus should land inside the dialog, not stay behind it").toBe(true);
34+
});
35+
36+
test("closing restores focus to the trigger", async ({ page }) => {
37+
await page.locator("#about-link").click();
38+
await expect(page.locator("#about-overlay")).toHaveClass(/visible/);
39+
await page.keyboard.press("Escape");
40+
await expect(page.locator("#about-overlay")).not.toHaveClass(/visible/);
41+
const id = await page.evaluate(() => document.activeElement?.id);
42+
expect(id, "focus should return to the element that opened the dialog").toBe("about-link");
43+
});
44+
45+
test("Tab is trapped inside the open dialog", async ({ page }) => {
46+
await page.locator("#about-link").click();
47+
await expect(page.locator("#about-overlay")).toHaveClass(/visible/);
48+
49+
// Walk well past the number of focusables; focus must never escape.
50+
for (let i = 0; i < 12; i++) {
51+
await page.keyboard.press("Tab");
52+
const inside = await page.evaluate(() => {
53+
const d = document.querySelector("#about-overlay [role=dialog]")!;
54+
return d.contains(document.activeElement);
55+
});
56+
expect(inside, `focus escaped the dialog after ${i + 1} Tab presses`).toBe(true);
57+
}
58+
// And backwards.
59+
for (let i = 0; i < 4; i++) {
60+
await page.keyboard.press("Shift+Tab");
61+
const inside = await page.evaluate(() => {
62+
const d = document.querySelector("#about-overlay [role=dialog]")!;
63+
return d.contains(document.activeElement);
64+
});
65+
expect(inside, `focus escaped backwards after ${i + 1} Shift+Tab presses`).toBe(true);
66+
}
67+
});
68+
});

0 commit comments

Comments
 (0)