Material-ui: [Autocomplete] Cannot delete multiple Chips with Escape Key

[jcafiero]

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

Current Behavior 😯

Currently, if you hit the Escape key while focused in a multi-select Autocomplete with the clearOnEscape prop it will not clear the Chips from the window.

One behavior I did notice was that if there is still active text in the field, hitting the Escape Key twice would clear everything from the Autocomplete.

Expected Behavior 🤔

If you hit the Escape key while focused in a multi-select Autocomplete with the clearOnEscape prop, it should clear the Chips, similar to how clicking on the 'X' icon will clear all of the Chips from the field. This behavior should be consistent whether or not there is partially filled text or not in the Autocomplete.

Steps to Reproduce 🕹

Here is a CodeSandbox: https://codesandbox.io/s/cranky-dhawan-2smfv?fontsize=14&hidenavigation=1&theme=dark

Steps:

1. Tab in to the Autocomplete in the example.
2. Hit the Escape key.
3. Note the Chips are still there.
4. Click on the clear icon in the Autocomplete component.
5. Note that the Chips are deleted.

Context 🔦

This is an accessibility bug that should be resolved to give keyboard users the same experience.

Your Environment 🌎

| Tech | Version |
| ----------- | ------- |
| Material-UI | v4.9.2 |
| React | latest |
| Browser | Chrome |

[oliviertassinari]

@jcafiero Brilliant, thanks for reporting this bug. It should be an easy one to fix and write tests for. Feel free to go ahead :).

[chaudharykiran]

As my understanding, esc should delete all the selected chips same as x button click. If that is the case, I like to work on this issue. cc. @jcafiero @oliviertassinari

[jcafiero]

@chaudharykiran that's fine with me. Yes, that is the desired behavior.

[oliviertassinari]

For anyone looking at the problem, the fix should be about:

diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index 9957ed76d..2f61a763e 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -628,7 +628,7 @@ export default function useAutocomplete(props) {
           // Avoid the Modal to handle the event.
           event.stopPropagation();
           handleClose(event);
-        } else if (clearOnEscape && inputValue !== '') {
+        } else if (clearOnEscape && (inputValue !== '' || (multiple && value.length > 0))) {
           // Avoid Opera to exit fullscreen mode.
           event.preventDefault();
           // Avoid the Modal to handle the event.

Then, it's about adding a test case to prevent regressions in the future.

[yann120]

Hi, can we do a PR to fix this tonight ?
I'm going to the meetup "Contribute by Doctolib" and I'm looking for 'good first issues' :)
Thanks

[oliviertassinari]

@yann120 I was working at Doctolib, sounds great! 😆

[chaudharykiran]

I am afraid, I may not be able to send PR for this tonight. cc. @oliviertassinari @yann120

[yann120]

Hey @oliviertassinari , we fix it yesterday at Doctolib event with your advices. But we write a new test and we can't make it pass... But we are working on it and try to understand why 👍