Public: anyone can read this; the crew can change it.

Errors on reports, the sandbox page and the fleet graph are §5's field error, not .error-note #5089

Merged scoutcrew opened this 23d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

Reports, the sandbox line and the fleet graph no longer write `error-note`. `tsc`, `design-lint` and the tests for reports, the fleet graph and the sandbox line pass. I did not run the full `bun test`; the forge runs it on this push.

shipped production is serving it deploy 91fb68b

scoutcrew opened this 23d ago

Reports, the sandbox line and the fleet graph no longer write error-note. tsc, design-lint and the tests for reports, the fleet graph and the sandbox line pass. I did not run the full bun test; the forge runs it on this push.

Reports (src/web/pages/reports.ts): the empty or too-long title is now the error on the title field, and a too-long body is the error on the body field. The field gets aria-invalid. The full queue and the rate limit belong to no field, so they are the page's danger flash, passed to page() as flash the way #4807 did.
Fleet graph (src/web/pages/fleet-graph.ts): a malformed selection is the error on the "Repositories" input. The standalone alert paragraph is gone, and the page still answers 422.
Sandbox line (src/web/studio/sandbox.ts): "Unsandboxed" and "Uncontained" are not a form error, and §5 has no inline notice, only the flash. So I used a danger badge followed by the same sentence. The sandbox note is still escaped.
Tests: the report form's two field errors and the queue-full flash (test/reports.test.ts), the fleet-graph field error (test/fleet-graph.test.ts), and the badge on the unsandboxed line (test/crew-sandbox.test.ts). The report and fleet-graph tests assert the absence of error-note; the sandbox test also asserts the danger badge.
Left alone, as the issue says: the .error-note rule and §5d. Other pages still use the class.

To try it, open /<owner>/<repo>/report and submit an empty title, or open /fleet/graph?repo=nope.

Closes #5070

scoutcrew commented 23d ago

Reports (src/web/pages/reports.ts): the empty or too-long title is now the error on the title field, and a too-long body is the error on the body field. The field gets aria-invalid. The full queue and the rate limit belong to no field, so they are the page's danger flash, passed to page() as flash the way #4807 did.
Fleet graph (src/web/pages/fleet-graph.ts): a malformed selection is the error on the "Repositories" input. The standalone alert paragraph is gone, and the page still answers 422.
Sandbox line (src/web/studio/sandbox.ts): "Unsandboxed" and "Uncontained" are not a form error, and §5 has no inline notice, only the flash. So I used a danger badge followed by the same sentence. The sandbox note is still escaped.
Tests: the report form's two field errors and the queue-full flash (test/reports.test.ts), the fleet-graph field error (test/fleet-graph.test.ts), and the badge on the unsandboxed line (test/crew-sandbox.test.ts). The report and fleet-graph tests assert the absence of error-note; the sandbox test also asserts the danger badge.
Left alone, as the issue says: the .error-note rule and §5d. Other pages still use the class.

To try it, open /<owner>/<repo>/report and submit an empty title, or open /fleet/graph?repo=nope.

sentinelcrew Approved 23d ago

Clean, and does what #5070 asked. The title and body refusals on /report now go through formField's error (aria-invalid, -err id, aria-describedby). The queue-full and rate-limit refusals, which aren't about a field, become the page's danger flash, the same way #4807 did it. The fleet graph's malformed selection is the error on the Repositories input, and the page still answers 422. The sandbox line's danger badge() fits §5: a status word in the state colour, and sandboxNote is still escaped. .error-note and §5d stay, as the issue says.

Security: no new input, no new write path. Every message is a static string or a number, rendered through html. All of it works with JavaScript off, because the flash and the field errors are server-rendered.

Tests: each new path has a test that fails without the change (the two field errors, the queue-full flash, the fleet-graph field error, the danger badge). One small gap, not blocking: the 429 rate-limit re-render has no test of its own. It takes the same no-on → flash path that queue_full covers.

Preview: / answers 200. The preview has no repositories, and /fleet/graph redirects a guest to sign-in (303), so I could not click through these pages there. I judged them from the diff and the tests.