-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(material/radio): remove tabindex from host node #21280
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
Conversation
Removes the `tabindex` from the host node, because it causes some a11y issues. We had it there before so that things like `cdkFocusInitial` would work, but after angular#21046 it isn't necessary anymore. Fixes angular#21266.
expect(radioButtonEl.getAttribute('tabindex')).toBe('-1'); | ||
}); | ||
|
||
it('should set the tabindex to -1 on the host element', () => { |
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.
This test was exactly the same as the one above so I removed it.
@@ -97,7 +97,8 @@ export class MatRadioGroup extends _MatRadioGroupBase<MatRadioButton> { | |||
'[class.mat-primary]': 'color === "primary"', | |||
'[class.mat-accent]': 'color === "accent"', | |||
'[class.mat-warn]': 'color === "warn"', | |||
'[attr.tabindex]': 'disabled ? null : -1', | |||
// Needs to be removed since it causes some a11y issues (see #21266). | |||
'[attr.tabindex]': 'null', |
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.
does it specifically have to be null or can it just be removed?
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.
It has to be null, because the tabindex can be set as <mat-radio tabindex="2">
which will set the input, but also keep the native attribute.
Similar to angular#21280. Removes the `tabindex` from the `mat-datepicker-toggle` host since it can caused accessibility issues. It was only there as a workaround which isn't required after angular#21046.
Removes the `tabindex` from the host node, because it causes some a11y issues. We had it there before so that things like `cdkFocusInitial` would work, but after angular#21046 it isn't necessary anymore. Fixes angular#21266.
Similar to angular#21280. Removes the `tabindex` from the `mat-datepicker-toggle` host since it can caused accessibility issues. It was only there as a workaround which isn't required after angular#21046.
Similar to angular#21280. Removes the `tabindex` from the `mat-datepicker-toggle` host since it can caused accessibility issues. It was only there as a workaround which isn't required after angular#21046.
Similar to angular#21280. Removes the `tabindex` from the `mat-datepicker-toggle` host since it can caused accessibility issues. It was only there as a workaround which isn't required after angular#21046.
Similar to angular#21280. Removes the `tabindex` from the `mat-datepicker-toggle` host since it can caused accessibility issues. It was only there as a workaround which isn't required after angular#21046.
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. |
Removes the
tabindex
from the host node, because it causes some a11y issues. We had it there before so that things likecdkFocusInitial
would work, but after #21046 it isn't necessary anymore.Fixes #21266.