[Autocomplete] Improve popup open logic

[Janpot]

• [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 😯

When an Autocomplete is active and I switch tabs, the menu is opened when I come back

Expected Behavior 🤔

The state of the UI is the same as before I switch tabs.

Steps to Reproduce 🕹

Steps:

1. go to https://material-ui.com/components/autocomplete/#combo-box
2. select an option
3. switch to another tab
4. switch back
5. observe

Context 🔦

I think we're hitting the same as https://stackoverflow.com/questions/10657176/changing-browser-tabs-undesirably-fires-the-focus-event-especially-in-google-ch

Your Environment 🌎

Tech	Version
Material-UI	v4.7.2
React
Browser	chrome 78.0.3904.108
TypeScript
etc.

[oliviertassinari]

What's wrong with this behavior?

[Janpot]

When I switch to another tab, I expect to find my page in the exact same state that I left it when I come back. If the menu of the active element is closed, it is because I wanted it to be closed. switching a tab is not an action that should be interpreted as "the user wants the menu of the active element to be open". It's confusing and so it's bad UX. I have a page where a filter happens automatically when the value of an autocomplete changes. So it happens fairly regularly that one of those is still active but closed at the time. There isn't really anything else to do on this page besides scrolling. I often come to this page in an existing tab to find a random menu open and it's kind of weird.

[oliviertassinari]

@Janpot One more question. Do you want to popup to open on keyboard focus (not taking the mouse into account)? For instance, when using Tab to navigate the page.

[Janpot]

~I'd probably consider that action equivalent to a mouse click on the component, so yes. But I have to admit I'm far from a UX/accessibility expert, this is just my gut feeling. I don't really mind if it doesn't open though.~

edit

Actually, scratch that, I'm 5 minutes further and I already changed my mind. Need to think more about this. Maybe it should behave more like a button that opens a modal. You wouldn't want the button to open the modal when tab navigating to it. I feel like when using tab I'm just navigating around, to get to the component I want to manipulate. It feels a bit intrusive when that starts popping up menus of elements that I want to skip. Possibly menus that cover op the element I'm trying to navigate to.

[Janpot]

@oliviertassinari How about:

1. keyboard focus by tab => menu stays closed
2. Enter key or start typing => open menu
   1. Enter key => select item + close menu
   2. Esc key => close menu

That seems to align with https://webaim.org/techniques/keyboard/

Probably something to look into as well: I was checking how <Select /> handles this and it seems to behave strange when you press spacebar when it's focused. It briefly flashes the popover and the readonly one even scrolls the page. The native select actually opens the menu on spacebar, it doesn't open on Enter though, while the Simple Select does.

edit just found out that on simple select I can hold spacebar to open the menu and use arrows to select an option, releasing spacebar will select that option. Is this intended? It feels counter intuitive.

[oliviertassinari]

How about:

Set disableOpenOnFocus and you will be good. Alternatively, you can control the open state and implement whatever logic works best for your context (onOpen/onClose).

I was checking how <Select />

Duplicate of another issue, fixed in master.

[Janpot]

Set disableOpenOnFocus and you will be good.

If I go by the demo, that also disables it on click, so not an option.

fixed in master.

Almost, the native select can also select options with spacebar after it is opened. The simple select can't.

[oliviertassinari]

If I go by the demo, that also disables it on click, so not an option.

@Janpot So you are looking for the same default behavior as react-select?

Almost, the native select can also select options with spacebar after it is opened. The simple select can't.

This is macOS specific, it won't happen on Linux nor Windows. We decided to go against it: https://github.com/mui-org/material-ui/pull/18754#issuecomment-563242809.

[oliviertassinari]

I think that a good solution would be to add a second reason argument to the onOpen and onClose callbacks so you can implement something custom, and on Material-UI side, to wait for more feedback to change the default tradeoff.

[Janpot]

So you are looking for the same default behavior as react-select?

I'm checking their docs pages and they open on click but not on keyboard focus, they also don't open menu for an active control after switching tabs. so wrt to this behavior, yes. I also think this would be the most sensible default for material-ui. I think it's especially bad accessibility to open the menu on keyboard focus by default. And I think material ui should mimic its own Select component, which is open on click but not on keyboard focus.

I saw they also don't allow typing a space in an empty control, wrt to that behavior, no.

[oliviertassinari]

@Janpot Would the reason approach work for you?

[Janpot]

Yes that would probably work if I can differentiate between an actual mouse click and another focus event.

You don't agree that the current behavior is not ideal accessibility-wise?

[oliviertassinari]

I believe the current logic will allow the make a difference between a focus and a touch/mouse click.

It seems that the Autocomplete behaves like the google.com's search, so I don't think that it's very important. For a11y, https://www.w3.org/TR/wai-aria-practices/examples/combobox/aria1.1pattern/listbox-combo.html is a good resource to look at.

[Janpot]

For me google.com's search doesn't open when I move keyboard focus to it. Neither does it open when it's active and I switch tabs. It does however also not open before I start typing, even when I click it.

is a good resource to look at.

I'm specifically referring to focus navigation:

3. Predictability of movement: Usability of a keyboard interface is heavily influenced by how readily users can guess where focus will land after a navigation key is pressed.

Point being that it's hard to predict where focus will land if a menu is covering some of those elements where focus may land.

Anyway, not that important to me 🙂. If I can implement it in userland that's fine, I just disagree with the default.

[oliviertassinari]

Right, you need to fill in a value in the search bar at the top of the page to get the Google's suggestions. It's only then that you will get a display of the popup on focus and an open back behavior when switching tabs.

I'm not sure to follow the focus covering issue. The popup should only open if the input is the active element. Does it behave differently in your case?

Yes, I would like to get a couple of more user's feedback before we change this aspect.

[Janpot]

oh ok, I was testing the one in the middle of the page when there's no search results, that one seems to behave differently.

In any case, I can't get that google box you are looking at in the same state as the matui one (active but closed popup) so I can't compare the tab switching behavior. Which ultimately is the main problem I'd like to solve, the keyboard accessibility is way low on my priority list.

However, the gmail searchbar has a autocomplete which can be closed while still staying active, and I can confirm it doesn't open when switching tabs. neither does it open for me on keyboard focus.

Edit:

Ok, I think I see now, you can look at it as a textfield with a menu. or as a select component with a search box. disableOpenOnFocus switches between the two approaches. I can live with that. But not with the tab switching behavior. I'd be happy with a reason parameter to get around this tab switching.

[oliviertassinari]

@Janpot I do agree that if the input is focused and the popup closed when leaving the tab, it would be better to return to the same state. However, I'm not sure about the code it would require nor that it would worth us to invest time in it.

For the reason we could do something like this, but I haven't tested it:

diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts
index 0628df0b1..d9114105b 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts
@@ -132,6 +132,7 @@ export interface UseAutocompleteProps {
    * Use in controlled mode (see open).
    *
    * @param {object} event The event source of the callback.
+   * @param {string} reason Can be: `"input"`, `"toggle"`, `"blur"`, `"escapeKeyDown"` or `"focusTag"``.
    */
   onClose?: (event: React.ChangeEvent<{}>) => void;
   /**
@@ -139,7 +140,7 @@ export interface UseAutocompleteProps {
    *
    * @param {object} event The event source of the callback.
    * @param {string} value The new value of the text input
-   * @param {string} reason One of "input" (user input) or "reset" (programmatic change)
+   * @param {string} reason Can be: `"input"` (user input) or `"reset"` (programmatic change)
    */
   onInputChange?: (event: React.ChangeEvent<{}>, value: any, reason: 'input' | 'reset') => void;
   /**
@@ -147,6 +148,7 @@ export interface UseAutocompleteProps {
    * Use in controlled mode (see open).
    *
    * @param {object} event The event source of the callback.
+   * @param {string} reason Can be: `"input"`, `"toggle"`, `"focus"` or `"keyDown"`.
    */
   onOpen?: (event: React.ChangeEvent<{}>) => void;
   /**
diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index 5b4ef231c..8e04019c3 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -376,26 +376,26 @@ export default function useAutocomplete(props) {
     changeHighlightedIndex('reset', 'next');
   }, [inputValue]); // eslint-disable-line react-hooks/exhaustive-deps

-  const handleOpen = event => {
+  const handleOpen = (event, reason = 'keyDown') => {
     if (open) {
       return;
     }

     if (onOpen) {
-      onOpen(event);
+      onOpen(event, reason);
     }
     if (!isOpenControlled) {
       setOpenState(true);
     }
   };

-  const handleClose = event => {
+  const handleClose = (event, reason) => {
     if (!open) {
       return;
     }

     if (onClose) {
-      onClose(event);
+      onClose(event, reason);
     }
     if (!isOpenControlled) {
       setOpenState(false);
@@ -436,7 +436,7 @@ export default function useAutocomplete(props) {
     }
     handleValue(event, newValue);
     if (!disableCloseOnSelect) {
-      handleClose(event);
+      handleClose(event, 'input');
     }

     resetInputValue(event, newValue);
@@ -481,7 +481,7 @@ export default function useAutocomplete(props) {
       return;
     }

-    handleClose(event);
+    handleClose(event, 'focusTag');

     let nextTag = focusedTag;

@@ -510,7 +510,7 @@ export default function useAutocomplete(props) {
   const handleClear = event => {
     handleValue(event, multiple ? [] : null);
     if (disableOpenOnFocus) {
-      handleClose();
+      handleClose(event, 'input');
     }
     setInputValue('');
   };
@@ -589,7 +589,7 @@ export default function useAutocomplete(props) {
           event.preventDefault();
           // Avoid the Modal to handle the event.
           event.stopPropagation();
-          handleClose(event);
+          handleClose(event, 'escapeKeyDown');
         } else if (clearOnEscape && inputValue !== '') {
           // Avoid Opera to exit fullscreen mode.
           event.preventDefault();
@@ -614,7 +614,7 @@ export default function useAutocomplete(props) {
     setFocused(true);

     if (!disableOpenOnFocus) {
-      handleOpen(event);
+      handleOpen(event, 'focus');
     }
   };

@@ -632,7 +632,7 @@ export default function useAutocomplete(props) {
       resetInputValue(event, value);
     }

-    handleClose(event);
+    handleClose(event, 'blur');
   };

   const handleInputChange = event => {
@@ -640,14 +640,14 @@ export default function useAutocomplete(props) {

     if (newValue === '') {
       if (disableOpenOnFocus) {
-        handleClose(event);
+        handleClose(event, 'input');
       }

       if (!disableClearable && !multiple) {
         handleValue(event, null);
       }
     } else {
-      handleOpen(event);
+      handleOpen(event, 'input');
     }

     if (inputValue === newValue) {
@@ -692,9 +692,9 @@ export default function useAutocomplete(props) {
     inputRef.current.focus();

     if (open) {
-      handleClose(event);
+      handleClose(event, 'toggle');
     } else {
-      handleOpen(event);
+      handleOpen(event, 'toggle');
     }
   };

@@ -719,7 +719,13 @@ export default function useAutocomplete(props) {

   const handleInputMouseDown = event => {
     if (inputValue === '' && (!disableOpenOnFocus || inputRef.current === document.activeElement)) {
-      handlePopupIndicator(event);
+      inputRef.current.focus();
+
+      if (open) {
+        handleClose(event, 'toggle');
+      } else {
+        handleOpen(event, 'toggle');
+      }
     }
   };

[Janpot]

@oliviertassinari could the following strategy work? When an autocomplete gets focus, add a focus event handler to the window. When that event on the window fires (when switched back to the tab), set a marker to ignore one focus event on the autocomplete. remove the window focus event handler when the autocomplete blurs.

[oliviertassinari]

@Janpot We have handled tab switch at two occasions in the past:

• https://github.com/mui-org/material-ui/blob/ad6fe1bfc569ee4230f2237e6639b22bdc4b7090/packages/material-ui/src/Snackbar/Snackbar.js#L202
• https://github.com/mui-org/material-ui/blob/ad6fe1bfc569ee4230f2237e6639b22bdc4b7090/packages/material-ui/src/utils/focusVisible.js#L123

But with more hindsight, I'm not sure what should be the expected behavior for the Autocomplete. Google's product seems to be highly inconsistent with this, it's almost like the behavior inherits from other decisions, the fruit of chance.

[Janpot]

@oliviertassinari Ok, I think I have a solution: https://codesandbox.io/s/material-demo-ofp6l It seems to be quite straightforward after all, and I can implement it in userland. Thanks for your guidance. If you don't feel like this should be the behavior of the Autocomplete component then feel free to close this issue.

[oliviertassinari]

@Janpot Great. I think that we should wait.

However, the reason argument could move forward. I think that we can leave this issue open until somebody else faces the problem and needs it.

[eps1lon]

When I switch to another tab, I expect to find my page in the exact same state that I left it when I come back

This is browser behavior though. When leaving the tab everything gets blurred. When you tab back focus events get fired again

This should not be applied to the autocomplete only. Every component doing "something" when focused would need this fix.

Considering how <input type="text" list="some-datalist-idref" /> behaves I'd much rather change the default of disableOpenOnFocus to true and handle click separately. The native variant does not open on focus. Only when you click in the textbox. I suspect this is also better for screen readers since you don't get this wall of text announced just by moving focus.

[oliviertassinari]

@eps1lon His sounds like a great plan 👍. I have noticed that Chrome autocomplete and autofill list boxes behave the same way.

One note regarding the prop, if we change the default to true, then we should change the prop name disableOpenOnFocus -> openOnFocus.

[haseebdaone]

Think I might give this a go, read through the whole thing and from what I've understood I just need to change disableOpenOnFocus = true is that correct?

[oliviertassinari]

@haseebdaone This is not a one-line change, it would have had the "good first issue" tag if it was the case.

[haseebdaone]

@oliviertassinari If you don't mind me asking what other changes are needed?

[sashberd]

Another good improvement for popup open logic is to disable it open on delete chip button click. Each time I delete Chip the popover is opened. If you have custom filter function and state that hold data for Autocomplete, the filter function will run before state is updated. So if such option could be added to development will be great

[meastes]

Another good improvement for popup open logic is to disable it open on delete chip button click. Each time I delete Chip the popover is opened. If you have custom filter function and state that hold data for Autocomplete, the filter function will run before state is updated. So if such option could be added to development will be great

I believe this is actually a bug. I've created an issue here: https://github.com/mui-org/material-ui/issues/19364

[oliviertassinari]

@haseebdaone See: https://github.com/mui-org/material-ui/issues/18815#issuecomment-569277122 @sashberd I have marked your comment as off-topic. If you have a reproduction, a new issue would be great. It's not supposed to behave this way, nor I can successfully reproduce it on https://material-ui.com/components/autocomplete/. @meastes I have marked your comment as off-topic. Autocomplete and Select are two very distinct components. Select aims to be a light replacement for a native <select>. Autocomplete aims to be feature-rich.

[oliviertassinari]

@haseebdaone I had a closer look at the changes. I think that a change, in this order, will solve our problem. Note that the tests will also need to be updated :)

diff --git a/docs/src/pages/components/autocomplete/GoogleMaps.tsx b/docs/src/pages/components/autocomplete/GoogleMaps.tsx
index 516010ff4..ac8ef4e60 100644
--- a/docs/src/pages/components/autocomplete/GoogleMaps.tsx
+++ b/docs/src/pages/components/autocomplete/GoogleMaps.tsx
@@ -109,7 +109,6 @@ export default function GoogleMaps() {
       autoComplete
       includeInputInList
       freeSolo
-      disableOpenOnFocus
       renderInput={params => (
         <TextField
           {...params}
diff --git a/docs/src/pages/components/autocomplete/Playground.tsx b/docs/src/pages/components/autocomplete/Playground.tsx
index c3cae2e6f..874f43bdf 100644
--- a/docs/src/pages/components/autocomplete/Playground.tsx
+++ b/docs/src/pages/components/autocomplete/Playground.tsx
@@ -88,10 +88,10 @@ export default function Playground() {
       />
       <Autocomplete
         {...defaultProps}
-        id="disable-open-on-focus"
-        disableOpenOnFocus
+        id="open-on-focus"
+        openOnFocus
         renderInput={params => (
-          <TextField {...params} label="disableOpenOnFocus" margin="normal" fullWidth />
+          <TextField {...params} label="openOnFocus" margin="normal" fullWidth />
         )}
       />
       <Autocomplete
diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
index 463ff0f2f..fd0097cdf 100644
--- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
+++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
@@ -252,7 +252,6 @@ const Autocomplete = React.forwardRef(function Autocomplete(props, ref) {
     disableCloseOnSelect = false,
     disabled = false,
     disableListWrap = false,
-    disableOpenOnFocus = false,
     disablePortal = false,
     filterOptions,
     filterSelectedOptions = false,
@@ -276,6 +275,7 @@ const Autocomplete = React.forwardRef(function Autocomplete(props, ref) {
     onInputChange,
     onOpen,
     open,
+    openOnFocus = false,
     openText = 'Open',
     options,
     PaperComponent = Paper,
@@ -566,10 +566,6 @@ Autocomplete.propTypes = {
    * If `true`, the list box in the popup will not wrap focus.
    */
   disableListWrap: PropTypes.bool,
-  /**
-   * If `true`, the popup won't open on input focus.
-   */
-  disableOpenOnFocus: PropTypes.bool,
   /**
    * Disable the portal behavior.
    * The children stay within it's parent DOM hierarchy.
@@ -691,6 +687,10 @@ Autocomplete.propTypes = {
    * Control the popup` open state.
    */
   open: PropTypes.bool,
+  /**
+   * If `true`, the popup will open on input focus.
+   */
+  openOnFocus: PropTypes.bool,
   /**
    * Override the default text for the *open popup* icon button.
    *
diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts
index e616b2b1a..81db8c032 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts
@@ -69,10 +69,6 @@ export interface UseAutocompleteCommonProps<T> {
    * If `true`, the list box in the popup will not wrap focus.
    */
   disableListWrap?: boolean;
-  /**
-   * If `true`, the popup won't open on input focus.
-   */
-  disableOpenOnFocus?: boolean;
   /**
    * A filter function that determines the options that are eligible.
    *
@@ -154,6 +150,10 @@ export interface UseAutocompleteCommonProps<T> {
    * Control the popup` open state.
    */
   open?: boolean;
+  /**
+   * If `true`, the popup will open on input focus.
+   */
+  openOnFocus?: boolean;
   /**
    * Array of options.
    */
diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index 3c01bb4b3..bb08631b4 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -1,7 +1,7 @@
 /* eslint-disable no-constant-condition */
 import React from 'react';
 import PropTypes from 'prop-types';
-import { setRef, useEventCallback, useControlled, ownerDocument } from '@material-ui/core/utils';
+import { setRef, useEventCallback, useControlled } from '@material-ui/core/utils';

 // https://stackoverflow.com/questions/990904/remove-accents-diacritics-in-a-string-in-javascript
 // Give up on IE 11 support for this feature
@@ -86,12 +86,12 @@ export default function useAutocomplete(props) {
     autoSelect = false,
     blurOnSelect = false,
     clearOnEscape = false,
+    componentName = 'useAutocomplete',
     debug = false,
     defaultValue = props.multiple ? [] : null,
     disableClearable = false,
     disableCloseOnSelect = false,
     disableListWrap = false,
-    disableOpenOnFocus = false,
     filterOptions = defaultFilterOptions,
     filterSelectedOptions = false,
     freeSolo = false,
@@ -105,13 +105,13 @@ export default function useAutocomplete(props) {
     multiple = false,
     onChange,
     onClose,
-    onOpen,
     onInputChange,
+    onOpen,
     open: openProp,
+    openOnFocus = false,
     options,
     selectOnFocus = !props.freeSolo,
     value: valueProp,
-    componentName = 'useAutocomplete',
   } = props;

   const [defaultId, setDefaultId] = React.useState();
@@ -655,7 +655,7 @@ export default function useAutocomplete(props) {
   const handleFocus = event => {
     setFocused(true);

-    if (!disableOpenOnFocus && !ignoreFocus.current) {
+    if (openOnFocus && !ignoreFocus.current) {
       handleOpen(event);
     }
   };
@@ -692,10 +692,6 @@ export default function useAutocomplete(props) {
     }

     if (newValue === '') {
-      if (disableOpenOnFocus) {
-        handleClose(event);
-      }
-
       if (!disableClearable && !multiple) {
         handleValue(event, null);
       }
@@ -783,8 +779,7 @@ export default function useAutocomplete(props) {
   };

   const handleInputMouseDown = event => {
-    const doc = ownerDocument(inputRef.current);
-    if (inputValue === '' && (!disableOpenOnFocus || inputRef.current === doc.activeElement)) {
+    if (inputValue === '') {
       handlePopupIndicator(event);
     }
   };
@@ -968,10 +963,6 @@ useAutocomplete.propTypes = {
    * If `true`, the list box in the popup will not wrap focus.
    */
   disableListWrap: PropTypes.bool,
-  /**
-   * If `true`, the popup won't open on input focus.
-   */
-  disableOpenOnFocus: PropTypes.bool,
   /**
    * A filter function that determins the options that are eligible.
    *
@@ -1051,6 +1042,10 @@ useAutocomplete.propTypes = {
    * Control the popup` open state.
    */
   open: PropTypes.bool,
+  /**
+   * If `true`, the popup will open on input focus.
+   */
+  openOnFocus: PropTypes.bool,
   /**
    * Array of options.
    */

[haseebdaone]

I'll give this a go