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] Fix: setSelection methods is always returned. #17894

Closed

Conversation

jainkuniya
Copy link
Contributor

@jainkuniya jainkuniya commented Feb 7, 2018

Motivation

Because mMostRecentEventCount < mNativeEventCount is always true as mMostRecentEventCount is not updated.

So determine whether text is set or not from getting current text. Also handle out of bound selection.

Test Plan

Use this component to test with & without this PR

Before this PR, on clicking Button, nothing will happen.
On this PR: text will be selected starting from 0 and ending at 3. (Just for testing I have barcoded start & end value)
Just created a component to test (below).

/* @flow */
import React, { PureComponent } from 'react';
import { View, TextInput, StyleSheet, TouchableHighlight, Text } from 'react-native';

const componentStyles = StyleSheet.create({
  input: {
    flex: 1,
  },
  button: {
    //position: 'absolute',
    right: 0,
    alignItems: 'center',
    padding: 10,
  },
  buttonText: {
    color: '#000',
  },
  row: {
    flexDirection: 'row',
    justifyContent: 'space-between',
  },
});

export default class PasswordInput extends PureComponent<Props, State> {
  props: Props;

  state: State;

  state = {
    selection: { start: 0, end: 0 },
  };

  handleSelectionChange = event => {
    const { selection } = event.nativeEvent;

    this.setState({
      selection,
    });
  };

  render() {
    const { selection } = this.state;

    return (
      <View>
        <View style={componentStyles.row}>
          <TextInput
            style={[componentStyles.input]}
            placeholder={'Start typing here'}
            placeholderTextColor={'green'}
            autoCorrect={false}
            autoCapitalize="none"
            selection={selection}
            onSelectionChange={event => this.handleSelectionChange(event)}
          />
        </View>
        <TouchableHighlight
          style={componentStyles.button}
          onPress={() => this.setState({ selection: { start: 0, end: 3 } })}
        >
          <Text style={componentStyles.buttonText}>
            {'Selection (Click me to select text starting from 0 to 3 in above textInput)'}
          </Text>
        </TouchableHighlight>
      </View>
    );
  }
}

Current behaviour
ezgif com-video-to-gif-5

This PR
ezgif com-video-to-gif-6

Release Notes

[ANDROID] [BUGFIX] [TextInput] Fix: setSelection methods is always returned.

Because mMostRecentEventCount < mNativeEventCount is always true as mMostRecentEventCount is not updated.

So determine whether text is set or not from getting current text. Also handle out of bound selection.
@pull-bot
Copy link

pull-bot commented Feb 7, 2018

Warnings
⚠️

📋 Test Plan - This PR appears to be missing a Test Plan.

@facebook-github-bot label Needs more information

@facebook-github-bot label Android

Generated by 🚫 dangerJS

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. cla signed labels Feb 7, 2018
@jainkuniya
Copy link
Contributor Author

Thanks! @facebook-open-source-bot I have updated and included Test Plan. 👍 :)

@jainkuniya
Copy link
Contributor Author

hi @janicduplessis please review this PR too. :)
#17851 is not affective until this issue is solved. :)

Copy link
Contributor

@shergin shergin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How removing mMostRecentEventCount can fix the problem which mMostRecentEventCount is supposed to address?

Copy link
Contributor Author

@jainkuniya jainkuniya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this and it worked. So I thought it might be the solution but later I got another solution which is more solid #17990. We can close this after #17990 gets resolved. :)

@sdwilsh sdwilsh removed the cla signed label Mar 1, 2018
@hramos hramos added Android and removed Android labels Mar 13, 2018
@react-native-bot react-native-bot added Missing Changelog This PR appears to be missing a changelog, or they are incorrectly formatted. Ran Commands One of our bots successfully processed a command. Android labels Mar 16, 2018
@react-native-bot react-native-bot added Platform: Android Android applications. Ran Commands One of our bots successfully processed a command. labels Mar 18, 2018
@facebook-github-bot
Copy link
Contributor

@jainkuniya do you have any updates for this pull request? It's been a while since the last update so wanted to check in and see if you've looked at the requested changes.

@hramos hramos added ✅Changelog and removed Missing Changelog This PR appears to be missing a changelog, or they are incorrectly formatted. labels Jan 15, 2019
@cpojer cpojer closed this Jan 29, 2019
@cpojer
Copy link
Contributor

cpojer commented Jan 29, 2019

Yeah this doesn't seem like the solution we are looking for here. I'm gonna close this PR.

@jainkuniya
Copy link
Contributor Author

Hey @cpojer can you please also review #17990

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Platform: Android Android applications. Ran Commands One of our bots successfully processed a command.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants