Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android Picker Not Consistently Firing onValueChange #15556

Closed
ghost opened this issue Aug 18, 2017 · 61 comments
Closed

Android Picker Not Consistently Firing onValueChange #15556

ghost opened this issue Aug 18, 2017 · 61 comments
Labels
Ran Commands One of our bots successfully processed a command.

Comments

@ghost
Copy link

ghost commented Aug 18, 2017

Is this a bug report?

Yes. I have also seen this issue on stackoverflow.

Have you read the Contributing Guidelines?

Yes

Environment

  1. react-native -v: 2.0.1
  2. node -v: v8.0.0
  3. npm -v: 5.0.0
  4. yarn --version: 0.24.5
  • Target Platform: Android
  • Development Operating System: macOS
  • Build tools: N/A

Steps to Reproduce

Using the Picker component in Android, the first onValueChange fires, but the next one does not. The one after does...this continues and results in about 33% of the time onValueChange does not fire.

I created a snack with a basic example.

Expected Behavior

I expected onValueChange to fire if the Picker item selected is different from the current selection.

Actual Behavior

About 33% of the time onValueChange will not fire, but the text on the Picker does change, which is confusing to the user. The other times, everything works as expected.

Reproducible Demo

I created a snack with a basic example. Select one option, then another and, in my experience, it will not consistently log to console.

@mell0kat
Copy link

mell0kat commented Sep 6, 2017

I am currently experiencing this error too, with about the same 33% statistic.

@dragos-panaitescu
Copy link

I also have exactly the same problem. Any solutions / workarounds?

@daniel-coman
Copy link

Unfortunately I have the same problem. Has anyone found any solution to resolve this behavior? Or at least an workaround.

@connercms
Copy link

+1 encountering this bug in two separate applications. happens about every other selection

@aligfl
Copy link

aligfl commented Oct 16, 2017

