mui-x icon indicating copy to clipboard operation
mui-x copied to clipboard

[pickers] Allows arrow navigation without needing `autofocus`

Open alexfauquette opened this issue 2 years ago • 2 comments

Fix #4900

The focused date is managed by the calendarState which seems to work quite well. The problem is that we need both:

  • static pickers with autofocus=false do not take the focus
  • static pickers with autofocus=false put the focus on the correct day when using keyboard navigation

Failing solution

To do so, I proposed keeping the previously focused day in the state. A day would receive the focus in two cases:

  • the parent has autofocus=true
  • the focused day has been modified

This does not work because the component containing all the days is rerendered when going from one month to another. So from it's point of view, there is no difference between a month modification, and the first render of the month (where autofocus is required to get the focus)

new solution

I propose to use the state reducer of the calendar. For now, it keeps in memory the focused day which by default is now.

I propose to stick more to the truth by setting the state to null if the focus is not set onto a date, and add another variable to keep in memory which date should have tabIndex={0}.

This includes dealing with blur events on day buttons. which I manage as follow:

  • If the blurred day is not the same as the saved one, it should be because the state has been modified, and another day took the focus, so I do nothing
  • If the blurred day si the same as the saved one, I put focused day to null because it has moved outside of the calendar

Can be tested on the static calendar: https://deploy-preview-5083--material-ui-x.netlify.app/x/react-date-pickers/date-picker/#sub-components

alexfauquette avatar Jun 01 '22 16:06 alexfauquette

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 312.1 529.7 410.7 414.4 84.087
Sort 100k rows ms 462.1 958.6 705.3 717.74 160.353
Select 100k rows ms 153 291.5 216.4 231.28 51.361
Deselect 100k rows ms 113.2 271.6 191 195.74 64.79

Generated by :no_entry_sign: dangerJS against a0f27983195a943b7848efb791eea63e873112c4

mui-bot avatar Jun 01 '22 16:06 mui-bot

@flaviendelangle I would like to have your opinion on this solution before going further (adapting range picker to the new calendar state, ...)

alexfauquette avatar Jun 02 '22 12:06 alexfauquette

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Aug 16 '22 09:08 github-actions[bot]

I tried to reuse the focusedDate in useCalendarState. But it's a wong strategy, because it's easier to manage focus inside each view. The focusedDate is only used for managing the transition between months when the focus goes out of the current month

Follow up PR #5820

alexfauquette avatar Aug 17 '22 10:08 alexfauquette