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

[popups] Add onClosed callback #1235

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

[popups] Add onClosed callback #1235

wants to merge 3 commits into from

Conversation

atomiks
Copy link
Contributor

@atomiks atomiks commented Dec 26, 2024

Closes #1208

@atomiks atomiks added component: select This is the name of the generic UI component, not the React module! component: menu This is the name of the generic UI component, not the React module! component: alert dialog This is the name of the generic UI component, not the React module! component: tooltip This is the name of the generic UI component, not the React module! component: dialog This is the name of the generic UI component, not the React module! component: popover The React component. component: preview card The React component. labels Dec 26, 2024
@mui-bot
Copy link

mui-bot commented Dec 26, 2024 •
edited
Loading

Netlify deploy preview

https://deploy-preview-1235--base-ui.netlify.app/

Generated by 🚫 dangerJS against b4d9197

@atomiks atomiks marked this pull request as ready for review December 26, 2024 11:37
@atomiks
Copy link
Contributor Author

atomiks commented Dec 27, 2024

Naming:

  • onHidden may be confusing since it doesn't relate to the open/closed concepts in the API. You can also hide the popup with data-anchor-hidden even though it's still open.
  • onClosed may be confusing since [data-closed] is applied as soon as open: false. Maybe onCloseComplete/onClosedComplete?
  • onOpenChangeComplete (as mentioned in the issue) is needlessly symmetric since you only need to care about open: false completing, not open: true (presumably? You can at least replicate it externally semi-easily)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@colmtuite colmtuite Awaiting requested review from colmtuite colmtuite is a code owner

@michaldudak michaldudak Awaiting requested review from michaldudak michaldudak is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
component: alert dialog This is the name of the generic UI component, not the React module! component: dialog This is the name of the generic UI component, not the React module! component: menu This is the name of the generic UI component, not the React module! component: popover The React component. component: preview card The React component. component: select This is the name of the generic UI component, not the React module! component: tooltip This is the name of the generic UI component, not the React module!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Dialog, Popover] Provide a callback for open/close completion
2 participants
@atomiks @mui-bot