-
Notifications
You must be signed in to change notification settings - Fork 6.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(material/radio): not checked on first click if partially visible #19505
fix(material/radio): not checked on first click if partially visible #19505
Conversation
@@ -5,7 +5,7 @@ | |||
<div class="mat-radio-container"> | |||
<div class="mat-radio-outer-circle"></div> | |||
<div class="mat-radio-inner-circle"></div> | |||
<input #input class="mat-radio-input cdk-visually-hidden" type="radio" | |||
<input #input class="mat-radio-input" type="radio" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jelbourn I don't remember why we ended up with cdk-visually-hidden
in the first place, but from my testing, screen readers treat it exactly the same as when the input is opacity: 0
. Given that we have the same issue in the other input-based components (slide toggle and checkbox), would it make sense to stop using it on native inputs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK we can't rely on just opacity: 0
. I looked through my notes with Google a11y folks and had this from 2018:
Accessibility tree visibility of elements with `opacity: 0` across screen readers?
- Cannot rely on this.
- Possibly could set to .001
- Probably JAWS also, potentially in IE only
Which I interpret as JAWS treating opacity: 0
as fully hidden.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that IE used to have some security restrictions if an input[type="file"]
has opacity: 0
. I wonder whether that's not related to your notes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does the MDC radio do here?
@@ -5,7 +5,7 @@ | |||
<div class="mat-radio-container"> | |||
<div class="mat-radio-outer-circle"></div> | |||
<div class="mat-radio-inner-circle"></div> | |||
<input #input class="mat-radio-input cdk-visually-hidden" type="radio" | |||
<input #input class="mat-radio-input" type="radio" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK we can't rely on just opacity: 0
. I looked through my notes with Google a11y folks and had this from 2018:
Accessibility tree visibility of elements with `opacity: 0` across screen readers?
- Cannot rely on this.
- Possibly could set to .001
- Probably JAWS also, potentially in IE only
Which I interpret as JAWS treating opacity: 0
as fully hidden.
MDC does the same for their radio button: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish I could test JAWS, but that machine is in the office. Since MDC does it, though, I guess it doesn't really matter.
LGTM
Currently the native `input` inside the radio button is collapsed down to 1px by 1px and moved to the bottom of the element which causes browsers to scroll the window down, rather than change the value, on the first click when the input is partially hidden. These changes fix the issue by having the input cover the entire button and hiding it with `opacity`. From my testing, using `opacity` versus `cdk-visually-hidden` doesn't make a difference for screen readers. Fixes angular#19397.
1159022
to
ff1564e
Compare
…19505) Currently the native `input` inside the radio button is collapsed down to 1px by 1px and moved to the bottom of the element which causes browsers to scroll the window down, rather than change the value, on the first click when the input is partially hidden. These changes fix the issue by having the input cover the entire button and hiding it with `opacity`. From my testing, using `opacity` versus `cdk-visually-hidden` doesn't make a difference for screen readers. Fixes #19397. (cherry picked from commit f80403c)
Fixes a regression caused by angular#19505 where clicking directly on the `input` element will prevent the click from reaching custom `click` listeners outside the radio button.
Fixes a regression caused by #19505 where clicking directly on the `input` element will prevent the click from reaching custom `click` listeners outside the radio button.
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/cdk](https://github.com/angular/components) | dependencies | patch | [`13.2.3` -> `13.2.4`](https://renovatebot.com/diffs/npm/@angular%2fcdk/13.2.3/13.2.4) | | [@angular/material](https://github.com/angular/components) | dependencies | patch | [`13.2.3` -> `13.2.4`](https://renovatebot.com/diffs/npm/@angular%2fmaterial/13.2.3/13.2.4) | --- ### Release Notes <details> <summary>angular/components</summary> ### [`v13.2.4`](https://github.com/angular/components/blob/HEAD/CHANGELOG.md#​1324-plastic-mug-2022-02-23) [Compare Source](angular/components@13.2.3...13.2.4) ##### cdk | Commit | Type | Description | | -- | -- | -- | | [74bae85bc5](angular/components@74bae85) | fix | **drag-drop:** incorrectly sorting element inside dialog with blocked scrolling ([#​14806](angular/components#14806)) | | [81898ca5f6](angular/components@81898ca) | fix | **drag-drop:** stop pointer events on placeholder ([#​24404](angular/components#24404)) | ##### material | Commit | Type | Description | | -- | -- | -- | | [6b76469b4a](angular/components@6b76469) | fix | **autocomplete:** closing immediately when input is focused programmatically ([#​21081](angular/components#21081)) | | [3ea76419c8](angular/components@3ea7641) | fix | **autocomplete:** use narrow value for aria-haspopup ([#​15361](angular/components#15361)) | | [9a12eabf6b](angular/components@9a12eab) | fix | **button-toggle:** unable to override elevation and high contrast styling applied incorrectly ([#​14722](angular/components#14722)) | | [cbd4b0ce4f](angular/components@cbd4b0c) | fix | **checkbox:** clear static aria attributes from host nodes ([#​17092](angular/components#17092)) | | [f6eaa7c1cf](angular/components@f6eaa7c) | fix | **form-field:** use correct color for form fields in high contrast mode ([#​24422](angular/components#24422)) | | [39d7834797](angular/components@39d7834) | fix | **radio:** clicks not propagating to wrapper elements ([#​24459](angular/components#24459)) | | [5988b8f77b](angular/components@5988b8f) | fix | **radio:** not checked on first click if partially visible ([#​19505](angular/components#19505)) | | [33716f124b](angular/components@33716f1) | fix | **select:** arrow highlighted state not updating in Safari ([#​15281](angular/components#15281)) | | [fc204e4f4d](angular/components@fc204e4) | fix | **sidenav:** prevent focus from entering hidden sidenav if child element has a visibility | | [5e41a0ad09](angular/components@5e41a0a) | fix | **tabs:** use buttons for paginator also tab-header and mdc ([#​24338](angular/components#24338)) | ##### material-experimental | Commit | Type | Description | | -- | -- | -- | | [4198f5b5dc](angular/components@4198f5b) | fix | **mdc-dialog:** align change detection with non-MDC version ([#​24451](angular/components#24451)) | | [45836f924d](angular/components@45836f9) | fix | **mdc-list:** fix typo in action-list css class ([#​24448](angular/components#24448)) | | [7ca02495cd](angular/components@7ca0249) | fix | **mdc-list:** use body-1 rather than subtitle-1 typography for list items ([#​24417](angular/components#24417)) | | [c9a15476e8](angular/components@c9a1547) | fix | **mdc-select:** target correct element with typography ([#​24258](angular/components#24258)) | | [bd3f39fb15](angular/components@bd3f39f) | perf | **mdc-table:** reduce bundle size ([#​24309](angular/components#24309)) | #### Special Thanks Alireza Ebrahimkhani, Arthur Ming, Jeri Peier, Kristiyan Kostadinov, Miles Malerba, Paul Gschwendtner and renovate\[bot] <!-- CHANGELOG SPLIT MARKER --> </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1188 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Currently the native
input
inside the radio button is collapsed down to 1px by 1px and moved to the bottom of the element which causes browsers to scroll the window down, rather than change the value, on the first click when the input is partially hidden. These changes fix the issue by having the input cover the entire button and hiding it withopacity
. From my testing, usingopacity
versuscdk-visually-hidden
doesn't make a difference for screen readers.Fixes #19397.