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

Allow async onRowsDelete #835

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pkmnct
Copy link
Contributor

@pkmnct pkmnct commented Aug 14, 2019

No description provided.

@gabrielliwerant gabrielliwerant added the needs review Useful to mark PRs for what's up next to review label Aug 15, 2019
@gabrielliwerant
Copy link
Collaborator

Hey @pkmnct, trying to get a handle on the exact use case here, as this was a spinoff from an issue that was asking specifically about a confirmation dialog. In my experimentation, there is no issue with using a confirmation dialog because it takes focus and prevents other code execution until the dialog is dealt with, allowing true/false to be returned from the callback.

Were you trying to do something other than use a confirmation dialog?

Additionally, I think this needs some babel plugin and configuration love, as I couldn't run any examples using onRowsDelete otherwise. I've done the work to set it up, so I can chip in on this if you like.

@gabrielliwerant gabrielliwerant added needs work and removed needs review Useful to mark PRs for what's up next to review labels Aug 16, 2019
@pkmnct
Copy link
Contributor Author

pkmnct commented Aug 16, 2019

@gabrielliwerant We were wanting to use a Material UI Dialog component, rather than a window.confirm dialog.

Here's an Code Sandbox: https://codesandbox.io/s/material-ui-datatables-async-onrowsdelete-example-wktpj

@gabrielliwerant
Copy link
Collaborator

@pkmnct Gotcha, that makes sense.

Any thoughts on how you want to handle the babel configs to handle async/await?

@pkmnct
Copy link
Contributor Author

pkmnct commented Aug 16, 2019

@gabrielliwerant I'm not having any issues using this PR branch locally testing, what error/warning/issue are you running into with the async/await and babel?

@gabrielliwerant
Copy link
Collaborator

gabrielliwerant commented Aug 16, 2019

Webpack can't run the sections with async because it's missing some plugins like babel-plugin-syntax-async-functions and babel-polyfill. It's possible that you have extra packages installed locally or browser updates which are helping. My research indicates that those are needed, and this does seem to fall in line with the fact that there have been no other uses of async in the library until now.

In particular, I get a regenerator runtime error (which is connected to async if I understand correctly), when I try to load the selectable-rows example in Chrome (v76).

If you can't reproduce this issue, it might make sense for me to create a branch based on this one that fixes it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants