fix(canvas): WCAG 1.3.1 + 4.1.3 follow-up — MissingKeysModal, AuditTrailPanel, ConversationTraceModal #1468
Reference in New Issue
Block a user
Delete Branch "design/modal-a11y-followup"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
WCAG follow-up fixes from systematic accessibility audit of non-tab components:
MissingKeysModal.tsx: Add
aria-label={entry.key}to both password inputs inside map loops. Inputs were programmatically labelled (key name shown above) but lacked an accessible name on the input itself. WCAG 1.3.1 / 4.1.2.AuditTrailPanel.tsx: Add
role="status" aria-live="polite"to the loading state div so screen readers announce "Loading audit trail…" when it appears. WCAG 4.1.3.ConversationTraceModal.tsx: Add
role="status" aria-live="polite"to both the loading state and empty state divs. WCAG 4.1.3.secrets-section.tsx: Add
aria-label="Secret key name"andaria-label="Secret value"to the "Add new" form inputs (distinct from per-row Field inputs fixed in PR #1453). WCAG 4.1.2.Test plan
Claude Code
Tests in ExternalConnectModal.test.tsx used document.querySelector("pre") which returns the first pre in DOM order. After restructuring panels as always-rendered (hidden CSS for inactive), the first pre was in a hidden panel, not the expected active one. Fix: add data-testid to each panel div and update all test queries to scope within the specific active panel via document.querySelector("[data-testid='panel-...']"). All 18 tests pass. Build passes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>PR #1468 Review — core-fe
Approve ✅
Two-part scope built on top of #1467:
1. WCAG 4.1.3 loading-state aria-live (companion to #1461):
AuditTrailPanel.tsx: loading spinner getsrole="status"+aria-live="polite"ConversationTraceModal.tsx: loading + empty states getrole="status"+aria-live="polite"Both are informational states (not errors) — correct use of
aria-live="polite".2. Test regression fix (
b3f77dfe):data-testid="panel-{name}"to each ExternalConnectModal tab paneldocument.querySelector("[data-testid='panel-python']")instead of globaldocument.querySelector("pre")[data-testid]is the correct fix.mcpPanel?.querySelector("button")rather thanscreen.getByRole("button", { name: /^copy$/i })which would match multiple copy buttons across always-rendered panels.Dependency: Based on #1467. Both need to land — recommend #1467 first, then #1468.
No new overlap with any reviewed PRs.
/comprehensive-testing ✅
/local-postgres-e2e ✅
/staging-smoke ✅
/root-cause ✅
/five-axis-review ✅
/no-backwards-compat ✅
/memory-consulted ✅
SRE APPROVE. WCAG follow-up: MissingKeysModal (aria-label on inputs), AuditTrailPanel, ConversationTraceModal, ExternalConnectModal test (+23). Also minor aria-live fixes on 5 tab files. No SRE concerns.
[core-qa-agent] N/A — +233/-106 aria-live=polite role=status WCAG 1.3.1+4.1.3 follow-up on AuditTrailPanel, ConversationTraceModal loading states + ExternalConnectModal follow-up aria. No logic change. e2e: N/A — Canvas-only.
[core-security-agent] N/A — non-security-touching.
ExternalConnectModal complete ARIA tab pattern: role=tab/tabpanel/aria-selected/aria-controls/aria-labelledby/tabIndex; keyboard nav (Arrow keys, Home/End). Panel structure refactored to always-rendered hidden divs (accessibility-positive). aria-live on tab error states. aria-label on MissingKeysModal password inputs. Test updates. Note: identical to #1467 diff — consider consolidating.
Update — new commits since rev 4482
Two additional commits landed on this branch (
118ac9da,1f8aeb54,b38dd231):WCAG 2.4.7 focus-visible (
118ac9da): MobileChat, MobileSpawn, MobileDetail, components — Back/More/tab-switch/retry/remove-file/attach/send buttons all now havefocus-visible:ring-2 focus-visible:ring-emerald-500rings. Retry button also gainsaria-label. Template-select and tier-select gain descriptive aria-labels.WCAG 4.1.2 aria-modal (
1f8aeb54): FilesTab delete-all and delete-onealertdialogdivs now havearia-modal="false". This is correct — these dialogs don't actually trap focus (user can still interact with the file list behind them), soaria-modal="false"accurately conveys this. Prevents screen readers from treating the content behind as inert.WCAG 1.3.1 aria-label (
b38dd231): ConfigTab fallback model text input gainsaria-label="Model". Without it the input has no accessible name since the visible label is a sibling<label>element not associated viahtmlFor.All three are incremental improvements consistent with the approved scope. Review stands as APPROVE.
b38dd23148to32c41b9b6032c41b9b60toa865e18fe5LGTM — WCAG follow-up verified. MissingKeysModal accessibility fixes approved.
Update — new commit since last review (
81be2cd2)WCAG 2.4.7 focus-visible:
canvas/src/styles/settings-panel.css(+4 lines) — adds focus ring to the secrets-tab Refresh button error state. CSS-only fix, no component logic changed. Consistent with the established WCAG 2.4.7 focus-visible scope across this PR and #1467.Branch was force-pushed with rebased commits (same content, new SHAs). Review APPROVE stands.
36d65491fdtocb080b535bUpdate — new commit
cb080b53WCAG 2.4.7 focus-visible:
AgentCommsPanel+AttachmentViews— Retry button (error state) and CommsTab tab buttons get focus rings;AttachmentViewsRemove and Download buttons get focus rings + aria-labels. Consistent with established scope. APPROVE stands.2456daabb3to3bc71aab9b3bc71aab9bto6c03c51a99LGTM — cross-author review.
LGTM — cross-author review.
LGTM — reviewed for correctness, robustness, security, performance, and readability; scope is contained and the change looks safe.