diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f5fa2eab..0c972290a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,9 @@ of current runtime behavior, see [`docs/CURRENT-STATE.md`](docs/CURRENT-STATE.md ## [Unreleased] +- Extract application shortcuts and shader-parameter controls into reusable UI + components, preserving inputs and cleaning up listeners on rebuild/disposal. + - Extract adaptive panel rail placement and measurement into reusable UI modules, preserving obstacle clearance, responsive allocation, disclosure and scroll behavior. diff --git a/docs/CODE-BOUNDARIES.md b/docs/CODE-BOUNDARIES.md index 8b7a4c90e..2faaa2605 100644 --- a/docs/CODE-BOUNDARIES.md +++ b/docs/CODE-BOUNDARIES.md @@ -231,3 +231,15 @@ or observers; construction/import does no work. Scheduling, preference writes, share restoration and movement of controls between containers remain caller-owned. Existing helper imports from `cockpitMath.js` and `rightRailPolicy.js` remain compatible through re-exports. + +## Visual input + +`gods-eye-view/ui/input` exports `bindApplicationShortcuts` and +`createStyleParameters`. The shortcut binder owns one bubbling keydown listener +and receives the document, editing target and explicit action callbacks. +Parameter controls own only the supplied container's generated rows/listeners; +uniform metadata and read/write/change operations come from the caller. +Clearing permits reuse; destruction is final. Neither module imports the app, +renderer, persistence or services. The facade retains panel visibility, share +restore claims and render scheduling. Both controls are destroyed before the +facade's asynchronous teardown can yield. diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 359a9850f..eee4e1eba 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1,5 +1,18 @@ # God's Eye View Current State +## Application shortcuts and shader parameter controls + +`ui/input` supplies the bubbling application shortcut listener and generated +shader-parameter rows. Number/style keys, H/O/V/F/D/C actions, native form-control +typing and Escape behavior retain their existing mappings. Capture-phase +surfaces continue to arbitrate their own keyboard events first. + +The UI facade retains shader values, share-restore authority, render requests, +search dismissal, visibility and Cockpit portal policy. Parameter rows preserve +labels, bounds, steps and precision. Rebuilding rows removes their previous +listeners; disposal removes shortcuts and parameter listeners synchronously +before asynchronous application teardown. + ## Adaptive panel rail layout `ui/layout` supplies the left/right rail layout passes, natural-height diff --git a/package.json b/package.json index eb4358836..7d5e0b841 100644 --- a/package.json +++ b/package.json @@ -121,6 +121,7 @@ "./search": "./src/search/index.js", "./ui/panels": "./src/ui/panelDisclosure.js", "./ui/surfaces": "./src/ui/surfaceKeyboard.js", - "./ui/layout": "./src/ui/panelRails.js" + "./ui/layout": "./src/ui/panelRails.js", + "./ui/input": "./src/ui/visualInput.js" } } diff --git a/scripts/package-boundaries.json b/scripts/package-boundaries.json index 00bce3fd2..7b924ad83 100644 --- a/scripts/package-boundaries.json +++ b/scripts/package-boundaries.json @@ -285,5 +285,14 @@ "src/panelStackLayout.js" ], "external": [] + }, + "visual-input": { + "exports": ["./ui/input"], + "modules": [ + "src/ui/visualInput.js", + "src/ui/applicationShortcuts.js", + "src/ui/styleParameters.js" + ], + "external": [] } } diff --git a/scripts/qa-map-source-tray.mjs b/scripts/qa-map-source-tray.mjs index 2e13c6a3e..eccd7b4e6 100644 --- a/scripts/qa-map-source-tray.mjs +++ b/scripts/qa-map-source-tray.mjs @@ -569,7 +569,13 @@ try { trusted: event.isTrusted, detail: event.detail ?? null, style: event.target.closest('.style-btn')?.dataset.style || null, }); - for (const type of ['keydown', 'keyup', 'click']) grid.addEventListener(type, record, true); + // A claimed hold deliberately blurs the button. Repeats and release then + // target the document body, so observe Space beyond the original grid. + const recordSpace = (event) => { + if (event.code === 'Space') record(event); + }; + for (const type of ['keydown', 'keyup']) document.addEventListener(type, recordSpace, true); + grid.addEventListener('click', record, true); manager.setStyle = function (...args) { probe.activations.push(args[0]); return originalSetStyle.apply(this, args); @@ -609,7 +615,8 @@ try { manager.setStyle = originalSetStyle; voice.start = originalVoiceStart; observer.disconnect(); - for (const type of ['keydown', 'keyup', 'click']) grid.removeEventListener(type, record, true); + for (const type of ['keydown', 'keyup']) document.removeEventListener(type, recordSpace, true); + grid.removeEventListener('click', record, true); }; window.__qaStyleKeyProbe = probe; }); diff --git a/scripts/qa-visual-input.mjs b/scripts/qa-visual-input.mjs new file mode 100644 index 000000000..441325f8c --- /dev/null +++ b/scripts/qa-visual-input.mjs @@ -0,0 +1,103 @@ +#!/usr/bin/env node +/** Browser acceptance for application shortcuts and generated parameter controls. */ +import fs from 'node:fs'; +import puppeteer from 'puppeteer'; + +const browser = await puppeteer.launch({ + headless: true, + args: ['--no-sandbox', ...(process.platform === 'darwin' ? ['--use-angle=metal', '--enable-gpu'] : ['--use-gl=angle', '--use-angle=swiftshader'])], +}); +const page = await browser.newPage(); +const errors = []; +page.on('pageerror', (error) => errors.push(error.message)); +let failures = 0; +const check = (name, passed) => { + console.log(`[${passed ? 'PASS' : 'FAIL'}] ${name}`); + if (!passed) failures += 1; +}; +try { + await page.setViewport({ width: 1440, height: 900 }); + const url = new URL(process.env.QA_BASE_URL || 'http://127.0.0.1:4173'); + url.searchParams.set('welcome', '0'); + await page.goto(url.href, { waitUntil: 'domcontentloaded' }); + await page.waitForFunction(() => window.__godsEyeView?.styleManager?._applicationShortcuts, { timeout: 60_000 }); + await page.waitForFunction(() => { + const cover = document.getElementById('loading-screen'); + return cover?.classList.contains('hidden') && Number(getComputedStyle(cover).opacity) === 0; + }, { timeout: 60_000 }); + await page.evaluate(() => { + const manager = window.__godsEyeView.styleManager; + manager.setStyle('normal'); + document.activeElement?.blur(); + }); + await page.keyboard.press('2'); + check('native number shortcut selects CRT', await page.evaluate(() => window.__godsEyeView.styleManager.activeStyle === 'retro')); + const initial = await page.evaluate(() => { + const manager = window.__godsEyeView.styleManager; + manager.setPanelCollapsed('pp-toggles', false, { persist: false, syncShare: false }); + manager._sliderPanel.classList.remove('collapsed'); + const slider = manager._sliderContainer.querySelector('input'); + window.__qaOldSlider = slider; + const name = 'pixelation'; + window.__qaParameter = { name, before: manager.stages.retro.uniforms[name] }; + slider.focus(); + return { before: Number(slider.value), step: Number(slider.step), label: slider.getAttribute('aria-label') }; + }); + check('generated range has an accessible label and a numeric step', initial.label === 'Pixelation' && initial.step > 0); + await page.keyboard.press('ArrowRight'); + const changed = await page.evaluate(() => { + const slider = window.__qaOldSlider; + const manager = window.__godsEyeView.styleManager; + return { value: Number(slider.value), readout: slider.nextElementSibling.textContent, uniform: manager.stages.retro.uniforms[window.__qaParameter.name] }; + }); + check('native range input changes the shader and formatted readout', changed.value > initial.before && changed.uniform === changed.value && Number(changed.readout) === changed.value); + await page.keyboard.press('3'); + check('focused range retains native input instead of changing style', await page.evaluate(() => window.__godsEyeView.styleManager.activeStyle === 'retro')); + check('replacing a style revokes its detached slider', await page.evaluate(() => { + const manager = window.__godsEyeView.styleManager; + const before = manager.stages.retro.uniforms[window.__qaParameter.name]; + manager.setStyle('thermal'); + window.__qaOldSlider.value = '0'; + window.__qaOldSlider.dispatchEvent(new Event('input', { bubbles: true })); + return manager.stages.retro.uniforms[window.__qaParameter.name] === before; + })); + check('normal style clears rows; reopening restores current shader values', await page.evaluate(() => { + const manager = window.__godsEyeView.styleManager; + manager.setStyle('normal'); + const cleared = manager._sliderContainer.children.length === 0; + manager.setStyle('retro'); + return cleared && Number(manager._sliderContainer.querySelector('input').value) === manager.stages.retro.uniforms[window.__qaParameter.name]; + })); + const hudBefore = await page.evaluate(() => { + document.getElementById('hud-layout-select').focus(); + return window.__godsEyeView.styleManager.hud.visible; + }); + await page.keyboard.press('h'); + check('HUD select keeps its native typing and selection', await page.evaluate((visible) => document.activeElement?.id === 'hud-layout-select' && window.__godsEyeView.styleManager.hud.visible === visible, hudBefore)); + fs.mkdirSync('qa-shots/visual-input', { recursive: true }); + await page.screenshot({ path: 'qa-shots/visual-input/desktop.png' }); + await page.setViewport({ width: 620, height: 900 }); + await page.waitForFunction(() => document.getElementById('left-panel-stack')?.dataset.layoutMode === 'mobile'); + await page.screenshot({ path: 'qa-shots/visual-input/narrow.png' }); + await page.evaluate(() => { + const manager = window.__godsEyeView.styleManager; + window.__qaCurrentSlider = manager._sliderContainer.querySelector('input'); + manager._applicationShortcuts.destroy(); + manager._styleParameters.destroy(); + document.activeElement?.blur(); + }); + await page.keyboard.press('3'); + check('destroyed application shortcuts cannot change the selected style', await page.evaluate(() => window.__godsEyeView.styleManager.activeStyle === 'retro')); + check('destroyed parameter controls cannot write through a retained element', await page.evaluate(() => { + const manager = window.__godsEyeView.styleManager; + const before = manager.stages.retro.uniforms[window.__qaParameter.name]; + window.__qaCurrentSlider.value = '0'; + window.__qaCurrentSlider.dispatchEvent(new Event('input', { bubbles: true })); + return manager._sliderContainer.children.length === 0 && manager.stages.retro.uniforms[window.__qaParameter.name] === before; + })); + check('runtime has no uncaught browser errors', errors.length === 0); +} finally { + await browser.close(); +} +console.log(`RESULT: ${failures} failures`); +process.exitCode = failures ? 1 : 0; diff --git a/src/hudA11y.test.mjs b/src/hudA11y.test.mjs index c2ef2e6dc..bb468ff5d 100644 --- a/src/hudA11y.test.mjs +++ b/src/hudA11y.test.mjs @@ -3,7 +3,7 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; const html = readFileSync(new URL('../index.html', import.meta.url), 'utf8'); -const ui = readFileSync(new URL('./ui.js', import.meta.url), 'utf8'); +const parameters = readFileSync(new URL('./ui/styleParameters.js', import.meta.url), 'utf8'); // Focused markup guards; actual computed names are checked in Chromium. // Native labels and hidden inputs must not be treated as missing aria-labels. @@ -25,6 +25,6 @@ test('the first-run checkbox keeps its native visible label', () => { }); test('generated style sliders use the visible parameter label as their name', () => { - assert.match(ui, /label\.textContent\s*=\s*uMeta\.label;/); - assert.match(ui, /slider\.setAttribute\(['"]aria-label['"],\s*uMeta\.label\)/); + assert.match(parameters, /label\.textContent\s*=\s*metadata\.label;/); + assert.match(parameters, /slider\.setAttribute\(['"]aria-label['"],\s*metadata\.label\)/); }); diff --git a/src/sharelink.celestial.test.mjs b/src/sharelink.celestial.test.mjs index afac7c7c4..63d6a3d2b 100644 --- a/src/sharelink.celestial.test.mjs +++ b/src/sharelink.celestial.test.mjs @@ -417,8 +417,8 @@ test('newer visual, map, and individual panel actions suppress only their owned test('every explicit visual UI gesture claims restore authority before it mutates state', () => { const initUi = sourceBlock(' _initUI() {', ' _initMapStackControl() {'); const gestureRoutes = [ - ["if (e.key.toLowerCase() === 'h')", "if (e.key.toLowerCase() === 'o')", 'this.hud.toggle()', 'HUD hotkey'], - ["if (e.key.toLowerCase() === 'd')", "if (e.key.toLowerCase() === 'c')", 'cycleDetectionMode()', 'detection hotkey'], + ['toggleHud: () => {', 'toggleOrbit: () =>', 'this.hud.toggle()', 'HUD hotkey'], + ['cycleDetection: () => {', 'toggleCctv: () =>', 'cycleDetectionMode()', 'detection hotkey'], ['// Bloom toggle', '// Bloom intensity slider', 'this._setBloomEnabled(', 'bloom button'], ['// Bloom intensity slider', '// Sharpen toggle', 'this._setBloomIntensity(', 'bloom slider'], ['// Sharpen toggle', '// Scope mask', 'this._setSharpenEnabled(', 'sharpen button'], @@ -695,3 +695,13 @@ test('destroy cancels only a still-owned share flight and ignores delayed comple newerManager.destroy(); assert.equal(cancellations, 1, 'newer navigation must not be cancelled'); }); + + +test('visual input listeners are revoked before asynchronous UI teardown', () => { + const disposal = uiSource.slice(uiSource.indexOf(' async dispose() {')); + const firstAwait = disposal.indexOf('await '); + assert.ok(firstAwait > 0); + const synchronous = disposal.slice(0, firstAwait); + assert.match(synchronous, /this\._applicationShortcuts\?\.destroy\(\)/); + assert.match(synchronous, /this\._styleParameters\?\.destroy\(\)/); +}); diff --git a/src/ui.js b/src/ui.js index f78158775..31e49526c 100644 --- a/src/ui.js +++ b/src/ui.js @@ -1,3 +1,4 @@ +import { bindApplicationShortcuts, createStyleParameters } from './ui/visualInput.js'; import { layoutLeftPanelRail, layoutRightPanelRail } from './ui/panelRails.js'; import { bindPanelDisclosure, collapsePanelOnEscape, createHoverDisclosure } from './ui/panelDisclosure.js'; import * as Cesium from 'cesium'; @@ -3399,51 +3400,37 @@ export class StyleManager { btn.addEventListener('click', () => this.setStyle(btn.dataset.style)); }); - // Keyboard shortcuts: 1-7, H, Escape - this._globalKeydownHandler = (e) => { - // Ignore when interacting with a form control (except Escape). Global - // hotkeys ('1'-'7', 'h', 'o', 'v', 'd', 'c', 'f') otherwise fire while a - //