Skip to content

Commit d297388

Browse files
authored
fix(a11y): finish the Notice conversion in the appraise form (#137)
* fix(a11y): finish the Notice conversion in the appraise form The last `&&`-gated Notice in the app, and the one PR #131 missed: a grep for the pattern across server pages found six, but this one is in a client component and reads `{state && !state.ok && <Notice/>}` rather than `{message && <Notice/>}`. It is the worst instance rather than an afterthought. `useActionState` keeps this component mounted across a rejection — that is the whole reason it exists (see the docblock) — so the surrounding tree never moves and the `&&` inserted a fresh role="alert" node carrying its own text into an otherwise stable document. In the server-page cases a document-less re-render at least changed several things at once; here the region's birth IS the only change, which is precisely the announcement AT handles least reliably. The visible behaviour is unchanged. The comment's argument — that `state === null` covers both "hasn't submitted yet" and "still pending", neither of which is a rejection worth announcing — is preserved exactly by moving the condition into the children. Empty children render the reserved `.notice-slot`, which is positioned out of flow, so the form-stack row gap does not open up. Cost to a user: an operator on a screen reader who pastes a malformed loot list into the appraise form is not reliably told the submit was rejected. The form stays where it is and the paste survives, so nothing else on screen reports it either. Gates: node-version, typecheck, lint, format:check, build, npm test (76 files / 1085 tests) all pass. docker build and test:e2e not run. * fix(a11y): drop the seal's alt text, which repeated the heading below it (#138) The login seal carried alt="<brand> emblem" two lines above <h1>{brand.name}</h1>. Every other decorative image in the app already uses alt="" — account/page.tsx:473, account/page.tsx:681, and, most directly, ui.tsx:117, which is the same brand mark rendered immediately before the wordmark that spells the brand out in text. /login was the one exception. Cost to a user: a screen-reader visitor to the only page an unauthenticated person can reach hears the brand name, then the brand name again, before reaching the motto, the scope disclosure, or the sign-in control. The emblem communicates nothing the h1 does not, which is the case alt="" exists for. The EVE SSO mark below keeps its alt: that image is the control's label, not decoration, and nothing else names it. The eslint-disable-next-line for @next/next/no-img-element now sits directly above the <img> again — the new comment goes above it, not between. Inserting it between broke the directive's adjacency and reintroduced the warning #123 cleared; lint is quoted below to show it is clean. Gates: node-version, typecheck, lint (0 warnings), format:check, build, npm test (76 files / 1085 tests) all pass. docker build and test:e2e not run.
1 parent a5294d9 commit d297388

2 files changed

Lines changed: 21 additions & 7 deletions

File tree

src/app/login/page.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,12 @@ export default async function LoginPage({
7575
return (
7676
<main className="launch">
7777
<div className="launch__panel">
78+
{/* Empty alt, like the same mark in `SiteHeader` (ui.tsx:117) and every
79+
other decorative image in the app: the `<h1>` directly below already
80+
carries `brand.name`, so `"<name> emblem"` made a screen reader read
81+
the brand twice before reaching anything actionable — on the one
82+
page an unauthenticated visitor can get to. The emblem adds nothing
83+
the heading does not already say, which is what `alt=""` is for. */}
7884
{/* eslint-disable-next-line @next/next/no-img-element -- what next/image
7985
would add here is a re-encode at its default quality 75, and
8086
PRODUCT.md principle 5 asks for this artwork at "full quality" or not
@@ -88,7 +94,7 @@ export default async function LoginPage({
8894
<img
8995
className="launch__seal"
9096
src={brand.sealUrl}
91-
alt={`${brand.name} emblem`}
97+
alt=""
9298
width={180}
9399
height={180}
94100
/>

src/app/payouts/[id]/appraise-form.tsx

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,12 +131,20 @@ export function AppraiseForm({
131131
const form = (
132132
<form action={formAction} className="form-stack">
133133
<RuleHead as="h3">Appraise a loot paste</RuleHead>
134-
{/* Only rendered once a submit has actually failed — `state === null`
135-
covers both "hasn't submitted yet" and "still pending", and neither
136-
of those is a rejection worth announcing. */}
137-
{state && !state.ok && (
138-
<Notice tone="bad">{lookupErrorMessage(OPERATION_ERRORS, state.code)}</Notice>
139-
)}
134+
{/* Mounted unconditionally, with the condition moved into the children:
135+
the text still appears only once a submit has actually failed —
136+
`state === null` covers both "hasn't submitted yet" and "still
137+
pending", and neither of those is a rejection worth announcing — but
138+
the live region now exists before the message does. `useActionState`
139+
keeps this component mounted across the rejection, so the `&&` form
140+
inserted a fresh `role="alert"` node with its text already inside it
141+
into an otherwise stable tree: exactly the shape `Notice`'s docblock
142+
(ui.tsx:255-270) calls out as the one that defeats the region it just
143+
asked for. Empty children render the reserved `.notice-slot`, which is
144+
out of flow and draws nothing, so the form's spacing is unchanged. */}
145+
<Notice tone="bad">
146+
{state && !state.ok ? lookupErrorMessage(OPERATION_ERRORS, state.code) : null}
147+
</Notice>
140148
<label className="form-stack__field">
141149
{/* Same hint as the composer's Loot field
142150
(`../new/new-operation-form.tsx`), which takes the same paste

0 commit comments

Comments
 (0)