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

[TextField] Fix TextField with Chinese input method #6562

Closed
wants to merge 1 commit into from

Conversation

halfcrazy
Copy link

@halfcrazy halfcrazy commented Apr 10, 2017

Use compositionstart event to prevent not selected words trigger event.

Closes #6527.

  • PR has tests / docs demo, and is linted.
  • Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
  • Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

ref facebook/react#3926

Use compositionstart event to prevent not selected words trigger event.
case 'compositionend': this.setState({isTyping: false}); break;
}
};

Copy link
Member

Choose a reason for hiding this comment

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

You also need to pass the event though to the appropriate composition prop if set. There are a couple of patterns for that, in use, either test if the prop is defined or (my preference) give the props empty arrow function defaults: () = {}.

@mbrookes mbrookes added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI and removed PR: Needs Review labels Apr 10, 2017
@@ -191,6 +191,9 @@ class TextField extends Component {
* @param {string} newValue The new value of the text field.
*/
onChange: PropTypes.func,
onCompositionEnd: PropTypes.func,
onCompositionStart: PropTypes.func,
onCompositionUpdate: PropTypes.func,
Copy link
Member

Choose a reason for hiding this comment

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

You can @ignore these.

@mbrookes mbrookes changed the title [TextField] Fix TextField with Chinese input method (#6527) [TextField] Fix TextField with Chinese input method Apr 10, 2017
@mbrookes
Copy link
Member

mbrookes commented Apr 10, 2017

facebook/react#3926 I found the same React issue before I realised you'd already linked to it! 😄

Reading that, we'll need to test carefully that this fixes the issue across browsers. It seems the event order is not consistent.

@@ -356,6 +361,14 @@ class TextField extends Component {
}
};

handleComposition = (event) => {
Copy link
Member

Choose a reason for hiding this comment

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

Please could you add a TODO comment explaining why this is needed (reference the FB issue), so we can revisit and remove the workaround if / when Facebook finally fix it (there is a PR pending review since Dec 2016).

@halfcrazy
Copy link
Author

After upgrade to v0.17.2 found this issue resolved by #6456

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants