mantinedev / mantine

A fully featured React components library
https://mantine.dev
MIT License
26.79k stars 1.9k forks source link

Popover `onOpen` does not work with controlled state. #6981

Closed crabvk closed 1 month ago

crabvk commented 1 month ago

Dependencies check up

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

truckcarr11 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

Fixed in 7.13.3