[pickers] Fix DigitalClock time options on a DST switch day
#10793
Conversation
LukasTy
added
bug 🐛
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
github-actions
bot
added
the
PR: out-of-date
| @@ -124,6 +124,21 @@ export const mergeDateAndTime = <TDate>( | |||
| return mergedDate; | |||
| }; | |||
|
|
|||
| export const isEqualTime = <TDate>( | |||
There was a problem hiding this comment.
I am afraid this is risky if we support ms one day.
Can the ms be different? If not we could check them as well
|
@LukasTy what is the current status of this PR? |
I shelved it while tackling more pressing matters. |
github-actions
bot
removed
the
PR: out-of-date
|
Deploy preview: https://deploy-preview-10793--material-ui-x.netlify.app/ |
LukasTy
changed the title
DigitalClock DST related problemDigitalClock time options on a DST switch day
| return ( | ||
| <ClockItem | ||
| key={formattedValue} | ||
| key={`${option.valueOf()}-${formattedValue}`} |
There was a problem hiding this comment.
Only using option.valueOf() fails on AdapterDayjs as it handles DST in a strange manner—they don't have duplicate options for the same hour, they format the duplicate offset time differently, but that's why we need to include the formattedValue in the key to ensure uniqueness. 🙈
There was a problem hiding this comment.
🤔 That is weird indeed
We could also format with the day I guess, but it's probably not worth the computation time that is higher than .valueOf +a formatted value that we are also using elsewhere.
|
I have updated the PR/solution and description with some extra notes. |
LukasTy
added
the
needs cherry-pick
| return ( | ||
| <ClockItem | ||
| key={formattedValue} | ||
| key={`${option.valueOf()}-${formattedValue}`} |
There was a problem hiding this comment.
🤔 That is weird indeed
We could also format with the day I guess, but it's probably not worth the computation time that is higher than .valueOf +a formatted value that we are also using elsewhere.
Fixes #10783
ChangeDigitalClocktime options building to usestartOfYearto avoidDSTissues.After some exploration, I decided to go with a solution proposed here: #10783 (comment).
In this way we are not fighting to not have duplicate time options as they technically make sense, we only ensure that options for the whole day are included (not limiting options to
24 hours*minutesStep)