[popups] Add onCloseComplete callback

[atomiks]

Closes #1208

[mui-bot]

Netlify deploy preview

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

Generated by 🚫 dangerJS against 8460035

[atomiks]

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)

[michaldudak]

onCloseComplete looks less confusing that onClosed IMO.

[michaldudak]

Suggested change

	let onClosedCalled = false;
	function notifyOnClosed() {
	onClosedCalled = true;
	}
	const notifyOnClosed = spy();

[michaldudak]

Sinon's spy can be used instead (it can also verify if the function was called exactly once)

[atomiks]

This technique was already used for the existing animation tests, it'd probably be worth updating them all simultaneously in a separate PR

[vladmoroz]

A bit late to barge in

• 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)

• You may want to start an in-content video or animation when the popup or panel open animation is complete
• Being closer to onOpenChange makes for a simpler, easier to remember API

[atomiks]

@vladmoroz I can see those points being valid, but reacting to close completing (when unmounting) requires this prop to exist, since you can't use onTransitionEnd/onAnimationEnd as they fail to get called in time. This doesn't happen for open: true. Is it too difficult to use those events externally? We'd have to use those props at the very least internally, adding extra bundle cost for a possibly rare use case

[vladmoroz]

I just think it's an odd API when one case is provided via a prop and a symmetrical one is fully DIY. Seems arbitrarily inflexible.

I agree that it's a less common use case

[atomiks]

Alright, let me try to make a new PR that reuses the existing node.getAnimations() util, but supports the open use case as well and rename to onOpenChangeComplete