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] Add shouldComponentUpdate function #3673

Merged
merged 1 commit into from
Mar 13, 2016

Conversation

nathanmarks
Copy link
Member

This fixes #3394.

edit: FYI, this change is somewhat of a band-aid since it does not address the root cause in TextField. However, this is a non-breaking change and the upcoming refactor will remove much of the internal state from the core component anyways. This gets the fix in without needing that rework right now.

Right now, when a TextField is controlled from outside of a portal (for eg, from outside of a Dialog), typing will prevent a double render that happens due to the internal setState call and some weirdness related to setState execution with components added through the ReactDOM API (render, unstable_renderSubtreeIntoContainer).

However, that internal setState call does not change any props, state, or context. So it is breaking IME composition for no reason. This addition prevents that rerender so the component retains the IME composition event in progress.

  • I spent a couple of hours trying to get tests for this working (failing) in karma/PhantomJS but they kept passing (it's a weird one). I can have another go at getting the tests working if you want
  • 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).

…omposition breaking in portals due to a double render
@alitaheri
Copy link
Member

@nathanmarks Don't bother with the test, this is a temporary fix, thanks a lot for getting it done 👍 👍

@callemall/material-ui Guys take a look

@oliviertassinari
Copy link
Member

That looks good to me 👍.

alitaheri added a commit that referenced this pull request Mar 13, 2016
[TextField] Add shouldComponentUpdate function
@alitaheri alitaheri merged commit 2cfe672 into mui:master Mar 13, 2016
@zannager zannager added core Infrastructure work going on behind the scenes component: text field This is the name of the generic UI component, not the React module! and removed core Infrastructure work going on behind the scenes labels Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: text field This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TextField] Breaks character composition
4 participants