fix: improve popup accessibility semantics#72
Merged
Conversation
cbulock
requested changes
Jun 30, 2026
cbulock
left a comment
cbulock
left a comment
Owner
There was a problem hiding this comment.
Two accessibility/interaction regressions stood out in this pass.
Please remove the popup semantics from the clear-date affordance, and avoid making the context-menu wrapper itself a second interactive control when consumers already slot a real button trigger.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves accessibility semantics for popup-style components in packages/core by making triggers keyboard focusable and wiring explicit ARIA relationships between triggers and their corresponding popup panels/menus.
Changes:
- Added stable generated IDs for menus/calendars and wired
aria-controls,aria-expanded, andaria-haspopupon relevant triggers. - Updated the date range picker to use a real
<button>trigger for the summary (instead of a focusable<div>), aligning keyboard behavior with native button semantics. - Added/extended component tests to cover the new accessibility relationships and keyboard interactions.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/components/date-range-picker/cindor-date-range-picker.ts | Adds a button-based summary trigger and wires popup ARIA relationships to the calendar panel. |
| packages/core/src/components/date-range-picker/cindor-date-range-picker.test.ts | Adds coverage asserting the summary trigger’s popup relationships and open-state behavior. |
| packages/core/src/components/date-picker/cindor-date-picker.ts | Wires popup ARIA relationships onto the readonly input trigger and icon-button controls. |
| packages/core/src/components/date-picker/cindor-date-picker.test.ts | Adds coverage asserting popup relationship attributes and their updates when opened. |
| packages/core/src/components/context-menu/cindor-context-menu.ts | Makes the trigger keyboard-focusable and adds ARIA relationships between trigger and menu. |
| packages/core/src/components/context-menu/cindor-context-menu.test.ts | Adds coverage for keyboard-open behavior and trigger/menu relationship attributes. |
Comment on lines
174
to
178
| <cindor-icon-button | ||
| aria-controls=${this.panelId} | ||
| aria-expanded=${String(this.open)} | ||
| aria-haspopup="grid" | ||
| label="Clear date" |
Comment on lines
145
to
149
| <cindor-icon-button | ||
| aria-controls=${this.panelId} | ||
| aria-expanded=${String(this.open)} | ||
| aria-haspopup="grid" | ||
| label=${this.open ? "Close calendar" : "Open calendar"} |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification