From b782934f3f2a80ae7e3872cc7d7a610aa6680ec4 Mon Sep 17 00:00:00 2001 From: Ruriko Araki Date: Thu, 19 Sep 2019 12:44:13 -0700 Subject: [PATCH] Fix excessive toggles on the Switch component (#26496) Summary: In Windows, if you clicked on a Switch component to toggle it, you could see it "shimmy" back and forth once before settling. The native Switch ends up toggling three times every time it's invoked. `Switch.js` prevents the native switch from toggling to the new value by explicitly setting the switch to `this.props.value` when it receives an `onChange` event. The re-setting of the value wasn't fast enough to prevent the `Switch` from starting to toggle, causing the visual shimmy. The solution is taken from `TextInput`. `TextInput.js` stores `_lastNativeText` when it receives an `onChange` event. In `componentDidUpdate`, it puts `this.props.text` back on the native textbox if the value of `this.props.text` isn't the same as `_lastNativeText`, which is how it ensures that it is a controlled component. Porting this to the `Switch` results in only one toggle happening per invoke, removing the shimmy, while preserving the controlled component behavior. This bug is not visible on Android or iOS, only Windows, however the code causing the bug was in `Switch.js` and it seems reasonable to avoid changing the value of the native switch excessively. ## Changelog [General] [Fixed] - Fix excessive toggles on the Switch component Pull Request resolved: https://github.com/facebook/react-native/pull/26496 Test Plan: Used RNTester on Android and iOS to test the Switch component and made sure that all scenarios behave as expected visually. Also ensured through the debugger that the value of the native switch is only being changed once, instead of three times. Reviewed By: TheSavior Differential Revision: D17468905 Pulled By: JoshuaGross fbshipit-source-id: 92bf511510306968c3573ee4eed6df009850fd77 --- Libraries/Components/Switch/Switch.js | 30 +++++++++++++++++++-------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/Libraries/Components/Switch/Switch.js b/Libraries/Components/Switch/Switch.js index 3eb9f092f883d6..26cbf55391bafd 100644 --- a/Libraries/Components/Switch/Switch.js +++ b/Libraries/Components/Switch/Switch.js @@ -92,6 +92,7 @@ class Switch extends React.Component { _nativeSwitchRef: ?React.ElementRef< typeof SwitchNativeComponent | typeof AndroidSwitchNativeComponent, >; + _lastNativeValue: ?boolean; render(): React.Node { const { @@ -196,19 +197,27 @@ class Switch extends React.Component { ); } - _handleChange = (event: SwitchChangeEvent) => { - if (this._nativeSwitchRef == null) { - return; + componentDidUpdate() { + // This is necessary in case native updates the switch and JS decides + // that the update should be ignored and we should stick with the value + // that we have in JS. + const nativeProps = {}; + const value = this.props.value === true; + + if (this._lastNativeValue !== value && typeof value === 'boolean') { + nativeProps.value = value; } - // Force value of native switch in order to control it. - const value = this.props.value === true; - if (Platform.OS === 'android') { - this._nativeSwitchRef.setNativeProps({on: value}); - } else { - this._nativeSwitchRef.setNativeProps({value}); + if ( + Object.keys(nativeProps).length > 0 && + this._nativeSwitchRef && + this._nativeSwitchRef.setNativeProps + ) { + this._nativeSwitchRef.setNativeProps(nativeProps); } + } + _handleChange = (event: SwitchChangeEvent) => { if (this.props.onChange != null) { this.props.onChange(event); } @@ -216,6 +225,9 @@ class Switch extends React.Component { if (this.props.onValueChange != null) { this.props.onValueChange(event.nativeEvent.value); } + + this._lastNativeValue = event.nativeEvent.value; + this.forceUpdate(); }; _handleSwitchNativeComponentRef = (