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] clearOnEscape causes exit from fullscreen #18107

Closed
2 tasks done
sclavijo93 opened this issue Oct 30, 2019 · 3 comments · Fixed by #18275
Closed
2 tasks done

[Autocomplete] clearOnEscape causes exit from fullscreen #18107

sclavijo93 opened this issue Oct 30, 2019 · 3 comments · Fixed by #18275
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

@sclavijo93
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 I'm testing the clearOnEscape variation of the Autocomplete component on docs and pressing escape causes the browser to exit from fullscreen

Expected Behavior 🤔

The browser state should not be affected by pressing escape

Steps to Reproduce 🕹

  • Codesandox
  • Select any option
  • Press escape to clear the value

Your Environment 🌎

Tech Version
Material-UI current master
Browser Opera v63.0.3368.107
OS macOs Mojave 10.14.6
@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 30, 2019

How do you trigger the "fullscreen" mode?

Browser | Opera v63.0.3368.107

Notice that we have no official support for this browser.

@oliviertassinari
Copy link
Member

What do you think of a fix like this one?

diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index bc02f10f1..2df5b1c6f 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -513,9 +513,15 @@ export default function useAutocomplete(props) {
         break;
       case 'Escape':
         if (popupOpen) {
+          // Avoid Opera to exit fullscreen mode.
+          event.preventDefault();
+          // Avoid the Modal to handle the event.
           event.stopPropagation();
           handleClose(event);
-        } else if (clearOnEscape) {
+        } else if (clearOnEscape && inputValue !== '') {
+          // Avoid Opera to exit fullscreen mode.
+          event.preventDefault();
+          // Avoid the Modal to handle the event.
           event.stopPropagation();
           handleClear(event);
         }

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: Autocomplete good first issue Great for first contributions. Enable to learn the contribution process. component: autocomplete This is the name of the generic UI component, not the React module! and removed component: Autocomplete labels Oct 30, 2019
@ssliman
Copy link
Contributor

ssliman commented Nov 8, 2019

@oliviertassinari working on this

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.

3 participants