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 handling of controlled state in toggle-able controls #3834

Merged

Conversation

rurikoaraki
Copy link
Collaborator

@rurikoaraki rurikoaraki commented Jan 14, 2025

Platforms Impacted

  • iOS
  • macOS
  • win32 (Office)
  • windows
  • android

Description of changes

The hook we use to handle toggle events, useAsToggleWithEvent, does not properly take controlled state into account. In particular it updates its internal state regardless of whether the check state is controlled and doesn't apply state applied from the parent correctly, which means that when its checked state is applied via not toggling the control directly, the state becomes out of sync with that the app believes its state should be.

For example, if a checkbox is meant to be controlled and toggling it fires an event to a parent, the behavior would look like:

  1. Click the checkbox: Internal state: checked, visual state: checked, tell parent checkbox is checked
  2. Parent tells checkbox it's checked, internal state: checked, visual state: checked
  3. Parent tells checkbox it's not checked, internal state: checked, visual state: not checked
  4. Click the checkbox: internal state: not checked, visual state: not checked <- bug

Fix is to use the useControllableValue hook to store current checked state, which takes controlled state into account when tracking a boolean's value.

Verification

New test added for the case where a controlled checkbox is inside a Pressable and both can toggle the checked state:
image

Ensured test above toggles as expected with various configurations of toggling via checkbox as well as external controls (Pressable, the buttons).
Tested uncontrolled checkboxes
Tested switches, toggle button

Pull request checklist

This PR has considered (when applicable):

  • Automated Tests
  • Documentation and examples
  • Keyboard Accessibility
  • Voiceover
  • Internationalization and Right-to-left Layouts

@rurikoaraki rurikoaraki requested a review from a team as a code owner January 14, 2025 19:58
@rurikoaraki rurikoaraki enabled auto-merge (squash) January 14, 2025 22:49
@rurikoaraki rurikoaraki merged commit b91697e into microsoft:main Jan 15, 2025
11 checks passed
@rurikoaraki rurikoaraki deleted the user/ruaraki/checkbox_controlled_fixes branch January 15, 2025 19:45
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.

2 participants