Skip to content

fix(cdk/drag-drop): resolve various event tracking issues inside the shadow dom #23026

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 127 additions & 32 deletions src/cdk/drag-drop/directives/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,28 +44,33 @@ const ITEM_WIDTH = 75;

describe('CdkDrag', () => {
function createComponent<T>(
componentType: Type<T>, providers: Provider[] = [], dragDistance = 0,
extraDeclarations: Type<any>[] = []): ComponentFixture<T> {
TestBed
.configureTestingModule({
imports: [DragDropModule, CdkScrollableModule],
declarations: [componentType, PassthroughComponent, ...extraDeclarations],
providers: [
{
provide: CDK_DRAG_CONFIG,
useValue: {
// We default the `dragDistance` to zero, because the majority of the tests
// don't care about it and drags are a lot easier to simulate when we don't
// have to deal with thresholds.
dragStartThreshold: dragDistance,
pointerDirectionChangeThreshold: 5
} as DragDropConfig
},
...providers
],
})
.compileComponents();
componentType: Type<T>, providers: Provider[] = [], dragDistance = 0,
extraDeclarations: Type<any>[] = [], encapsulation?: ViewEncapsulation): ComponentFixture<T> {
TestBed.configureTestingModule({
imports: [DragDropModule, CdkScrollableModule],
declarations: [componentType, PassthroughComponent, ...extraDeclarations],
providers: [
{
provide: CDK_DRAG_CONFIG,
useValue: {
// We default the `dragDistance` to zero, because the majority of the tests
// don't care about it and drags are a lot easier to simulate when we don't
// have to deal with thresholds.
dragStartThreshold: dragDistance,
pointerDirectionChangeThreshold: 5
} as DragDropConfig
},
...providers
],
});

if (encapsulation != null) {
TestBed.overrideComponent(componentType, {
set: {encapsulation}
});
}

TestBed.compileComponents();
return TestBed.createComponent<T>(componentType);
}

Expand Down Expand Up @@ -2010,6 +2015,54 @@ describe('CdkDrag', () => {
});
}));

it('should calculate the index if the list is scrolled while dragging inside the shadow DOM',
fakeAsync(() => {
// This test is only relevant for Shadow DOM-supporting browsers.
if (!_supportsShadowDom()) {
return;
}

const fixture = createComponent(DraggableInScrollableVerticalDropZone, [], undefined, [],
ViewEncapsulation.ShadowDom);
fixture.detectChanges();
const dragItems = fixture.componentInstance.dragItems;
const firstItem = dragItems.first;
const thirdItemRect = dragItems.toArray()[2].element.nativeElement.getBoundingClientRect();
const list = fixture.componentInstance.dropInstance.element.nativeElement;

startDraggingViaMouse(fixture, firstItem.element.nativeElement);
fixture.detectChanges();

dispatchMouseEvent(document, 'mousemove', thirdItemRect.left + 1, thirdItemRect.top + 1);
fixture.detectChanges();

list.scrollTop = ITEM_HEIGHT * 10;
dispatchFakeEvent(list, 'scroll');
fixture.detectChanges();

dispatchMouseEvent(document, 'mouseup');
fixture.detectChanges();
flush();
fixture.detectChanges();

expect(fixture.componentInstance.droppedSpy).toHaveBeenCalledTimes(1);

const event = fixture.componentInstance.droppedSpy.calls.mostRecent().args[0];

// Assert the event like this, rather than `toHaveBeenCalledWith`, because Jasmine will
// go into an infinite loop trying to stringify the event, if the test fails.
expect(event).toEqual({
previousIndex: 0,
currentIndex: 12,
item: firstItem,
container: fixture.componentInstance.dropInstance,
previousContainer: fixture.componentInstance.dropInstance,
isPointerOverContainer: jasmine.any(Boolean),
distance: {x: jasmine.any(Number), y: jasmine.any(Number)},
dropPoint: {x: jasmine.any(Number), y: jasmine.any(Number)}
});
}));

