fix(overlays,picker): remove invalid aria-hidden attribute #30563
Conversation
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
…dex="-1" This is needed in order to prevent A11Y issues, in order to prevent to be possible to navigate through each option item through tab navigation, structure must change into this.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
lgtm, please merge |
brandyscarney
changed the title
brandyscarney
changed the title
brandyscarney
approved these changes
Sep 2, 2025
core/src/components/picker-column-option/test/a11y/picker-column-option.e2e.ts
Outdated
Show resolved
Hide resolved
…mn-option.e2e.ts Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
core/src/components/picker-column-option/test/a11y/picker-column-option.e2e.ts
Outdated
Show resolved
Hide resolved
…mn-option.e2e.ts Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
2 tasks
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 24, 2025
Issue number: resolves #30684 --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> Recently, we [fixed some issues with aria-hidden in modals](#30563), unfortunately at this time we neglected modals that opt out of focus trapping. As a result, a lot of modals that disable focus trapping still have it happening and it doesn't get cleaned up properly on dismiss. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> We're now properly checking for and skipping focus traps on modals that do not want them. ## Does this introduce a breaking change? - [ ] Yes - [X] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information I created regression tests for Angular in this to prevent this from happening again. I initially tried to do this with core, but the issue doesn't seem to reproduce with core. <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> Current dev build: ``` 8.7.5-dev.11758652700.103435a3 ```
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.
Issue number: resolves #30040
What is the current behavior?
aria-hidden="true"attributes both on overlay and picker components was causing some console error/warnings messages.What is the new behavior?
There is no need of making usage of this attribute due the facts:
display: none;through the selector:host(.overlay-hidden), which by itself will disable overlay content for the screen readers.Removed overlay tests since they were basically checking about
aria-hiddenattribute.Updated PickerColumn and PickerColumnOption structure in order to fit the a11y needs.
Added a focus management system to better drive users when making usage of keyboard navigation inside picker.
Skip A11Y test validation when reported violation is color contrast related.
tabIndex="-1"andaria-hidden="true"- Which turns into an A11Y violation when it gets focused.It happens that it gets focused dynamically in order to open the native numeric keyboard once user selects highlighted picker values zone, in order to allow users to insert numeric values through the keyboard.
If this
aria-hiddenandtabindexare removed/updated, the existing functionality will be lost since ScreenReaders will start to ignore the updated value through the PickerChange and will be focused onto the focused input.This mentioned input has an onChange event that's used to update the
aria-valuetexton eachpicker-columnwhich is being capture by the ScreenReader.That said, this new devTools reported issue/warning is a false positive since A11Y behaviour is being covered through a different perspective.
Does this introduce a breaking change?
Testing:
Other Information
Dev build:
8.7.4-dev.11756388042.1a103a79