From 9e1ea6558d3acd0f54f8cd9da505c952ce4bf683 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sun, 6 Dec 2020 12:15:26 +0100 Subject: [PATCH] fix(cdk/drag-drop): don't stop event propagation unless nested Some time ago we added logic to stop event propagation so that nested drag items don't trigger multiple sequences. Stopping propagation for all events seems to interfere multiple other use cases so these changes add some logic so that we only stop propagation when items are nested. There was something similar in #19334, but I decided not to move forward with it, because it required consumers to know the internals of the `drag-drop` module, whereas this approach can do it automatically. Fixes #19333. --- src/cdk/drag-drop/directives/drag.spec.ts | 18 +++++++++++++++--- src/cdk/drag-drop/directives/drag.ts | 6 ++++-- src/cdk/drag-drop/drag-ref.ts | 12 ++++++++---- tools/public_api_guard/cdk/drag-drop.d.ts | 5 +++-- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/cdk/drag-drop/directives/drag.spec.ts b/src/cdk/drag-drop/directives/drag.spec.ts index 80eab4e8f871..5ed0b9566180 100644 --- a/src/cdk/drag-drop/directives/drag.spec.ts +++ b/src/cdk/drag-drop/directives/drag.spec.ts @@ -691,7 +691,7 @@ describe('CdkDrag', () => { expect(styles.touchAction || (styles as any).webkitUserDrag).toBeFalsy(); })); - it('should stop propagation for the drag sequence start event', fakeAsync(() => { + it('should not stop propagation for the drag sequence start event by default', fakeAsync(() => { const fixture = createComponent(StandaloneDraggable); fixture.detectChanges(); const dragElement = fixture.componentInstance.dragElement.nativeElement; @@ -702,7 +702,7 @@ describe('CdkDrag', () => { dispatchEvent(dragElement, event); fixture.detectChanges(); - expect(event.stopPropagation).toHaveBeenCalled(); + expect(event.stopPropagation).not.toHaveBeenCalled(); })); it('should not throw if destroyed before the first change detection run', fakeAsync(() => { @@ -5486,7 +5486,6 @@ describe('CdkDrag', () => { describe('with nested drags', () => { it('should not move draggable container when dragging child (multitouch)', fakeAsync(() => { - const fixture = createComponent(NestedDragsComponent, [], 10); fixture.detectChanges(); @@ -5534,7 +5533,20 @@ describe('CdkDrag', () => { .toHaveBeenCalledTimes(containerDragMovedCount); expect(fixture.componentInstance.containerDragReleasedSpy) .toHaveBeenCalledTimes(containerDragReleasedCount); + })); + + it('should stop event propagation when dragging a nested item', fakeAsync(() => { + const fixture = createComponent(NestedDragsComponent); + fixture.detectChanges(); + const dragElement = fixture.componentInstance.item.nativeElement; + + const event = createMouseEvent('mousedown'); + spyOn(event, 'stopPropagation').and.callThrough(); + + dispatchEvent(dragElement, event); + fixture.detectChanges(); + expect(event.stopPropagation).toHaveBeenCalled(); })); }); }); diff --git a/src/cdk/drag-drop/directives/drag.ts b/src/cdk/drag-drop/directives/drag.ts index 8884997743b1..312a09edfb40 100644 --- a/src/cdk/drag-drop/directives/drag.ts +++ b/src/cdk/drag-drop/directives/drag.ts @@ -192,13 +192,15 @@ export class CdkDrag implements AfterViewInit, OnChanges, OnDestroy { @Optional() @Inject(CDK_DRAG_CONFIG) config: DragDropConfig, @Optional() private _dir: Directionality, dragDrop: DragDrop, private _changeDetectorRef: ChangeDetectorRef, - @Optional() @Self() @Inject(CDK_DRAG_HANDLE) private _selfHandle?: CdkDragHandle) { + @Optional() @Self() @Inject(CDK_DRAG_HANDLE) private _selfHandle?: CdkDragHandle, + @Optional() @SkipSelf() @Inject(CDK_DRAG_PARENT) parentDrag?: CdkDrag) { this._dragRef = dragDrop.createDrag(element, { dragStartThreshold: config && config.dragStartThreshold != null ? config.dragStartThreshold : 5, pointerDirectionChangeThreshold: config && config.pointerDirectionChangeThreshold != null ? config.pointerDirectionChangeThreshold : 5, - zIndex: config?.zIndex + zIndex: config?.zIndex, + parentDragRef: parentDrag?._dragRef }); this._dragRef.data = this; diff --git a/src/cdk/drag-drop/drag-ref.ts b/src/cdk/drag-drop/drag-ref.ts index da18f013d7dd..3cb0d626c522 100644 --- a/src/cdk/drag-drop/drag-ref.ts +++ b/src/cdk/drag-drop/drag-ref.ts @@ -36,6 +36,9 @@ export interface DragRefConfig { /** `z-index` for the absolutely-positioned elements that are created by the drag item. */ zIndex?: number; + + /** Ref that the current drag item is nested in. */ + parentDragRef?: DragRef; } /** Options that can be used to bind a passive event listener. */ @@ -783,10 +786,11 @@ export class DragRef { * @param event Browser event object that started the sequence. */ private _initializeDragSequence(referenceElement: HTMLElement, event: MouseEvent | TouchEvent) { - // Always stop propagation for the event that initializes - // the dragging sequence, in order to prevent it from potentially - // starting another sequence for a draggable parent somewhere up the DOM tree. - event.stopPropagation(); + // Stop propagation if the item is inside another + // draggable so we don't start multiple drag sequences. + if (this._config.parentDragRef) { + event.stopPropagation(); + } const isDragging = this.isDragging(); const isTouchSequence = isTouchEvent(event); diff --git a/tools/public_api_guard/cdk/drag-drop.d.ts b/tools/public_api_guard/cdk/drag-drop.d.ts index 7422c8819f87..03e7c1c311fb 100644 --- a/tools/public_api_guard/cdk/drag-drop.d.ts +++ b/tools/public_api_guard/cdk/drag-drop.d.ts @@ -42,7 +42,7 @@ export declare class CdkDrag implements AfterViewInit, OnChanges, OnDes constructor( element: ElementRef, dropContainer: CdkDropList, - _document: any, _ngZone: NgZone, _viewContainerRef: ViewContainerRef, config: DragDropConfig, _dir: Directionality, dragDrop: DragDrop, _changeDetectorRef: ChangeDetectorRef, _selfHandle?: CdkDragHandle | undefined); + _document: any, _ngZone: NgZone, _viewContainerRef: ViewContainerRef, config: DragDropConfig, _dir: Directionality, dragDrop: DragDrop, _changeDetectorRef: ChangeDetectorRef, _selfHandle?: CdkDragHandle | undefined, parentDrag?: CdkDrag); getFreeDragPosition(): { readonly x: number; readonly y: number; @@ -55,7 +55,7 @@ export declare class CdkDrag implements AfterViewInit, OnChanges, OnDes reset(): void; static ngAcceptInputType_disabled: BooleanInput; static ɵdir: i0.ɵɵDirectiveDefWithMeta, "[cdkDrag]", ["cdkDrag"], { "data": "cdkDragData"; "lockAxis": "cdkDragLockAxis"; "rootElementSelector": "cdkDragRootElement"; "boundaryElement": "cdkDragBoundary"; "dragStartDelay": "cdkDragStartDelay"; "freeDragPosition": "cdkDragFreeDragPosition"; "disabled": "cdkDragDisabled"; "constrainPosition": "cdkDragConstrainPosition"; "previewClass": "cdkDragPreviewClass"; }, { "started": "cdkDragStarted"; "released": "cdkDragReleased"; "ended": "cdkDragEnded"; "entered": "cdkDragEntered"; "exited": "cdkDragExited"; "dropped": "cdkDragDropped"; "moved": "cdkDragMoved"; }, ["_previewTemplate", "_placeholderTemplate", "_handles"]>; - static ɵfac: i0.ɵɵFactoryDef, [null, { optional: true; skipSelf: true; }, null, null, null, { optional: true; }, { optional: true; }, null, null, { optional: true; self: true; }]>; + static ɵfac: i0.ɵɵFactoryDef, [null, { optional: true; skipSelf: true; }, null, null, null, { optional: true; }, { optional: true; }, null, null, { optional: true; self: true; }, { optional: true; skipSelf: true; }]>; } export interface CdkDragDrop { @@ -321,6 +321,7 @@ export declare class DragRef { export interface DragRefConfig { dragStartThreshold: number; + parentDragRef?: DragRef; pointerDirectionChangeThreshold: number; zIndex?: number; }