Skip to content

Commit

Permalink
fix(cdk/drag-drop): don't stop event propagation unless nested (#21227)
Browse files Browse the repository at this point in the history
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.

(cherry picked from commit e42d502)
  • Loading branch information
crisbeto authored and annieyw committed Jan 6, 2021
1 parent deb149c commit 87ee0b8
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 11 deletions.
18 changes: 15 additions & 3 deletions src/cdk/drag-drop/directives/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(() => {
Expand Down Expand Up @@ -5520,7 +5520,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();

Expand Down Expand Up @@ -5568,7 +5567,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();
}));
});
});
Expand Down
6 changes: 4 additions & 2 deletions src/cdk/drag-drop/directives/drag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,15 @@ export class CdkDrag<T = any> 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;

Expand Down
12 changes: 8 additions & 4 deletions src/cdk/drag-drop/drag-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -783,10 +786,11 @@ export class DragRef<T = any> {
* @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);
Expand Down
5 changes: 3 additions & 2 deletions tools/public_api_guard/cdk/drag-drop.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export declare class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDes
constructor(
element: ElementRef<HTMLElement>,
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;
Expand All @@ -55,7 +55,7 @@ export declare class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDes
reset(): void;
static ngAcceptInputType_disabled: BooleanInput;
static ɵdir: i0.ɵɵDirectiveDefWithMeta<CdkDrag<any>, "[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<CdkDrag<any>, [null, { optional: true; skipSelf: true; }, null, null, null, { optional: true; }, { optional: true; }, null, null, { optional: true; self: true; }]>;
static ɵfac: i0.ɵɵFactoryDef<CdkDrag<any>, [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<T, O = T> {
Expand Down Expand Up @@ -321,6 +321,7 @@ export declare class DragRef<T = any> {

export interface DragRefConfig {
dragStartThreshold: number;
parentDragRef?: DragRef;
pointerDirectionChangeThreshold: number;
zIndex?: number;
}
Expand Down

0 comments on commit 87ee0b8

Please sign in to comment.