mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
3.91k stars 1.19k forks source link

[pickers] Popper touch the edge #10009

Open oliviertassinari opened 10 months ago

oliviertassinari commented 10 months ago

Steps to reproduce 🕹

Link to live example:

Steps:

  1. Open https://codesandbox.io/s/falling-bush-pdrn2j?file=/demo.tsx

Current behavior 😯

The PickersPopper open on the very edge.

Expected behavior 🤔

Leave a space with the edge

Context 🔦

I first observed this at https://flights.teamout.com/

Screenshot 2023-06-17 at 21 00 40

would it feel much better with some padding?

Screenshot 2023-08-11 at 22 48 25

cc @lildesert

Your environment 🌎

v6.11.1

Order ID or Support key 💳 (optional)

No response

oliviertassinari commented 10 months ago

I think we depends on a fix in Base UI: https://github.com/mui/base-ui/issues/39.

LukasTy commented 10 months ago

I've looked into this case and it seems that having a popper modifier is quite a tricky way to achieve such behavior. What do you think about simply adding a horizontal margin to the popper Paper instead? 🤔 Could be tested by adding the follow snippet to your demo.

slotProps={{
  desktopPaper: {
    sx: {
      mx: 1
    }
  }
}}
oliviertassinari commented 10 months ago

What do you think about simply adding a horizontal margin to the popper Paper instead? 🤔

This also shifts the anchor point no? I tried this with the Material UI Tooltip https://github.com/mui/base-ui/issues/413, doesn't work.

LukasTy commented 10 months ago

This also shifts the anchor point no?

Doesn't seem to shift it in the pickers case. 🤔

https://github.com/mui/mui-x/assets/4941090/96ef8bc0-818b-496c-ab93-53c86f8ae5f7

oliviertassinari commented 10 months ago

You would need to test with a placement like bottom-start, it won't be seen when centered.

For example, here we see it's 2px off https://mui.com/material-ui/react-tooltip/#positioned-tooltips

Screenshot 2023-08-14 at 14 38 25
LukasTy commented 10 months ago

Thanks for pointing it out. Indeed, it's not a problem only when center placement is used. In any case, wouldn't we expect that if this is implemented via @mui/base then it would have our preferred default (probably 8px) and would implicitly resolve this issue at the same time? 🤔

oliviertassinari commented 9 months ago

In any case, wouldn't we expect that if this is implemented via @mui/base then it would have our preferred default (probably 8px) and would implicitly resolve this issue at the same time? 🤔

@LukasTy Oh, yeah, we could do this. It would fix this issue at the same time. But it depends on what default value we go with. If it's 4px to cover the tooltip use case, it won't be enough.