Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ahellam opened this issue Jan 19, 2024 · 11 comments
Labels
bug 🐛 Something doesn't work component: DateRangePicker The React component. component: pickers This is the name of the generic UI component, not the React module! plan: Pro Impact at least one Pro user support: pro standard Support request from a Pro standard plan user. https://mui.com/legal/technical-support-sla/

Comments

@ahellam
Copy link

ahellam commented Jan 19, 2024

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

@ahellam ahellam added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 19, 2024
@zannager zannager added component: pickers This is the name of the generic UI component, not the React module! support: pro standard Support request from a Pro standard plan user. https://mui.com/legal/technical-support-sla/ labels Jan 19, 2024
@LukasTy
Copy link
Member

LukasTy commented Jan 22, 2024

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. 🙏

@LukasTy LukasTy added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jan 22, 2024
@LukasTy LukasTy changed the title DateCalendar calendars prop [pickers] DateCalendar calendars prop Jan 22, 2024
@ahellam
Copy link
Author

ahellam commented Jan 22, 2024

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!

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Jan 22, 2024
@michelengelen michelengelen removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 23, 2024
@ahellam
Copy link
Author

ahellam commented Jan 31, 2024

@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
Copy link
Member

LukasTy commented Feb 2, 2024

@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. 👌

@LukasTy LukasTy added plan: Pro Impact at least one Pro user component: DateRangePicker The React component. labels Feb 6, 2024
@LukasTy LukasTy changed the title [pickers] DateCalendar calendars prop [DateRangePicker] Strange focus behavior with a selected value when switching months Feb 6, 2024
@LukasTy LukasTy added the bug 🐛 Something doesn't work label Feb 20, 2024
@kealjones-wk
Copy link
Contributor

kealjones-wk commented May 21, 2024

Pretty sure that I accidentally introduced this issue when i attempted to fix the autofocus behavior back in this pr:
#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
Copy link
Member

LukasTy commented May 22, 2024

@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
Copy link
Contributor

kealjones-wk commented May 22, 2024

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
Copy link
Member

LukasTy commented May 23, 2024

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

  • select a date;
  • change month;
  • get back to the month where the date is selected;
  • move focus to the calendar;
  • notice the focus on the first or last enabled date (depending on month switching navigation).

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
Copy link
Contributor

@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:

focus-date-range-calendar.mp4

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)

@larsrijnen-whyellow
Copy link

@kealjones-wk and @LukasTy,

I'm reaching out to inquire about any recent updates or progress regarding the previously mentioned visual bug. Our Quality Assurance has identified and flagged this issue, but we found that is was still an open issue for mui.
Any further information or guidance would be greatly appreciated.

@LukasTy
Copy link
Member

LukasTy commented Jan 15, 2025

Thank you for reaching out, @larsrijnen-whyellow. 🙏
We will be looking into these issues and hopefully solve it for the v8 stable release.
The extra incentive is that we will change the behavior of range Pickers from tooltip to dialog (#14786), and issues like these will immediately become more important. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: DateRangePicker The React component. component: pickers This is the name of the generic UI component, not the React module! plan: Pro Impact at least one Pro user support: pro standard Support request from a Pro standard plan user. https://mui.com/legal/technical-support-sla/
Projects
None yet
Development

No branches or pull requests

6 participants