Skip to content

Commit

Permalink
fix(cdk/drag-drop): only block dragstart event on event targets (#24581)
Browse files Browse the repository at this point in the history
Currently we block the `dragstart` event on the entire drag element which doesn't account for its disabled state and for any existing handles.

Fixes #24533.

(cherry picked from commit 45fae71)
  • Loading branch information
crisbeto committed Mar 12, 2022
1 parent d1476ec commit e4c64dd
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 12 deletions.
34 changes: 34 additions & 0 deletions src/cdk/drag-drop/directives/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,19 @@ describe('CdkDrag', () => {

expect(event.defaultPrevented).toBe(true);
}));

it('should not prevent the default dragstart action when dragging is disabled', fakeAsync(() => {
const fixture = createComponent(StandaloneDraggable);
fixture.detectChanges();
fixture.componentInstance.dragInstance.disabled = true;
const event = dispatchFakeEvent(
fixture.componentInstance.dragElement.nativeElement,
'dragstart',
);
fixture.detectChanges();

expect(event.defaultPrevented).toBe(false);
}));
});

describe('touch dragging', () => {
Expand Down Expand Up @@ -1613,6 +1626,27 @@ describe('CdkDrag', () => {
dragElementViaMouse(fixture, handleChild, 50, 100);
expect(dragElement.style.transform).toBe('translate3d(50px, 100px, 0px)');
}));

it('should prevent default dragStart on handle, not on entire draggable', fakeAsync(() => {
const fixture = createComponent(StandaloneDraggableWithHandle);
fixture.detectChanges();

const draggableEvent = dispatchFakeEvent(
fixture.componentInstance.dragElement.nativeElement,
'dragstart',
);
fixture.detectChanges();

const handleEvent = dispatchFakeEvent(
fixture.componentInstance.handleElement.nativeElement,
'dragstart',
true,
);
fixture.detectChanges();

expect(draggableEvent.defaultPrevented).toBe(false);
expect(handleEvent.defaultPrevented).toBe(true);
}));
});

describe('in a drop container', () => {
Expand Down
37 changes: 25 additions & 12 deletions src/cdk/drag-drop/drag-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,7 @@ export class DragRef<T = any> {
this._ngZone.runOutsideAngular(() => {
element.addEventListener('mousedown', this._pointerDown, activeEventListenerOptions);
element.addEventListener('touchstart', this._pointerDown, passiveEventListenerOptions);
// Usually this isn't necessary since the we prevent the default action in `pointerDown`,
// but some cases like dragging of links can slip through (see #24403).
element.addEventListener('dragstart', preventDefault, activeEventListenerOptions);
element.addEventListener('dragstart', this._nativeDragStart, activeEventListenerOptions);
});
this._initialTransform = undefined;
this._rootElement = element;
Expand Down Expand Up @@ -631,9 +629,7 @@ export class DragRef<T = any> {

// Delegate the event based on whether it started from a handle or the element itself.
if (this._handles.length) {
const targetHandle = this._handles.find(handle => {
return event.target && (event.target === handle || handle.contains(event.target as Node));
});
const targetHandle = this._getTargetHandle(event);

if (targetHandle && !this._disabledHandles.has(targetHandle) && !this.disabled) {
this._initializeDragSequence(targetHandle, event);
Expand Down Expand Up @@ -1287,7 +1283,7 @@ export class DragRef<T = any> {
private _removeRootElementListeners(element: HTMLElement) {
element.removeEventListener('mousedown', this._pointerDown, activeEventListenerOptions);
element.removeEventListener('touchstart', this._pointerDown, passiveEventListenerOptions);
element.removeEventListener('dragstart', preventDefault, activeEventListenerOptions);
element.removeEventListener('dragstart', this._nativeDragStart, activeEventListenerOptions);
}

/**
Expand Down Expand Up @@ -1512,6 +1508,28 @@ export class DragRef<T = any> {

return this._previewRect;
}

/** Handles a native `dragstart` event. */
private _nativeDragStart = (event: DragEvent) => {
if (this._handles.length) {
const targetHandle = this._getTargetHandle(event);

if (targetHandle && !this._disabledHandles.has(targetHandle) && !this.disabled) {
event.preventDefault();
}
} else if (!this.disabled) {
// Usually this isn't necessary since the we prevent the default action in `pointerDown`,
// but some cases like dragging of links can slip through (see #24403).
event.preventDefault();
}
};

/** Gets a handle that is the target of an event. */
private _getTargetHandle(event: Event): HTMLElement | undefined {
return this._handles.find(handle => {
return event.target && (event.target === handle || handle.contains(event.target as Node));
});
}
}

/**
Expand Down Expand Up @@ -1564,8 +1582,3 @@ function matchElementSize(target: HTMLElement, sourceRect: ClientRect): void {
target.style.height = `${sourceRect.height}px`;
target.style.transform = getTransform(sourceRect.left, sourceRect.top);
}

/** Utility to prevent the default action of an event. */
function preventDefault(event: Event): void {
event.preventDefault();
}

0 comments on commit e4c64dd

Please sign in to comment.