+1 seeing the exact same thing :(. any workarounds to this problem?

@ghost
Copy link

ghost commented Oct 24, 2017

+1 Unbelievable that this error occurs. Do I have to try and rewrite my app not using the Picker just so that it will reliable? Crazy world. Any workarounds would be appreciated.

@ghost
Copy link
Author

ghost commented Nov 2, 2017

did you try adding a state? demo: https://snack.expo.io/r1JfNfKRZ

@laurent22
Copy link

Still happening in 0.49. Honestly this Picker component should be removed from the doc or marked as "alpha", as it simply doesn't work and shouldn't be relied on. If I had known from the start that it was broken, I could have used a third party picker or make my own.

@tombailey
Copy link

tombailey commented Nov 6, 2017

Can anyone point to a related commit that might have broken this? I can probably look at this if someone can point me in the right direction

@sandropoluan
Copy link

+1
Same problem... in RN 0.48.1

@tombailey
Copy link

FYI: tested on 'left hand'/'right hand' example above and this appears to be fixed on master; no idea which commit actually fixed it

@vbhv19
Copy link

vbhv19 commented Nov 22, 2017

+1 any workaround for this?

@YJ1007
Copy link

YJ1007 commented Nov 22, 2017

+1 Any temporary workaround around this problem ??

@ThaJay
Copy link

ThaJay commented Nov 23, 2017

Same problem.
Used to be good until recently.
on react-native 0.47.2 atm.
In what version is this fixed?
(I could not upgrade to latest yet because gradle scripts seem to be broken / missing)
the workaround suggested by sandropoluan does not work for me.

@ThaJay
Copy link

ThaJay commented Nov 27, 2017

@tombailey any chance you are able to figure out what the commit was so it can be proposed for the next release?

adding setState to onValueChange does not work, onValueChange is not called half of the time, so changing it does not make a difference.

@tombailey
Copy link

@ThaJay I have looked but I have no idea which commit resolved this. I didn't do an upgrade on my project, I just created a new one with the example above and pointed at the latest version of RN. Can someone else verify it works for them too?

@ThaJay
Copy link

ThaJay commented Nov 27, 2017

I just made a test project and was unable to reproduce:
https://github.com/ThaJay/react-native-picker-test
It worked in the same version of react-native I have this problem with in my app, so this does not get me closer to figuring it out. Anyway, it's not something obvious.

If I remove state and setState it still seems to call onValueChange with the right value but the selected value in the picker does not change any more.
Funny thing is, when I have this issue in the app, the value of the picker DOES change, even though onValueChange does not seem to get called. (in my app I use redux to store the value)

@ThaJay
Copy link

ThaJay commented Nov 28, 2017

Further testing learns that the behavior does not change on the latest version (v0.50.4)
I just updated my project and the picker fires onValueChange exactly half the time, while the view element actually updates every time just like on 0.47.2 (but it should not update because onValueChange sets the state it depends on, so it's like the Android picker changes but nothing else).

As stated above I was unable to reproduce this in an empty project but I'm still stunned as of how this can happen and it's a blocking bug for the next release that's waiting to be approved.

The example OP gives does not work, the Picker never changes because it has no selectedValue.
If I change it as below, it works normally.
Perhaps it has something to do with Redux.

import React, {Component} from 'react'
import {Picker, View} from 'react-native'

export default class App extends Component {
  constructor(props) {
    super(props)
    this.state = {hand:'right'}
  }

  render () {
    return (
      <View>
        <Picker
          onValueChange={hand => {
            this.setState({hand})
            console.log('changed to', hand)
          }}
          selectedValue={this.state.hand}
          style={{width:160}}
          mode='dropdown'
        >
          <Picker.Item label='Right Hand' value='right' />
          <Picker.Item label='Left Hand' value='left' />
        </Picker>
      </View>
    )
  }
}

@ThaJay
Copy link

ThaJay commented Nov 28, 2017

I added redux to the test repo and was still unable to reproduce
https://github.com/ThaJay/react-native-picker-test

@tombailey
Copy link

@ThaJay given that this is Android specific and seems to happen 50% of the time, I am inclined to blame https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/views/picker/ReactPicker.java#L133

mSuppressNextEvent in particular

I don't have the time to look at this properly so if anyone else does that would be great.

@ThaJay
Copy link

ThaJay commented Nov 29, 2017

If I disable mSuppressNextEvent in ReactPicker it does seem to work.
It also works if I use state and no redux in our app (but adding redux did not reproduce in the test app).

So there must be something going wrong with the re-render

@ThaJay
Copy link

ThaJay commented Nov 29, 2017

I have found the folowing workaround using componentDidUpdate:
so I don't have to change react-native

  constructor (props) {
    super(props)
    this.state = {value:this.props.eventId}
  }

  componentDidUpdate () {
    if (
      this.state.value &&
      this.props.eventId !== this.state.value
    ) {
      this.props.chooseEvent(this.state.value)
    }
  }

  onValueChange = value => {
    this.setState({value})
  }

with

          <Picker
            selectedValue={this.state.value}
            onValueChange={this.onValueChange}
          >

@sandropoluan
Copy link

This issue still happening until this days... unbelievable!

@diegolaciar
Copy link

+1 fix for this. The same issue with Android Picker.

@tombailey
Copy link

For anyone still having issues with this, you might be interested in https://github.com/tombailey/react-native-fixed-android-picker as a work around

@diegolaciar
Copy link

I found a solution. I my case i was using redux-form.
Looks like this component must use state and setState.

something like..

Note this lines:
this.setState({ itemValue });
onChange(itemValue);

constructor(props) {
    super(props);
    this.state = { itemValue: this.prop.initialSelectedValue };
  }
render () {
const {
      input: { value, onChange, ...inputProps },
    } = this.props;
return (
       <Picker
          selectedValue={this.state.itemValue}
          onValueChange={itemValue => {
            this.setState({ itemValue });
            onChange(itemValue);
          }}
          {...inputProps}
        >
);
}

Cheers

@toussa
Copy link

toussa commented Jan 25, 2018

I struggled with this problem during several hours and @diegolaciar 's workaround works !

It may be related how the Picker handles the render process, and in which order it updates its internal selected value.
Indeed, it looks like the internal selected value is updated before the check to know if the value has changed or not.

Well, at least we have a workaround now :) Thanks again diego

@AliZeynalov
Copy link

AliZeynalov commented Jan 28, 2018

It seems you should add width property of style to picker in Android. Try adding width and it should work on Android.

@ThaJay
Copy link

ThaJay commented Mar 28, 2018

@AliaMYH Take a look at my example too, both this.state and this.setState are needed to make it work. Anything else does not work. Props do not work.

@nhohb07
Copy link

nhohb07 commented Apr 8, 2018

I working on React Native v0.54.0, this bug still appeared, the action onValueChange on Android does not fire when the user selected the first item on the picker.

@ThaJay
Copy link

ThaJay commented Apr 10, 2018

I'm on react-native 0.55.0 and the issue is back even with my workaround. What do I do now? :/

@tombailey
Copy link

@ThaJay until the root cause is determined and fixed, you probably want to try #15556 (comment)

@ThaJay
Copy link

ThaJay commented Apr 11, 2018

@tombailey I had a similar workaround in place. #15556 (comment)
It stopped working.
So I decided I am done with this picker component and built my own. Easy peasy, some stuff with a
TouchableOpacity and a Modal with a list.

@tombailey
Copy link

@ThaJay the solution I posted isn't a "workaround" in the same way your's was. It does actually what you propose "TouchableOpacity and a Modal with a list".

@ThaJay
Copy link

ThaJay commented Apr 11, 2018

@tombailey Oh sorry, I thought you meant @diegolaciar's post below yours. That contains the same workaround.

Your repo is indeed similar to what I built yesterday. It's nice to have control over styling, isn't it? Finally my picker dialog is dark in night mode. 👍

@sandropoluan
Copy link

@hramos , can you reopen this issue?

@hungtranvupycogroup
Copy link

I find this line


Can we start with selectedIndex = -1 ?

@jnadeauNn
Copy link

I have the same issues. I think this issues should be reopen. Thank you very much

@hafizalfaza
Copy link

I use setTimeout to achieve it, it's a bit tricky but it works

onValueChange={(itemValue, itemIndex) => { setTimeout(() => {this.onValueChange(itemValue, itemIndex)}, 10)}}

@kigawas
Copy link

kigawas commented Sep 27, 2018

Folks, I think it should be used with state instead of props like this.props.value in redux-form.

@akashanand708
Copy link

I use setTimeout to achieve it, it's a bit tricky but it works

onValueChange={(itemValue, itemIndex) => { setTimeout(() => {this.onValueChange(itemValue, itemIndex)}, 10)}}

It helped, thanks.

@wahabtobibello
Copy link

@hafizalfaza Youu solution worked although i just needed to use:
onValueChange={(itemValue, itemIndex) => { setTimeout(() => {this.onValueChange(itemValue, itemIndex)}, 0)}}

I really don't understand how this works fine but the picker doesn't work well. my react-native version i s 0.57

@AdamZaczek
Copy link

For people revisiting, @hafizalfaza 's solution is the way to go. Picker's onValueChange needs setTimeout or some other way of adding to event loop to work correctly

@54vanya
Copy link

54vanya commented Jan 16, 2019

Why is this even closed? Issue is still exist, @hafizalfaza 's solutions isn't that I really want to use in my life

@ThaJay
Copy link

ThaJay commented Jan 17, 2019

Then don't use it. I have had a different Picker component in my app for ages. This is not likely to get fixed any time soon.

edit: LOL 1 day later it got fixed

@AdamZaczek
Copy link

What do you recommend @ThaJay ?

@ThaJay
Copy link

ThaJay commented Jan 18, 2019

I made my own. I think there was someone in this thread who made his own and published it as a module, but I just made a gist out of my code as an example of how this could be done.
https://gist.github.com/ThaJay/80ee8647940e0cb753a9da67f55b14df

I just pasted 2 files together and removed duplicate imports so it will not work as is, but maybe you can use parts of it in your own project.

facebook-github-bot pushed a commit that referenced this issue Jan 18, 2019
…ondition (#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: #22821

Differential Revision: D13731979

Pulled By: cpojer

fbshipit-source-id: e06bd9aa62463b66c8f3fd7214485898d5375054
matt-oakes pushed a commit to matt-oakes/react-native that referenced this issue Feb 7, 2019
…ondition (facebook#22821)

Summary:
Changelog:
----------

[Android] [Fixed] - Fix Picker.onValueChange sometimes not fired due to race condition. Fixes facebook#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: facebook#22821

Differential Revision: D13731979

Pulled By: cpojer

fbshipit-source-id: e06bd9aa62463b66c8f3fd7214485898d5375054
@ghost
Copy link

ghost commented Mar 12, 2019

This issue still reproduces. Picker does not trigger onValueChange for first item when there is not selected value. Even if we set selectedValue={this.state.value} where this.state.value = -1 or this.state.value = null it does not trigger onValueChange for first item. So how we can define Picker without any pre-selected value?

@ghost
Copy link

ghost commented Mar 12, 2019

Guys, I solved this problem with unselectable first item by setting fake unselectable item with value = -1. Then when user picks valid item,fake item with value = -1 dissapears.

// this.state.selectedIndex initially setted to -1
<Picker
    selectedValue={this.state.selectedIndex}
    onValueChange={(value, index) => { this.setState({selectedIndex: index}) }}
>
    // first fake item
    if(Platform.OS === 'android' && this.state.selectedIndex === -1) {
        <PickerIOS label={'CANCEL'} value={-1} />
    }

    <Picker.Item label={'First'} value={1} />
    <Picker.Item label={'Second'} value={2} />
    <Picker.Item label={'Third'} value={3} />
</Picker>

@Rana-Muhammad-Amir
Copy link

following code working fine on android but in "onValueChange" this.changeCityArray() not calling on IOS platform please help.....

<Select
iosText={this.state.nok_state}
iosTouchStyle={styles.iosPickerStyle}
onPress={() => showActionSheetState()}
onValueChange={(itemValue, itemIndex) => ( this.changeCityArray(itemIndex), this.setState({nok_state: itemValue, key: itemIndex}) )}
render={this.stateOptions} selectedValue={this.state.nok_state} style={styles.pickerStyle}
/>

@bjmiao
Copy link

bjmiao commented May 17, 2020

I use setTimeout to achieve it, it's a bit tricky but it works

onValueChange={(itemValue, itemIndex) => { setTimeout(() => {this.onValueChange(itemValue, itemIndex)}, 10)}}

Great! A little bit tricky but it works xd
Can someone explain why it works?

andrejcesen added a commit to SowaLabs/react-native-formik that referenced this issue Nov 5, 2020
Empty value is prepended to Picker by library mainters in PickerModal.js to fix this issue: facebook/react-native#15556
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ran Commands One of our bots successfully processed a command.
Projects
None yet
Development

No branches or pull requests