Material-ui: [TextField] Breaks character composition

[imtherealk]

TextField component breaks composite chracters.
When I set value property from the container's state it breaks character composition.
For example, I have a react component like this

class SomeForm extends React.Component {
  constructor() {
    super();
    this.state = {value: ''};
  }
  render() {
    return (
      <div>
        <TextField value={this.state.value}
                          onChange={({target: {value}}) => this.setState({value})} />
      </div>
      );
  }
}

In korean, to input charater '가', I have to enter 'ㄱ' and then 'ㅏ', and they are composed into '가'
But, when value changed to non-empty value from empty value, it breaks a composition. So I can't enter '가' to the TextField

[alitaheri]

This is not good. We'll try to look into this. You're welcome to send a PR if you get to the bottom of it :grin: or if you find what part of the code is responsible. Since we don't have Korean keyboard it might not be so easy for us to reproduce. your help is appreciated :sweat_smile:

[newoga]

Geeze this is bad!

I used to study Mandarin in college and I just changed my keyboard layout to a Chinese one and reproduced this...don't have a clue how to fix it though.

Edit: Nope, nevermind. I couldn't reproduce it. Turns out I just configured the Chinese keyboard wrong on my mac. Looks like it's entering characters fine. I guess we'll have to rely on @therealk2 to help reproduce steps.

[imtherealk]

I had some test and I found out this problem only appears when I have TextField inside Dialog.
It works well outside of Dialog. This is my code to reproduce it.

class Help extends React.Component {
  constructor() {
    super();
    this.state = {
      value1: '',
      value2: ''
    }
  }
  onChange({target: {value}}, name) {
    this.setState(_.assign({}, this.state, {
      [name]: value
    }));
  }
  render() {
    return (
      <div>
        <Dialog open={true}>
          <TextField value={this.state.value1}
                     onChange={(e) => this.onChange(e, 'value1')} />
          <br/>
          <TextField value={this.state.value2}
                     onChange={(e) => this.onChange(e, 'value2')} />
        </Dialog>
      </div>
    );
  }
}

[nathanmarks]

@imtherealk is there a way I can easily type korean on my computer? :smile:

[imtherealk]

@nathanmarks I hope this post will be helpful. You can enable 2-Set Korean input method on your Mac. http://www.macinfo.us/how-to-type-korean-characters-on-your-mac.php

[nathanmarks]

@imtherealk thanks! I'm looking forwards to trying this :smile:

[nathanmarks]

@imtherealk 가 think I got the korean keyboard working, time to try this out :+1:

[nathanmarks]

@imtherealk @newoga

I've found what is causing the issue... eh, sort of. Basically, the <Dialog /> re-rendering is breaking the composition somehow.

I found if I typed quickly enough but also on some randomish timing pattern, I could actually trigger a 가 composition. Looking at your code above @imtherealk , you'll see that the component rendering the <Dialog /> is also the same component rendering the <TextField />. That means that every time an input is registered, the state changes and re-renders <Dialog />. Something happening there is causing the problem.

I tested this by wrapping the <TextField /> (and the state) in another component, here's the result (working as it should):

Here's the wrapper I used to test:

class WrappedTextField extends React.Component {
  state = {
    value1: '',
  };
  shouldComponentUpdate(nextProps) {
    return nextProps.value !== this.props.value;
  }
  onChange({target: {value}}, name) {
    this.setState(Object.assign({}, this.state, {
      [name]: value,
    }));
  }
  render() {
    return (
      <TextField
        name="woof"
        value={this.props.value}
        onChange={(event) => this.onChange(event, 'value1')}
      />
    );
  }
}

WrappedTextField.propTypes = {
  value: React.PropTypes.string,
};

@newoga any idea why this would happen specifically with the dialog?

[nathanmarks]

@newoga It is specifically a combination between our TextField and the portal pattern implementation. A regular input has no such issue.

Here is a demo just using RenderToLayer with a TextField and an input:

<div>
  <TextField value={this.state.value1} onChange={(event) => this.onChange(event, 'value1')} />
  <input value={this.state.value2} onChange={(event) => this.onChange(event, 'value2')} />
</div>

@callemall/material-ui Do you have any idea why this might be happening (off the top of your head)? I'm going to continue debugging it after I get home etc tonight.

[newoga]

@newoga It is specifically a combination between our TextField and the portal pattern implementation.

That was the first thought I had when you mentioned it had to do with Dialog, but haven't had a chance to investigate it yet (just got to hotel :airplane:)

[nathanmarks]

@newoga no worries! Was just checking if anyone knew, I plan on investigating tonight

[nathanmarks]

Found the problem. Writing a failing test.

removed my last reply... still verifying via the test

[nathanmarks]

I can't seem to get the phantomjs test to fail, sigh. I'll have to fix this without a test for now. Here's the main symptom of the issue though:

The double render is due to some issues with portal pattern (RenderToLayer) + setState and/or event propagation (if I remove setState in the textfield handleChange the issue is gone). But for some reason i'm having difficulty reproducing the same issue in phantomjs.

