bug(drag-drop): Root element pointer down listeners causing excessive change detection

[Achilles1515]

Reproduction

https://stackblitz.com/edit/angular-kwisjn?file=src/app/cdk-drag-drop-overview-example.html

The StackBlitz demo above is a slightly modified version of the "Basic Drag&Drop" official example with a change detection counter added and a second draggable box.

Simply clicking (and not even dragging) an element containing the cdkDrag directive causes app-wide change detection to occur. This behavior is exemplified by clicking the second box and witnessing the change detection counter increase.

The first draggable box in the demo has had its root element listeners (mousedown and touchstart) modified to run outsize Angular zone.

drag-ref.ts code:

/**
   * Sets an alternate drag root element. The root element is the element that will be moved as
   * the user is dragging. Passing an alternate root element is useful when trying to enable
   * dragging on an element that you might not have access to.
   */
  withRootElement(rootElement: ElementRef<HTMLElement> | HTMLElement): this {
    const element = coerceElement(rootElement);

    if (element !== this._rootElement) {
      if (this._rootElement) {
        this._removeRootElementListeners(this._rootElement);
      }

      element.addEventListener('mousedown', this._pointerDown, activeEventListenerOptions);
      element.addEventListener('touchstart', this._pointerDown, passiveEventListenerOptions);
      this._initialTransform = undefined;
      this._rootElement = element;
    }

    return this;
  }

Is there any reason these event listeners can't be instantiated outside the Angular zone? Like so:

  withRootElement(rootElement: ElementRef<HTMLElement> | HTMLElement): this {
    const element = coerceElement(rootElement);

    if (element !== this._rootElement) {
      if (this._rootElement) {
        this._removeRootElementListeners(this._rootElement);
      }

      this._ngZone.runOutsideAngular(() => {
        element.addEventListener('mousedown', this._pointerDown, activeEventListenerOptions);
        element.addEventListener('touchstart', this._pointerDown, passiveEventListenerOptions);
      });
      this._initialTransform = undefined;
      this._rootElement = element;
    }

    return this;
  }

Expected Behavior

No change detection occurs when simply clicking a draggable item.

Actual Behavior

Change detection occurs when simply clicking a draggable item.

Environment

• Angular:
• CDK/Material: 9.1.1

[crisbeto]

There are some data bindings that need to be updated once dragging starts (e.g. .cdk-drag-dragging), hence why we run the events inside the NgZone.

[Achilles1515]

There are some data bindings that need to be updated once dragging starts (e.g. .cdk-drag-dragging), hence why we run the events inside the NgZone.

Good to know, I wasn't thinking about host bindings.

But with the current implementation, I still don't see the need to run the pointerdown events inside NgZone. If you look at the StackBlitz demo I posted and watch the DOM elements update in dev tools, the .cdk-drag-dragging class still gets added to both boxes at the same time.

.cdk-drag-dragging won't get added until _hasStartedDragging is true, which happens here:

/** Handler that is invoked when the user moves their pointer after they've initiated a drag. */
  private _pointerMove = (event: MouseEvent | TouchEvent) => {
    // ...

    if (!this._hasStartedDragging) {
     //...
      if (isOverThreshold) {
        //...
        if (!this._dropContainer || !this._dropContainer.isDragging()) {
          this._hasStartedDragging = true;
          this._ngZone.run(() => this._startDragSequence(event));
        }
      }

      return;
    }
//...
}

Immediately following this._hasStartedDragging = true;, _startDragSequence emits the started event, setting the directive host view and its ancestors as dirty, and the _ngZone.run() above will trigger change detection and update the bindings.

private _handleEvents(ref: DragRef<CdkDrag<T>>) {
    ref.started.subscribe(() => {
      this.started.emit({source: this});

      // Since all of these events run outside of change detection,
      // we need to ensure that everything is marked correctly.
      this._changeDetectorRef.markForCheck();
    });
//...
}

Does anything else come to mind that relies on the pointerdown events being run insize NgZone?

[crisbeto]

Looking into it deeper, you're right that it's not necessary. I think this is a leftover from when dragging didn't have a threshold so it had to trigger change detection immediately.

[Achilles1515]

Looking into it deeper, you're right that it's not necessary. I think this is a leftover from when dragging didn't have a threshold so it had to trigger change detection immediately.

Great. Thanks for looking into it.

[angular-automatic-lock-bot]

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.}