n.siegfried/Project-SDE-WP-Suite
3
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

T5.8 - S1 (wizard): errors at the field, and the last dialog is gone

Browse Source 
S1 has two halves and they are easy to conflate.

One is that validation was a native dialog: "Please complete all required
fields: Project Name, Number, Client, Division, and Site Location." names five
fields at once, highlights none of them, and scrolls nowhere. The other is that
validateStep guarded steps 1, 5 and 6 while the MARKUP marks required fields on
1, 3, 5, 6 and 7 - so two steps' asterisks meant nothing at all, which is worse
than no asterisk.

Both fixed, and the second is the one worth measuring: the probe reads the
required-field list out of work-package-suite.html rather than out of STEP_GATES,
because a probe that read the table would agree with whatever the table says and
prove nothing. Both notations count - an asterisk in a <label>, and the red span
beside step 3's role titles.

Now: an error per FIELD, rendered at it, associated by aria-describedby, marked
aria-invalid, announced through role="alert", and the first one focused and
scrolled into view. The error boxes are BUILT from the gate table rather than
written into the markup twelve times - adding a required field is one row, and
its error element, its association and its announcement all follow. A
markup-side error box somebody forgets to add is an error nobody ever sees.

An error clears as you type rather than on the next submit. An error still
showing over a field you have just corrected teaches people to ignore errors.

And nothing paints a step you have not tried to leave: the rail asks
stepGateMet(), which reads the same fields and marks none of them.

The thirteen dialogs

  Every one is now the thing it should have been - an error at the field it is
  about, or an announcement in a live region with the role T4.5 established:
  errors interrupt, confirmations do not.

  A dialog is not merely ugly. It blocks the page, cannot be placed or styled, a
  screen reader can present it only as a modal interruption, and it is one OK
  button whatever it says - so "sample data loaded" and "you cannot do that"
  arrived identically.

  Two deserve naming. The empty-comment alert became an inline error on the
  feedback textarea. And showAnalytics() was a confirm() carrying the entire
  usage summary as its body - a wall of text in a dialog whose only dismissal
  was also the download button. The summary is the useful part, so it is shown,
  with the download offered as an action beside it. That function has no caller
  in the wizard's markup (the "Usage data" button is the creator's, calling the
  creator's own showAnalytics), and it was converted rather than deleted:
  deleting a feature is not what this task was asked to do, and its dialog
  counted toward the number this task has to drive to zero.

  html/work-package-suite.html       #wp-toast, an error box on the textarea
  html/work-package-suite-app.js     STEP_GATES widened; per-field messages;
                                     ensureErrorBoxes; wizardToast; 13 removals
  html/work-package-suite-styles.css .wp-toast
  tests/validation_check.py          new - 81 checks
  tests/stepper_check.py             its "the alert T5.8 still owns" check now
                                     asserts the opposite, by name

Done when
  [x] every step with required fields validates them - 5 steps, from the markup
  [x] each error renders at its field and is associated via aria-describedby
  [x] submitting an invalid step focuses AND scrolls to the first error
      (scroll checked by bounding box, not by trusting scrollIntoView)
  [x] errors announce to screen readers
  [x] the wizard's native dialog count is 0

The count, recorded both ways because BL-017 says the metric counts prose

  work-package-suite-app.js   0 raw, 0 with comments stripped
  app-wide                   64 raw, 64 stripped, against wave 0's 79
                             wp-creation-app.js 43 (wave 7), users.js 10 and
                             admin.js 6 and index.html 5 (wave 9)

The wizard contributes none of what is left, which the probe asserts rather
than leaving to the total.

Verified one at a time
  validation_check 81/81  new
  stepper_check    71/71
  sections_check   88/88
  browser_check    71/71
  a11y             22/22
  url_state        23/23
  autosave         34/34
  locations        58/58
  aggregates       16/16
  pipeline         43/43
  launcher         58/58
  f_items          F1-F5 FIXED, F6 REPRODUCES (T7.2)

