diff --git a/docs/reference/file-map.md b/docs/reference/file-map.md index a476db2..0747381 100644 --- a/docs/reference/file-map.md +++ b/docs/reference/file-map.md @@ -220,6 +220,18 @@ python tests/autosave_check.py # S2/B5 — does unsaved work survive? python tests/a11y_check.py # S10/S11/S12 — announce, legible, focus 22 checks ``` +Wave 5 added one more, for the same reason: + +```bash +python tests/stepper_check.py # A4/S9 — ten real buttons, keyboard operable 70 checks +``` + +It drives the rail with **real** key events over `Input.dispatchKeyEvent` rather than +`page.key()`, which dispatches a synthetic `KeyboardEvent` on `document`. That event +never reaches a listener bound to the rail and never triggers a button's native +Enter/Space activation, so a rail with no keyboard support at all would have reported a +clean pass — the same class of false green as `a11y_check.py`'s focus emulation. + Each tests what was *broken* rather than what is easy to assert. `aggregates_check.py` poisons localStorage and demands the dashboard still report the server's total; a test that only checked the totals were correct would have passed before `B4` was built. `a11y_check.py` runs @@ -239,9 +251,23 @@ one. | 8 | `pushState` | 0 | **2** (6 call sites via `wp-url.js`) | `T4.2` | | — | `outline: none` in stylesheets | 6 | **1**, with its replacement one rule above | `T3.4`, `T4.7` | | — | helper-text contrast, worst case | 3.01:1 | **4.56:1** | `T4.6` | +| 3 | `
` | 12 | **2** | `T5.1` | +| 1 | native dialogs app-wide | 79 | **78** | `T5.1` (2 removed), wave 4 (1 added) | -Metrics 1–4 and 6 (native dialogs, `
`, `.help-tip` badges) are wave 9's to move -and are unchanged. +Metrics 2, 4 and 6 (creator dialogs, ``, `.help-tip` badges) are wave 9's to +move and are unchanged. + +**Metric 3 after `T5.1`:** the two survivors are `wp-creation-index.html:383` +(`.cmt-overlay`) and `work-package-suite-app.js:1046` (the constraint-library row). Both +are wave 7 / wave 9 work. Nothing in the SOP wizard's markup carries a click handler on a +non-interactive element any more. + +**Metric 1 is noisier than it looks** — the command matches `alert(` inside a *comment* as +readily as inside code, and waves 3 and 4 left several comments explaining dialogs they +were removing. That is why the figure was 80 before `T5.1` rather than the 79 wave 0 +recorded. `T5.1` removed two real calls (`validateStep`'s three conditions collapsed to +one) and reworded its own comments so they do not inflate it. Logged as **BL-017**; +`T5.8` records a comment-stripped figure alongside the raw one. `server/smoketest.py` is the one that is **not** self-contained: it drives a server you point it at and aborts unless `WP_SMOKE_USER` and `WP_SMOKE_PASSWORD` are set, because every route diff --git a/docs/waves/backlog.md b/docs/waves/backlog.md index 3568808..d1520e6 100644 --- a/docs/waves/backlog.md +++ b/docs/waves/backlog.md @@ -264,6 +264,40 @@ deliberately deferred. - **Why not now:** out of `A5`'s stated scope, and `A4`/`S9` rebuild the stepper. - **Suggested wave or follow-up:** `T7.x`, with the stepper rebuild. +### BL-016 — Back to a URL with no `step` leaves the wizard on the step it was on + +- **Found during:** T5.1 +- **Where:** `html/work-package-suite-app.js`, the `WPUrl.onChange` handler +- **What:** the popstate handler reads `parseInt(state.step, 10)` and acts only when + the result is `>= 1`. Going from `?project=X&step=6` back to `?project=X` yields + `NaN`, so nothing happens and the wizard stays on step 6 while the address bar says + step 1. `T4.2`'s own probe never caught it because it moves between two URLs that + both carry a `step`, so the NaN branch was never taken. The fix is one expression — + treat a missing `step` as 1 — but it is `S3`'s code and `T4.2`'s done-whens, not + `A4`'s. +- **Why not now:** `CLAUDE.md` — do not fix what you notice in passing. `T5.1`'s rail + makes it easier to reach (ten keyboard-reachable buttons instead of ten chips) but + does not cause it, and folding an `S3` correction into an `A4` diff makes both + unreviewable. +- **Suggested wave or follow-up:** wave 9, with `C2`. `tests/stepper_check.py` pins the + current behaviour with a named check so the fix has a test waiting for it. + +### BL-017 — The native-dialog baseline metric counts prose + +- **Found during:** T5.1 +- **Where:** `docs/reference/file-map.md` §4, metric 1 +- **What:** the metric is `grep -ohE '\b(alert|confirm|prompt)\(' *.js *.html`, which + matches those words inside comments as readily as inside code. Four comments written + during `T5.1` — every one of them *about* removing a dialog — pushed the count from + 80 to 82 while the task was deleting two real calls. They were reworded, but the next + person to explain a dialog in a comment will move the number again, and `T5.8` and + wave 9 both have to drive it to a target. +- **Why not now:** redefining a wave 0 baseline mid-plan is worse than the noise; the + count is comparable to itself as long as everyone measures it the same way. +- **Suggested wave or follow-up:** `T5.8`, which owns the wizard's count, should record + a comment-stripped figure alongside the raw one and state both. Wave 9 sets the + target against the stripped figure. + ### BL-014 — Four controls fall back to the browser's default focus ring - **Found during:** T3.4 diff --git a/html/work-package-suite-app.js b/html/work-package-suite-app.js index a4e951e..71c87ce 100644 --- a/html/work-package-suite-app.js +++ b/html/work-package-suite-app.js @@ -436,8 +436,10 @@ function loadSampleData(){ track('sample_loaded'); alert('✓ Sample data loaded!\n\nNavigate through the SOP steps to see example values. You can edit or replace any field.'); - - // Switch to step 1 + + // Switch to step 1. Every step now holds sample content, so the rail marks them + // all as visited — otherwise a fully populated wizard shows ten unstarted steps. + for(let i = 1; i <= 10; i++) _visitedSteps.add(i); currentStep = 1; updateStepUI(); updateProjectDisplay(); @@ -468,6 +470,9 @@ function restoreSavedSOP(){ renderSequenceSteps(); renderSources(); repopulateForm(); + // A completed SOP has been all the way through: the rail marks every step done + // rather than showing ten unstarted steps over a finished configuration. + for(let i = 1; i <= 10; i++) _visitedSteps.add(i); if(typeof onSOPReady === 'function') onSOPReady(sop); } @@ -531,9 +536,6 @@ function switchTool(tool, opts){ document.querySelectorAll('.tool').forEach(t=>t.classList.remove('active')); document.getElementById(`tool-${contentTool}`).classList.add('active'); - // Reset step counter - document.getElementById('total-steps').textContent = (tool === 'sop') ? '10' : '—'; - if(contentTool === 'wp') renderWPTab(isDash); // Only go full-bleed when the creator is actually showing. With the SOP // incomplete this tab shows a short 'complete the SOP first' gate; making the @@ -1198,6 +1200,65 @@ function addSource(){ renderSources(); } +// ── STEP GATES (A4 / S9) ────────────────────────────────────────────────────── +// validateStep() guarded steps 1, 5 and 6 with three hand-written conditions and +// three hand-written messages. The rail needs the same answer for every step, not +// just the one you are standing on, so the list is data now: one source that both +// the guard and the rail read. A rail that offers a step the guard then refuses is +// worse than no rail. +// +// The predicate reads the DOM, not `state`. collectStepData() only ever collects +// the CURRENT step, so `state.governance.woformat` is stale for any step you have +// not visited — and the rail has to judge all ten. Every step's markup is in the +// document at all times (steps are shown and hidden with display), so the fields +// are always readable. +// +// T5.8 widens this to every step whose markup marks a field required, and replaces +// the native dialog with inline errors. The shape is chosen so that is an edit to this +// table rather than to the four functions below it. +const STEP_GATES = { + 1: {fields: ['proj_name', 'proj_number', 'proj_client', 'proj_division', 'proj_site'], + msg: 'Please complete all required fields: Project Name, Number, Client, Division, and Site Location.'}, + 5: {fields: ['gov_woformat'], msg: 'Please enter a Work Package Number Format.'}, + 6: {fields: ['qual_qcreq'], msg: 'Please select a QC requirement.'} +}; + +const STEP_LABELS = {1:'Project', 2:'Team', 3:'Sign-offs', 4:'WP types', 5:'Governance', + 6:'Quality', 7:'Platforms', 8:'Sequence', 9:'Constraints', 10:'Sources'}; + +// Steps you have actually been on. A stepper's tick means "done", and a step you +// have never opened is not done however its defaults happen to read — step 6's QC +// dropdown, for instance, is never empty. +// +// Deliberately NOT part of `state`: state is fingerprinted by sopIsDirty(), so +// recording navigation in it would make merely looking at a step count as an +// unsaved change and fire T4.3's unsaved-work guard on the way out. +const _visitedSteps = new Set([1]); + +function stepGateMet(n){ + const gate = STEP_GATES[n]; + if(!gate) return true; + return gate.fields.every(id => { + const el = document.getElementById(id); + return !!(el && String(el.value || '').trim()); + }); +} + +// What the rail may offer, stated as the guard already behaved rather than as +// something stricter. goToStep()/nextStep() have one rule: you may leave the step +// you are on once ITS required fields are filled. So the unreachable set is +// "everything ahead of here, while here is incomplete" — nothing more elaborate. +// +// The temptation is to lock every step after the first unmet gate anywhere in the +// wizard. That is a rule the guard does not enforce: with step 1 blank you can +// still jump 3 -> 5, because validateStep() only ever looks at the step you are +// standing on. A rail that showed a padlock the Next button then walked straight +// past would be the drift this table exists to prevent. +function stepReachable(n){ + if(n <= currentStep) return true; // going back is never gated (previousStep never was) + return stepGateMet(currentStep); +} + // ── STEP NAVIGATION ──────────────────────────────────────────────────────────── function goToStep(n, opts){ const fromUrl = !!(opts && opts.fromUrl); @@ -1205,7 +1266,13 @@ function goToStep(n, opts){ // the forward-navigation guard. validateStep() ends in alert() when a required // field is empty, which on a freshly-loaded deep link is ALWAYS - so a shared // link to step 3 opened a modal dialog before the page had finished booting. - if(!fromUrl && !validateStep(currentStep)) return; + // + // Neither is going BACKWARDS. previousStep() has never validated, so a rail that + // did would trap you on an incomplete step with no way out but the Back button — + // and the rail's whole point is that you can move around. + if(!fromUrl && n > currentStep && !validateStep(currentStep)) return; + railMessage(''); + _visitedSteps.add(n); currentStep = n; if(typeof WPAutosave !== 'undefined') WPAutosave.flush('step'); // S3: the step you are on survives a refresh and a shared link. @@ -1228,32 +1295,173 @@ if(typeof WPUrl !== 'undefined'){ function nextStep(){ if(!validateStep(currentStep)) return; if(currentStep < 10){ + railMessage(''); currentStep++; + _visitedSteps.add(currentStep); updateStepUI(); } } function previousStep(){ if(currentStep > 1){ + railMessage(''); currentStep--; + _visitedSteps.add(currentStep); updateStepUI(); } } +// ── THE STEP RAIL (A4 / S9) ─────────────────────────────────────────────────── +// One state per step, each told apart by a word and a marker shape as well as a +// colour (C1). The rail is static markup; this only re-labels it. +const _STEP_STATE_TEXT = {current: 'Current step', complete: 'Complete', locked: 'Locked', todo: ''}; + +function railMessage(text){ + const el = document.getElementById('step-rail-msg'); + if(!el) return; + // Re-setting identical text does not re-announce, and clearing then setting in + // the same tick is a no-op to most screen readers. Only touch it on a change. + if(el.textContent === text) return; + el.textContent = text; +} + +function stepRailState(n){ + if(n === currentStep) return 'current'; + if(!stepReachable(n)) return 'locked'; + // Visited AND valid. A step you have never opened is not "complete" however its + // defaults happen to read — step 6's QC dropdown, for one, is never empty. + if(_visitedSteps.has(n) && stepGateMet(n)) return 'complete'; + return 'todo'; +} + +function renderStepRail(){ + const list = document.getElementById('step-rail-list'); + if(!list) return; + list.querySelectorAll('.step-btn').forEach(btn => { + const n = parseInt(btn.dataset.step, 10); + const st = stepRailState(n); + btn.classList.toggle('is-current', st === 'current'); + btn.classList.toggle('is-complete', st === 'complete'); + btn.classList.toggle('is-locked', st === 'locked'); + if(st === 'current') btn.setAttribute('aria-current', 'step'); + else btn.removeAttribute('aria-current'); + // aria-disabled, not disabled: the button stays focusable so a keyboard user + // can reach it and be told what is in the way. `disabled` would remove it from + // the tab order and from most screen readers' element lists entirely. + if(st === 'locked'){ + btn.setAttribute('aria-disabled', 'true'); + btn.title = `Finish step ${currentStep}, ${STEP_LABELS[currentStep]}, first.`; + } else { + btn.removeAttribute('aria-disabled'); + btn.removeAttribute('title'); + } + const marker = btn.querySelector('.step-btn-marker'); + if(marker) marker.textContent = st === 'complete' ? '✓' : String(n); + const stateEl = btn.querySelector('.step-btn-state'); + if(stateEl) stateEl.textContent = _STEP_STATE_TEXT[st]; + }); + const pos = document.getElementById('step-rail-pos'); + if(pos) pos.textContent = currentStep; + const here = document.getElementById('step-rail-here'); + if(here) here.textContent = STEP_LABELS[currentStep] || ''; +} + +// Clicking a step you cannot reach yet says why, and puts the cursor in the field +// that is in the way. Saying "no" and leaving you where you were, with no idea +// which of five inputs was empty, is what the old native dialog did. +// +// The blocker is always the step you are standing on — that is the only thing +// stepReachable() gates on — so there is nowhere to navigate to. +function railBlockedClick(n){ + const gate = STEP_GATES[currentStep]; + railMessage(`Step ${n}, ${STEP_LABELS[n]}, is not available yet — finish step ${currentStep}, ${STEP_LABELS[currentStep]}, first.`); + if(!gate) return; + const missing = gate.fields + .map(id => document.getElementById(id)) + .find(el => el && !String(el.value || '').trim()); + if(missing){ + try { missing.scrollIntoView({block: 'center', behavior: 'smooth'}); } catch(e) { } + missing.focus(); + } +} + +function collapseRailIfNarrow(){ + const rail = document.getElementById('step-rail'); + const toggle = document.getElementById('step-rail-toggle'); + if(!rail || !toggle) return; + // Only when the disclosure is the live control. Above the breakpoint the toggle + // is display:none and the list is always shown, so collapsing would set a class + // nothing reads and leave aria-expanded describing a control nobody can see. + if(!toggle.offsetParent) return; + rail.classList.add('is-collapsed'); + toggle.setAttribute('aria-expanded', 'false'); +} + +// Every field any gate depends on, flattened once. Typing into one of these +// changes what the rail is allowed to offer, and a rail that only refreshes when +// you navigate is a rail that says "locked" over a form you have just filled in — +// which is worse than the strip it replaced, because that one at least lied +// consistently. +const _GATE_FIELD_IDS = new Set( + Object.keys(STEP_GATES).reduce((all, n) => all.concat(STEP_GATES[n].fields), [])); + +function railWatchField(e){ + const t = e.target; + if(!t || !t.id || !_GATE_FIELD_IDS.has(t.id)) return; + // The refusal message is about a state that no longer holds once the field it + // named has been filled. + if(stepGateMet(currentStep)) railMessage(''); + renderStepRail(); +} + +document.addEventListener('DOMContentLoaded', function(){ + const list = document.getElementById('step-rail-list'); + const toggle = document.getElementById('step-rail-toggle'); + document.addEventListener('input', railWatchField); + document.addEventListener('change', railWatchField); + if(toggle){ + toggle.addEventListener('click', function(){ + const rail = document.getElementById('step-rail'); + const open = rail.classList.toggle('is-collapsed') === false; + toggle.setAttribute('aria-expanded', open ? 'true' : 'false'); + }); + } + if(!list) return; + list.addEventListener('click', function(e){ + const btn = e.target.closest('.step-btn'); + if(!btn) return; + const n = parseInt(btn.dataset.step, 10); + if(btn.getAttribute('aria-disabled') === 'true'){ railBlockedClick(n); return; } + goToStep(n); + collapseRailIfNarrow(); + }); + // Enter and Space come free with - 1 / 10 +
@@ -63,21 +65,45 @@
- -
-
-
Project
-
Team
-
Sign-Offs
-
WP Types
-
Governance
-
Quality
-
Platforms
-
Sequence
-
Constraints
-
Sources
-
-
+ +
diff --git a/tests/stepper_check.py b/tests/stepper_check.py new file mode 100644 index 0000000..b50f18c --- /dev/null +++ b/tests/stepper_check.py @@ -0,0 +1,535 @@ +#!/usr/bin/env python3 +"""The SOP wizard's step rail — A4 / S9 / C1 (T5.1). + +The rail was ten div elements carrying onclick inside a horizontal scroller: not +in the tab order, unreachable by keyboard, and silent about progress. The one +thing on the page that did say where you were was a "1 / 10" pill in the app bar, +detached from the control it described. + +Every one of T5.1's done-whens is checked here, because nothing that already +existed could check any of them: + + 1. all ten steps are