mui-x
mui-x copied to clipboard
[pickers] Allows arrow navigation without needing `autofocus`
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
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
@flaviendelangle I would like to have your opinion on this solution before going further (adapting range picker to the new calendar state, ...)
This pull request has conflicts, please resolve those before we can evaluate the pull request.
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