No colour literal added. The toast says "error" by a red rule AND by staying
until dismissed where a confirmation times out - two channels, not one (C1).

Question for the PR, per CLAUDE.md: step 3's two role TITLES are validated
because the markup marks them required, but the two role NAME pickers beside
them are not marked and so are not gated. A sign-off role with nobody in it is
arguably the more useful thing to catch. The markup is what was built to; if the
intent was the names, that is two rows in STEP_GATES.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Nick Siegfried
2026-08-16 12:30:32 -05:00
parent 3bc3abe209
commit 55caefb099
6 changed files with 706 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
tests/stepper_check.py
View File

@@ -343,13 +343,14 @@ def run(page, base, tok):
[r["step"] for r in rows if r["ariaDisabled"] == "true"])
chk("...and the guard refuses the same move",
page.eval("(() => { const n = currentStep; goToStep(4); return currentStep === n; })()"))
# That call went through validateStep() directly rather than through the rail,
# so it hit the wizard's surviving alert. Recorded here so the fact is on the
# page rather than hidden, and cleared so check 8 below still means something.
# Until T5.8 this call raised the wizard's surviving alert, and this check
# recorded that rather than hiding it. T5.8 replaced it with an error at the
# field, so the same call now refuses silently and marks the field instead.
guard_dialogs = json.loads(page.eval("JSON.stringify(window.__dialogs||[])"))
chk("...via the wizard's alert, which T5.8 still owns",
len(guard_dialogs) == 1 and guard_dialogs[0][0] == "alert", guard_dialogs)
page.eval("window.__dialogs = []")
chk("...with no dialog at all, since T5.8", not guard_dialogs, guard_dialogs)
chk("...and an error at the field that is in the way",
bool((page.eval("(document.getElementById('proj_site_err')||{}).textContent||''")).strip()),
page.eval("(document.getElementById('proj_site_err')||{}).textContent||''"))
chk("...while going BACKWARDS is still allowed, so nobody is trapped",
page.eval("(() => { currentStep = 4; updateStepUI(); goToStep(2); return currentStep; })()") == 2,
page.eval("currentStep"))

419
tests/validation_check.py Normal file
View File

