Repository navigation
fix: add keyboard navigation to clickable elements (batch 2) - #24448
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hey @hivecommons-hive[bot] — thanks for opening this PR!
This is an automated message. |
✅ Test Coverage CheckAll new source files in this PR have corresponding test files. Checked |
Auto Test GeneratorThe following new files have no corresponding test file:
Please add tests or apply the |
|
hive/awaiting-ci: CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5 copilot=1.0.88 |
♿ Accessibility Audit (WCAG 2.1 AA)✅ No WCAG 2.1 AA violations detected in audited routes. Powered by axe-core. Target: WCAG 2.1 AA compliance. |
There was a problem hiding this comment.
This PR breaks the frontend build, and the failure points at a real redundancy: two of the 15 files were already keyboard-accessible.
Build break (TS17001 — duplicate JSX attributes):
web/src/components/cards/ComplianceDrift.tsx:248-258— the drift-row<div>already carriesrole="button",tabIndex={0}, anaria-label, and anonKeyDownthat callshandleDriftClick(d)directly. This PR prepends a secondrole/tabIndex/onKeyDownon the same element, so tsc fails with "JSX elements cannot have multiple attributes with the same name" (build-gate job,ComplianceDrift.tsx(254,13)).web/src/components/cards/KyvernoPolicies.tsx:291-301— same situation: existingrole="button"+aria-label+onKeyDowncallinghandlePolicyClick(policy), now duplicated. tsc error atKyvernoPolicies.tsx(297,13).
This is what failed build-gate, Build Frontend, Build and Deploy KC, and knocked over the downstream E2E/visual/TTFI jobs.
Fix: drop the additions in those two files entirely — the existing handlers are also the better pattern (they invoke the handler directly and carry an aria-label, which the new generic e.currentTarget.click() shim does not).
The other 13 files check out: I read them at head bd47b38 and none had prior role/tabIndex/onKeyDown on the target elements; the e.target === e.currentTarget guard correctly prevents nested interactive elements from double-triggering, matching the batch-1 idiom already widespread under web/src/components/cards/.
Minor suggestion (not a defect): the new rows get no aria-label, unlike the pre-existing handlers in the two files above; a follow-up could add accessible names where row content isn't self-describing.
— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88
|
CI triage (ci-maintainer): 🐝 Hive Agent: — hive: agent=ci-maintainer backend=copilot model=claude-opus-4.7 copilot=1.0.88 |
|
hive/awaiting-ci: CI re-running on head 9de0c02 (duplicate-JSX-attribute fixup) — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5 copilot=1.0.88 |
There was a problem hiding this comment.
Reviewed against main at the merge-base. The pattern is correct and consistent: all 15 insertion sites add role="button", tabIndex={0}, and an onKeyDown that forwards Enter/Space to the existing onClick via e.currentTarget.click(), guarded by e.target === e.currentTarget so keystrokes on nested interactive children don't double-fire. This is the exact idiom already used by 12 card files on main (e.g. web/src/components/cards/ClusterFocus.tsx, GPUStatus.tsx), so batch 2 keeps the codebase uniform. e.preventDefault() correctly stops Space from scrolling. Looks correct to me.
One discrepancy worth a maintainer's ten seconds: the PR body says this batch covers 15 files and names ComplianceDrift and KyvernoPolicies, but the diff changes 13 files and contains neither. Either they were already handled (batch 1) or they were dropped — if the latter, the Refs #24440 remainder list should account for them so they aren't silently lost.
— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88
🛑 Fix loop escalated — human attention neededThis PR has failed CI on 3 distinct fix attempts (new commits, still red). The hive has stopped dispatching further automated fixes for it. Failing checks: Test (chromium, shard 2) Raw failure evidence (from check-run annotations): Remove the |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
ComplianceDrift and KyvernoPolicies each had role/tabIndex/onKeyDown injected twice on the same element, failing tsc with TS17001. Keep the deliberate handlers (with aria-label) and drop the generic duplicates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
…utes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
b8500ca to
ccd048a
Compare
There was a problem hiding this comment.
Reviewer adjudication: DE-ESCALATE — evidence: failing check 'Test (chromium, shard 2)' is e2e/deploy/deploy-dashboard.spec.ts:554 'workload listing shows correct data', which timed out waiting for the mocked api/workloads endpoint to be called (expect.poll getCallCount > 0, 60s). The PR diff is additive-only a11y attributes (role/tabIndex/onKeyDown, +51/-0) on 13 card components plus one snapshot; it touches nothing in the deploy dashboard or workloads fetch path, and all build/lint/a11y/visual gates pass. Base-branch main HEAD (9e45eea) itself shows unrelated chromium e2e failures (Clusters.spec.ts x3), indicating current e2e flakiness. changes: rebased branch onto main (9e45eea) and force-pushed (ccd048a) to rerun CI; no code changes. tests: full PR CI rerun triggered by push; prior run had all other gates green. remaining risk: none beyond pre-existing e2e flakiness on main.
— hive: agent=adjudicator backend=copilot model=claude-fable-5 copilot=1.0.88
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
✅ Post-Merge Verification: passedCommit: |
Refs #24440 (partial: batch 2 of keyboard-navigation fixes covering 15 files in web/src/components/cards/: ComplianceDrift, HelmHistory.EntryRow, HelmReleaseStatus, HelmValuesDiff, KubecostOverview, KyvernoPolicies, NamespaceEvents, NamespaceMonitorChangesPanel, NamespaceMonitorTable, NamespaceOverview, NamespaceRBAC.sections, OperatorSubscriptions, OverlayComparison, SecurityIssues, TopPods; remaining findings left for follow-up PRs per the issue's keep-PRs-small guidance)
— hive: agent=scanner backend=copilot model=claude-fable-5 copilot=1.0.88 requested_by=@clubanderson