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] Provide options to control behaviour when cleared #18873

Closed
1 task done
monbrey opened this issue Dec 16, 2019 · 9 comments · Fixed by #18889
Closed
1 task done

[Autocomplete] Provide options to control behaviour when cleared #18873

monbrey opened this issue Dec 16, 2019 · 9 comments · Fixed by #18889
Labels
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. new feature New feature or request

Comments

@monbrey
Copy link
Contributor

monbrey commented Dec 16, 2019

  • I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

While this issue is similar to #18280 I don't believe its a duplicate.

It would be good if developers had the ability to customize the behavior of the autocomplete when the selection is cleared via the Clear button.

Options:

  • openOnClear={false} - revert the Autocomplete to it's null/placeholder state
  • onClear event handler

Examples 🌈

<Autocomplete
    options={["A", "B", "C", "D"]}
    openOnClear={false}
    renderInput={params => (
        <TextField {...params} label="Letter" variant="outlined" fullWidth />
    )}
/>
@monbrey monbrey changed the title [Autocomplete] Provide options to control behaviour when cleared / focused [Autocomplete] Provide options to control behaviour when cleared Dec 16, 2019
@oliviertassinari oliviertassinari added the component: autocomplete This is the name of the generic UI component, not the React module! label Dec 16, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 16, 2019

@monbrey

  • What's your motivation for such behavior?
  • What's wrong with disableOpenOnFocus?
  • What about a solution like this one: [Autocomplete] Improve popup open logic #18815 (comment)? Basically, we could give a reason argument to the onOpen callback and you could ignore the "clear" events (require to control the open state).

@monbrey
Copy link
Contributor Author

monbrey commented Dec 16, 2019

I'm not really sure how to respond to my "motivation" other than to say that clearing a value doesn't always imply the user wants to select another one - if they did, I would think they'd be less likely to focus specifically on the clear button, and would just open the select anywhere.

Regarding disableOpenOnFocus - I did have some luck preventing it from opening by binding its boolean to a state value which was updated onChange - if the value became null, don't open the dropdown. However this doesn't prevent the Autocomplete input from being focused on clear, and doesn't revert it to the placeholder state.

The reason argument might also help in a similar way, however I tried a similar state-based approach with the onOpen and onChange events and encountered the same focus-related issues as above.

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 16, 2019

Thanks for the extra information, I better understand the motivation now. My previous suggestions were a bit off the target. I can see two different levels of solution.

  1. We might want to change the default behavior to match the one you are describing. It does sound better.
  2. We can add a reason argument to the onChange event. You would be able to switch the open prop.

@oliviertassinari oliviertassinari added the new feature New feature or request label Dec 16, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 16, 2019

What do you think of the following diff?

diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index ca8afabdf..4466e4d50 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -108,6 +108,7 @@ export default function useAutocomplete(props) {
     setDefaultId(`mui-autocomplete-${Math.round(Math.random() * 1e5)}`);
   }, []);

+  const ignoreFocus = React.useRef(false);
   const firstFocus = React.useRef(true);
   const inputRef = React.useRef(null);
   const listboxRef = React.useRef(null);
@@ -509,10 +510,8 @@ export default function useAutocomplete(props) {
   };

   const handleClear = event => {
+    ignoreFocus.current = true;
     handleValue(event, multiple ? [] : null);
-    if (disableOpenOnFocus) {
-      handleClose();
-    }
     setInputValue('');
   };

@@ -614,14 +613,17 @@ export default function useAutocomplete(props) {
   const handleFocus = event => {
     setFocused(true);

-    if (!disableOpenOnFocus) {
+    if (!disableOpenOnFocus && !ignoreFocus.current) {
       handleOpen(event);
     }
   };

   const handleBlur = event => {
     setFocused(false);
     firstFocus.current = true;
+    ignoreFocus.current = false;

     if (debug && inputValue !== '') {
       return;
@@ -706,8 +708,6 @@ export default function useAutocomplete(props) {
   });

   const handlePopupIndicator = event => {
-    inputRef.current.focus();
-
     if (open) {
       handleClose(event);
     } else {
@@ -715,13 +715,14 @@ export default function useAutocomplete(props) {
     }
   };

+  // Prevent input blur when interacting with the combobox
   const handleMouseDown = event => {
     if (event.target.nodeName !== 'INPUT') {
-      // Prevent blur
       event.preventDefault();
     }
   };

+  // Focus the input when first interacting with the combobox
   const handleClick = () => {
     if (
       firstFocus.current &&

Do you want to submit a pull request? It would also be great to add a new test case :).

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Dec 16, 2019
@monbrey
Copy link
Contributor Author

monbrey commented Dec 16, 2019

What do you think of the following diff?

Look interesting - if I'm reading it correctly it appears to be changing the default behaviour as you suggested. I'll try this out then have a go at doing the PR myself - I don't have a lot of experience writing test cases for TS/JS but it never hurts to learn.

@MartinSkovsen
Copy link

Is there any particular reason that Mui enforce focus on clear? What if the end user wishes to have a clear state and no focus after clear?

@oliviertassinari
Copy link
Member

@MartinSkovsen The focus comes from:

https://github.com/mui-org/material-ui/blob/576ac632946dae1a021a336a4df0688e047bc743/packages/material-ui/src/useAutocomplete/useAutocomplete.js#L905

The clear button receives the focus on mouseDown, and is unmounted on mouseUp, so if we didn't focus the input, the focus would move to the outermost element that can receive a focus, often the <body>.

Where do you expect the activeElement to be if an end-user clears a combobox?

@MartinSkovsen
Copy link

Good point. I don't have any specific expectation for the activeElement; Keeping the current functionality is certainly a sensible default. However, I would like to optionally provide an alternative activeElement. Think of this scenario:

I have an optional entry in my form. The user uses a combobox for providing the desired value. The user might decide to delete the selected value for which ever reason, and then want to be out of focus. I currently have this case, and ideally I don't interfere with how the component works, particularly since a work around will be devious (and surely confusing). -- Perhaps this is just an anti pattern suggestion...

@oliviertassinari
Copy link
Member

@MartinSkovsen Thanks for the extra details, feel free to override this behavior in your application. I assume you would need to call prevent default on mouse down to keep the focus where it was, and extend the component to support https://next.material-ui.com/components/autocomplete/#events.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. new feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants