Skip to content

test(admin): extend the accessibility scan to the main admin pages - #1424

Merged
solarssk merged 7 commits into
mainfrom
test/a11y-admin-surfaces
Sep 24, 2026
Merged

solarssk merged 7 commits into
mainfrom
test/a11y-admin-surfaces

Conversation

@solarssk

Copy link
Copy Markdown
Owner

Description

Until now the automated accessibility scan only looked at the login page and the operator check-in page. This extends it to the main admin pages, where staff spend most of their time, and fixes the real contrast problems it found there. Several pieces of text were too pale to meet the accessibility minimum: inactive tab labels, the green "Live" badge on an event's Overview, green success buttons, and the "issued" state text in an attendee's items list. They are now darker; nothing else about their look changes.

Technical changes:

  • apps/admin/e2e/seed.ts: seeds a superadmin (bootstrapSuperadmin; on re-runs its MFA is reset). admin-login.ts (new) signs it in over the API, walking the forced TOTP enrollment (login, totp/enroll, totp/confirm, backup-codes/complete) with a TOTP computed via node:crypto.
  • a11y.spec.ts: new "admin pages" test scanning the events list, event overview, attendees, attendee detail, event settings and communication. serious/critical fail the test on every surface. The scan excludes iframes: the only ones are the srcdoc mail-template previews on the Communication page, whose content is the template's own HTML, and axe hangs forever trying to scan them (found and confirmed by timing each step).
  • CSS, all with existing tokens: .at-tab --text-muted -> --text-secondary (4.34:1 -> 6.9:1); .at-btn--success and .overview-live-indicator background --status-ok -> --status-ok-fg (white on it 2.74:1 -> 5.4:1; the success button hover is derived from it); .attendee-items-row__state--ok text --status-ok -> --status-ok-fg.
  • ci.yml: the E2E job's path filter now covers apps/admin/src/** and packages/ui/src/**, since any change there can regress the scan. This makes the job (a few minutes) run on most admin/UI PRs; accepted.
  • CHANGELOG.md: operator-facing bullet under [Unreleased]. apps/admin/README.md updated.

