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.

Sign up for GitHub

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

Jump to bottom

[material-ui][Dialog] Prevent onClick on the root element from being overwritten #41881

Merged
merged 6 commits into from
Apr 15, 2024
Merged

[material-ui][Dialog] Prevent onClick on the root element from being overwritten #41881

merged 6 commits into from
Apr 15, 2024

Conversation

ryanburr
Copy link
Contributor

This PR fixes #41417

  • Prevents the onClick prop from being overwritten by destructing and invoking on local handler.

oliviertassinari
oliviertassinari reviewed Apr 13, 2024
View reviewed changes
Copy link
Member

@oliviertassinari oliviertassinari left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, surprised we still have bugs of this class after so many years of battle testing.

packages/mui-material/src/Dialog/Dialog.d.ts Outdated Show resolved Hide resolved
@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: dialog This is the name of the generic UI component, not the React module! labels Apr 13, 2024
ryanburr and others added 2 commits April 13, 2024 18:52
@ryanburr @oliviertassinari
chore(dialog): ignore onClick comments
c02e2c9 
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Signed-off-by: Ryan Burr <ryan.burr2@gmail.com>
@ryanburr
docs(dialog): generate proper docs
b47aa61
ZeeshanTamboli
ZeeshanTamboli reviewed Apr 14, 2024
View reviewed changes
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Left one comment.

packages/mui-material/src/Dialog/Dialog.d.ts Outdated Show resolved Hide resolved
@ZeeshanTamboli ZeeshanTamboli added the package: material-ui Specific to @mui/material label Apr 14, 2024
@mui-bot
Copy link

mui-bot commented Apr 15, 2024 •
edited
Loading

Netlify deploy preview

https://deploy-preview-41881--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 071a543

@ZeeshanTamboli ZeeshanTamboli changed the title [material-ui][Dialog] Prevent onClick from being overwritten [material-ui][Dialog] Prevent onClick on the root element from being overwritten Apr 15, 2024
ZeeshanTamboli
ZeeshanTamboli approved these changes Apr 15, 2024
View reviewed changes
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pull request!

@ZeeshanTamboli ZeeshanTamboli added the needs cherry-pick The PR should be cherry-picked to master after merge label Apr 15, 2024
@ZeeshanTamboli ZeeshanTamboli merged commit 418fb02 into mui:next Apr 15, 2024
20 checks passed
github-actions bot pushed a commit that referenced this pull request Apr 15, 2024
@ryanburr @ZeeshanTamboli @web-flow
[material-ui][Dialog] Prevent onClick on the root element from being …
eb31ac2 
…overwritten (#41881)

Signed-off-by: Ryan Burr <ryan.burr2@gmail.com>
Co-authored-by: ZeeshanTamboli <zeeshan.tamboli@gmail.com>
@github-actions github-actions bot mentioned this pull request Apr 15, 2024
Merged
@ryanburr
Copy link
Contributor Author

@ZeeshanTamboli thanks for the assist!

@ryanburr ryanburr deleted the ryanburr/fix/dialog-onclick-backdrop branch April 16, 2024 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZeeshanTamboli ZeeshanTamboli ZeeshanTamboli approved these changes

@oliviertassinari oliviertassinari Awaiting requested review from oliviertassinari

Assignees
No one assigned
Labels
bug 🐛 Something doesn't work component: dialog This is the name of the generic UI component, not the React module! needs cherry-pick The PR should be cherry-picked to master after merge package: material-ui Specific to @mui/material
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[material-ui][Dialog] Specifying onClick prop disables backdrop clicks
4 participants
@ryanburr @mui-bot @oliviertassinari @ZeeshanTamboli