angular / components

Component infrastructure and Material Design components for Angular
https://material.angular.io
MIT License
24.4k stars 6.76k forks source link

fix(material/slider): avoid error on touch events in pointerMove and pointerUp #25272

Closed ethaneckstein closed 2 years ago

ethaneckstein commented 2 years ago

Is this a regression?

The previous version in which this bug was not present was

No response

Description

In fix(material/slider): avoid error on some touchstart events #23823 there was a fix implemented to address the errors on touchstart events. This needs to be implemented on all event.preventDefault() occurrences. We are still experiencing these errors in our application and you can still reproduce them in the utilizing the previous StackBlitz https://stackblitz.com/edit/angular-ivy-hg59w4?file

Reproduction

Steps to reproduce:

  1. Use mat-slider in template
  2. Scroll vertically into the mat-slider and interact with it. It can be hard to get the interaction with the vertical scroll so try a few different ways.

Expected Behavior

No "[Intervention] Ignored attempt to cancel a touchstart event with cancelable=false, for example because scrolling is in progress and cannot be interrupted." in the console when interacting with the mat-slider control.

Actual Behavior

"[Intervention] Ignored attempt to cancel a touchstart event with cancelable=false, for example because scrolling is in progress and cannot be interrupted." repeatedly erroring in the console when interacting with the mat-slider control and slider doesn't move in deployed mobile device.

Environment

crisbeto commented 2 years ago

I wasn't able to reproduce this locally or in the Stackblitz. Are you only seeing this for touchstart events? Because I don't think that we can do much more to avoid it since there's already a cancelable check around the preventDefault call there.

ethaneckstein commented 2 years ago

The cancelable check is only present in the _pointerDown function. This error also occurs in the _pointerMove and _pointerUp functions.

 /**
   * Called when the user has moved their pointer after
   * starting to drag. Bound on the document level.
   */
  private _pointerMove = (event: TouchEvent | MouseEvent) => {
    if (this._isSliding === 'pointer') {
      const pointerPosition = getPointerPositionOnPage(event, this._touchId);

      if (pointerPosition) {
        // Prevent the slide from selecting anything else.
        //  Add cancelable check here
        event.preventDefault();
        const oldValue = this.value;
        this._lastPointerEvent = event;
        this._updateValueFromPosition(pointerPosition);

        // Native range elements always emit `input` events when the value changed while sliding.
        if (oldValue != this.value) {
          this._emitInputEvent();
        }
      }
    }
  };

  /** Called when the user has lifted their pointer. Bound on the document level. */
  private _pointerUp = (event: TouchEvent | MouseEvent) => {
    if (this._isSliding === 'pointer') {
      if (
        !isTouchEvent(event) ||
        typeof this._touchId !== 'number' ||
        // Note that we use `changedTouches`, rather than `touches` because it
        // seems like in most cases `touches` is empty for `touchend` events.
        findMatchingTouch(event.changedTouches, this._touchId)
      ) {
        //  Add cancelable check here
        event.preventDefault();
        this._removeGlobalEvents();
        this._isSliding = null;
        this._touchId = undefined;

        if (this._valueOnSlideStart != this.value && !this.disabled) {
          this._emitChangeEvent();
        }

        this._valueOnSlideStart = this._lastPointerEvent = null;
      }
    }
  };
crisbeto commented 2 years ago

The warning you posted above only mentions the touchstart event, that's why I was asking if you're seeing it for anything else.

ethaneckstein commented 2 years ago

Ah I see. I copied the title from the previous issue. I probably should have been more specific. I will update that.

angular-automatic-lock-bot[bot] commented 2 years ago

This issue has been automatically locked due to inactivity. Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.