Skip to content

Commit e8066a1

Browse files
crisbetommalerba
authored andcommitted
fix(cdk/drag-drop): resolve various event tracking issues inside the shadow dom (#23026)
The `drag-drop` module depends on a single document-level `scroll` listener in order to adjust itself when the page or an element is scrolled. The problem is that the events won't be picked up if they come from inside a shadow root. These changes add some logic to bind an extra event at the shadow root level. Furthermore, they fix several places where we were reading the `Event.target` while not accounting for shadow DOM. Fixes #22939. (cherry picked from commit bd08e93)
1 parent 85591e2 commit e8066a1

File tree

7 files changed

+215
-71
lines changed

7 files changed

+215
-71
lines changed

src/cdk/drag-drop/directives/drag.spec.ts

Lines changed: 127 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -44,28 +44,33 @@ const ITEM_WIDTH = 75;
4444

4545
describe('CdkDrag', () => {
4646
function createComponent<T>(
47-
componentType: Type<T>, providers: Provider[] = [], dragDistance = 0,
48-
extraDeclarations: Type<any>[] = []): ComponentFixture<T> {
49-
TestBed
50-
.configureTestingModule({
51-
imports: [DragDropModule, CdkScrollableModule],
52-
declarations: [componentType, PassthroughComponent, ...extraDeclarations],
53-
providers: [
54-
{
55-
provide: CDK_DRAG_CONFIG,
56-
useValue: {
57-
// We default the `dragDistance` to zero, because the majority of the tests
58-
// don't care about it and drags are a lot easier to simulate when we don't
59-
// have to deal with thresholds.
60-
dragStartThreshold: dragDistance,
61-
pointerDirectionChangeThreshold: 5
62-
} as DragDropConfig
63-
},
64-
...providers
65-
],
66-
})
67-
.compileComponents();
47+
componentType: Type<T>, providers: Provider[] = [], dragDistance = 0,
48+
extraDeclarations: Type<any>[] = [], encapsulation?: ViewEncapsulation): ComponentFixture<T> {
49+
TestBed.configureTestingModule({
50+
imports: [DragDropModule, CdkScrollableModule],
51+
declarations: [componentType, PassthroughComponent, ...extraDeclarations],
52+
providers: [
53+
{
54+
provide: CDK_DRAG_CONFIG,
55+
useValue: {
56+
// We default the `dragDistance` to zero, because the majority of the tests
57+
// don't care about it and drags are a lot easier to simulate when we don't
58+
// have to deal with thresholds.
59+
dragStartThreshold: dragDistance,
60+
pointerDirectionChangeThreshold: 5
61+
} as DragDropConfig
62+
},
63+
...providers
64+
],
65+
});
66+
67+
if (encapsulation != null) {
68+
TestBed.overrideComponent(componentType, {
69+
set: {encapsulation}
70+
});
71+
}
6872

73+
TestBed.compileComponents();
6974
return TestBed.createComponent<T>(componentType);
7075
}
7176

@@ -2010,6 +2015,54 @@ describe('CdkDrag', () => {
20102015
});
20112016
}));
20122017

