Skip to content
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: refactor radio group to better manage disabled state and interactions #6601

Merged
merged 6 commits into from
Feb 14, 2023

Conversation

chrisdholt
Copy link
Member

Pull Request

📖 Description

This PR proposes a slight refactor of radio group in order to support the disabling of the radiogroup without modifying the state of individual radio children. This PR does a few things:

  1. Removes click handling behavior for the radio itself and manages via the group. Per the APG, radios which are not selected (primarily on page-load scenarios) can be selected via "Space" so that behavior remains in tact.

  2. Removes behavior modifying each disabled items disabled state. For the sake of ensuring we don't break with future changes to the disabledChanged method, I've left this enumerated as protected for now.

  3. Assigns a tabindex of -1 to prevent focus from being brought to the element and its children.

  4. To ensure that focus does not pass to the radiogroup when clicked, a mousedown event has been added to prevent focus when disabled.

🎫 Issues

fixes #6567

👩‍💻 Reviewer Notes

📑 Test Plan

Tests have been added for most all pertinent scenarios

✅ Checklist

General

  • I have included a change request file using $ yarn change
  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

⏭ Next Steps

Copy link
Collaborator

@radium-v radium-v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

radio-group has some special interaction behaviors when in a toolbar, so it'd be good to just double-check that everything here works in that scenario as well (it probably does).

@chrisdholt chrisdholt force-pushed the users/chhol/radio-group-disabled-state branch from 69bd45e to 6327c55 Compare February 13, 2023 19:20
@chrisdholt chrisdholt merged commit e504d09 into master Feb 14, 2023
@chrisdholt chrisdholt deleted the users/chhol/radio-group-disabled-state branch February 14, 2023 00:09
janechu pushed a commit that referenced this pull request Jun 10, 2024
…tions (#6601)

* fix radio group disabled handling

* add disabled mousedown event to prevent focus events on click when disabled

* Change files

* remove commented out disabledChanged method and leave protected

* api report

* fix tabindex template interpolation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix: disabled state in radio-group
3 participants