-
Notifications
You must be signed in to change notification settings - Fork 81
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 KRadioButton autofocus on dynamic rendering #492
Fix KRadioButton autofocus on dynamic rendering #492
Conversation
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.
Thank you @AlexVelezLl. good work! Code seems to be fine. Just left one minor note about the changelog. I tested manually and confirm it works.
Is there a corresponding issue in Kolibri in which scope we would introduce this?
CHANGELOG.md
Outdated
@@ -7,6 +7,17 @@ Changelog is rather internal in nature. See release notes for the public overvie | |||
|
|||
## Version 2.0.0 | |||
|
|||
- [#492] | |||
- **Description:** Add autofocus directive on KRadioButton to fix autofocus behavior on dynamic rendering. | |||
- **Products impact:** Kolibri's Setup Wizard |
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.
Could you please change this to "bugfix"? This is not as much about the place it fixes, but rather a general category. There are some examples in the PR template you can choose from.
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.
Oh, hadn't noticed that. Thanks!
- **Components:** KRadioButton | ||
- **Breaking:** no | ||
- **Impacts a11y:** yes | ||
- **Guidance:** Add "autofocus" prop on KRadioButton. This change improves keyboard navigation on forms where a KRadioButton jumps into the DOM. |
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.
Nice, thanks!
Also, one note on the process, after we merge a KDS PR, it is customary to update KDS version on Kolibri's |
Yes, it should help address the issue Kolibri#11458, I already added the |
Ah great, in that case you can install this KDS version in your existing Kolibri PR. Let me know if you needed help. |
Description
Add autofocus directive on KRadioButton to fix autofocus behavior on dynamic rendering.
Issue addressed
Closes #489
Before/after screenshots
Before
Compartir.pantalla.-.2023-11-16.09_06_25.mp4
After
Compartir.pantalla.-.2023-11-23.11_42_57.mp4
Changelog
Steps to test
Testing checklist
Reviewer guidance
After review
CHANGELOG.md