[nathanmarks]

@newoga

I did some investigation last night and figured out that the compositionEnd event was never firing during the IME character input when using a TextField within a RenderToLayer component, controlled from the outside.

Here's where the composition event appeared to break:

I wondered why that was happening during the composition step, so I took a closer look at TextField and saw that it was re-rendering twice after every key input (even with roman characters) instead of once.

This was only happening by controlling the TextField from outside of the portal. The onChange handler in TextField has a setState() call. This call appears to be triggering a re-render separately from the setState call in the component controlling the TextField, as there are no updated props or state.

There are several options here (not mutually exclusive):

1. Address this issue in the TextField refactor where we aim to remove as much internal state as we can. Removing that setState call stopped the issue in my testing.
2. Try figure out why this only happens in the portal pattern (lol) (It also happens using the core ReactDOM.render() function, not just ReactDOM.unstable_renderSubtreeIntoContainer())
3. Implement a shouldComponentUpdate function

  shouldComponentUpdate(nextProps, nextState, nextContext) {
    return (
      !shallowEqual(this.props, nextProps) ||
      !shallowEqual(this.state, nextState) ||
      !shallowEqual(this.context, nextContext)
    );
  }

[alitaheri]

@nathanmarks Wow great job :+1: :+1: This wasn't an easy task! I think it's best if we remove the setState(). will that be a breaking change?

[nathanmarks]

@alitaheri Thanks! It will break some behaviour for using it uncontrolled :angry: lol :smile:

[alitaheri]

Oops :sweat_smile: Well @imtherealk How urgent is this? If it's not so urgent we plan on removing state from our components so it will be automatically fixed. otherwise, follow 2. @nathanmarks 3 will not be possible, with our inline styles that function will always return true

[nathanmarks]

@alitaheri It works fine unless the user is passing their own inline styles, that's only the case with the leaf nodes that TextField passes styles too when it actually updates.

[alitaheri]

Oh right. it's a good fix for now :+1: atleast it will give @imtherealk a good work around for now. But it's not concrete. We have to fix it at the core :grin:

[nathanmarks]

@alitaheri RIP IT ALL OUT! :smile:

[alitaheri]

YEAH :rage3: :rage3: :+1: :laughing:

[nathanmarks]

Ok, so you think we should implement the shouldComponentUpdate until the refactor?

[alitaheri]

yeah, that should give our Korean users a temporary work around so they can go on with their work. After we do the migration this will automatically be fixed by design :+1:

[nathanmarks]

will do :+1:

[alitaheri]

Thanks a lot :+1: :+1: :grin:

[imtherealk]

@nathanmarks @alitaheri not in a hurry, actually. I just found a bug and wanted u know it. thanks a lot for the fix :)

[newoga]

Great stuff @nathanmarks, sorry, I've been limited on my travels (my Internet isn't so great as well at the hotel). Thanks for taking this on!

Also yay for no state! :smile:

[nathanmarks]

@imtherealk It's a pretty annoying bug!

[ghostpanda]

@nathanmarks @alitaheri
when I use reduxForm module.
some props injects, like active dirty error and so on.
when I inputs first Chinese character, active dirty error props changes, so shallowEqual(this.props, nextProps) should be false, error appear again.

🙏 Hopes that you can give me some advice. Thanks a lot!
My temporary solution is delete this line this.setState({hasValue: isValid(event.target.value), isClean: false});
TextField.js

[Beingbook]

This bug still happens when I write first-letter. Compositing Korean is disable for first-letter.

(ㄱㅏ나안 should be 가나안)
Anyone have this experience...? It looks TextField has a bug because normal input element or draftJS works well.

I see this bug on Chrome for OSX El Cap and Windows 10. Safari doesn't make this bug.

[pgonee]

(sorry for my english 😢 )

@nathanmarks

@Beingbook is right. This bug still happen.

When i typed first letter to TextField inside Dialog, TextField still re-rendered twice.

Because typing first letter cause changing of TextField's states. (isClean to false and hasValue to true)

And then props changed.

So, TextField's shouldComponentUpdate() return true twice when i typed first letter.

[nathanmarks]

@pgonee I will make sure this doesn't on the next branch, and I'll see if I can get a chance to address it again on 15.x

[wanhongfu]

Is there any updates on this issue? seems it still happened in version v0.16.1, thanks.

[madlordory]

happened again，may it resovled

[oliviertassinari]

I've found what is causing the issue... eh, sort of. Basically, the

re-rendering is breaking the composition somehow.

@nathanmarks That issue seems highly correlated to #4430.
For some reason, #5540 seems to have addressed the issue.
Unless that bug is still present on the v0.16.5 release, I'm closing the issue as well as #5688.

Address this issue in the TextField refactor where we aim to remove as much internal state as we can. Removing that setState call stopped the issue in my testing.

I agree, that would be the best fix IMHO.

[lgan1989]

My temporary workaround for the 'first letter' issue is

   <TextField
            value={this.state.name || ' '}

[jzz4012650]

Any progress on this bug? Still got Chinese IME trouble on version 0.17.1