[popups] Require Portal part

[atomiks]

Closes #1215

• Each component has its own separate Portal component, rather than a shared one now, as the context cannot be shared.
• Removed keepMounted prop on Positioner and Popup since it's specified by Portal in all scenarios.

[mui-bot]

Netlify deploy preview

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

Generated by 🚫 dangerJS against 2d41b5e

[michaldudak]

What makes the context non-shareable between different components' portals?

[atomiks]

What makes the context non-shareable between different components' portals?

We need to determine if the Portal part is missing on the correct component, with the right error message. One exception is you can still nest Dialog/Popover and the detection won't work however since it's able to detect the parent one exists. In this case, some kind of id to link them would be needed but not sure if that's worth the trouble

Another thing is I plan to make a new Lite portal component for those that don't need focus management (Tooltip, Preview Card). So it also can't be shared in that case - it just makes more sense to split them at this point

[michaldudak]

It feels like a step backward in terms of bundle size. #1248 tries to save a couple of bytes by removing a few strings but here we're adding a couple of components that are pretty much the same.

How about we ensure that each PortalContext is consumed instead? In dev mode only, the Portal can set a flag in the nearest context marking it as read. If a context isn't read after the effect phase, a Portal must be missing.

[atomiks]

It feels like a step backward in terms of bundle size

The PortalContext and then the effect suggestion is basically going to match split components - especially with the new Lite one for Tooltip and PreviewCard. Select already also needs to perform splitting. At that point, the difference is negligible and these duplicated components are small enough for it not to be a concern

How about we ensure that each PortalContext is consumed instead? In dev mode only, the Portal can set a flag in the nearest context marking it as read. If a context isn't read after the effect phase, a Portal must be missing.

Throwing an error in render ensures the whole app doesn't render and lets them know the tree is invalid in a very obvious way, which won't occur with the effect technique (will only appear as an error in the console, but otherwise the app runs fine)

[atomiks]

I found the main cause of why omitting nested popup/portal combinations doesn't properly work in Floating UI. I can also explore fixing that as an alternative to this PR, but the keepMounted change being limited to just the Portal part simplifies the API nicely imo

[michaldudak]

but the keepMounted change being limited to just the Portal part simplifies the API nicely imo

Yeah, having keepMounted on just one part is a nice improvement. It does feel, however, that portalling and keeping children mounted are separate concerns. If we made the Portal optional, we could move this prop to the Root part.

[atomiks]

I think it's a good idea to still require the Portal part. If they need to retain the popup's location in the DOM tree, they can still specify the container prop to be a ref where it's located.

There are unresolved bugs like floating-ui/floating-ui#3060. In this case, a parent Popover that's not portaled with a portaled Popover child will close unexpectedly.

While FloatingTree is a solution, it's hard to generalize it across different popup components, as it works better for things like Menu where there's clear ownership between the root and submenus (or manually added by the user themselves)