-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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(radio): radio with modern syntax is keyboard navigable #27530
Conversation
Run & review this pull request in StackBlitz Codeflow. |
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.
Good to go once my other comment has been addressed.
Can you make the commit title more descriptive too? Instead of "tab to the next radio group" maybe we say "fix(radio): radio with modern syntax is keyboard navigable"?
Great job!
core/src/components/radio/radio.tsx
Outdated
@@ -135,7 +135,7 @@ export class Radio implements ComponentInterface { | |||
ev.stopPropagation(); | |||
ev.preventDefault(); | |||
|
|||
this.el.focus(); | |||
this.nativeInput.focus(); |
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 looks like this causes some issues with the legacy syntax just in Safari. Doing a Shift+Tab to get back to the previous radio group requires pressing Shift+Tab twice.
this.nativeInput.focus(); | |
const element = this.legacyFormController.hasLegacyControl() ? this.el : this.nativeInput; | |
element.focus(); |
Let's keep focusing this.el
only when in the legacy syntax.
Issue number: resolves #27268
What is the current behavior?
When tabbing on a radio group (modern syntax), it focuses on the next radio option inside the radio group. It replicates the behavior of the up/down keys.
What is the new behavior?
A spike ticket has been created to further investigate web accessibility and browser compatibility.
Does this introduce a breaking change?
Other information
N/A