it('should calculate the index if the viewport is scrolled while dragging', fakeAsync(() => {
const fixture = createComponent(DraggableInDropZone);

Expand Down Expand Up @@ -2260,6 +2313,54 @@ describe('CdkDrag', () => {
cleanup();
}));

it('should update the boundary if a parent is scrolled while dragging inside the shadow DOM',
fakeAsync(() => {
// This test is only relevant for Shadow DOM-supporting browsers.
if (!_supportsShadowDom()) {
return;
}

const fixture = createComponent(DraggableInScrollableParentContainer, [], undefined, [],
ViewEncapsulation.ShadowDom);
fixture.componentInstance.boundarySelector = '.cdk-drop-list';
fixture.detectChanges();

const container: HTMLElement = fixture.nativeElement.shadowRoot
.querySelector('.scroll-container');
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
const list = fixture.componentInstance.dropInstance.element.nativeElement;
const cleanup = makeScrollable('vertical', container);
container.scrollTop = 10;

// Note that we need to measure after scrolling.
let listRect = list.getBoundingClientRect();

startDraggingViaMouse(fixture, item);
startDraggingViaMouse(fixture, item, listRect.right, listRect.bottom);
flush();
dispatchMouseEvent(document, 'mousemove', listRect.right, listRect.bottom);
fixture.detectChanges();

const preview = fixture.nativeElement.shadowRoot
.querySelector('.cdk-drag-preview')! as HTMLElement;
let previewRect = preview.getBoundingClientRect();

// Different browsers round the scroll position differently so
// assert that the offsets are within a pixel of each other.
expect(Math.abs(previewRect.bottom - listRect.bottom)).toBeLessThan(2);

container.scrollTop = 0;
dispatchFakeEvent(container, 'scroll');
fixture.detectChanges();
listRect = list.getBoundingClientRect(); // We need to update these since we've scrolled.
dispatchMouseEvent(document, 'mousemove', listRect.right, listRect.bottom);
fixture.detectChanges();
previewRect = preview.getBoundingClientRect();

expect(Math.abs(previewRect.bottom - listRect.bottom)).toBeLessThan(2);
cleanup();
}));

it('should clear the id from the preview', fakeAsync(() => {
const fixture = createComponent(DraggableInDropZone);
fixture.detectChanges();
Expand Down Expand Up @@ -5520,7 +5621,8 @@ describe('CdkDrag', () => {
return;
}

const fixture = createComponent(ConnectedDropZonesInsideShadowRoot);
const fixture = createComponent(ConnectedDropZones, [], undefined, [],
ViewEncapsulation.ShadowDom);
fixture.detectChanges();

const groups = fixture.componentInstance.groupedDragItems;
Expand Down Expand Up @@ -5651,7 +5753,8 @@ describe('CdkDrag', () => {
return;
}

const fixture = createComponent(ConnectedDropZonesInsideShadowRoot);
const fixture = createComponent(ConnectedDropZones, [], undefined, [],
ViewEncapsulation.ShadowDom);
fixture.detectChanges();
const item = fixture.componentInstance.groupedDragItems[0][1];

Expand Down Expand Up @@ -5961,7 +6064,7 @@ class DraggableInDropZone implements AfterViewInit {
// Firefox preserves the `scrollTop` value from previous similar containers. This
// could throw off test assertions and result in flaky results.
// See: https://bugzilla.mozilla.org/show_bug.cgi?id=959812.
this._elementRef.nativeElement.querySelector('.scroll-container').scrollTop = 0;
this.dropInstance.element.nativeElement.scrollTop = 0;
}
}

Expand Down Expand Up @@ -6363,14 +6466,6 @@ class ConnectedDropZones implements AfterViewInit {
}
}

@Component({
encapsulation: ViewEncapsulation.ShadowDom,
styles: CONNECTED_DROP_ZONES_STYLES,
template: CONNECTED_DROP_ZONES_TEMPLATE
})
class ConnectedDropZonesInsideShadowRoot extends ConnectedDropZones {
}

@Component({
encapsulation: ViewEncapsulation.ShadowDom,
styles: CONNECTED_DROP_ZONES_STYLES,
Expand Down
4 changes: 2 additions & 2 deletions src/cdk/drag-drop/drag-drop-registry.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ describe('DragDropRegistry', () => {

it('should dispatch `scroll` events if the viewport is scrolled while dragging', () => {
const spy = jasmine.createSpy('scroll spy');
const subscription = registry.scroll.subscribe(spy);
const subscription = registry.scrolled().subscribe(spy);
const item = new DragItem();

registry.startDragging(item, createMouseEvent('mousedown'));
Expand All @@ -236,7 +236,7 @@ describe('DragDropRegistry', () => {

it('should not dispatch `scroll` events when not dragging', () => {
const spy = jasmine.createSpy('scroll spy');
const subscription = registry.scroll.subscribe(spy);
const subscription = registry.scrolled().subscribe(spy);

dispatchFakeEvent(document, 'scroll');

Expand Down
43 changes: 41 additions & 2 deletions src/cdk/drag-drop/drag-drop-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import {Injectable, NgZone, OnDestroy, Inject} from '@angular/core';
import {DOCUMENT} from '@angular/common';
import {normalizePassiveListenerOptions} from '@angular/cdk/platform';
import {Subject} from 'rxjs';
import {merge, Observable, Observer, Subject} from 'rxjs';

/** Event options that can be used to bind an active, capturing event. */
const activeCapturingEventOptions = normalizePassiveListenerOptions({
Expand Down Expand Up @@ -62,7 +62,11 @@ export class DragDropRegistry<I extends {isDragging(): boolean}, C> implements O
*/
readonly pointerUp: Subject<TouchEvent | MouseEvent> = new Subject<TouchEvent | MouseEvent>();

/** Emits when the viewport has been scrolled while the user is dragging an item. */
/**
* Emits when the viewport has been scrolled while the user is dragging an item.
* @deprecated To be turned into a private member. Use the `scrolled` method instead.
* @breaking-change 13.0.0
*/
readonly scroll: Subject<Event> = new Subject<Event>();

constructor(
Expand Down Expand Up @@ -185,6 +189,41 @@ export class DragDropRegistry<I extends {isDragging(): boolean}, C> implements O
return this._activeDragInstances.indexOf(drag) > -1;
}

/**
* Gets a stream that will emit when any element on the page is scrolled while an item is being
* dragged.
* @param shadowRoot Optional shadow root that the current dragging sequence started from.
* Top-level listeners won't pick up events coming from the shadow DOM so this parameter can
* be used to include an additional top-level listener at the shadow root level.
*/
scrolled(shadowRoot?: DocumentOrShadowRoot | null): Observable<Event> {
const streams: Observable<Event>[] = [this.scroll];

if (shadowRoot && shadowRoot !== this._document) {
// Note that this is basically the same as `fromEvent` from rjxs, but we do it ourselves,
// because we want to guarantee that the event is bound outside of the `NgZone`. With
// `fromEvent` it'll only happen if the subscription is outside the `NgZone`.
streams.push(new Observable((observer: Observer<Event>) => {
return this._ngZone.runOutsideAngular(() => {
const eventOptions = true;
const callback = (event: Event) => {
if (this._activeDragInstances.length) {
observer.next(event);
}
};

(shadowRoot as ShadowRoot).addEventListener('scroll', callback, eventOptions);

return () => {
(shadowRoot as ShadowRoot).removeEventListener('scroll', callback, eventOptions);
};
});
}));
}

return merge(...streams);
}

ngOnDestroy() {
this._dragInstances.forEach(instance => this.removeDragItem(instance));
this._dropInstances.forEach(instance => this.removeDropContainer(instance));
Expand Down
18 changes: 10 additions & 8 deletions src/cdk/drag-drop/drag-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
} from './drag-styling';
import {getTransformTransitionDurationInMs} from './transition-duration';
import {getMutableClientRect, adjustClientRect} from './client-rect';
import {ParentPositionTracker} from './parent-position-tracker';
import {getEventTarget, ParentPositionTracker} from './parent-position-tracker';
import {deepCloneNode} from './clone-node';

/** Object that can be used to configure the behavior of DragRef. */
Expand Down Expand Up @@ -623,7 +623,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 => {
const target = event.target;
const target = getEventTarget(event);
return !!target && (target === handle || handle.contains(target as HTMLElement));
});

Expand Down Expand Up @@ -851,6 +851,7 @@ export class DragRef<T = any> {
const isTouchSequence = isTouchEvent(event);
const isAuxiliaryMouseButton = !isTouchSequence && (event as MouseEvent).button !== 0;
const rootElement = this._rootElement;
const target = getEventTarget(event);
const isSyntheticEvent = !isTouchSequence && this._lastTouchEventTime &&
this._lastTouchEventTime + MOUSE_EVENT_IGNORE_TIME > Date.now();

Expand All @@ -860,7 +861,7 @@ export class DragRef<T = any> {
// it's flaky and it fails if the user drags it away quickly. Also note that we only want
// to do this for `mousedown` since doing the same for `touchstart` will stop any `click`
// events from firing on touch devices.
if (event.target && (event.target as HTMLElement).draggable && event.type === 'mousedown') {
if (target && (target as HTMLElement).draggable && event.type === 'mousedown') {
event.preventDefault();
}

Expand All @@ -884,9 +885,9 @@ export class DragRef<T = any> {
this._removeSubscriptions();
this._pointerMoveSubscription = this._dragDropRegistry.pointerMove.subscribe(this._pointerMove);
this._pointerUpSubscription = this._dragDropRegistry.pointerUp.subscribe(this._pointerUp);
this._scrollSubscription = this._dragDropRegistry.scroll.subscribe(scrollEvent => {
this._updateOnScroll(scrollEvent);
});
this._scrollSubscription = this._dragDropRegistry
.scrolled(this._getShadowRoot())
.subscribe(scrollEvent => this._updateOnScroll(scrollEvent));

if (this._boundaryElement) {
this._boundaryRect = getMutableClientRect(this._boundaryElement);
Expand Down Expand Up @@ -1084,7 +1085,8 @@ export class DragRef<T = any> {
return this._ngZone.runOutsideAngular(() => {
return new Promise(resolve => {
const handler = ((event: TransitionEvent) => {
if (!event || (event.target === this._preview && event.propertyName === 'transform')) {
if (!event || (getEventTarget(event) === this._preview &&
event.propertyName === 'transform')) {
this._preview.removeEventListener('transitionend', handler);
resolve();
clearTimeout(timeout);
Expand Down Expand Up @@ -1379,7 +1381,7 @@ export class DragRef<T = any> {
const scrollDifference = this._parentPositions.handleScroll(event);

if (scrollDifference) {
const target = event.target as Node;
const target = getEventTarget(event);

// ClientRect dimensions are based on the scroll position of the page and its parent node so
// we have to update the cached boundary ClientRect if the user has scrolled. Check for
Expand Down
Loading