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] Don't close the popup when Ctrl/Shift clicks #19654

Closed
bishonen opened this issue Feb 11, 2020 · 10 comments · Fixed by #22696
Closed

[Autocomplete] Don't close the popup when Ctrl/Shift clicks #19654

bishonen opened this issue Feb 11, 2020 · 10 comments · Fixed by #22696
Labels
accessibility a11y component: autocomplete This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@bishonen
Copy link

bishonen commented Feb 11, 2020

In an Autocomplete component that contains the multiple prop, it would be a great feature to have a ctrl click functionality. E.g. simply clicking with the mouse should select a single item. Only when ctrl+click is used the popup shouldn't close, allowing to select more options.

@oliviertassinari oliviertassinari added component: autocomplete This is the name of the generic UI component, not the React module! low priority waiting for 👍 Waiting for upvotes and removed low priority labels Feb 11, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 11, 2020

I have seen a related request in select2/select2#5302 for range selection but there is low traction for it.
I haven't seen it implemented anywhere, see the benchmarking list: https://www.notion.so/mui-org/e6efee7c4e6a467487b5efb9b07b9fc3?v=b4467d9174084d8286548631756d4795&p=96963c221d994c9ab9fbb3db5517faab.

@oliviertassinari oliviertassinari added the component: select This is the name of the generic UI component, not the React module! label Feb 11, 2020
@oliviertassinari

This comment has been minimized.

@henrymorton

This comment has been minimized.

@kevin-brown
Copy link

kevin-brown commented May 5, 2020

It's useful to note that Select2 has implemented this many many years ago with the CTRL key (recently adding the meta key in select2/select2#5222).

#19654 (comment) The ticket which was linked is for using SHIFT to select a range at once instead of a single one, which Select2 does not currently support.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 21, 2020

@kevin-brown Thanks for mentioning this! What do you think about this diff for the Autocomplete?

diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index feea18c89f..a12ff69dc2 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -563,7 +563,7 @@ export default function useAutocomplete(props) {
     resetInputValue(event, newValue);

     handleValue(event, newValue, reason, { option });
-    if (!disableCloseOnSelect) {
+    if (!disableCloseOnSelect && !event.ctrlKey && !event.metaKey) {
       handleClose(event, reason);
     }

Do you want to work on a pull request? :) (we would also need new tests)

Regarding the Select, we are working on it in #21753.

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Jul 21, 2020
@oliviertassinari oliviertassinari changed the title Select Component: Multiple - Allow for Ctrl/Shift clicks [Select][Autocomplete] Don't close the popup when Ctrl/Shift clicks Jul 21, 2020
@montelius
Copy link
Contributor

Hello! :) Can I work on a pull request for this?

@oliviertassinari
Copy link
Member

@montelius yes :)

@montelius
Copy link
Contributor

@oliviertassinari Nice, thanks! :) So this is the first time I'm working with Material-UI, and I'm a bit confused about how the unit tests are structured. What tests would be suitable to implement for this feature? Could you maybe refer me to unit tests of other components that are similar so that I could get an idea of what would be a good test case? Tried to find something myself but didn't manage, would be grateful for your help!

@montelius
Copy link
Contributor

@oliviertassinari Yes thanks, thought the tests should go in useAutocomplete.test.js but that makes sense, I'll create a PR soon!

@oliviertassinari oliviertassinari changed the title [Select][Autocomplete] Don't close the popup when Ctrl/Shift clicks [Autocomplete] Don't close the popup when Ctrl/Shift clicks Oct 30, 2021
@oliviertassinari oliviertassinari removed the component: select This is the name of the generic UI component, not the React module! label Oct 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility a11y component: autocomplete This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants