♿ Fix event handler accessibility parity on clickable divs#3959
♿ Fix event handler accessibility parity on clickable divs#3959clubanderson merged 4 commits intomainfrom
Conversation
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hey @Copilot — thanks for opening this PR!
This is an automated message. |
❌ PR Title Verification FailedYour PR title does not follow the required format. Current title: Required FormatPR titles must start with one of these emoji prefixes:
How to FixEdit your PR title to start with the appropriate emoji. For example:
You can edit the title by clicking the Edit button next to your PR title. This comment was automatically posted by the PR Title Verifier workflow. |
… clickable divs Fixes #3577 Add role="presentation" to 9 clickable div elements that lacked proper ARIA roles, resolving the Auto-QA accessibility parity findings: - ClusterGrid.tsx:171 - container div with stopPropagation - ClusterAssignmentPanel.tsx:182 - dropdown backdrop overlay - PayloadCard.tsx:203 - dropdown backdrop overlay - Deploy.tsx:265 - modal backdrop overlay - Deploy.tsx:266 - modal content container (stopPropagation) - CardWrapper.tsx:1261 - modal backdrop overlay - CardWrapper.tsx:1262 - modal content container (stopPropagation) - ClusterSelectionDialog.tsx:90 - modal backdrop overlay - ClusterSelectionDialog.tsx:91 - modal content container (stopPropagation) Using role="presentation" is semantically correct for these elements: - Backdrop overlays are visual-only; keyboard users use the Close button - StopPropagation containers are structural wrappers, not interactive Signed-off-by: copilot <copilot@github.com> Agent-Logs-Url: https://github.com/kubestellar/console/sessions/000497be-b91c-488a-a6a2-446abb9f4061 Co-authored-by: clubanderson <407614+clubanderson@users.noreply.github.com>
|
[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 |
Per code review feedback, modal content containers should use role="dialog" aria-modal="true" rather than role="presentation" to ensure screen readers announce modal content correctly. Backdrop/overlay divs keep role="presentation" as they are visual-only. Signed-off-by: copilot <copilot@github.com> Agent-Logs-Url: https://github.com/kubestellar/console/sessions/000497be-b91c-488a-a6a2-446abb9f4061 Co-authored-by: clubanderson <407614+clubanderson@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses accessibility lint/parity issues by adding ARIA roles to non-semantic elements that handle click interactions (e.g., modal backdrops, dropdown backdrops, and click-swallowing containers) across the React frontend.
Changes:
- Add
role="presentation"to several backdrop / container<div>elements withonClick. - Add
role="dialog"andaria-modal="true"to a few modal content containers.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/components/missions/ClusterSelectionDialog.tsx | Adds role="presentation" to the overlay and role="dialog" aria-modal="true" to the dialog container. |
| web/src/components/mission-control/PayloadCard.tsx | Adds role="presentation" to the priority dropdown backdrop. |
| web/src/components/mission-control/ClusterAssignmentPanel.tsx | Adds role="presentation" to the cluster picker backdrop. |
| web/src/components/deploy/Deploy.tsx | Adds role="presentation" to the modal backdrop and role="dialog" aria-modal="true" to the modal container. |
| web/src/components/clusters/components/ClusterGrid.tsx | Adds role="presentation" to a click-swallowing controls wrapper. |
| web/src/components/cards/CardWrapper.tsx | Adds role="presentation" to the modal backdrop and role="dialog" aria-modal="true" to the modal container. |
| <div className="fixed inset-0 z-50 flex items-center justify-center bg-black/60 backdrop-blur-2xl" role="presentation" onClick={onCancel}> | ||
| <div className="w-full max-w-lg rounded-lg border border-border bg-card shadow-xl flex flex-col max-h-[80vh]" role="dialog" aria-modal="true" onClick={e => e.stopPropagation()}> | ||
| {/* Header */} | ||
| <div className="flex items-center justify-between px-4 py-3 border-b border-border flex-shrink-0"> | ||
| <div> |
There was a problem hiding this comment.
role="dialog" elements should have an accessible name. This dialog currently lacks aria-label/aria-labelledby, so screen readers will announce it as an unnamed dialog. Consider adding aria-labelledby pointing to the header <h3> (and giving the <h3> a stable id, e.g. via useId()).
web/src/components/deploy/Deploy.tsx
Outdated
| <div className="bg-card border border-border rounded-xl shadow-2xl w-full max-w-sm mx-4 p-5" role="dialog" aria-modal="true" onClick={e => e.stopPropagation()}> | ||
| <h3 className="text-base font-medium text-foreground mb-1"> |
There was a problem hiding this comment.
This modal uses role="dialog" aria-modal="true" but does not provide an accessible name (aria-label or aria-labelledby). Add aria-labelledby referencing the modal heading (or an aria-label) so assistive tech can announce what dialog opened.
| <div className="bg-card border border-border rounded-xl shadow-2xl w-full max-w-sm mx-4 p-5" role="dialog" aria-modal="true" onClick={e => e.stopPropagation()}> | |
| <h3 className="text-base font-medium text-foreground mb-1"> | |
| <div | |
| className="bg-card border border-border rounded-xl shadow-2xl w-full max-w-sm mx-4 p-5" | |
| role="dialog" | |
| aria-modal="true" | |
| aria-labelledby="group-picker-dialog-title" | |
| onClick={e => e.stopPropagation()} | |
| > | |
| <h3 id="group-picker-dialog-title" className="text-base font-medium text-foreground mb-1"> |
| <div className="bg-card border border-border rounded-xl shadow-2xl max-w-lg w-full mx-4 max-h-[80vh] overflow-y-auto p-6" role="dialog" aria-modal="true" onClick={e => e.stopPropagation()}> | ||
| <div className="flex items-center justify-between mb-4"> | ||
| <h3 className="text-lg font-semibold">{showInstallGuide.mission.mission?.title ?? `Install ${installInfo?.project ?? 'Component'}`}</h3> | ||
| <button onClick={() => setShowInstallGuide(null)} className="p-1 hover:bg-secondary rounded"><X className="w-4 h-4" /></button> |
There was a problem hiding this comment.
The dialog container has role="dialog" aria-modal="true" but no accessible name, and the icon-only close <button> lacks an accessible label. Add aria-labelledby (or aria-label) for the dialog, and add an aria-label (or visible text) to the close button so screen readers can announce it correctly.
| <div className="bg-card border border-border rounded-xl shadow-2xl max-w-lg w-full mx-4 max-h-[80vh] overflow-y-auto p-6" role="dialog" aria-modal="true" onClick={e => e.stopPropagation()}> | |
| <div className="flex items-center justify-between mb-4"> | |
| <h3 className="text-lg font-semibold">{showInstallGuide.mission.mission?.title ?? `Install ${installInfo?.project ?? 'Component'}`}</h3> | |
| <button onClick={() => setShowInstallGuide(null)} className="p-1 hover:bg-secondary rounded"><X className="w-4 h-4" /></button> | |
| <div | |
| className="bg-card border border-border rounded-xl shadow-2xl max-w-lg w-full mx-4 max-h-[80vh] overflow-y-auto p-6" | |
| role="dialog" | |
| aria-modal="true" | |
| aria-labelledby="install-guide-title" | |
| onClick={e => e.stopPropagation()} | |
| > | |
| <div className="flex items-center justify-between mb-4"> | |
| <h3 | |
| id="install-guide-title" | |
| className="text-lg font-semibold" | |
| > | |
| {showInstallGuide.mission.mission?.title ?? `Install ${installInfo?.project ?? 'Component'}`} | |
| </h3> | |
| <button | |
| onClick={() => setShowInstallGuide(null)} | |
| className="p-1 hover:bg-secondary rounded" | |
| aria-label="Close dialog" | |
| > | |
| <X className="w-4 h-4" /> | |
| </button> |
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 2 code suggestion(s) and 1 general comment(s). @copilot Please apply all of the following code review suggestions:
Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
- Deploy.tsx: add aria-labelledby="group-picker-dialog-title" to dialog, add id="group-picker-dialog-title" to h3 - CardWrapper.tsx: add aria-labelledby="install-guide-title" to dialog, add id="install-guide-title" to h3, add aria-label="Close dialog" to icon-only close button - ClusterSelectionDialog.tsx: add aria-labelledby="cluster-selection-dialog-title" to dialog, add id="cluster-selection-dialog-title" to h3 Signed-off-by: copilot <copilot@github.com> Agent-Logs-Url: https://github.com/kubestellar/console/sessions/679eab4c-3066-439b-95c7-ec47afc42084 Co-authored-by: clubanderson <407614+clubanderson@users.noreply.github.com>
|
Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits. 📝 Please follow instructions in the contributing guide to update your commits with the DCO Full details of the Developer Certificate of Origin can be found at developercertificate.org. The list of commits missing DCO signoff:
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Applied all suggestions in commit
All three dialogs now have accessible names that screen readers will announce when the dialog opens. |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
Auto-QA flagged 9
divelements withonClickbut no ARIAroleoronKeyDown, failing accessibility parity checks.📌 Fixes
📝 Summary of Changes
Adds appropriate ARIA roles and accessible names to all 9 flagged clickable
divelements across 6 files. No behavioral changes — purely semantic markup fixes.Changes Made
role="presentation"Deploy.tsx,CardWrapper.tsx,ClusterSelectionDialog.tsx,ClusterAssignmentPanel.tsx,PayloadCard.tsxrole="dialog" aria-modal="true" aria-labelledbyDeploy.tsx,CardWrapper.tsx,ClusterSelectionDialog.tsxrole="presentation"ClusterGrid.tsxaria-label="Close dialog"CardWrapper.tsxRationale: Backdrop overlays are visual-only — keyboard users close via the dedicated Close button, so
role="presentation"is correct. Modal content containers getrole="dialog" aria-modal="true"so screen readers announce them correctly. Each dialog is also given an accessible name viaaria-labelledbypointing to its heading<h3>, and the icon-only close button inCardWrapperreceivesaria-label="Close dialog".ClusterGrid.tsx:171— structural wrapper →role="presentation"ClusterAssignmentPanel.tsx:182— dropdown backdrop →role="presentation"PayloadCard.tsx:203— dropdown backdrop →role="presentation"Deploy.tsx:265,266— modal backdroprole="presentation"+ contentrole="dialog" aria-modal="true" aria-labelledby="group-picker-dialog-title"CardWrapper.tsx:1261,1262— modal backdroprole="presentation"+ contentrole="dialog" aria-modal="true" aria-labelledby="install-guide-title"; icon-only close buttonaria-label="Close dialog"ClusterSelectionDialog.tsx:90,91— modal backdroprole="presentation"+ contentrole="dialog" aria-modal="true" aria-labelledby="cluster-selection-dialog-title"Checklist
git commit -s)Screenshots or Logs (if applicable)
N/A — markup-only changes with no visual impact.
👀 Reviewer Notes
No logic changes. All
onClickhandlers are unchanged — only ARIA roles and accessible names added. The three modal content wrappers (Deploy,CardWrapper,ClusterSelectionDialog) userole="dialog" aria-modal="true"witharia-labelledbypointing to their respective<h3>headings, ensuring screen readers can announce the dialog name when it opens.