Skip to content

fix(#1718): drop prefix-tool a11y allowlist entries — subsumed by #1720#1736

Open
Kpa-clawbot wants to merge 1 commit into
masterfrom
fix/1718-prefix-tool-allowlist
Open

fix(#1718): drop prefix-tool a11y allowlist entries — subsumed by #1720#1736
Kpa-clawbot wants to merge 1 commit into
masterfrom
fix/1718-prefix-tool-allowlist

Conversation

@Kpa-clawbot

Copy link
Copy Markdown
Owner

Summary

PR #1720 (merged 2026-06-13) consolidated active button states onto the
shared .btn-active-accent rule that paints
background: var(--accent-strong) (#2563eb) +
color: var(--text-on-accent) (#f9fafb) = 4.95:1, WCAG AA pass in
both themes. That subsumes the #ptCheckBtn / #ptGenBtn
color-contrast violations issue #1718 tracked, so the allowlist entries
are stale.

Change

Drop the two issue: 1718 entries from tests/a11y-allowlist.yaml:

- route: '/analytics?tab=prefix-tool'
  selector: '#ptCheckBtn'
  rule: color-contrast
  issue: 1718
  expires_at: 2026-09-11
- route: '/analytics?tab=prefix-tool'
  selector: '#ptGenBtn'
  rule: color-contrast
  issue: 1718
  expires_at: 2026-09-11

No other tabs touched. #1715 dark-theme work and other
expires_at: 2026-09-11 entries are out of scope — separate issues,
separate PRs. No production CSS/JS modified (PR #1720 did the
substantive fix).

Verification

The CI a11y gate (test-a11y-axe-1668.js) is the authoritative check.
It re-renders /analytics?tab=prefix-tool in dark+light × desktop+
mobile and asserts zero net violations against the trimmed allowlist.
With this PR the entries are gone — if PR #1720's fix were ever
reverted, the gate fails immediately with no allowlist masking it.

Local repro not attempted: sandbox chromium lacks the
@axe-core/playwright module (matches the documented limitation in
PR #1730 / PR #1723). CI is the source of truth for this gate.

TDD note

Config-change exemption per workspace AGENTS.md:

Mirrors the exact pattern accepted in PR #1722 (clock-health),
PR #1723 (subpaths), PR #1730 (nodes), and PR #1731 (rf-health) —
same allowlist-drop shape, same upstream PR #1720 fix.

Preflight

bash ~/.openclaw/skills/pr-preflight/scripts/run-all.sh origin/master
— config-only change; PII grep on diff clean.

Fixes #1718.

Refs PR #1720, PR #1722, PR #1723, PR #1730, PR #1731.

PR #1720 consolidated active button states (incl. `.btn-active-accent`)
onto `var(--accent-strong)` + `var(--text-on-accent)` (4.95:1, WCAG AA).
The `#ptCheckBtn` / `#ptGenBtn` color-contrast violations that issue
#1718 tracked are gone, so the allowlist entries are stale.

Drops the two `issue: 1718` entries from `tests/a11y-allowlist.yaml`:

```yaml
- route: '/analytics?tab=prefix-tool'
  selector: '#ptCheckBtn'
  rule: color-contrast
  issue: 1718
  expires_at: 2026-09-11
- route: '/analytics?tab=prefix-tool'
  selector: '#ptGenBtn'
  rule: color-contrast
  issue: 1718
  expires_at: 2026-09-11
```

No other tabs touched (#1715 dark-theme work remains — separate issue).

## Verification

The CI a11y gate (`test-a11y-axe-1668.js`) is the authoritative check.
It re-renders `/analytics?tab=prefix-tool` in dark+light × desktop+
mobile and asserts zero net violations against the trimmed allowlist.
With this PR the entries are gone — if PR #1720's fix were ever
reverted, the gate fails immediately with no allowlist masking it.

Local repro not attempted: sandbox chromium lacks the
`@axe-core/playwright` module (matches the documented limitation in
PR #1730 / PR #1723). CI is the source of truth.

## TDD note

Config-change exemption per workspace AGENTS.md:
- No test files modified.
- No production code modified.
- Config-only allowlist trim; CI must stay green without test edits.
- The gate itself is the test — dropping the allowlist entries IS the
  red→green transition (entries gone → axe runs unfiltered → must
  remain pass because #1720 fixed the root cause).

Mirrors the exact pattern accepted in PR #1722, PR #1723, PR #1730 and
PR #1731 (same allowlist-drop shape, same upstream PR #1720 fix).

Fixes #1718.

Refs PR #1720, PR #1731.
@Kpa-clawbot

Copy link
Copy Markdown
Owner Author

Polish review — trivially safe, CI flake is pre-existing on master

Verdict: 0 BLOCKER / 0 MAJOR. Diff is the deletion of two tests/a11y-allowlist.yaml entries (#ptCheckBtn, #ptGenBtn). No production code touched.

Verified on origin/master:

CI: 🎭 Playwright failure is the pre-existing test-issue-1630-reach-mobile-e2e.js fixture flake (Error: no repeater with reach links found in fixture). Same failure on most-recent master run (27637454126, 18:23Z) before this PR existed. Not blocked by this change. Tracked separately.

Not auto-merging: mergeStateStatus=UNSTABLE (the unrelated flake). Operator: safe to gh pr merge 1736 --squash --admin once the e2e fixture is addressed, or merge through the flake if you want this in immediately. Pure YAML deletion, zero behavior risk.

@Kpa-clawbot

Copy link
Copy Markdown
Owner Author

Parent-direct review (trivial 10-line allowlist delete) — round 1

Verdict: APPROVED (conditional on CI)

Diff: removes 2 expired-2026-09-11 allowlist entries for #ptCheckBtn / #ptGenBtn on /analytics?tab=prefix-tool, citing PR #1720 (merged 2026-06-13) which consolidated active button states onto .btn-active-accent (--accent-strong #2563eb on --text-on-accent #f9fafb = 4.95:1 AA).

  • ✅ Body cites the consolidation rule + measured contrast ratio.
  • ✅ Scope is exactly the diff; no incidental churn.
  • ✅ No TDD red-commit required (allowlist removal is a tightening of constraints — if a11y regresses, existing color-contrast tests will catch it as a positive failure).

Out-of-scope (not blocking this PR):

  • Playwright job is failing on master HEAD (no repeater with reach links found in fixture) — pre-existing fixture issue, unrelated to this diff. Auto-merge gate (requires CLEAN) cannot land this until master CI is green; recommend operator address the fixture seed independently.

No must-fix items.

@Kpa-clawbot

Copy link
Copy Markdown
Owner Author

🤖 Hourly PR-watch review

Verdict: NEEDS-VERIFY (then APPROVE) — CI blocked by #1747

Removes 2 prefix-tool color-contrast allowlist entries claimed to be subsumed by #1720. Diff is a clean deletion.

Verification gap

The PR body asserts #1720 already raised contrast on #ptCheckBtn / #ptGenBtn to pass axe. Worth confirming by running the a11y axe check locally against /analytics?tab=prefix-tool to make sure those selectors don't re-fail once the allowlist forgives them — otherwise the next CI run will surface a regression that the operator will have to chase.

CI status

Playwright failing on the e2e fixture aging issue (#1630 reach), not on this change. #1747 unblocks it.

TDD note

Pure config deletion. The a11y E2E test IS the assertion gate; if it stays green post-#1747-merge after rebase, the change is proven.

Findings: 0 BLOCKER / 1 MINOR

Reviewed by: tufte (UI surface).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

a11y: /analytics?tab=prefix-tool contrast violations exposed by #1706 coverage

1 participant