[Popper] Support marginThreshold prop like in Popover

[oliviertassinari]

Summary 💡

The Popper component is missing the equivalent of the marginThreshold prop of the Popover.

As we are trying to revamp the Popover component, I think we must port this feature back. We have seen in #413 and mui/mui-x#10009 that this prop matters, we can't get away without it.

Examples 🌈

• Radix UI solves this with a custom collisionPadding prop on the Popper.

  • demo: https://codesandbox.io/p/sandbox/zen-fast-kdcg9t?file=%2FApp.jsx%3A17%2C29
  • docs: https://www.radix-ui.com/docs/primitives/components/popover
  • source: https://github.com/radix-ui/primitives/blob/eca6babd188df465f64f23f3584738b85dba610e/packages/react/popper/src/Popper.tsx#L169

Motivation 🔦

Great UX.

This also connects a bit with the effort to modernize the Popper component of mui/material-ui#32587 (comment).

cc @michaldudak

[michaldudak]

we are trying to deprecate & remove the Popover component

Are we? Is there an issue about this? I thought the Popover was intended to be a high-level building block for popups, dialogs, select listboxes, etc.

[oliviertassinari]

@michaldudak I have updated the description, great point.

I'm not sure about "dialogs" though. I don't see how it would fit. The problems we need to consider:

• Positioning: Solved with Popup/Popper
• Animation: Solved with animation components
• Focus: Solved with TrapFocus
• Modal vs. non-modal: Solved with Modal, with Portal built in.

I would imagine that the value of Popover is to remove the duplication for very frequent combination of the above. Dialog feels hyper specialized, it could directly compose these 4 components. What I see frequently needed is a slick way to display small element elements on the page, typically, it's always the same animation between Context Menu, Menu, Select, Dropdown, Combobox, Date Picker, Color Pickers, Data Grid pannel.

Now, I think Popover source should be very simple, so developers can easily eject and built their own.

[oliviertassinari]

mui/mui-x#10009 (comment) raises the question on what the default value for this prop should be.

[michaldudak]

The new Popover (and other popover-like components) supports the collisionPadding prop that defines a distance from the popover to its collision boundary.

https://master--base-ui.netlify.app/components/react-popover/

[oliviertassinari]

Nice 👍, 5px might be enough, we will see. I guess we don't need a demo for this.

For the prop name, "padding" feels strange, it's more of a "margin"?

[michaldudak]

Depends how you look at it, I guess - it's a padding of a collision box. Maybe margin would be a bit more intuitive though, it's hard to say.
cc @colmtuite

[colmtuite]

@michaldudak We should stick with collisionPadding for easier migration from Radix. I think the default should be 0, though it's not really important. Radix also supports an object for setting independent values for each side, that's something to consider.

[oliviertassinari]

We should stick with collisionPadding for easier migration from Radix

@colmtuite Are people frequently using this prop? If not, we could argue that it's an opportunity to see what the optimal name could be.

We could also have a deprecated prop with the Radix prop name (actually true for everything else) so it's super easy to migrate. Maybe to consider? Actually, could do the same with Material UI prop names. Idea moved to #601.

I think the default should be 0, though it's not really important.

The initial proposal for not using 0 is that it seems to create confusing UIs:

• [tooltip] Tooltip touch the edge #413
• [pickers] Popper touch the edge mui-x#10009

I'm not sure about the use cases for being right on the edges of the screen?