Skip to content

Commit 310cc38

Browse files
Kudofacebook-github-bot
authored andcommitted
Fix PickerAndroid will reset selected value during items update. (#24793)
Summary: Fixes #13351 Two root causes: 1. Android Spinner will reset selection to undefined after setAdapter() which will trigger onValueChange(). The behavior is not expected for RN. And the solution is to setSelection() explicitly 2. In original implementation, it setups `items` immediately, but delays the `selected` setup after update transaction. There will be some race condition and incosistency if update `items` only. The fix will do the setup all after update transaction. [Android] [Fixed] - Fix #13351 PickerAndroid will reset selected value during items update. Pull Request resolved: #24793 Differential Revision: D15293516 Pulled By: cpojer fbshipit-source-id: 5a99a60015c7e1b2968252cdc0b2661d52a15b9d
1 parent ebeb893 commit 310cc38

File tree

2 files changed

+32
-21
lines changed

2 files changed

+32
-21
lines changed

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

+29-18
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import android.view.View;
1414
import android.widget.AdapterView;
1515
import android.widget.Spinner;
16+
import android.widget.SpinnerAdapter;
1617

1718
import com.facebook.react.common.annotations.VisibleForTesting;
1819

@@ -23,6 +24,7 @@ public class ReactPicker extends AppCompatSpinner {
2324
private int mMode = Spinner.MODE_DIALOG;
2425
private @Nullable Integer mPrimaryColor;
2526
private @Nullable OnSelectListener mOnSelectListener;
27+
private @Nullable SpinnerAdapter mStagedAdapter;
2628
private @Nullable Integer mStagedSelection;
2729

2830
private final OnItemSelectedListener mItemSelectedListener = new OnItemSelectedListener() {
@@ -111,33 +113,42 @@ public void setOnSelectListener(@Nullable OnSelectListener onSelectListener) {
111113
return mOnSelectListener;
112114
}
113115

116+
/* package */ void setStagedAdapter(final SpinnerAdapter adapter) {
117+
mStagedAdapter = adapter;
118+
}
119+
114120
/**
115-
* Will cache "selection" value locally and set it only once {@link #updateStagedSelection} is
121+
* Will cache "selection" value locally and set it only once {@link #commitStagedData} is
116122
* called
117123
*/
118-
public void setStagedSelection(int selection) {
124+
/* package */ void setStagedSelection(int selection) {
119125
mStagedSelection = selection;
120126
}
121127

122-
public void updateStagedSelection() {
123-
if (mStagedSelection != null) {
124-
setSelectionWithSuppressEvent(mStagedSelection);
125-
mStagedSelection = null;
126-
}
127-
}
128-
129128
/**
130-
* Set the selection while suppressing the follow-up {@link OnSelectListener#onItemSelected(int)}
131-
* event. This is used so we don't get an event when changing the selection ourselves.
132-
*
133-
* @param position the position of the selected item
129+
* Used to commit staged data into ReactPicker view.
130+
* During this period, we will disable {@link OnSelectListener#onItemSelected(int)} temporarily,
131+
* so we don't get an event when changing the items/selection ourselves.
134132
*/
135-
private void setSelectionWithSuppressEvent(int position) {
136-
if (position != getSelectedItemPosition()) {
137-
setOnItemSelectedListener(null);
138-
setSelection(position, false);
139-
setOnItemSelectedListener(mItemSelectedListener);
133+
/* package */ void commitStagedData() {
134+
setOnItemSelectedListener(null);
135+
136+
final int origSelection = getSelectedItemPosition();
137+
if (mStagedAdapter != null && mStagedAdapter != getAdapter()) {
138+
setAdapter(mStagedAdapter);
139+
// After setAdapter(), Spinner will reset selection and cause unnecessary onValueChange event.
140+
// Explicitly setup selection again to prevent this.
141+
// Ref: https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/widget/AbsSpinner.java#123
142+
setSelection(origSelection, false);
143+
mStagedAdapter = null;
140144
}
145+
146+
if (mStagedSelection != null && mStagedSelection != origSelection) {
147+
setSelection(mStagedSelection, false);
148+
mStagedSelection = null;
149+
}
150+
151+
setOnItemSelectedListener(mItemSelectedListener);
141152
}
142153

143154
public @Nullable Integer getPrimaryColor() {

ReactAndroid/src/main/java/com/facebook/react/views/picker/ReactPickerManager.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ public void setItems(ReactPicker view, @Nullable ReadableArray items) {
4444
}
4545
ReactPickerAdapter adapter = new ReactPickerAdapter(view.getContext(), data);
4646
adapter.setPrimaryTextColor(view.getPrimaryColor());
47-
view.setAdapter(adapter);
47+
view.setStagedAdapter(adapter);
4848
} else {
49-
view.setAdapter(null);
49+
view.setStagedAdapter(null);
5050
}
5151
}
5252

@@ -77,7 +77,7 @@ public void setSelected(ReactPicker view, int selected) {
7777
@Override
7878
protected void onAfterUpdateTransaction(ReactPicker view) {
7979
super.onAfterUpdateTransaction(view);
80-
view.updateStagedSelection();
80+
view.commitStagedData();
8181
}
8282

8383
@Override

0 commit comments

Comments
 (0)