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

fix(ComboBox): switch from onInputValueChange to onStateChange #3646

Merged

Conversation

asudoh
Copy link
Contributor

@asudoh asudoh commented Aug 5, 2019

Downshift's onInputValueChange seems to be called from React state updater function and thus calling setState() in onInputValueChange handler causes React's "An update was scheduled from inside an update function" warning.

Fixes #2543.
Fixes #3637.

Changelog

Changed

  • Switch from onInputValueChange to onStateChange in our <ComboBox> code for keeping track of user-typed value of <input>.

Testing / Reviewing

Testing should make sure our <Combobox> is not broken.

Downshift `v1`'s `onInputValueChange` seems to be called from React
state updater function and thus calling `setState()` in
`onInputValueChange` handler causes React's "An update was scheduled
from inside an update function" warning.

Fixes carbon-design-system#2543.
Fixes carbon-design-system#3637.
@asudoh asudoh force-pushed the combobox-oninputvaluechange branch from 3417836 to a566b61 Compare August 5, 2019 05:07
@netlify
Copy link

netlify bot commented Aug 5, 2019

Deploy preview for the-carbon-components ready!

Built with commit 3417836

https://deploy-preview-3646--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Aug 5, 2019

Deploy preview for carbon-components-react ready!

Built with commit 3417836

https://deploy-preview-3646--carbon-components-react.netlify.com

@netlify
Copy link

netlify bot commented Aug 5, 2019

Deploy preview for carbon-elements ready!

Built with commit 3417836

https://deploy-preview-3646--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Aug 5, 2019

Deploy preview for the-carbon-components ready!

Built with commit fa146e9

https://deploy-preview-3646--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Aug 5, 2019

Deploy preview for carbon-elements ready!

Built with commit fa146e9

https://deploy-preview-3646--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Aug 5, 2019

Deploy preview for carbon-components-react ready!

Built with commit fa146e9

https://deploy-preview-3646--carbon-components-react.netlify.com

Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

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

looks good to me, tested locally with example app from the original issue

Copy link
Member

@tw15egan tw15egan left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @asudoh! 👍

@tw15egan tw15egan merged commit 141a85f into carbon-design-system:master Aug 5, 2019
@asudoh asudoh deleted the combobox-oninputvaluechange branch August 5, 2019 22:43
asudoh added a commit to asudoh/carbon-components that referenced this pull request Jan 6, 2020
This change backs out carbon-design-system#3646 partially given moving `inputValue` state
syncing code from `handleOnInputValueChange` to `handleOnStateChange`
seems to break IME presumably because the different rendering timing
the change causes in-composition string (of IME) overriden by an older
value.
asudoh added a commit to asudoh/carbon-components that referenced this pull request Jan 6, 2020
This change backs out carbon-design-system#3646 partially given moving `inputValue` state
syncing code from `handleOnInputValueChange` to `handleOnStateChange`
causes in-composition string (of IME) getting lost, because the
different rendering sequence yielded by such change causes Downshift to
(temporarily) render an older (stale) value to the `<input>`.

Fixes carbon-design-system#4947.
asudoh added a commit that referenced this pull request Jan 8, 2020
This change backs out #3646 partially given moving `inputValue` state
syncing code from `handleOnInputValueChange` to `handleOnStateChange`
causes in-composition string (of IME) getting lost, because the
different rendering sequence yielded by such change causes Downshift to
(temporarily) render an older (stale) value to the `<input>`.

Fixes #4947.
joshblack pushed a commit to joshblack/carbon that referenced this pull request Jan 13, 2020
This change backs out carbon-design-system#3646 partially given moving `inputValue` state
syncing code from `handleOnInputValueChange` to `handleOnStateChange`
causes in-composition string (of IME) getting lost, because the
different rendering sequence yielded by such change causes Downshift to
(temporarily) render an older (stale) value to the `<input>`.

Fixes carbon-design-system#4947.
joshblack pushed a commit to joshblack/carbon that referenced this pull request Jan 14, 2020
This change backs out carbon-design-system#3646 partially given moving `inputValue` state
syncing code from `handleOnInputValueChange` to `handleOnStateChange`
causes in-composition string (of IME) getting lost, because the
different rendering sequence yielded by such change causes Downshift to
(temporarily) render an older (stale) value to the `<input>`.

Fixes carbon-design-system#4947.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants