ng-matero / extensions

Angular Material Extensions Library.
https://ng-matero.github.io/extensions/
MIT License
393 stars 48 forks source link

[mtxDatetimepicker] input "mtxDatetimepickerFilter" partially work #244

Closed apic-jeremy closed 7 months ago

apic-jeremy commented 8 months ago

Hi,

I'm trying to "disable" some value of the mtxDatetimepicker in "time" mode.

I use the "mtxDatetimepickerFilter". It make less visible the corresponding value (like a disabled option) but we can still select them. mtxDatetimepickerFilter

There the code of the function that i use :

HTML PART

<mat-form-field>
  <mtx-datetimepicker #startTimePicker [type]="'time'" [touchUi]="false" [twelvehour]="false" [timeInput]="true"></mtx-datetimepicker>
  <input [mtxDatetimepicker]="startTimePicker"[mtxDatetimepickerFilter]="startDateAllowed" matInput required>
  <mtx-datetimepicker-toggle [for]="startTimePicker" matSuffix></mtx-datetimepicker-toggle>
</mat-form-field>

TS PART

startDateAllowed = (d: moment.Moment) => {
  return [15, 30, 45].indexOf(d.get('minute')) != -1
}

PACKAGE.JSON

"@ng-matero/extensions": "^14.8.1",
"@ng-matero/extensions-moment-adapter": "^14.0.3",
nzbin commented 8 months ago

@JelleBruisten Can you help me to check this issue?

JelleBruisten commented 8 months ago

@nzbin I will take a look next week

JelleBruisten commented 8 months ago

@apic-jeremy I have not started yet on this ( busy week 😢 ), however I did notice that you mentioned that it is on @ng-matero/extensions version 14.8.1. Do you happen to have tested this on the latest versions?

apic-jeremy commented 8 months ago

no i haven't because the application is in angular 14. And I didn't find a "stackblitz" with the latest versions of MtxDatetimepicker.

JelleBruisten commented 7 months ago

Sorry for the late investigation 😶However I did some investigation and this problem is also present in current versions of ng-matero extensions, the problem lies in how we handle filter vs min/max dates:

For example:

  handleMinuteInputChange(value: NumberInput) {
    const minute = coerceNumberProperty(value);
    if (minute || minute === 0) {
      const newValue = this._adapter.createDatetime(
        this._adapter.getYear(this.activeDate),
        this._adapter.getMonth(this.activeDate),
        this._adapter.getDate(this.activeDate),
        this._adapter.getHour(this._activeDate),
        minute
      );
      this._activeDate = this._adapter.clampDate(newValue, this.minDate, this.maxDate);
      this.activeDateChange.emit(this.activeDate);

      // If previously we did set [mtxValue]="40" and the input changed to 30, and the clamping
      // will make it "40" again then the minuteInputDirective will not have been updated
      // since "40" === "40" same reference so no change detected by directly setting it within
      // this handler, we handle this usecase
      if (this.minuteInputDirective) {
        this.minuteInputDirective.timeValue = this.minute;
      }
    }
  }

on the line:

this._activeDate = this._adapter.clampDate(newValue, this.minDate, this.maxDate);

We make sure here that the newValue falls in between a minDate and a maxDate, but never in this code is there a check for if this value is valid with the filtering. So if a value is not valid, we would need to do something here.

Few options:

edit: The above code is for the input, in the clock code there is no restriction

JelleBruisten commented 7 months ago

restrict is likely the easiest way, but what do you think @nzbin and @apic-jeremy?

Here a example if we restrict it: Animation

nzbin commented 7 months ago

restrict is likely the easiest way, but what do you think @nzbin and @apic-jeremy?

Here a example if we restrict it: Animation

It's great.

nzbin commented 7 months ago

Please try the new version 14.8.2

apic-jeremy commented 7 months ago

Hi !! Thank a lot, it seems work for me :)

apic-jeremy commented 7 months ago

@nzbin I don't know if I can comment this thread or create an other, sorry

Hi, during my test, I found an unexpectable comportement since your correction : Animation

endDateAllowed = (d: moment.Moment) => {
  return [0, 15, 30, 45].indexOf(d.get('minute')) != -1 || (d.get('hour') == 23 && d.get('minute') == 59)
}

As you can see : At the begginin, the endDte is set to "23:59" When I try clic an other hour than "23" it's block (I can select "23h" and then select only "00, 15, 30, 45 or 59" as exprected in the function)

Then, if i set the minute to "00", I can finaly select an other hour.

I understand it's because no other hour is allowed to "59", but it's problematique in my case

JelleBruisten commented 7 months ago

I did try it with your filter function but indeed it does not seems to work try this:

(date: moment.Moment | null, type: MtxDatetimepickerFilterType) => {
      if (date === null) {
        return true;
      }

      switch (type) {
        case MtxDatetimepickerFilterType.DATE:
          return true;
        case MtxDatetimepickerFilterType.HOUR:
          return true;
        case MtxDatetimepickerFilterType.MINUTE:
          return (
            [0, 15, 30, 45].includes(date.get('minute')) || 
            (date.get('hour') === 23 && date.get('minute') === 59)
          );
      }
    }

So you want to enable all dates, every hour and just restrict minutes to be 0, 15, 30 or 45 and allow 59 if the hour is 23?

Also general recommendation is to always use === and !== might save you a few headaches, with some random buggy behavior.

apic-jeremy commented 7 months ago

Hi !

I factorized the code to :

if (date !== null && type === MtxDatetimepickerFilterType.MINUTE) {
      return (
        [0, 15, 30, 45].includes(date.get('minute')) || 
        (date.get('hour') === 23 && date.get('minute') === 59)
      );
    } else {
      return true;
    }

I can now select an other hour, and the selection is well restrict, but I have still a problem see : Animation2 The error "maxDatetimepickerFilter" is not fire anymore (I don't allow 21h59 or 22h04) :

<mat-error *ngIf="CalendarEditForm.get('startDate').hasError('mtxDatetimepickerFilter') || CalendarEditForm.get('endDate').hasError('mtxDatetimepickerFilter')" class="font-12">
              {{ 'template-planning.pop.hint.date-step' | translate }}
          </mat-error>

Before your correction (in 14.8.1) I have this behavior : Animation3

May be I need to create a subscription on valueChange of the input to control it and fire a custom Validator ? What do you think ?

apic-jeremy commented 7 months ago

EDIT : I add a custom validator and it seem's patch my "issue" :

export function stepValidator(): ValidatorFn { 
  return (control: AbstractControl): ValidationErrors | null => { 
    let date: any = control.value;
    if(!moment.isMoment(date) || ([0, 15, 30, 45].includes(date.get('minute')) || (date.get('hour') === 23 && date.get('minute') === 59))){
      return null;
    }   
    return { 'errorStep': true }    
  } 
}
<mat-error *ngIf="CalendarEditForm.get('startDate').hasError('mtxDatetimepickerFilter') || CalendarEditForm.get('endDate').hasError('mtxDatetimepickerFilter') || CalendarEditForm.get('endDate').hasError('errorStep')" class="font-12">
              {{ 'template-planning.pop.hint.date-step' | translate }}
</mat-error>

Animation4

What do you think ? Is there a issue with the component or is my case too complex ?