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] blurOnSelect consistency #20137

Closed
2 tasks done
vince1995 opened this issue Mar 16, 2020 · 7 comments · Fixed by #20314
Closed
2 tasks done

[Autocomplete] blurOnSelect consistency #20137

vince1995 opened this issue Mar 16, 2020 · 7 comments · Fixed by #20314
Labels
bug 🐛 Something doesn't work 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

@vince1995
Copy link
Contributor

  • 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 a Autocomplete has blurOnSelect it doesn't blur when an option is selected via keyboard (arrows and return). The popup closes.

Expected Behavior 🤔

It should also blur.

Steps to Reproduce 🕹

Go to https://codesandbox.io/s/material-demo-smkqh and select an option with the keyboard.

@oliviertassinari oliviertassinari added the component: autocomplete This is the name of the generic UI component, not the React module! label Mar 18, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 19, 2020

@vince1995 The prop was introduced for the mobile experience. It's frequent to want to hide the virtual keyboard after the selection of an option. We have never considered the use case on a desktop with the keyboard. Could you share more on why you want to implement such behavior?

Without more information on the motivation for the end-users of the component, I would be in favor of denying the request, and implement the following breaking change:

-blurOnSelect?: 'touch' | 'mouse' | true | false;
+blurOnSelect?: 'touch' | false;

@rassie
Copy link

rassie commented Mar 19, 2020

I've stumbled upon this too, even though my use case is probably a bit different.

I want to enable user to change a setting quickly from predefined options. If the user were to use a keyboard, he'd focus the input field, type something in, select an option with arrows and confirm with return. Now the value of the selected entry is in the text input field and the cursor is positioned behind the text, so that selecting another entry is not possible without deleting the value manually. I would like to have the ability to start typing again right away to select a new option.

With a mouse, the input field would blur and with selectOnFocus set, focusing the field allows typing again. So basically, I'm missing a selectOnSelect options (select text on option selection), but until that's available, a consistent blurOnSelect would help quite a lot.

@oliviertassinari oliviertassinari added ready to take Help wanted. Guidance available. There is a high chance the change will be accepted good first issue Great for first contributions. Enable to learn the contribution process. breaking change and removed ready to take Help wanted. Guidance available. There is a high chance the change will be accepted labels Mar 19, 2020
@vince1995
Copy link
Contributor Author

Because we expect that if a value is selected the user finished his input. It would be confusing if the input is still focused after the selection.

And it's consistent to mobile device logic.

@oliviertassinari
Copy link
Member

Ok, thanks for these insights. I think that we can move forward with consistency. @vince1995 Do you want to work on a pull request? :)

diff --git a/docs/src/pages/components/autocomplete/Playground.tsx b/docs/src/pages/components/autocomplete/Playground.tsx
index e7d99762b..aab11932d 100644
--- a/docs/src/pages/components/autocomplete/Playground.tsx
+++ b/docs/src/pages/components/autocomplete/Playground.tsx
@@ -104,6 +104,12 @@ export default function Playground() {
         disablePortal
         renderInput={params => <TextField {...params} label="disablePortal" margin="normal" />}
       />
+      <Autocomplete
+        {...defaultProps}
+        id="blur-on-select"
+        blurOnSelect
+        renderInput={params => <TextField {...params} label="blurOnSelect" margin="normal" />}
+      />
     </div>
   );
 }
diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index f915e3ca0..53d5234a2 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -451,6 +451,8 @@ export default function useAutocomplete(props) {
     setValue(newValue);
   };

+  const isTouch = React.useRef(false);
+
   const selectNewValue = (event, option, reasonProp = 'select-option', origin = 'options') => {
     let reason = reasonProp;
     let newValue = option;
@@ -489,6 +491,14 @@ export default function useAutocomplete(props) {
     if (!disableCloseOnSelect) {
       handleClose(event, reason);
     }
+
+    if (
+      blurOnSelect === true ||
+      (blurOnSelect === 'touch' && isTouch.current) ||
+      (blurOnSelect === 'mouse' && !isTouch.current)
+    ) {
+      inputRef.current.blur();
+    }
   };

   function validTagIndex(index, direction) {
@@ -736,8 +746,6 @@ export default function useAutocomplete(props) {
     setHighlightedIndex(index, 'mouse');
   };

-  const isTouch = React.useRef(false);
-
   const handleOptionTouchStart = () => {
     isTouch.current = true;
   };
@@ -745,15 +753,6 @@ export default function useAutocomplete(props) {
   const handleOptionClick = event => {
     const index = Number(event.currentTarget.getAttribute('data-option-index'));
     selectNewValue(event, filteredOptions[index], 'select-option');
-
-    if (
-      blurOnSelect === true ||
-      (blurOnSelect === 'touch' && isTouch.current) ||
-      (blurOnSelect === 'mouse' && !isTouch.current)
-    ) {
-      inputRef.current.blur();
-    }
-
     isTouch.current = false;
   };

@oliviertassinari oliviertassinari changed the title [Autocmplete] doesn't blur if it should [Autocomplete] blurOnSelect consistency Mar 19, 2020
@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label Mar 19, 2020
@alexbarkin
Copy link
Contributor

Hey, @oliviertassinari I would be happy to take this one on, is it still available?

@oliviertassinari
Copy link
Member

@alexbarkin Definitely :)

@RickMeasham
Copy link

Thanks for fixing this. I use an autocomplete in my menubar to quickly search and navigate to a record. Without blurOnSelect the autocomplete was still active after changing the currently viewed object (as the page doesn't get re-rendered). Using inputValue="" cleared the value after someone selected something, but left the cursor flashing.

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! 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