-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[DateRangeCalendar] Do not update the previewed day when hover a day and the value is empty #16819
Conversation
…and the value is empty
(event: React.MouseEvent<HTMLDivElement>, newPreviewRequest: PickerValidDate) => { | ||
if (!isWithinRange(utils, newPreviewRequest, valueDayRange)) { | ||
setRangePreviewDay(newPreviewRequest); | ||
(event: React.MouseEvent<HTMLDivElement>, newRangePreviewDay: PickerValidDate) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is one scenario where this does not work: if you hover a day while value === [null, null]
, you don't move your mouse and you have an update of the value (from some server update).
Before this PR, the preview range would be applied.
Now it would not.
I think the trade-off is fine here, especially given than before this PR is would break already if you hovered inside the range and the value updates (since we do setRangePreviewDay(null)
on line 476).
I could store the hovered date in a ref if it's not causing any preview and check when the value change, but tbh it feels like wasted complexity.
Deploy preview: https://deploy-preview-16819--material-ui-x.netlify.app/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cherry-pick PRs will be created targeting branches: v7.x |
…and the value is empty (#16819)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice improvement! 👏
We could still optimze in other ways - not updating every single day when hoving over days with a selected value.
Screen.Recording.2025-03-10.at.13.50.09.mov
If you can create a dedicated issue 🙏 |
Fixes #16815
TODO: Apply the same logic to #16069 if needed