forked from facebook/react-native
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix Android Picker ArrayOutOfBoundsException during Picker.Item updat…
…e from a long list (facebook#25276) Summary: axe-fb reported this side effect from my previous commit in facebook#24793 (comment) After revisited the implementation of Android Spinner, it seems the formal way to update existing adapter is mutating it, i.e. `arrayAdapter.clear()` & `arrayAdapter.addAll()` to update a Spinner Adapter. `setAdapter()` will reset everything including `mDataChanged`. A race condition may happens between rendering a long picker list and reseting adapter. Here is a code snippet: https://snack.expo.io/kudochien/80f810 To reproduce the issue, please select large item (e.g. 500) first and click the button right hand side. Please not to verify this on Expo directly in the meantime, because Expo with RN 0.59 does not include my previous commit. ## Changelog [Android] [Fixed] - Fix Picker ArrayOutOfBoundsException during Picker.Item update from a long list Pull Request resolved: facebook#25276 Test Plan: 1. Check the test case https://snack.expo.io/kudochien/80f810 will have exception or not. 2. Regression of https://snack.expo.io/Sy1JClEag from facebook#13351 3. Regression of https://snack.expo.io/kudochien/android-picker-issue from facebook#22821 4. RNTester Picker example Reviewed By: mdvacca Differential Revision: D15857426 Pulled By: axe-fb fbshipit-source-id: 8ef902447fdd1b8aeab50ad061545cd14c735e51
- Loading branch information
Showing
4 changed files
with
146 additions
and
94 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
68 changes: 68 additions & 0 deletions
68
ReactAndroid/src/main/java/com/facebook/react/views/picker/ReactPickerAdapter.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| package com.facebook.react.views.picker; | ||
|
|
||
| import android.content.Context; | ||
| import android.view.LayoutInflater; | ||
| import android.view.View; | ||
| import android.view.ViewGroup; | ||
| import android.widget.ArrayAdapter; | ||
| import android.widget.TextView; | ||
|
|
||
| import com.facebook.infer.annotation.Assertions; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import javax.annotation.Nullable; | ||
|
|
||
| /* package */ | ||
| class ReactPickerAdapter extends ArrayAdapter<ReactPickerItem> { | ||
|
|
||
| private final LayoutInflater mInflater; | ||
| private @Nullable | ||
| Integer mPrimaryTextColor; | ||
|
|
||
| public ReactPickerAdapter(Context context, List<ReactPickerItem> data) { | ||
| super(context, 0, data); | ||
|
|
||
| mInflater = (LayoutInflater) Assertions.assertNotNull( | ||
| context.getSystemService(Context.LAYOUT_INFLATER_SERVICE)); | ||
| } | ||
|
|
||
| @Override | ||
| public View getView(int position, View convertView, ViewGroup parent) { | ||
| return getView(position, convertView, parent, false); | ||
| } | ||
|
|
||
| @Override | ||
| public View getDropDownView(int position, View convertView, ViewGroup parent) { | ||
| return getView(position, convertView, parent, true); | ||
| } | ||
|
|
||
| private View getView(int position, View convertView, ViewGroup parent, boolean isDropdown) { | ||
| ReactPickerItem item = getItem(position); | ||
| if (convertView == null) { | ||
| int layoutResId = isDropdown | ||
| ? android.R.layout.simple_spinner_dropdown_item | ||
| : android.R.layout.simple_spinner_item; | ||
| convertView = mInflater.inflate(layoutResId, parent, false); | ||
| } | ||
|
|
||
| TextView textView = (TextView) convertView; | ||
| textView.setText(item.label); | ||
| if (!isDropdown && mPrimaryTextColor != null) { | ||
| textView.setTextColor(mPrimaryTextColor); | ||
| } else if (item.color != null) { | ||
| textView.setTextColor(item.color); | ||
| } | ||
|
|
||
| return convertView; | ||
| } | ||
|
|
||
| public @Nullable Integer getPrimaryTextColor() { | ||
| return mPrimaryTextColor; | ||
| } | ||
|
|
||
| public void setPrimaryTextColor(@Nullable Integer primaryTextColor) { | ||
| mPrimaryTextColor = primaryTextColor; | ||
| notifyDataSetChanged(); | ||
| } | ||
| } |
39 changes: 39 additions & 0 deletions
39
ReactAndroid/src/main/java/com/facebook/react/views/picker/ReactPickerItem.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| package com.facebook.react.views.picker; | ||
|
|
||
| import com.facebook.react.bridge.ReadableArray; | ||
| import com.facebook.react.bridge.ReadableMap; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| import javax.annotation.Nullable; | ||
|
|
||
| /* package */ | ||
| class ReactPickerItem { | ||
| public final String label; | ||
| @Nullable | ||
| public final Integer color; | ||
|
|
||
| public ReactPickerItem(final ReadableMap jsMapData) { | ||
| label = jsMapData.getString("label"); | ||
|
|
||
| if (jsMapData.hasKey("color") && !jsMapData.isNull("color")) { | ||
| color = jsMapData.getInt("color"); | ||
| } else { | ||
| color = null; | ||
| } | ||
| } | ||
|
|
||
| @Nullable | ||
| public static List<ReactPickerItem> createFromJsArrayMap(final ReadableArray jsArrayMap) { | ||
| if (jsArrayMap == null) { | ||
| return null; | ||
| } | ||
|
|
||
| final List<ReactPickerItem> items = new ArrayList<>(jsArrayMap.size()); | ||
| for (int i = 0; i < jsArrayMap.size(); ++i) { | ||
| items.add(new ReactPickerItem(jsArrayMap.getMap(i))); | ||
| } | ||
| return items; | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters