Material-ui: [Autocomplete] GitHub's demo isn't identical to GitHub implementation

Created on 2 Sep 2020  路  8Comments  路  Source: mui-org/material-ui


Events bound to DOM siblings of Autocomplete are ignored.

Codesandbox link

  • [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 馃槸


For elements placed next to the Autocomplete component, any event on them is ignored.

Expected Behavior 馃


To only capture event on Autocomplete and let the other elements pass their events.
Or add another autocomplete close reason.

Steps to Reproduce 馃暪

Steps:

  1. Place a link outside of autocomplete on a popper
  2. Click the link
  3. Autocomplete popper closes without capturing the event.

Context 馃敠


I'm simply trying to place a helper link above my autocomplete popper so those users who are unsure could navigate to the link and read about more information.

Your Environment 馃寧

| Tech | Version |
| ----------- | ------- |
| Material-UI core | v4.11.0 |
| Material-UI lab | v4.0.0-alpha.52 |
| React | v16.13.1 |
| Browser | Chrome |
| TypeScript | v4.0.2 |

Autocomplete docs good first issue

All 8 comments

Thanks for the report. We currently close the whole Popper if the Autocomplete considers itself closed. You can reproduce the same issue with https://material-ui.com/components/autocomplete/#githubs-picker:

  1. Open label popper
  2. click on "apply labels to this pull request"
  3. popper closes (on github.com the popper does not close)

@eps1lon yup, that's basically the bug I'm experiencing, Autocomplete considers itself closed even though I click on a link above it.

Expected behaviour for Autocomplete should be to consider clicks outside the Popper as blur. Or perhaps leave the open/close to Popper.

@bexic You are observing the current behavior because this advanced demo was designed to have the Autocomplete component control the open/close state of the Popper (<Autocomplete onClose={} />). Nothing stops you from controlling the open/close state with a ClickAwayListener + Escape listener instead.

I'd argue that we should fix this in our demo. The demo we showcase works different than the actual GitHub implementation. It's a bit misleading to reproduce it visually ("This demo reproduces the GitHub's label picker:") but don't implement the basic behavior.

That's fair. GitHub doesn't close on blur and has an edit button at the bottom.

@bexic You should be able to reproduce the behavior of GitHub's label picker with this diff:

diff --git a/docs/src/pages/components/autocomplete/GitHubLabel.tsx b/docs/src/pages/components/autocomplete/GitHubLabel.tsx
index d3d5c72b02..2c0fab5758 100644
--- a/docs/src/pages/components/autocomplete/GitHubLabel.tsx
+++ b/docs/src/pages/components/autocomplete/GitHubLabel.tsx
@@ -8,6 +8,7 @@ import {
   createStyles,
 } from '@material-ui/core/styles';
 import Popper from '@material-ui/core/Popper';
+import ClickAwayListener from '@material-ui/core/ClickAwayListener';
 import SettingsIcon from '@material-ui/icons/Settings';
 import CloseIcon from '@material-ui/icons/Close';
 import DoneIcon from '@material-ui/icons/Done';
@@ -141,13 +142,7 @@ export default function GitHubLabel() {
     setAnchorEl(event.currentTarget);
   };

-  const handleClose = (
-    event: React.ChangeEvent<{}>,
-    reason: AutocompleteCloseReason,
-  ) => {
-    if (reason === 'toggleInput') {
-      return;
-    }
+  const handleClose = () => {
     setValue(pendingValue);
     if (anchorEl) {
       anchorEl.focus();
@@ -190,11 +185,22 @@ export default function GitHubLabel() {
         placement="bottom-start"
         className={classes.popper}
       >
-        <div className={classes.header}>Apply labels to this pull request</div>
+        <ClickAwayListener onClickAway={handleClose}>
+          <div>
+            <div className={classes.header}>
+              Apply labels to this pull request
+            </div>
             <Autocomplete
               open
-          onClose={handleClose}
               multiple
+              onClose={(
+                event: React.ChangeEvent<{}>,
+                reason: AutocompleteCloseReason,
+              ) => {
+                if (reason === 'escape') {
+                  handleClose();
+                }
+              }}
               classes={{
                 paper: classes.paper,
                 option: classes.option,
@@ -258,6 +264,8 @@ export default function GitHubLabel() {
                 />
               )}
             />
+          </div>
+        </ClickAwayListener>
       </Popper>
     </React.Fragment>
   );

What do you think about it? Do you want to work on a pull request? :)

can I work on this? :)

@hmaddisb Sure :)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ghost picture ghost  路  3Comments

revskill10 picture revskill10  路  3Comments

activatedgeek picture activatedgeek  路  3Comments

FranBran picture FranBran  路  3Comments

anthony-dandrea picture anthony-dandrea  路  3Comments