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

[material-ui][Autocomplete] broken when rendered inside a DataGrid row #43976

Closed
ddolcimascolo opened this issue Oct 3, 2024 · 19 comments · May be fixed by mui/mui-x#15128
Closed

[material-ui][Autocomplete] broken when rendered inside a DataGrid row #43976

ddolcimascolo opened this issue Oct 3, 2024 · 19 comments · May be fixed by mui/mui-x#15128
Assignees
Labels
component: autocomplete This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material regression A bug, but worse

Comments

@ddolcimascolo
Copy link

ddolcimascolo commented Oct 3, 2024

Steps to reproduce

Link to live example: https://codesandbox.io/p/sandbox/serene-parm-nqtpx5

Steps:

  1. Click the Edit icon on any row
  2. Try to select movies with the mouse, try to enter some text in the input. All fail

Current behavior

Autocomplete is unusable

Expected behavior

Autocomplete works as usual

Context

Nothing special, expect the kinda complex setup with one Dialog per DataGrid row

Your environment

npx @mui/envinfo
  System:
    OS: Linux 6.8 Linux Mint 21.3 (Virginia)
  Binaries:
    Node: 20.9.0 - ~/.nvm/versions/node/v20.9.0/bin/node
    npm: 10.1.0 - ~/.nvm/versions/node/v20.9.0/bin/npm
    pnpm: 8.12.1 - ~/.nvm/versions/node/v20.9.0/bin/pnpm
  Browsers:
    Chrome: 129.0.6668.89

Search keywords: autocomplete multiple broken datagrid

@ddolcimascolo ddolcimascolo added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Oct 3, 2024
@ddolcimascolo
Copy link
Author

I'd say it's a regression of #42494 but I'm not sure. This works fine in v6.1.1

@zannager zannager added the component: autocomplete This is the name of the generic UI component, not the React module! label Oct 3, 2024
@ddolcimascolo
Copy link
Author

Hi @zannager @mj12albert, did you get a chance to look into this regression?

For what it's worth, I'm a mui-x premium customer.

@ddolcimascolo
Copy link
Author

While waiting for a response, I worked around the issue by restoring pre #42494 behavior of using the "click" event instead of the "mousedown" event using a custom component wrapping TextField that you can use as a drop-in replacement in your Autocomplete renderInput function.

// https://github.com/mui/material-ui/issues/43976
// https://github.com/mui/material-ui/pull/42494
export default function NonBuggyTextField({ InputProps, inputProps, ...rest }) {
  function restorePre612Behavior(props) {
    return {
      ...props,
      onMouseDown: undefined,
      onClick: props.onMouseDown
    };
  }

  return (
    <TextField
      {...rest}
      slotProps={{
        input: restorePre612Behavior(InputProps),
        htmlInput: restorePre612Behavior(inputProps)
      }}
    />
  );
}

Hope this helps someone. And please team can we get a word on this?

Cheers,
David

@DiegoAndai DiegoAndai assigned DiegoAndai and unassigned mj12albert Oct 10, 2024
@DiegoAndai DiegoAndai added regression A bug, but worse package: material-ui Specific to @mui/material and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Oct 10, 2024
@DiegoAndai DiegoAndai changed the title [v6.1.2] Autocomplete broken when rendered inside a DataGrid row [material-ui][Autocomplete] broken when rendered inside a DataGrid row Oct 10, 2024
@DiegoAndai
Copy link
Member

Hey @ddolcimascolo, thanks for the report and for pinging me. I'm sorry for the delay. I'm looking into this.

@ddolcimascolo
Copy link
Author

Thank you very much. Anything you need from me just ask.

@DiegoAndai
Copy link
Member

@ZeeshanTamboli if you have some time may I ask you to take a look at this one?

I've been debugging, and so far I'm following this lead:

  • Click the autocomplete so listbox opens
  • Input 'a' on the keyboard
  • Listbox closes

Video:

Screen.Recording.2024-10-10.at.17.24.46.mov

This happens as the useAutocomplete's onBlur callback triggers, something else is grabbing focus and I'm not sure what yet. I'll continue debugging tomorrow.

@DiegoAndai
Copy link
Member

cc: @cherniavskii in case you have any suggestions on why this might be happening or where to look for debugging.

@DiegoAndai
Copy link
Member

I also thought it might have been the use of hooks inside renderCell, but wrapping them in a component doesn't solve the issue: https://codesandbox.io/p/sandbox/blazing-river-hhr38h?file=%2Fsrc%2FDemo.js%3A34%2C13

@ddolcimascolo
Copy link
Author

Thanks a lot for investigating.
Considering this was introduced in a patch release, don't you think it's worth reverting, adding a test for this use case and work on a better fix for #42494 later?

Cheers,
David

@cherniavskii
Copy link
Member

@DiegoAndai The DataGrid focus management logic kicks in and moves focus back to the cell, this is why Autocomplete doesn't react to keyboard or mouse events.
I would suggest the following options:

  1. Use Data Grid Editing feature – this means splitting the component from renderCell into two: view (renderCell) and edit (renderEditCell).
  2. Lift the Dialog with Autocomplete up, so it's not rendered inside of the cell.

@ddolcimascolo
Copy link
Author

@cherniavskii

Thx for the suggestions, I already know I can refactor these components. But you obviously understood that I provided a very minimal reproduction (that took time to provide) and that the actual components in the application for which I use and pay for mui-x are far much complex, and there are many of them as this paradigm of editing in a Dialog is quite common...

This was working in the previous patch release, this has to be considered as a regression.

I'm totally fine with refactoring my code if any breaking change in any major version happen, I've been doing this for a long time now. But that's not OK in a patch release.

I really love mui and mui-x and I'm willing to keep on supporting the development of both libraries, please consider reverting the change in #42494.

Thanks in advance,
Regards,

David

@cherniavskii
Copy link
Member

@ddolcimascolo I should've clarified in my previous comment that reverting the Autocomplete behavior change is still an option.

For what it's worth, I'm open to changes on the Data Grid side if necessary.

@DiegoAndai
Copy link
Member

DiegoAndai commented Oct 11, 2024

Reverting #42494 is the best-looking option at the moment. #42494 introduced other bugs besides this one. There's already a PR for this: #43982, I'm working on it. The reason I haven't merged it yet is because:

  • I still haven't figured out what's happening in #42432. #43982 fixes it, but I have not been able to implement a test for it. The actual one isn't covering it properly.
  • I haven't found a way to add a test for this use case. @cherniavskii do you have any ideas on how to create a minimal reproduction without using the DataGrid that we could use in a test for it?

I'll come back to it next week. Thanks for your patience.

cc: @ZeeshanTamboli

@ddolcimascolo
Copy link
Author

Thanks a lot to both of you guys for your work on this issue, and for the status update.

@cherniavskii
Copy link
Member

@DiegoAndai I think I can set up a test for it, let me take a look.

@ddolcimascolo
Copy link
Author

Hi team,

Did you get a chance to complete the patch?

Thx,
David

@DiegoAndai
Copy link
Member

@ddolcimascolo it's on review (#43982), hopefully it will be out with next week's release.

@DiegoAndai
Copy link
Member

#43982 has been merged, and this should be fixed with it. The fix will be included in next week's release.

@ddolcimascolo, thank you for your patience. If you have any issues next week, please reach out to @aarongarciah. I'll be out for a couple of weeks, so if you tag me, I won't be able to respond.

Copy link

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

Note

@ddolcimascolo How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

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! package: material-ui Specific to @mui/material regression A bug, but worse
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants