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

Popover onOpen does not work with controlled state. #6981

Closed
1 of 2 tasks
crabvk opened this issue Oct 15, 2024 · 4 comments
Closed
1 of 2 tasks

Popover onOpen does not work with controlled state. #6981

crabvk opened this issue Oct 15, 2024 · 4 comments
Labels
Fixed patch Completed issues that will be published with next patch (1.0.X)

Comments

@crabvk
Copy link

crabvk commented Oct 15, 2024

Dependencies check up

  • I have verified that I use latest version of all @mantine/* packages

What version of @mantine/* packages do you have in package.json?

7.13.2

What package has an issue?

@mantine/core

What framework do you use?

Vite

In which browsers you can reproduce the issue?

All

Describe the bug

Using controlled state with useDisclosure and onOpen callback of Popover component.
The callback isn't being called.

If possible, include a link to a codesandbox with a minimal reproduction

https://codesandbox.io/p/sandbox/dh3ddv

Possible fix

No response

Self-service

  • I would be willing to implement a fix for this issue
@truckcarr11
Copy link

Diving into the source code, it looks like this is intentional. Can see that here

Only when the popover is un-controlled does it call the hooks for onOpen and onClose.

If I had to guess, it is because you can react to changes using useEffect. You can see an example of that here

@crabvk
Copy link
Author

crabvk commented Oct 16, 2024

The commit message says:

Popover: Fix onClose function being called twice with controlled state

so, seems like onClose called somewhere else, but onOpen doesn't.
Looks strange 🤔

@truckcarr11
Copy link

Good catch on the commit message, I missed that. So yeah it seems it used to (be called even when controlled), but it was doing it twice and there was an attempt to fix it.

I modified your example to add the onClose as well and that doesn't seem to trigger either.

@rtivital rtivital added the Fixed patch Completed issues that will be published with next patch (1.0.X) label Oct 16, 2024
@rtivital
Copy link
Member

Fixed in 7.13.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed patch Completed issues that will be published with next patch (1.0.X)
Projects
None yet
Development

No branches or pull requests

3 participants