Skip to content

Commit

Permalink
refactor(cdk/drag-drop): Remove use of zone onStable (#28681)
Browse files Browse the repository at this point in the history
* refactor(cdk/drag-drop): Remove use of zone onStable

* test: fix tests
  • Loading branch information
mmalerba authored Mar 12, 2024
1 parent e10a07d commit 9509ad5
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 19 deletions.
19 changes: 13 additions & 6 deletions src/cdk/drag-drop/directives/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
ChangeDetectionStrategy,
Component,
ElementRef,
ErrorHandler,
Input,
NgZone,
Provider,
Expand Down Expand Up @@ -1166,12 +1167,18 @@ describe('CdkDrag', () => {
expect(Math.floor(elementRect.left)).toBe(50);
}));

it('should throw if drag item is attached to an ng-container', fakeAsync(() => {
expect(() => {
createComponent(DraggableOnNgContainer).detectChanges();
flush();
}).toThrowError(/^cdkDrag must be attached to an element node/);
}));
it('should throw if drag item is attached to an ng-container', () => {
const errorHandler = jasmine.createSpyObj(['handleError']);
createComponent(DraggableOnNgContainer, [
{
provide: ErrorHandler,
useValue: errorHandler,
},
]).detectChanges();
expect(errorHandler.handleError.calls.mostRecent().args[0].message).toMatch(
/^cdkDrag must be attached to an element node/,
);
});

it('should cancel drag if the mouse moves before the delay is elapsed', fakeAsync(() => {
// We can't use Jasmine's `clock` because Zone.js interferes with it.
Expand Down
31 changes: 18 additions & 13 deletions src/cdk/drag-drop/directives/drag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import {Directionality} from '@angular/cdk/bidi';
import {DOCUMENT} from '@angular/common';
import {
AfterViewInit,
Directive,
ElementRef,
EventEmitter,
Expand All @@ -27,6 +26,10 @@ import {
Self,
InjectionToken,
booleanAttribute,
afterNextRender,
AfterViewInit,
inject,
Injector,
} from '@angular/core';
import {coerceElement, coerceNumberProperty} from '@angular/cdk/coercion';
import {BehaviorSubject, Observable, Observer, Subject, merge} from 'rxjs';
Expand Down Expand Up @@ -207,6 +210,8 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
},
);

private _injector = inject(Injector);

constructor(
/** Element that the draggable is attached to. */
public element: ElementRef<HTMLElement>,
Expand Down Expand Up @@ -296,35 +301,35 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
}

ngAfterViewInit() {
// Normally this isn't in the zone, but it can cause major performance regressions for apps
// using `zone-patch-rxjs` because it'll trigger a change detection when it unsubscribes.
this._ngZone.runOutsideAngular(() => {
// We need to wait for the zone to stabilize, in order for the reference
// element to be in the proper place in the DOM. This is mostly relevant
// for draggable elements inside portals since they get stamped out in
// their original DOM position and then they get transferred to the portal.
this._ngZone.onStable.pipe(take(1), takeUntil(this._destroyed)).subscribe(() => {
// We need to wait until after render, in order for the reference
// element to be in the proper place in the DOM. This is mostly relevant
// for draggable elements inside portals since they get stamped out in
// their original DOM position, and then they get transferred to the portal.
afterNextRender(
() => {
this._updateRootElement();
this._setupHandlesListener();

if (this.freeDragPosition) {
this._dragRef.setFreeDragPosition(this.freeDragPosition);
}
});
});
},
{injector: this._injector},
);
}

ngOnChanges(changes: SimpleChanges) {
const rootSelectorChange = changes['rootElementSelector'];
const positionChange = changes['freeDragPosition'];

// We don't have to react to the first change since it's being
// handled in `ngAfterViewInit` where it needs to be deferred.
// handled in the `afterNextRender` queued up in the constructor.
if (rootSelectorChange && !rootSelectorChange.firstChange) {
this._updateRootElement();
}

// Skip the first change since it's being handled in `ngAfterViewInit`.
// Skip the first change since it's being handled in the `afterNextRender` queued up in the
// constructor.
if (positionChange && !positionChange.firstChange && this.freeDragPosition) {
this._dragRef.setFreeDragPosition(this.freeDragPosition);
}
Expand Down

0 comments on commit 9509ad5

Please sign in to comment.