Skip to content

Commit

Permalink
Fix excessive toggles on the Switch component (#26496)
Browse files Browse the repository at this point in the history
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: #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
  • Loading branch information
rurikoaraki authored and facebook-github-bot committed Sep 19, 2019
1 parent 9f0dede commit b782934
Showing 1 changed file with 21 additions and 9 deletions.
30 changes: 21 additions & 9 deletions Libraries/Components/Switch/Switch.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ class Switch extends React.Component<Props> {
_nativeSwitchRef: ?React.ElementRef<
typeof SwitchNativeComponent | typeof AndroidSwitchNativeComponent,
>;
_lastNativeValue: ?boolean;

render(): React.Node {
const {
Expand Down Expand Up @@ -196,26 +197,37 @@ class Switch extends React.Component<Props> {
);
}

_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);
}

if (this.props.onValueChange != null) {
this.props.onValueChange(event.nativeEvent.value);
}

this._lastNativeValue = event.nativeEvent.value;
this.forceUpdate();
};

_handleSwitchNativeComponentRef = (
Expand Down

0 comments on commit b782934

Please sign in to comment.