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.15k stars 1.3k forks source link

[DateRangePicker] Strange focus behavior with a selected value when switching months #11756

Open ahellam opened 9 months ago

ahellam commented 9 months ago

Summary

Goal: to make something similar to the week picker but my selection may overflow into the next month so seeing two calendars would be helpful.

Request: Add a calendars prop similar to the range to choose how many calendars are visible.

Examples

No response

Motivation

Our customers have different kinds of pay periods which might be weekly, every two weeks etc and these can start on any day of the week. This means the pay period might overflow into the next month and it would be nice to see the whole pay period highlighted.

Search keywords: DateCalendar calendars prop Order ID: 78587

LukasTy commented 9 months ago

Hello @ahellam, have you tried using DateRangeCalendar to achieve the behavior you need? 🤔 This is a rough idea of how it could behave. Let me know if you see such an option as a viable alternative. 🙏

ahellam commented 9 months ago

Thanks @LukasTy, The example you provided is pretty close, I would just need to set up preview behavior on hover similar to the weekpicker example in your docs. Thanks for the response!

ahellam commented 8 months ago

@LukasTy, I seem to be running into a little bit of a strange focus effect which happens in your sandbox example as well. When the calendars automatically scroll to the previous/next month when the range spans across a month, it will apply focus to a date that is outside of the range that I am not hovering over. Any advice/thoughts on this?

https://i.gyazo.com/5f62a7934481242125daa9fb9203ad80.gif

edit: looks like it happens on month change even if a new selection isn't made https://i.gyazo.com/525886de49ffee845a109a7ca144097b.gif

any way to disable this if a selection exists?

LukasTy commented 8 months ago

@ahellam Thank you for reporting this. I can confirm that this behavior seems weird. I'd reckon that it would make the most sense if the focus would preferably jump to the selected value if it exists in the given month. We'll investigate it. 👌

kealjones-wk commented 5 months ago

Pretty sure that I accidentally introduced this issue when i attempted to fix the autofocus behavior back in this pr: https://github.com/mui/mui-x/pull/11273

Funnily enough the issue outlined here in 11756 is actually more annoying that what i "fixed" can we just revet that old PR of mine? @LukasTy

Or at the very least check props.autoFocus first?

LukasTy commented 5 months ago

@kealjones-wk is there a reason why you suspect that it was your change? 🤔 The problem is identical on v6 and v7, but the autoFocus prop rule is different on v7. As far as I can see the problem is not tied to the autoFocus prop being present. 🤷

kealjones-wk commented 5 months ago

Not sure what you mean by the autoFocus prop is different between the two versions...

they look basically like the same logic to me... (left is master, right is 6.19.12) Screenshot 2024-05-22 at 4 44 10 PM

ok after some testing I see what you mean it seems like PickersDay's autoFocus which is set by that component is the one at fault. ~Maybe its WrappedDay's onFocus prop that is set to handleFocus in x-date-pickers/src/DateCalendar/DayCalendar.tsx~ sorry im tired idk what im talking about ill investigate more tomorrow but this is super annoying lol

LukasTy commented 5 months ago

After more exploration, I think that the problem is relevant to the overall calendar state (not only DateRangeCalendar). The DateCalendar has a similar issue:

In this case, IMHO, the focus should have the following priority. Move focus to:

  1. selected day if it exists;
  2. today if no date is selected;
  3. the current closestEnabledDate logic that is now present.
kealjones-wk commented 5 months ago

@LukasTy I dont think i was able to reproduce the particular issue you lay out in your last comment... I do like your focus priority but believe recently an issue was resolved about the reference date getting the focus so maybe something like:

Assuming that props.autoFocus is set to true on the calendar:

  1. selected day if one exists;
  2. reference date if it is set;
  3. today if it is in the current month;
  4. the current closestEnabledDate logic that is now present.

Here is a little video of the issue I am currently facing:

https://github.com/mui/mui-x/assets/41018730/b25db1d4-01f0-4526-9b0e-a73d315be5ea

In the video you can see that using the keyboard I focus the previous month navigation button and hit Enter, only to then have the next month steal the focus from the navigation button. Imagine a keyboard user is trying to quickly run through months this would be incredibly annoying to have that nav button constantly lose focus.

I don't know exactly how to handle this as i do like the focus behavior for first displayed but think that navigating through that months shouldn't cause the focus to change like that.

For some context: I am wrapping DateRangePicker to create our libraries own custom DateRangePicker that works more similarly to DatePickers (has toggle buttons and ability to focus the calendar via keyboard and such)