dhutaryan / ngx-mat-timepicker

Material timepicker based on material design
https://dhutaryan.github.io/ngx-mat-timepicker/
MIT License
73 stars 8 forks source link

Clock View needs double click on hour to switch to minute on Android/iOS #161

Closed Sagato closed 5 months ago

Sagato commented 5 months ago

I am using Capacitor. In the browser on mobile devices and on chrome/safari the selection works fine. On Android and iOS, when I am using Capacitor to create cross platform apps, it works as the title states. I have to click twice on the hour in order to make it switch to minutes.

k3nsei commented 5 months ago

As iOS and Android native timepickers are best user experience shouldn't lib be using them in mobile envs? By excluding Android and iOS from requirements library probably could benefit with slimer bundle size and code to maintain.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/time#handling_browser_support

Sagato commented 5 months ago

Actually I found that it was only a changedetection issue. I just forked the repo to create a very very small PR.

k3nsei commented 5 months ago

@Sagato I had same issue when using ChangeDetectionStrategy.OnPush, that after clicking trigger button content of popup/dialog wasn't rendering markForChanges() needed to be executed. But as I'm using it in ngx-formly I thought it was a problem of our project setup. But from what your are saying it could be a bug of this lib.

I solved it that way:

image

image

Sagato commented 5 months ago

@k3nsei Actually I forked the repo earlier and played around with it. I found that onHourChanged in: https://github.com/dhutaryan/ngx-mat-timepicker/blob/master/projects/mat-timepicker/src/lib/clock-dials.ts gets fed from an event of the hours-clock-dial.ts Which gets emitted in this function on the bottom. The variable gets changed in clock-dials.ts subscription (ngOnInit), but cd is not called. This leads to this glitch I'd say.

/** Handles mouse and touch events on dial and document. */
  _onUserAction(event: MouseEvent | TouchEvent): void {
    if (event.cancelable) {
      event.preventDefault();
    }

    this._setHour(event);

    const eventsSubscription = merge(
      fromEvent<MouseEvent>(this._document, 'mousemove'),
      fromEvent<TouchEvent>(this._document, 'touchmove'),
    )
      .pipe(debounceTime(0))
      .subscribe({
        next: (event) => {
          event.preventDefault();
          this._setHour(event);
        },
      });

    merge(
      fromEvent<MouseEvent>(this._document, 'mouseup'),
      fromEvent<TouchEvent>(this._document, 'touchend'),
    )
      .pipe(take(1))
      .subscribe({
        next: () => {
          eventsSubscription.unsubscribe();
          this.selectedChange.emit({
            hour: this.selectedHour,
            changeView: true,
          });
        },
      });
  }

Here is the PR: https://github.com/dhutaryan/ngx-mat-timepicker/pull/162#issue-2335470264

k3nsei commented 5 months ago

I think it should be just here https://github.com/dhutaryan/ngx-mat-timepicker/blob/eaaf5b83cf8f99c8a514d3af4c151393fb0595d8/projects/mat-timepicker/src/lib/timepicker-base.ts#L438 where markForCheck() is missing.

Sagato commented 5 months ago

Let me check this!

k3nsei commented 5 months ago

Anyway I'm just reporting my findings as had similar issues in my project. It's up to @dhutaryan as library maintainer to look into it :)

dhutaryan commented 5 months ago

@k3nsei could you provide a repro on stackblitz with the issue you are talking about? I can't find a way why it might not work.

dhutaryan commented 5 months ago

Fixed in: 17.5.1 16.3.1 15.3.1

@Sagato thank you