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

[Autocomplete] Input characters are added even when the IME is unsettled with freeSolo and multiple #19435

Closed
2 tasks done
teramotodaiki opened this issue Jan 28, 2020 · 12 comments · Fixed by #19499
Closed
2 tasks done
Labels
bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! ready to take Help wanted. Guidance available. There is a high chance the change will be accepted

Comments

@teramotodaiki
Copy link
Contributor

An IME (Input Method Editor) holds internal state of keys are pressed and it confirms characters when the Enter is pressed. Then the Autocomplete adds into its values because the Enter is detected. It causes input characters is left on the input element but the characters has already added.

Easy to see in GIF: https://gyazo.com/4c1c5607d5500eed6845018540e0d7c6

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

When the Enter is pressed, input characters are added into Autocomplete values.
I think there are 2 ways to get that behavior.

  1. Open popup and highlight something then hit Enter. (difficult to reproduce)
  2. Make freeSolo and multiple props true. Type something then hit Enter. (easy to reproduce)

Expected Behavior 🤔

DON'T add characters into Autocomplete values while the IME is unsettled.
To detect IME is unsettled, I am recommending to use isComposing flag. It works fine on my environments (chrome, safari and firefox). You can try it on codesandbox.io by link below.

Steps to Reproduce 🕹

Steps:

  1. See https://codesandbox.io/s/autocomplete-ime-behavior-9qgq2
  2. Make IME on.
  3. Type something in the above Autocomplete then hit Enter.
  4. It added but unsettled characters are left on TextField.
  5. You can try another behavior which I expect on the next Autocomplete.

macOS/Chrome and Safari cause this problem. Firefox does not. I didn't test other browsers.

Context 🔦

I am using an autocomplete in Japanese. The Autocomplete works almost fine but IME makes it chaos.
I resolved this problem by below code, but I think also it helps community.

<Autocomplete
  multiple
  freeSolo
  options={[]}
  defaultValue={[]}
  renderInput={params => (
    <TextField
      {...params}
      onKeyDownCapture={e =>
        e.key === 'Enter' && e.nativeEvent.isComposing
          ? e.stopPropagation()
          : undefined
      }
    />
  )}
/>

As far as I can see, I may be able to fix the Autocomplete behavior.
I think that inserting && !event.nativeEvent.isComposing into here will be fine.

https://github.com/mui-org/material-ui/blob/cbe079c4a4779778736f1c2d065158fb3d29320e/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js#L586-L606

Your Environment 🌎

Tech Version
material-ui/core v4.9.0
material-ui/lab v4.0.0-alpha.40
React v16.12.0
Browser macOS, Chrome 79.0.3945.130/Safari 13.0.4/Firefox 72.0.2
@oliviertassinari oliviertassinari added component: autocomplete This is the name of the generic UI component, not the React module! bug 🐛 Something doesn't work labels Jan 28, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 28, 2020

Thanks for the issue, IME is an important concern. I don't have any objection with your proposed patch. However, if you could explore the following, we would highly appreciate it:
How does the other implementations solve the problem? We could use this benchmark list: https://trello.com/c/qrI0FOna/2335-autocomplete.

@teramotodaiki
Copy link
Contributor Author

I'm going to research when I have time. :)

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 28, 2020

Thank you, it feels odd that we need to dive into the event.nativeEvent, I would hope there is a simpler solution. Ant Design might be a good place to start looking as they have a strong audience that uses IME.

@teramotodaiki
Copy link
Contributor Author

I'm back. This is the survey of how the Benchmarks resolve this problem.

react-components ❌

UI Select (AngularUI) ❌

Ant Design ✅

🆕 rc-select (react-component/select) ✅

Blueprint 🤔

  • https://blueprintjs.com/docs/#select/select-component
  • It confirms when the Enter key is hit even while IME is unsettled.
  • But unsettled characters will been cleared. It looks good. ✅
  • This way is faster than my approach because user hit Enter key only once. ✅
  • Sometimes we need to type something after confirm IME. But it can't. ❌
    Image from Gyazo

UI Fablic ❌

NEXT SURVEY if I have time...

https://ej2.syncfusion.com/react/documentation/auto-complete/getting-started/?no-cache=1
https://github.com/downshift-js/downshift
https://github.com/jquense/react-widgets
https://github.com/moroshko/react-autosuggest
https://jqueryui.com/autocomplete/
https://react-select.com/
https://rsuitejs.com/en/components/input-picker
https://select2.org/getting-started/installation
https://ui.reach.tech/combobox/
https://vuetifyjs.com/en/components/combobox
https://sancho-ui.com/components/combobox
https://www.telerik.com/kendo-react-ui/components/dropdowns/
https://www.w3.org/TR/wai-aria-practices/#combobox
https://evergreen.segment.com/components/tag-input/
https://youtube.com/
https://google.com/
https://github.com/
https://semantic-ui.com/modules/dropdown.html

Is there anything you are particularly interested in?

@oliviertassinari
Copy link
Member

@teramotodaiki
Copy link
Contributor Author

Hmm... It's better way on real world. event.keyCode is legacy way but still working.

@teramotodaiki
Copy link
Contributor Author

I think there are 2 ways to fix.

  1. Deprecated, but most browsers have event.keyCode (DOM)
    https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode

  2. Modern, and the best API for this purpose is event.isComposing (DOM)
    https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/isComposing

React wraps event.keyCode (DOM) internally to make event.which (React). So we might use event.which (React) instead of event.isComposing (DOM).

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 28, 2020

@teramotodaiki 👍 for going with 2.

@oliviertassinari oliviertassinari added the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Jan 28, 2020
@teramotodaiki
Copy link
Contributor Author

@oliviertassinari Do you mean using event.nativeEvent.isComposing?
I agree to 2. This is confirmation.

@oliviertassinari
Copy link
Member

Yes, unless somebody wants to weigh into another approach, it seems to be the more reliable approach.

@teramotodaiki
Copy link
Contributor Author

teramotodaiki commented Jan 29, 2020

I'm sorry but 1 might be better because macOS/Safari returns isComposing==false when the IME is confirmed. This is a cheat sheet of how does browsers do when IME is confirmed.

browser fire onKeyDown event.key event.which event.nativeEvent.isComposing
macOS/Chrome yes Enter 229 true
macOS/Safari yes Enter 229 false
macOS/Firefox yes Process 229 true
Win10/Edge no - - -
Win10/Chrome yes Process 229 true

event: React.KeyboardEvent

The event.which may be only solution of this problem. 😅
What do you think about this? @oliviertassinari

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 29, 2020

Thanks for not giving up and keep digging. No problem for me, I believe event.which Is how Ant Design and select2 solve this problem, both projects have a strong Chinese community, I would expect that solved the issue relatively well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! ready to take Help wanted. Guidance available. There is a high chance the change will be accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants