-
Notifications
You must be signed in to change notification settings - Fork 6.8k
bug(DateAdapter): Date comparison is only checking partial info #28726
-
Is this a regression?
- Yes, this behavior used to work in the previous version
The previous version in which this bug was not present was
No response
Description
I'm developing an app with Material DatePicker and Luxon Date Adapter.
I gave my users the possibility to input seconds, milliseconds or full ISO date strings into my app's date inputs.
This works well except when pasting certain dates without deleting the input's value first (replacing value with CTRL+V).
I found out why, I was pasting dates which were on the same day (but different time values) as the previous date.
Video of the problem:
datepicker-bug
https://github.com/angular/components/assets/15314924/f19fc5e0-f019-474c-b854-85a177b86703
Checking the source code reveals that in the date-adapter.ts file, dates are only compared using year/month/day instead of relying on the native's type when applicable.
components/src/material/core/datetime/date-adapter.ts
Lines 243 to 249 in facd027
And the method sameDate() is called by MatDatePickerInputBase, and its result is based on the previous method's result:
components/src/material/core/datetime/date-adapter.ts
Lines 258 to 268 in facd027
Neither method is overriden in luxon-date-adapter to use something more robust.
Using my own date adapter worked as expected with this snippet:
@Injectable() export class DemoDateAdapter extends LuxonDateAdapter { public override compareDate(first: DateTime<boolean>, second: DateTime<boolean>): number { return +first - +second; } }
This fixed the problem for Luxon but for any other adapter not overloading this method (including native dates) the problem arises.
The DateAdapter interface and class does not account for milliseconds (there is no dateAdapter.getMillis), perhaps it should ?
Reproduction
I have a demo app of a lib I'm managing having the problem :
https://thomasprioul.github.io/mdl-angular-libs/#/home/forms
Steps to reproduce:
- Use a material app with datepicker inputs and Luxon Date Adapter
- Use the following provider (in app or component) to support long date formats
export const DEFAULT_DATEFORMAT_PROVIDER: Provider = { provide: MAT_DATE_FORMATS, useValue: <MatDateFormats>{ ...MAT_LUXON_DATE_FORMATS, display: { ...MAT_LUXON_DATE_FORMATS.display, monthLabel: 'LLL yyyy', dateInput: ['D', 'D T', 'D TT', 'D TT.SSS', 'D TT.SSSZ'], }, parse: { ...MAT_LUXON_DATE_FORMATS.parse, dateInput: ['D TT.SSSZ', 'D TT.SSS', 'D TT', 'D T', 'D'], }, }, };
- Input a date with a timestamp
- CTRL+V another date with same day but different timestamp
- Form value does not change and date is reset to previous value when losing focus on the input
Expected Behavior
Correct detection of "different date" on CTRL+V
Actual Behavior
New input is considered as "equal date" when it is not
Environment
- Angular: 17.1.1
- CDK/Material: 17.1.1
- Browser(s): any
- Operating System (e.g. Windows, macOS, Ubuntu): any
Beta Was this translation helpful? Give feedback.
All reactions
Replies: 3 comments 1 reply
-
I believe this is intentional. We only check that the year, month and day match, because the datepicker only selects dates and not dates + times.
Beta Was this translation helpful? Give feedback.
All reactions
-
It's actually not only about the time that is ignored but everything that is stored in the date instance that adapter provides. For example luxon's DateTime
is also keeping track of the outputCalendar
and their DateTime.equals
is checking that as well.
In my case my initial value for the FormControl
bound to the DatePicker
is a DateTime
with an outputCalendar
of null
. The DateAdapter<luxon.DateTime>
creates DateTime
instances with the outputCalendar
set to gregory
by default. But since compareDate
is not taking the "metadata" into account my form is never updated. With the range picker this gets even worse because one of the dates might get updated but not the other potentially resulting in start and end dates with completely different meaning. And from a user perspective this is not clear at all.
Since the DatePicker
provides the full type of the DateAdapter
and not only something representing year, month, day it should also consult the DateAdapter
to perform the comparison.
And the fix for this is straightforward if the custom adapters simply (and optionally) override the compareDate
function. I'm more than happy to provide a MR for at least the luxon adapter.
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
Hello @mpflanzer ,
Thank you for the detailed information.
I gave my users the possibility to input seconds, milliseconds or full ISO date strings into my app's date inputs.
I don't think MatDatepicker has any requirement to work with time input or support time at all. I think the recommended solution might be for you to make your own adapter for your specific use case.
I'm going to convert this to discussion. That way this issue doesn't get locked, and you will still be able to post comments.
-Zach
Beta Was this translation helpful? Give feedback.
All reactions
-
Thanks for opening the discussion @zarend.
Just avoid confusion, I described a situation which is not using more than a date (without time) but might still lead to wrong behavior. For example the start and end date might use to different calendars depending on which initial values have been set to FormControls
linked to the date picker. The result would be that the range selected by the user is not at all represented by the values the date picker sets.
Beta Was this translation helpful? Give feedback.