2018+
it('should calculate the index if the list is scrolled while dragging inside the shadow DOM',
2019+
fakeAsync(() => {
2020+
// This test is only relevant for Shadow DOM-supporting browsers.
2021+
if (!_supportsShadowDom()) {
2022+
return;
2023+
}
2024+
2025+
const fixture = createComponent(DraggableInScrollableVerticalDropZone, [], undefined, [],
2026+
ViewEncapsulation.ShadowDom);
2027+
fixture.detectChanges();
2028+
const dragItems = fixture.componentInstance.dragItems;
2029+
const firstItem = dragItems.first;
2030+
const thirdItemRect = dragItems.toArray()[2].element.nativeElement.getBoundingClientRect();
2031+
const list = fixture.componentInstance.dropInstance.element.nativeElement;
2032+
2033+
startDraggingViaMouse(fixture, firstItem.element.nativeElement);
2034+
fixture.detectChanges();
2035+
2036+
dispatchMouseEvent(document, 'mousemove', thirdItemRect.left + 1, thirdItemRect.top + 1);
2037+
fixture.detectChanges();
2038+
2039+
list.scrollTop = ITEM_HEIGHT * 10;
2040+
dispatchFakeEvent(list, 'scroll');
2041+
fixture.detectChanges();
2042+
2043+
dispatchMouseEvent(document, 'mouseup');
2044+
fixture.detectChanges();
2045+
flush();
2046+
fixture.detectChanges();
2047+
2048+
expect(fixture.componentInstance.droppedSpy).toHaveBeenCalledTimes(1);
2049+
2050+
const event = fixture.componentInstance.droppedSpy.calls.mostRecent().args[0];
2051+
2052+
// Assert the event like this, rather than `toHaveBeenCalledWith`, because Jasmine will
2053+
// go into an infinite loop trying to stringify the event, if the test fails.
2054+
expect(event).toEqual({
2055+
previousIndex: 0,
2056+
currentIndex: 12,
2057+
item: firstItem,
2058+
container: fixture.componentInstance.dropInstance,
2059+
previousContainer: fixture.componentInstance.dropInstance,
2060+
isPointerOverContainer: jasmine.any(Boolean),
2061+
distance: {x: jasmine.any(Number), y: jasmine.any(Number)},
2062+
dropPoint: {x: jasmine.any(Number), y: jasmine.any(Number)}
2063+
});
2064+
}));
2065+
20132066
it('should calculate the index if the viewport is scrolled while dragging', fakeAsync(() => {
20142067
const fixture = createComponent(DraggableInDropZone);
20152068

@@ -2260,6 +2313,54 @@ describe('CdkDrag', () => {
22602313
cleanup();
22612314
}));
22622315

2316+
it('should update the boundary if a parent is scrolled while dragging inside the shadow DOM',
2317+
fakeAsync(() => {
2318+
// This test is only relevant for Shadow DOM-supporting browsers.
2319+
if (!_supportsShadowDom()) {
2320+
return;
2321+
}
2322+
2323+
const fixture = createComponent(DraggableInScrollableParentContainer, [], undefined, [],
2324+
ViewEncapsulation.ShadowDom);
2325+
fixture.componentInstance.boundarySelector = '.cdk-drop-list';
2326+
fixture.detectChanges();
2327+
2328+
const container: HTMLElement = fixture.nativeElement.shadowRoot
2329+
.querySelector('.scroll-container');
2330+
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
2331+
const list = fixture.componentInstance.dropInstance.element.nativeElement;
2332+
const cleanup = makeScrollable('vertical', container);
2333+
container.scrollTop = 10;
2334+
2335+
// Note that we need to measure after scrolling.
2336+
let listRect = list.getBoundingClientRect();
2337+
2338+
startDraggingViaMouse(fixture, item);
2339+
startDraggingViaMouse(fixture, item, listRect.right, listRect.bottom);
2340+
flush();
2341+
dispatchMouseEvent(document, 'mousemove', listRect.right, listRect.bottom);
2342+
fixture.detectChanges();
2343+
2344+
const preview = fixture.nativeElement.shadowRoot
2345+
.querySelector('.cdk-drag-preview')! as HTMLElement;
2346+
let previewRect = preview.getBoundingClientRect();
2347+
2348+
// Different browsers round the scroll position differently so
2349+
// assert that the offsets are within a pixel of each other.
2350+
expect(Math.abs(previewRect.bottom - listRect.bottom)).toBeLessThan(2);
2351+
2352+
container.scrollTop = 0;
2353+
dispatchFakeEvent(container, 'scroll');
2354+
fixture.detectChanges();
2355+
listRect = list.getBoundingClientRect(); // We need to update these since we've scrolled.
2356+
dispatchMouseEvent(document, 'mousemove', listRect.right, listRect.bottom);
2357+
fixture.detectChanges();
2358+
previewRect = preview.getBoundingClientRect();
2359+
2360+
expect(Math.abs(previewRect.bottom - listRect.bottom)).toBeLessThan(2);
2361+
cleanup();
2362+
}));
2363+
22632364
it('should clear the id from the preview', fakeAsync(() => {
22642365
const fixture = createComponent(DraggableInDropZone);
22652366
fixture.detectChanges();
@@ -5520,7 +5621,8 @@ describe('CdkDrag', () => {
55205621
return;
55215622
}
55225623

5523-
const fixture = createComponent(ConnectedDropZonesInsideShadowRoot);
5624+
const fixture = createComponent(ConnectedDropZones, [], undefined, [],
5625+
ViewEncapsulation.ShadowDom);
55245626
fixture.detectChanges();
55255627

55265628
const groups = fixture.componentInstance.groupedDragItems;
@@ -5651,7 +5753,8 @@ describe('CdkDrag', () => {
56515753
return;
56525754
}
56535755

5654-
const fixture = createComponent(ConnectedDropZonesInsideShadowRoot);
5756+
const fixture = createComponent(ConnectedDropZones, [], undefined, [],
5757+
ViewEncapsulation.ShadowDom);
56555758
fixture.detectChanges();
56565759
const item = fixture.componentInstance.groupedDragItems[0][1];
56575760

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

@@ -6363,14 +6466,6 @@ class ConnectedDropZones implements AfterViewInit {
63636466
}
63646467
}
63656468

6366-
@Component({
6367-
encapsulation: ViewEncapsulation.ShadowDom,
6368-
styles: CONNECTED_DROP_ZONES_STYLES,
6369-
template: CONNECTED_DROP_ZONES_TEMPLATE
6370-
})
6371-
class ConnectedDropZonesInsideShadowRoot extends ConnectedDropZones {
6372-
}
6373-
63746469
@Component({
63756470
encapsulation: ViewEncapsulation.ShadowDom,
63766471
styles: CONNECTED_DROP_ZONES_STYLES,

src/cdk/drag-drop/drag-drop-registry.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ describe('DragDropRegistry', () => {
224224

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

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

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

241241
dispatchFakeEvent(document, 'scroll');
242242

src/cdk/drag-drop/drag-drop-registry.ts

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import {Injectable, NgZone, OnDestroy, Inject} from '@angular/core';
1010
import {DOCUMENT} from '@angular/common';
1111
import {normalizePassiveListenerOptions} from '@angular/cdk/platform';
12-
import {Subject} from 'rxjs';
12+
import {merge, Observable, Observer, Subject} from 'rxjs';
1313

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

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

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

192+
/**
193+
* Gets a stream that will emit when any element on the page is scrolled while an item is being
194+
* dragged.
195+
* @param shadowRoot Optional shadow root that the current dragging sequence started from.
196+
* Top-level listeners won't pick up events coming from the shadow DOM so this parameter can
197+
* be used to include an additional top-level listener at the shadow root level.
198+
*/
199+
scrolled(shadowRoot?: DocumentOrShadowRoot | null): Observable<Event> {
200+
const streams: Observable<Event>[] = [this.scroll];
201+
202+
if (shadowRoot && shadowRoot !== this._document) {
203+
// Note that this is basically the same as `fromEvent` from rjxs, but we do it ourselves,
204+
// because we want to guarantee that the event is bound outside of the `NgZone`. With
205+
// `fromEvent` it'll only happen if the subscription is outside the `NgZone`.
206+
streams.push(new Observable((observer: Observer<Event>) => {
207+
return this._ngZone.runOutsideAngular(() => {
208+
const eventOptions = true;
209+
const callback = (event: Event) => {
210+
if (this._activeDragInstances.length) {
211+
observer.next(event);
212+
}
213+
};
214+
215+
(shadowRoot as ShadowRoot).addEventListener('scroll', callback, eventOptions);
216+
217+
return () => {
218+
(shadowRoot as ShadowRoot).removeEventListener('scroll', callback, eventOptions);
219+
};
220+
});
221+
}));
222+
}
223+
224+
return merge(...streams);
225+
}
226+
188227
ngOnDestroy() {
189228
this._dragInstances.forEach(instance => this.removeDragItem(instance));
190229
this._dropInstances.forEach(instance => this.removeDropContainer(instance));

src/cdk/drag-drop/drag-ref.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
} from './drag-styling';
2323
import {getTransformTransitionDurationInMs} from './transition-duration';
2424
import {getMutableClientRect, adjustClientRect} from './client-rect';
25-
import {ParentPositionTracker} from './parent-position-tracker';
25+
import {getEventTarget, ParentPositionTracker} from './parent-position-tracker';
2626
import {deepCloneNode} from './clone-node';
2727

2828
/** Object that can be used to configure the behavior of DragRef. */
@@ -623,7 +623,7 @@ export class DragRef<T = any> {
623623
// Delegate the event based on whether it started from a handle or the element itself.
624624
if (this._handles.length) {
625625
const targetHandle = this._handles.find(handle => {
626-
const target = event.target;
626+
const target = getEventTarget(event);
627627
return !!target && (target === handle || handle.contains(target as HTMLElement));
628628
});
629629

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

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

@@ -884,9 +885,9 @@ export class DragRef<T = any> {
884885
this._removeSubscriptions();
885886
this._pointerMoveSubscription = this._dragDropRegistry.pointerMove.subscribe(this._pointerMove);
886887
this._pointerUpSubscription = this._dragDropRegistry.pointerUp.subscribe(this._pointerUp);
887-
this._scrollSubscription = this._dragDropRegistry.scroll.subscribe(scrollEvent => {
888-
this._updateOnScroll(scrollEvent);
889-
});
888+
this._scrollSubscription = this._dragDropRegistry
889+
.scrolled(this._getShadowRoot())
890+
.subscribe(scrollEvent => this._updateOnScroll(scrollEvent));
890891

891892
if (this._boundaryElement) {
892893
this._boundaryRect = getMutableClientRect(this._boundaryElement);
@@ -1084,7 +1085,8 @@ export class DragRef<T = any> {
10841085
return this._ngZone.runOutsideAngular(() => {
10851086
return new Promise(resolve => {
10861087
const handler = ((event: TransitionEvent) => {
1087-
if (!event || (event.target === this._preview && event.propertyName === 'transform')) {
1088+
if (!event || (getEventTarget(event) === this._preview &&
1089+
event.propertyName === 'transform')) {
10881090
this._preview.removeEventListener('transitionend', handler);
10891091
resolve();
10901092
clearTimeout(timeout);
@@ -1379,7 +1381,7 @@ export class DragRef<T = any> {
13791381
const scrollDifference = this._parentPositions.handleScroll(event);
13801382

13811383
if (scrollDifference) {
1382-
const target = event.target as Node;
1384+
const target = getEventTarget(event);
13831385

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

0 commit comments

Comments
 (0)