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/
4.52k stars 1.31k forks source link

[pickers] Picker doesn't open when clicking on label position (& mouse up vs. mouse down) #9386

Closed cherniavskii closed 1 year ago

cherniavskii commented 1 year ago

Duplicates

Latest version

Steps to reproduce πŸ•Ή

Link to live example: https://codesandbox.io/s/sweet-voice-6g6cyk?file=/demo.tsx

Steps:

  1. Click on the text field in the area between the placeholder and the top border

Current behavior 😯

Depending on the click position, the picker might not open:

https://github.com/mui/mui-x/assets/13808724/d1af8dad-8e61-445c-8694-03b9e3af93db

Expected behavior πŸ€”

The picker should open regardless of the click position

Context πŸ”¦

I think it only happens when clicking over the hidden label:

Interestingly, I can reproduce the issue in Chrome and Safari, but not in Firefox.


Also, I checked if the Autocomplete component suffers from the same issue, but it doesn't: https://mui.com/material-ui/react-autocomplete/#combo-box

Your environment 🌎

npx @mui/envinfo ``` Don't forget to mention which browser you used. Output from `npx @mui/envinfo` goes here. ```

Order ID or Support key πŸ’³ (optional)

No response

LukasTy commented 1 year ago

Thank you for opening this issue! πŸ™ It looks like a regression as compared to v5. From the initial exploration, it looks like we are not providing props the the TextField components in the most correct manner after the refactoring. πŸ™ˆ

LukasTy commented 1 year ago

It looks like we have the same regression for the regular pickers as well. πŸ™ˆ

P.S. The regression practically negates the following fix: #6074

LukasTy commented 1 year ago

Well, this has been an adventure... πŸ™ˆ I've finally been able to narrow down the cause of regression: https://github.com/mui/material-ui/pull/36892. After this change, the onClick passed to the TextField component no longer gets applied to the root element, but only the input element thus negating our solution of passing the onClick callback to the root element to trigger opening of the picker even when clicked on the invisible future label area. πŸ€”

cherniavskii commented 1 year ago

Interesting, I'm curious why Autocomplete is not affected?

LukasTy commented 1 year ago

Because they are using onMouseDown instead of onClick. πŸ™ˆ We've gone with the same approach initially, until it proved problematic with the TimeClock.

oliviertassinari commented 1 year ago

I think the behavior in https://mui.com/x/react-date-pickers/date-range-picker/#basic-usage is not optimal, if the popup opens on focus, I think it should be on mouseDown, not mouseUp to feel responsive. But it's controversial, not all UI will agree, e.g. Google search doesn't agree with this (they say it should be mouseUp). Maybe it's because for big popups, you can say, better add ~30ms of latency (mouseDown -> mouseUp), to allow users to change their mind (doing the mouseUp on a different element).

LukasTy commented 1 year ago

if the popup opens on focus, I think it should be on mouseDown, not mouseUp to feel responsive.

Am I missing something, but where did you see the popup opening on focus? πŸ€”

Maybe it's because for big popups, you can say, better add ~30ms of latency (mouseDown -> mouseUp), to allow users to change their mind (doing the mouseUp on a different element).

Yes, I would also be in the "same mind camp" as you mentioned, to me, executing logic on mouseDown feels a bit hacky, especially considering bigger actions. onClick also behaves like this, it will not be triggered until the mouseUp happens. πŸ€”

oliviertassinari commented 1 year ago

@LukasTy Note that a <select> opens on mousedown on macOS and Windows. I replicated this in Material UI https://mui.com/material-ui/react-select/, and it looks like @michaldudak replicated this on Base UI https://mui.com/base-ui/react-select/, which I think is great for small widgets like this (we kept touchEnd though, not touchStart).

For a menu, I'm not sure, Radix made it trigger on mouse down & touch start https://www.radix-ui.com/primitives/docs/components/dropdown-menu, Vercel mouse down and touch end https://vercel.com/design/menu, but on a calendar they made it mouse up https://vercel.com/design/calendar.

I guess it's really a tradeoff between moving fast for end-users, and aborting click mistakes.

LukasTy commented 1 year ago

@oliviertassinari IMHO, those who want speedy behavior (form filling) would tend to favor keyboard interactions most of the time. In such a case, the only thing that matters is the good field experience that we offer.

I've checked the native behavior of <input type="date" /> on Chrome, Safari, and Firefox and all seem to open the popup only on mouseUp. There are some resources discussing the differences and which event should be used when: source 1, source 2. There are benefits to going with mouseDown, but it has its own implications as well. πŸ€” IMHO, I'd say that ideally, it would be best to align the approach amongst all the library components. What do you think?

oliviertassinari commented 1 year ago

would tend to favor keyboard interactions most of the time.

@LukasTy It depends on what you have in mind by "most", in my mind, it would be like 60% of the time keyboard, 40% pointer to get the fastest to my goal.

I've checked the native behavior of on Chrome, Safari, and Firefox and all seem to open the popup only on mouseUp.

I think for me, here, the main difference is that on the native pickers, it's the click on a button that triggers the popup, while on our date range picker, it feels like it's the focus on the input that opens is (focus is mouse down). There is a bit of a surprise, after mouse down, and you see the input focused, you might expect to type text, but on the mouse up, something else pops.

IMHO, I'd say that ideally, it would be best to align the approach amongst all the library components.

I'm not sure we can, sometimes, end-users need the action to be cancellable, e.g. a form button submit click, but some other times, it makes the UI feel less responsive, e.g. input focus.


I would personally vote to follow https://react-select.com/ rather than https://reactdatepicker.com/, but it might be a nonsignificant detail, so we can stay on the status quo, wait to get more data.