How to test

  • Locally (disposable database): npm run e2e -w @admitto/admin gives 4 passed with 0 violations on all 8 scanned surfaces. Before the CSS fixes the same scan reported color-contrast violations on the events list, overview and attendee detail (details in the commit message).
  • The PR touches apps/admin/src/**, so its CI runs the E2E job.
  • Visual check by eye is still worth a glance: the success button (used in a few places) and the Live badge are now a darker green; hover on the success button is derived by mixing 15% black into that green.

What stays / known limitations

  • Only these 8 surfaces are scanned. Settings pages other than event settings, reports, requirements, users and the setup wizard are not.
  • iframes are excluded from the scan, so the rendered mail-template preview content is not checked.
  • Other uses of --status-ok as a text or fill colour that are not on scanned pages (for example the mobile check-in result overlay) were not touched.
  • Issue Add automated accessibility testing (axe-core on the existing Playwright surface) #1411 can be closed on merge; extending to more pages would be a new issue.

Documentation impact

  • Wiki updated
  • No Wiki update needed - colour values only; no workflow, role, terminology or availability change.

Checklist

  • No secrets / keys / passwords in the diff
  • No real personal data (seed/sample data uses synthetic @example.com addresses)
  • Tests pass locally (npm test; optional: npm run coverage to match CI) - E2E suite, typecheck, lint, baseline and docs checks run locally; unit suites run in CI
  • New or changed functionality is covered by tests added to the automated suite - the a11y spec is the test
  • New fields containing personal data are justified and minimised - N/A
  • No PII in logs; token/QR contains no personal data
  • DB schema changes include a migration - N/A

🤖 Generated with Claude Code

Completes the scope of #1411: the scan covered only login and the
operator check-in page. It now also signs in as a seeded superadmin
(admin roles need MFA, so admin-login.ts walks the forced TOTP
enrollment over the API; the seed resets MFA on re-runs) and scans the
events list, event overview, attendees, attendee detail, event settings
and communication. serious/critical violations fail the test on all of
them; the axe scan excludes iframes, because axe hangs indefinitely on
the srcdoc mail-template previews on the Communication page (and that
content is the template's own HTML, not the app's UI).

The scan found real contrast problems (fixed here, all with existing
tokens): inactive .at-tab labels (--text-muted -> --text-secondary),
.at-btn--success and .overview-live-indicator (white on #2fb344, 2.74:1,
now --status-ok-fg), and .attendee-items-row__state--ok (green text on
white, now --status-ok-fg). The E2E job's path filter now covers
apps/admin/src/** and packages/ui/src/**, since any change there can
regress the scan.

Verified locally: 4 tests pass, 0 violations on all 8 scanned surfaces.

Refs #1411.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@solarssk solarssk added this to the 0.7.3 milestone Sep 23, 2026
@solarssk solarssk added type: chore Tooling, config, CI, deps — no production logic change area: infra CI/CD, GitHub Actions, repo config, hosting labels Sep 23, 2026
@solarssk solarssk self-assigned this Sep 23, 2026
@codecov

codecov Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

solarssk and others added 2 commits September 24, 2026 00:04
… test retry-safe

CI on this PR found a violation the local run had missed: the muted
item-state text ("Not yet", --text-disabled #94a3b8 on white = 2.56:1) only
shows for an attendee who has not been admitted, and locally the attendee had
been admitted by an earlier run. It is real information, not a disabled
control, so it now uses --text-muted (4.76:1). The admin test now re-seeds
first, which puts the attendee back to "not admitted" (so this state is what
gets scanned, deterministically) and resets the admin's MFA (CI retries a
failed test once, and a second TOTP enrollment was impossible - the retry
died with next=mfa_required). Verified: reverting the CSS makes the local
scan fail, and two consecutive local runs against the same database pass.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…roducible

Correction to the previous commit's message, which claimed that reverting
the "Not yet" CSS fix makes the scan fail locally. It did not, and that
claim was untested: locally the attendee kept the "Issued" badge state from
an earlier run's admit (the seed reset the check-ins but not the per-item
state), so the muted "Not yet" label never rendered, while CI's fresh
database always shows it. The seed now also deletes the attendee's item
state, so a local run starts from the same "Not yet" state as CI. Verified
this time in both directions: with the fix 4 tests pass; with the CSS
reverted the admin scan fails on attendee detail (1 serious violation).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@solarssk

Copy link
Copy Markdown
Owner Author

Correction: my previous commit message claimed the local scan fails when the --text-muted fix is reverted. That was not verified and was wrong: locally the attendee kept the 'Issued' item state from an earlier run, so the muted 'Not yet' label (which CI's fresh database always shows) never rendered. The seed now also resets item state; verified in both directions (with the fix 4 pass, with the CSS reverted the admin scan fails on attendee detail). CI found the real issue, and it also exposed that a retried admin test could not enroll TOTP twice, fixed by re-seeding at the start of the test.

solarssk and others added 2 commits September 24, 2026 00:25
…e a11y scan

SonarCloud S9332 on PR #1424: the networkidle wait (with a 4 s cap, since
pages that hold a live SSE stream never go idle) is an unreliable readiness
signal. The scan now waits for a heading and for every shared Spinner
(.at-spinner) to disappear. Verified both ways again: 4 tests pass, and with
the "Not yet" CSS fix reverted the attendee-detail scan still fails, so the
page really is fully loaded when scanned.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@solarssk

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1fe03531e6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/admin/e2e/a11y.spec.ts
Comment thread .github/workflows/ci.yml
…mations ended

Two review findings on PR #1424, both valid:
- Readiness "no spinner" could pass before a page's data arrived (Overview
  does not use .at-spinner; the events picker delays its spinner), so axe
  could scan placeholders. Each surface now lists the API requests that
  carry its data, and the scan waits for those responses (registered before
  navigating), then for the heading, spinners, two animation frames and any
  finite animation to end.
- The E2E path filter covers apps/web/src/admin/** (all admin API
  handlers) instead of only the check-in ones.

Waiting for real data immediately paid off: the Communication page's info
notice was reported at 3.07:1, but that was axe reading it mid fade-in (the
measured colour changed between runs, #8591a4 then #6e7989). Once the scan
waits for finite animations the notice passes with no CSS change, so no CSS
was touched for it. Verified: all 8 surfaces 0 violations, stable across two
runs; the "Not yet" CSS revert still makes the attendee-detail scan fail.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@solarssk

Copy link
Copy Markdown
Owner Author

Both P2s were valid and are fixed in the latest commit: each admin page now waits for the API responses that carry its data (plus spinners, two animation frames and any finite animation) before it is scanned, and the E2E path filter covers apps/web/src/admin/**. Side finding: waiting for real data made the Communication page's info notice show up at 3.07:1, but that was axe measuring it mid fade-in; with animations finished it passes and no CSS was changed for it.

@sonarqubecloud

Copy link
Copy Markdown

@solarssk
solarssk merged commit 8634fa0 into main Sep 24, 2026
24 checks passed
@solarssk
solarssk deleted the test/a11y-admin-surfaces branch September 24, 2026 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: infra CI/CD, GitHub Actions, repo config, hosting type: chore Tooling, config, CI, deps — no production logic change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant