react-native/Libraries/Components/Picker/PickerAndroid.android.js
Kudo Chien 6a3d9c06ce Fix Picker.onValueChange on Android sometimes not fired due to race condition (#22821)
Summary:
Changelog:
----------

[Android] [Fixed] - Fix Picker.onValueChange sometimes not fired due to race condition. Fixes #15556

Root Cause:
------------

Please check my code snippet at https://snack.expo.io/kudochien/android-picker-issue
and try to select different items on Picker to see if console.log() hit.
If calling setState() with some latency, e.g. setTimeout() or changes from redux,
the second time changing picker item on UI, the onValueChange() will be not fired.

The root cause comes from the `forceUpdate` in PickerAndroid.android.js.
If user's setState() update comes after forceUpdate(), the flow will be:
1. First time select picker item
2. onValueChange + forceUpdate
3. user's setState() + componentDidUpdate + setNativeProps({ selected: ... })
4. mSuppressNextEvent = true
5. Second time select picker item
6. Since mSuppressNextEvent is true, the onValueChange will not be fired.

Solution:
---------

Like other controlled components, disable change listener during setup values from JS.
Android Spinner `setSelection(int position)` is asynchronous call, i.e. will fire onItemSelected in next run loop and is not suitable for us.
`setSelection(int position, boolean animate)`, however, is synchronous call which I used.

Some more references about setSelection:
https://stackoverflow.com/a/43512925/2590265
http://androidxref.com/8.1.0_r33/xref/frameworks/base/core/java/android/widget/AbsSpinner.java#276
The two arguments version will use `setSelectionInt()` which set mBlockLayoutRequests as true to prevent onItemSelected call from next layout().

I also moved the setOnItemSelectedListener() call after onLayout to prevent onValueChange() during intialization.
Pull Request resolved: https://github.com/facebook/react-native/pull/22821

Differential Revision: D13731979

Pulled By: cpojer

fbshipit-source-id: e06bd9aa62463b66c8f3fd7214485898d5375054
2019-01-18 08:12:12 -08:00

152 lines
4.4 KiB
JavaScript

/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @format
* @flow strict-local
*/
'use strict';
const AndroidDropdownPickerNativeComponent = require('AndroidDropdownPickerNativeComponent');
const AndroidDialogPickerNativeComponent = require('AndroidDialogPickerNativeComponent');
const React = require('React');
const StyleSheet = require('StyleSheet');
const processColor = require('processColor');
const REF_PICKER = 'picker';
const MODE_DROPDOWN = 'dropdown';
import type {SyntheticEvent} from 'CoreEventTypes';
import type {TextStyleProp} from 'StyleSheet';
type PickerAndroidChangeEvent = SyntheticEvent<
$ReadOnly<{|
position: number,
|}>,
>;
type PickerAndroidProps = $ReadOnly<{|
children?: React.Node,
style?: ?TextStyleProp,
selectedValue?: ?(number | string),
enabled?: ?boolean,
mode?: ?('dialog' | 'dropdown'),
onValueChange?: ?(itemValue: ?(string | number), itemIndex: number) => mixed,
prompt?: ?string,
testID?: string,
|}>;
type Item = $ReadOnly<{|
label: string,
value: ?(number | string),
color?: ?number,
|}>;
type PickerAndroidState = {|
selectedIndex: number,
items: $ReadOnlyArray<Item>,
|};
/**
* Not exposed as a public API - use <Picker> instead.
*/
class PickerAndroid extends React.Component<
PickerAndroidProps,
PickerAndroidState,
> {
static getDerivedStateFromProps(
props: PickerAndroidProps,
): PickerAndroidState {
let selectedIndex = 0;
const items = React.Children.map(props.children, (child, index) => {
if (child.props.value === props.selectedValue) {
selectedIndex = index;
}
const childProps = {
value: child.props.value,
label: child.props.label,
};
if (child.props.color) {
/* $FlowFixMe(>=0.78.0 site=react_native_android_fb) This issue was
* found when making Flow check .android.js files. */
childProps.color = processColor(child.props.color);
}
return childProps;
});
return {selectedIndex, items};
}
state = PickerAndroid.getDerivedStateFromProps(this.props);
render() {
const Picker =
this.props.mode === MODE_DROPDOWN
? AndroidDropdownPickerNativeComponent
: AndroidDialogPickerNativeComponent;
const nativeProps = {
enabled: this.props.enabled,
items: this.state.items,
mode: this.props.mode,
onSelect: this._onChange,
prompt: this.props.prompt,
selected: this.state.selectedIndex,
testID: this.props.testID,
style: [styles.pickerAndroid, this.props.style],
/* $FlowFixMe(>=0.78.0 site=react_native_android_fb) This issue was found
* when making Flow check .android.js files. */
accessibilityLabel: this.props.accessibilityLabel,
};
return <Picker ref={REF_PICKER} {...nativeProps} />;
}
_onChange = (event: PickerAndroidChangeEvent) => {
if (this.props.onValueChange) {
const position = event.nativeEvent.position;
if (position >= 0) {
const children = React.Children.toArray(this.props.children);
const value = children[position].props.value;
/* $FlowFixMe(>=0.78.0 site=react_native_android_fb) This issue was
* found when making Flow check .android.js files. */
this.props.onValueChange(value, position);
} else {
this.props.onValueChange(null, position);
}
}
// The picker is a controlled component. This means we expect the
// on*Change handlers to be in charge of updating our
// `selectedValue` prop. That way they can also
// disallow/undo/mutate the selection of certain values. In other
// words, the embedder of this component should be the source of
// truth, not the native component.
if (
this.refs[REF_PICKER] &&
this.state.selectedIndex !== event.nativeEvent.position
) {
this.refs[REF_PICKER].setNativeProps({
selected: this.state.selectedIndex,
});
}
};
}
const styles = StyleSheet.create({
pickerAndroid: {
// The picker will conform to whatever width is given, but we do
// have to set the component's height explicitly on the
// surrounding view to ensure it gets rendered.
// TODO would be better to export a native constant for this,
// like in iOS the RCTDatePickerManager.m
height: 50,
},
});
module.exports = PickerAndroid;