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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Switch] make sure wasOn is set properly #1871

Merged
merged 1 commit into from
Jul 6, 2023
Merged

Conversation

Saadnajmi
Copy link
Collaborator

@Saadnajmi Saadnajmi commented Jul 6, 2023

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 馃憤
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 馃憤
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

Separating out a change from #1867 to reduce the footprint of that PR.

When a <Switch> is created on macOS with an initial value of on, it would not report it's change the first time you toggled it. This seems to be because an internal variable _wasOn was not set properly on macOS, which the onChange handler relied on. Comparing Appkit to UIKit, I think this is because in UIKit, setting the property on will internally call setOn:Animated, which RN overrides to set _wasOn. We had to reimplement both on and setOn:Animated for macOS. Let's just have the setter for on call setOn:Animated to fix this issue.

Changelog

Pick one each for the category and type tags:

[MACOS] [FIXED] - Make sure wasOn is set properly in Switch

Test Plan

Before (from #1867):

1.mov

After:

Screen.Recording.2023-07-05.at.10.46.08.PM.mov

@Saadnajmi Saadnajmi merged commit 28dd130 into microsoft:main Jul 6, 2023
17 of 18 checks passed
@Saadnajmi Saadnajmi deleted the switch branch July 6, 2023 16:56
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.

None yet

3 participants