@@ -0,0 +1,419 @@
#!/usr/bin/env python3
"""Inline validation in the SOP wizard — S1, wizard half (T5.8).
The defect S1 records has two halves and they are easy to conflate. One is that
validation was a native dialog. The other is that it guarded three steps while
the markup marked required fields on five — so two steps' asterisks meant
nothing at all, which is worse than no asterisk.
1. every step with required fields validates them (1, 3, 5, 6, 7 — counted
from the markup, not from the old guard)
2. each error renders at its field and is associated via aria-describedby
3. submitting an invalid step focuses and scrolls to the first error
4. errors announce to screen readers
5. the wizard's native dialog count is 0
Check 5 is measured two ways, because the wave 0 metric counts the word
`alert(` inside a comment as readily as inside code (BL-017). Both figures are
recorded: raw, and with comments stripped.
Every dialog is stubbed before anything is driven, so a survivor is caught and
reported rather than hanging the session.
Exit 0 all passed, 1 a failure, 2 could not run.
"""
import json
import os
import re
import subprocess
import sys
import tempfile
import time
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
import cdp # noqa: E402
from browser_check import seed, start_server, chk, _PASS, _FAIL, _c # noqa: E402
ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
STUB = """
window.__dialogs = [];
window.alert = function (m) { window.__dialogs.push(['alert', String(m)]); };
window.confirm = function (m) { window.__dialogs.push(['confirm', String(m)]); return false; };
window.prompt = function (m) { window.__dialogs.push(['prompt', String(m)]); return null; };
true
"""
# Read out of the markup, not out of the app's gate table: the S1 defect is
# precisely that the two disagreed, so a probe that read the table would agree
# with whatever the table says and prove nothing.
STEP_FIELDS = {
1: ["proj_name", "proj_number", "proj_client", "proj_division", "proj_site"],
3: ["role_super_title", "role_foreman_title"],
5: ["gov_woformat", "gov_discmode"],
6: ["qual_qcreq"],
7: ["plat_tracking", "plat_commissioning"],
}
NO_REQUIRED = [2, 4, 8, 9, 10, 11, 12]
def settle(seconds=1.4):
time.sleep(seconds)
def ascii_(v):
"""Windows consoles are cp1252, and a failure message carrying the toast's
close glyph crashes the reporter instead of reporting the failure. Losing the
glyph costs nothing; losing the diagnosis costs the run."""
return str(v).encode("ascii", "replace").decode("ascii")
def open_wizard(page, base, tok, query="?project=projA"):
page.clear_cookies()
page.set_cookie("wp_session", tok["root"])
page.goto(base + "/work-package-suite.html" + query)
settle(2.0)
page.eval(STUB)
def required_fields_in_markup():
"""Which steps mark a field required, straight from work-package-suite.html.
Two notations: an asterisk in the <label> text, and a red <span>*</span> beside
an input (step 3's role titles). Both count — the markup was making a promise
either way."""
src = open(os.path.join(ROOT, "html", "work-package-suite.html"), encoding="utf-8").read()
bounds = [(m.start(), int(m.group(1))) for m in re.finditer(r'id="sop-step-(\d+)"', src)]
bounds.append((len(src), None))
marked = {}
for k in range(len(bounds) - 1):
start, n = bounds[k]
block = src[start:bounds[k + 1][0]]
starred = len(re.findall(r"<label[^>]*>[^<]*\*", block))
starred += len(re.findall(r'var\(--danger\)[^>]*>\*<', block))
if starred:
marked[n] = starred
return marked
def field_state(page, ids):
return json.loads(page.eval("""JSON.stringify(%s.map(id => {
const el = document.getElementById(id);
const box = document.getElementById(id + '_err');
return {
id: id,
exists: !!el,
invalid: el ? el.getAttribute('aria-invalid') : null,
describedby: el ? (el.getAttribute('aria-describedby') || '') : '',
error: box ? (box.textContent || '').trim() : null,
role: box ? box.getAttribute('role') : null,
};
}))""" % json.dumps(ids)))
def blank(page, ids):
page.eval("""(() => {
for (const id of %s) {
const el = document.getElementById(id);
if (!el) continue;
if (el.tagName === 'SELECT') {
if (![...el.options].some(o => o.value === '')) el.add(new Option('', ''), 0);
el.value = '';
} else { el.value = ''; }
el.dispatchEvent(new Event('input', {bubbles: true}));
el.dispatchEvent(new Event('change', {bubbles: true}));
}
return true;
})()""" % json.dumps(ids))
def run(page, base, tok):
print("\n1. every step the MARKUP marks required is validated")
marked = required_fields_in_markup()
chk("the markup marks required fields on 5 steps",
sorted(marked) == sorted(STEP_FIELDS), sorted(marked))
open_wizard(page, base, tok)
chk("the page boots with no JavaScript errors", not page.js_errors(), page.js_errors())
gated = json.loads(page.eval(
"JSON.stringify(Object.keys(STEP_GATES).map(Number).sort((a,b)=>a-b))"))
chk("...and the guard now covers exactly those 5, not the old 3",
gated == sorted(STEP_FIELDS), gated)
for n in sorted(STEP_FIELDS):
got = json.loads(page.eval("JSON.stringify(stepGateFields(%d))" % n))
chk("step %-2d gates on %d field(s), the ones the markup marks" % (n, len(STEP_FIELDS[n])),
sorted(got) == sorted(STEP_FIELDS[n]), got)
for n in NO_REQUIRED:
chk("step %-2d has no required fields and gates on none" % n,
page.eval("JSON.stringify(stepGateFields(%d))" % n) == "[]",
page.eval("JSON.stringify(stepGateFields(%d))" % n))
print("\n2 + 3 + 4. an invalid step marks its fields, focuses the first, announces")
for n in sorted(STEP_FIELDS):
ids = STEP_FIELDS[n]
page.eval("goToStep(%d, {fromUrl:true}); updateStepUI()" % n)
settle(0.5)
blank(page, ids)
ok = page.eval("validateStep(%d)" % n)
settle(0.4)
st = field_state(page, ids)
chk("step %-2d refuses while its required fields are empty" % n, ok is False, ok)
chk(" ...an error at every one of them",
all(f["error"] for f in st), [f["id"] for f in st if not f["error"]])
chk(" ...each naming its own field, not one message for the step",
len({f["error"] for f in st}) == len(st), [f["error"] for f in st])
chk(" ...associated through aria-describedby",
all(f["id"] + "_err" in f["describedby"] for f in st),
[(f["id"], f["describedby"]) for f in st])
chk(" ...marked aria-invalid", all(f["invalid"] == "true" for f in st),
[(f["id"], f["invalid"]) for f in st])
chk(" ...announced through a live region",
all(f["role"] == "alert" for f in st), [(f["id"], f["role"]) for f in st])
chk(" ...and focus is on the FIRST empty one",
page.eval("(document.activeElement||{}).id") == ids[0],
page.eval("(document.activeElement||{}).id"))
print("\n an error clears as the field is corrected, not on the next submit")
page.eval("goToStep(1, {fromUrl:true}); updateStepUI()")
settle(0.4)
blank(page, STEP_FIELDS[1])
page.eval("validateStep(1)")
settle(0.3)
chk("the error is showing to begin with",
bool(field_state(page, ["proj_name"])[0]["error"]))
page.eval("""(() => {
const el = document.getElementById('proj_name');
el.value = 'Typed';
el.dispatchEvent(new Event('input', {bubbles: true}));
return true;
})()""")
settle(0.4)
after = field_state(page, ["proj_name", "proj_number"])
chk("...and clears the moment that field is filled", not after[0]["error"], after[0])
chk("...without clearing the ones still empty", bool(after[1]["error"]), after[1])
print("\n the first error is scrolled to, not only focused")
page.eval("goToStep(7, {fromUrl:true}); updateStepUI(); window.scrollTo(0, 0)")
settle(0.5)
blank(page, STEP_FIELDS[7])
page.eval("validateStep(7)")
settle(0.9)
chk("the focused field is inside the viewport", page.eval("""(() => {
const el = document.getElementById('plat_tracking');
const r = el.getBoundingClientRect();
return r.top >= -2 && r.bottom <= window.innerHeight + 2;
})()"""))
print("\n Next is refused, and the rail agrees with the refusal")
page.eval("goToStep(1, {fromUrl:true}); updateStepUI()")
settle(0.4)
blank(page, STEP_FIELDS[1])
page.eval("nextStep()")
settle(0.5)
chk("Next does not leave an invalid step", page.eval("currentStep") == 1,
page.eval("currentStep"))
chk("...and marks the fields, the same way validateStep does",
all(f["error"] for f in field_state(page, STEP_FIELDS[1])))
chk("...while the rail shows the steps ahead as unavailable", page.eval(
"[...document.querySelectorAll('#step-rail-list .step-btn[aria-disabled=true]')].length")
== 11,
page.eval("[...document.querySelectorAll('#step-rail-list .step-btn[aria-disabled=true]')]"
".map(b => b.dataset.step)"))
print("\n merely LOOKING at a step does not paint it red")
open_wizard(page, base, tok)
for n in sorted(STEP_FIELDS):
page.eval("goToStep(%d, {fromUrl:true}); updateStepUI()" % n)
settle(0.3)
marks = json.loads(page.eval(
"JSON.stringify([...document.querySelectorAll('.field-error')]"
".filter(e => (e.textContent||'').trim()).map(e => e.id))"))
chk("no error is shown on a step nobody has tried to leave", not marks, marks)
print("\n5. the wizard opens no native dialog at all")
open_wizard(page, base, tok)
# Drive every path that used to raise one.
page.eval("loadSampleData()")
settle(1.6)
chk("loading the sample announces instead of interrupting",
page.eval("!document.getElementById('wp-toast').hidden"))
chk("...politely, because it is a confirmation not an error",
page.eval("document.getElementById('wp-toast').getAttribute('role')") == "status")
chk("...and says what happened",
"Sample data loaded" in (page.eval(
"(document.getElementById('wp-toast')||{}).textContent||''")),
ascii_(page.eval("(document.getElementById('wp-toast')||{}).textContent||''")))
chk("...and can be dismissed from the keyboard", page.eval("""(() => {
const b = document.querySelector('#wp-toast .wp-toast-close');
if (!b) return false;
b.focus();
const focused = document.activeElement === b;
b.click();
return focused && document.getElementById('wp-toast').hidden;
})()"""))
page.eval("switchTool('wp'); loadSampleData()")
settle(1.2)
chk("loading the sample on the wrong tab interrupts (role=alert)",
page.eval("document.getElementById('wp-toast').getAttribute('role')") == "alert",
ascii_(page.eval("(document.getElementById('wp-toast')||{}).textContent||''")))
page.eval("switchTool('sop')")
settle(0.6)
# Through the control a person uses — the modal's text input — not through the
# library helper, which has no duplicate message to give.
page.eval("""(() => {
showConstraintLibrary();
const i = document.getElementById('custom-constraint-input');
i.value = 'Probe constraint'; addCustomConstraintText();
i.value = 'Probe constraint'; addCustomConstraintText();
return true;
})()""")
settle(0.6)
chk("adding a duplicate constraint says so without a dialog",
"already in the list" in (page.eval(
"(document.getElementById('wp-toast')||{}).textContent||''")),
ascii_(page.eval("(document.getElementById('wp-toast')||{}).textContent||''")))
page.eval("closeConstraintModal()")
page.eval("toggleComments(); document.getElementById('comment-text').value=''; submitComment()")
settle(0.6)
chk("an empty comment is refused at the field",
bool((page.eval("(document.getElementById('comment-text_err')||{}).textContent||''")).strip()),
ascii_(page.eval("(document.getElementById('comment-text_err')||{}).textContent||''")))
chk("...announced", page.eval(
"document.getElementById('comment-text_err').getAttribute('role')") == "alert")
chk("...and focused", page.eval("(document.activeElement||{}).id") == "comment-text")
page.eval("exportComments()")
settle(0.5)
chk("exporting nothing says so without a dialog",
"no feedback to export" in (page.eval(
"(document.getElementById('wp-toast')||{}).textContent||''")).lower(),
ascii_(page.eval("(document.getElementById('wp-toast')||{}).textContent||''")))
page.eval("showAnalytics()")
settle(0.5)
chk("the usage summary is shown rather than crammed into a confirmation",
"Step 1:" in (page.eval("(document.getElementById('wp-toast')||{}).textContent||''")),
ascii_(page.eval("(document.getElementById('wp-toast')||{}).textContent||''"))[:100])
chk("...with the download offered as an action beside it",
page.eval("""(() => {
const b = document.querySelector('#wp-toast .wp-toast-action');
return !!b && /download/i.test(b.textContent);
})()"""))
fired = json.loads(page.eval("JSON.stringify(window.__dialogs||[])"))
chk("nothing anywhere in that opened alert / confirm / prompt", not fired, fired[:4])
print("\n and the source agrees — the count, measured both ways")
src = open(os.path.join(ROOT, "html", "work-package-suite-app.js"), encoding="utf-8").read()
raw = len(re.findall(r"\b(alert|confirm|prompt)\(", src))
code = "\n".join(ln for ln in src.splitlines() if not ln.strip().startswith("//"))
stripped = len(re.findall(r"\b(alert|confirm|prompt)\(", code))
chk("work-package-suite-app.js: 0 native dialogs, raw count", raw == 0, raw)
chk("...and 0 with comments stripped too", stripped == 0, stripped)
app_raw = app_stripped = 0
per_file = {}
for name in sorted(os.listdir(os.path.join(ROOT, "html"))):
if not name.endswith((".js", ".html")):
continue
text = open(os.path.join(ROOT, "html", name), encoding="utf-8").read()
r = len(re.findall(r"\b(alert|confirm|prompt)\(", text))
c = len(re.findall(r"\b(alert|confirm|prompt)\(",
"\n".join(ln for ln in text.splitlines()
if not ln.strip().startswith("//"))))
app_raw += r
app_stripped += c
if r:
per_file[name] = (r, c)
print(" app-wide: %d raw, %d with comments stripped (wave 0 recorded 79)"
% (app_raw, app_stripped))
for name, (r, c) in sorted(per_file.items()):
print(" %-26s %3d raw %3d stripped" % (name, r, c))
chk("the app-wide count went DOWN against the wave 0 baseline of 79",
app_raw < 79, app_raw)
chk("...and the wizard contributes none of what is left",
"work-package-suite-app.js" not in per_file, sorted(per_file))
print("\n both widths")
for w, label in ((390, "390px"), (1440, "1440px")):
page.viewport(w, 900, mobile=(w == 390))
open_wizard(page, base, tok)
blank(page, STEP_FIELDS[1])
page.eval("validateStep(1)")
settle(0.6)
chk("%s: the error renders under its field" % label, page.eval("""(() => {
const f = document.getElementById('proj_name').getBoundingClientRect();
const e = document.getElementById('proj_name_err').getBoundingClientRect();
return e.height > 0 && e.top >= f.bottom - 2;
})()"""))
page.eval("wizardToast('probe message at %s')" % label)
settle(0.4)
chk("%s: an announcement stays inside the viewport" % label, page.eval("""(() => {
const r = document.getElementById('wp-toast').getBoundingClientRect();
return r.left >= -1 && r.right <= window.innerWidth + 1 && r.width > 0;
})()"""), page.eval("JSON.stringify(document.getElementById('wp-toast')"
".getBoundingClientRect())"))
chk("%s: the page does not scroll sideways" % label,
page.eval("document.documentElement.scrollWidth <= window.innerWidth + 1"))
page.viewport(1400, 1000)
def main():
exe = cdp.find_browser()
if not exe:
print("no headless-capable browser found; set WP_BROWSER.")
return 2
tmpdir = tempfile.mkdtemp(prefix="wpsuite-validation-")
db_path = os.path.join(tmpdir, "check.db")
server = None
try:
tok = seed(db_path)
port = cdp.free_port()
base = "http://127.0.0.1:%d" % port
server = start_server(port, db_path)
if server is None:
print("the test server would not start.")
return 2
print("\nInline validation — S1 (wizard)\nTarget: %s" % base)
browser = cdp.Browser(exe)
page = browser.page()
try:
run(page, base, tok)
finally:
page.close()
browser.close()
finally:
if server:
server.kill()
try:
server.wait(timeout=10)
except subprocess.TimeoutExpired:
pass
try:
from server.db import engine
engine.dispose()
except Exception:
pass
import shutil
for _ in range(10):
shutil.rmtree(tmpdir, ignore_errors=True)
if not os.path.exists(tmpdir):
break
time.sleep(0.3)
total = len(_PASS) + len(_FAIL)
print("\n%s\n%d/%d checks passed." % ("-" * 54, len(_PASS), total))
if _FAIL:
for f in _FAIL:
print(" - " + f)
return 1
print("\nResult: " + _c("ALL PASS — errors at the field, and no dialogs left.", "32") + "\n")
return 0
if __name__ == "__main__":
sys.exit(main())