Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

bug(DateAdapter): Date comparison is only checking partial info #28726

ThomasPrioul started this conversation in Ideas
Discussion options

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.

compareDate(first: D, second: D): number {
return (
this.getYear(first) - this.getYear(second) ||
this.getMonth(first) - this.getMonth(second) ||
this.getDate(first) - this.getDate(second)
);
}

And the method sameDate() is called by MatDatePickerInputBase, and its result is based on the previous method's result:

sameDate(first: D | null, second: D | null): boolean {
if (first && second) {
let firstValid = this.isValid(first);
let secondValid = this.isValid(second);
if (firstValid && secondValid) {
return !this.compareDate(first, second);
}
return firstValid == secondValid;
}
return first == second;
}

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:

  1. Use a material app with datepicker inputs and Luxon Date Adapter
  2. 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'],
 },
 },
};
  1. Input a date with a timestamp
  2. CTRL+V another date with same day but different timestamp
  3. 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
You must be logged in to vote

Replies: 3 comments 1 reply

Comment options

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.

You must be logged in to vote
0 replies
Comment options

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.

You must be logged in to vote
0 replies
Comment options

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

You must be logged in to vote
1 reply
Comment options

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Category
Ideas
Labels
needs triage This issue needs to be triaged by the team
Converted from issue

This discussion was converted from issue #28613 on March 14, 2024 21:35.

AltStyle によって変換されたページ (->オリジナル) /