call onValueChange only when value changes

[a-c-sreedhar-reddy]

Summary

OnValueChange function of Picker is called when Picker is initialized.

Changelog

[Android][fixed] - OnValueChange will be called only when the selectedValue changes.

Test Plan

export default class App extends Component<Props> {
  constructor(props) {
    super(props);
    this.state = {language:"js",test:1}
  }
  render() {
    return (
      <View style={styles.container}>
        <Picker
          selectedValue={this.state.language}
          style={{height: 50, width: 100}}
          onValueChange={(itemValue, itemIndex) =>{
            this.setState({language: itemValue,test:2});
          }
        }>
          <Picker.Item label="Java" value="java" />
          <Picker.Item label="JavaScript" value="js" />
        </Picker>
        <Text>{this.state.test}</Text>
      </View>
    );
  }
}

Here state.test must be 1 when the app loads. But the onValueChange is called initially which changes state.test to 2. This pull request fixes the issue.

[analysis-bot]

Code analysis results:

• eslint found some issues. Run yarn lint --fix to automatically fix problems.

[cpojer]

Yeah this makes sense to me. Thank you for the PR!

[cpojer]

cc @Kudo since you recently fixed an issue with onValueChange.

[facebook-github-bot]

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[react-native-bot]

This pull request was successfully merged by @a-c-sreedhar-reddy in 82148da.

^{When will my fix make it into a release? | Upcoming Releases}

[Kudo]

Thanks @cpojer.

Hey @a-c-sreedhar-reddy, just to clarify which RN version you used.
My change landed after RN 0.59, so that if you tested on RN 0.58 or before, there might be another story.
I've tested your code, but seems works fine on RN 0.59.
Could you please double check your RN version and if RN 0.59 works for you?

[a-c-sreedhar-reddy]

Thanks @cpojer.

Hey @a-c-sreedhar-reddy, just to clarify which RN version you used.
My change landed after RN 0.59, so that if you tested on RN 0.58 or before, there might be another story.
I've tested your code, but seems works fine on RN 0.59.
Could you please double check your RN version and if RN 0.59 works for you?

    "react": "16.8.3",
    "react-native": "0.59.5"

@Kudo This is what I tested with.

[Kudo]

That's weird to me since I tested with the exact same version.
Supposedly this issue should not happen as it is handled by native side.
https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/views/picker/ReactPicker.java#L98-L103

  @Override
  protected void onLayout(boolean changed, int left, int top, int right, int bottom) {
    super.onLayout(changed, left, top, right, bottom);

    // 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);
  }

[a-c-sreedhar-reddy]

@Kudo You are right. May be I did not update node modules. Sorry for this. Should I make a PR removing this change?

[Kudo]

@a-c-sreedhar-reddy Sorry missed this. Your fix is fine for me.
In fact, from the semantic of onValueChange, it is no hurt to double check for value inequality.
Just to clarify if I was doing wrong in Java native side.