kylefang/react-native
1
0
Fork 0
mirror of https://github.com/zhigang1992/react-native.git synced 2026-04-19 13:08:11 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fix Picker.onValueChange on Android sometimes not fired due to race condition (#22821)

Browse Source 
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
This commit is contained in:
Kudo Chien
2019-01-18 08:09:07 -08:00
committed by Facebook Github Bot
parent 2191c9ed58
commit 6a3d9c06ce
2 changed files with 42 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

62
ReactAndroid/src/main/java/com/facebook/react/views/picker/ReactPicker.java
View File

@@ -7,8 +7,6 @@
package com.facebook.react.views.picker;
import javax.annotation.Nullable;
import android.content.Context;
import android.util.AttributeSet;
import android.view.View;
@@ -17,14 +15,31 @@ import android.widget.Spinner;
import com.facebook.react.common.annotations.VisibleForTesting;
import javax.annotation.Nullable;
public class ReactPicker extends Spinner {
private int mMode = MODE_DIALOG;
private @Nullable Integer mPrimaryColor;
private boolean mSuppressNextEvent;
private @Nullable OnSelectListener mOnSelectListener;
private @Nullable Integer mStagedSelection;
private final OnItemSelectedListener mItemSelectedListener = new OnItemSelectedListener() {
@Override
public void onItemSelected(AdapterView<?> parent, View view, int position, long id) {
if (mOnSelectListener != null) {
mOnSelectListener.onItemSelected(position);
}
}
@Override
public void onNothingSelected(AdapterView<?> parent) {
if (mOnSelectListener != null) {
mOnSelectListener.onItemSelected(-1);
}
}
};
/**
* Listener interface for ReactPicker events.
*/
@@ -75,31 +90,19 @@ public class ReactPicker extends Spinner {
post(measureAndLayout);
}
public void setOnSelectListener(@Nullable OnSelectListener onSelectListener) {
if (getOnItemSelectedListener() == null) {
// onItemSelected gets fired immediately after layout because checkSelectionChanged() in
// AdapterView updates the selection position from the default INVALID_POSITION. To match iOS
// behavior, we don't want the event emitter for onItemSelected to fire right after layout.
mSuppressNextEvent = true;
setOnItemSelectedListener(
new OnItemSelectedListener() {
@Override
public void onItemSelected(AdapterView<?> parent, View view, int position, long id) {
if (!mSuppressNextEvent && mOnSelectListener != null) {
mOnSelectListener.onItemSelected(position);
}
mSuppressNextEvent = false;
}
@Override
protected void onLayout(boolean changed, int left, int top, int right, int bottom) {
super.onLayout(changed, left, top, right, bottom);
@Override
public void onNothingSelected(AdapterView<?> parent) {
if (!mSuppressNextEvent && mOnSelectListener != null) {
mOnSelectListener.onItemSelected(-1);
}
mSuppressNextEvent = false;
}
});
}
// onItemSelected gets fired immediately after layout because checkSelectionChanged() in
// AdapterView updates the selection position from the default INVALID_POSITION.
// To match iOS behavior, which no onItemSelected during initial layout.
// We setup the listener after layout.
if (getOnItemSelectedListener() == null)
setOnItemSelectedListener(mItemSelectedListener);
}
public void setOnSelectListener(@Nullable OnSelectListener onSelectListener) {
mOnSelectListener = onSelectListener;
}
@@ -130,8 +133,9 @@ public class ReactPicker extends Spinner {
*/
private void setSelectionWithSuppressEvent(int position) {
if (position != getSelectedItemPosition()) {
mSuppressNextEvent = true;
setSelection(position);
setOnItemSelectedListener(null);
setSelection(position, false);
setOnItemSelectedListener(mItemSelectedListener);
}
}