From 78c6941caf7ade520892d869ba0877cf3e8389bb Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 6 Aug 2024 17:54:26 +0200 Subject: [PATCH] refactor(cdk/drag-drop): remove unnecessary generics (#29542) Previously we used generics in the `DragDropRegistry` to avoid circular dependencies. Now that we can use type-only imports, we don't need the generics anymore. --- src/cdk/drag-drop/drag-drop-registry.spec.ts | 43 ++++++++----------- src/cdk/drag-drop/drag-drop-registry.ts | 30 ++++++------- src/cdk/drag-drop/drag-drop.ts | 2 +- src/cdk/drag-drop/drag-ref.ts | 2 +- src/cdk/drag-drop/drop-list-ref.ts | 2 +- .../drag-drop/sorting/mixed-sort-strategy.ts | 2 +- .../sorting/single-axis-sort-strategy.ts | 2 +- tools/public_api_guard/cdk/drag-drop.md | 24 +++++------ 8 files changed, 50 insertions(+), 57 deletions(-) diff --git a/src/cdk/drag-drop/drag-drop-registry.spec.ts b/src/cdk/drag-drop/drag-drop-registry.spec.ts index 603b47a0117a..e53b4893f1e8 100644 --- a/src/cdk/drag-drop/drag-drop-registry.spec.ts +++ b/src/cdk/drag-drop/drag-drop-registry.spec.ts @@ -9,10 +9,11 @@ import { } from '../testing/private'; import {DragDropRegistry} from './drag-drop-registry'; import {DragDropModule} from './drag-drop-module'; +import {DragRef} from './drag-ref'; describe('DragDropRegistry', () => { let fixture: ComponentFixture; - let registry: DragDropRegistry; + let registry: DragDropRegistry; beforeEach(fakeAsync(() => { TestBed.configureTestingModule({ @@ -22,13 +23,13 @@ describe('DragDropRegistry', () => { fixture = TestBed.createComponent(BlankComponent); fixture.detectChanges(); - inject([DragDropRegistry], (c: DragDropRegistry) => { + inject([DragDropRegistry], (c: DragDropRegistry) => { registry = c; })(); })); it('should be able to start dragging an item', () => { - const item = new DragItem(); + const item = new DragItem() as unknown as DragRef; expect(registry.isDragging(item)).toBe(false); registry.startDragging(item, createMouseEvent('mousedown')); @@ -36,7 +37,7 @@ describe('DragDropRegistry', () => { }); it('should be able to stop dragging an item', () => { - const item = new DragItem(); + const item = new DragItem() as unknown as DragRef; registry.startDragging(item, createMouseEvent('mousedown')); expect(registry.isDragging(item)).toBe(true); @@ -46,7 +47,7 @@ describe('DragDropRegistry', () => { }); it('should stop dragging an item if it is removed', () => { - const item = new DragItem(); + const item = new DragItem() as unknown as DragRef; registry.startDragging(item, createMouseEvent('mousedown')); expect(registry.isDragging(item)).toBe(true); @@ -58,7 +59,7 @@ describe('DragDropRegistry', () => { it('should dispatch `mousemove` events after starting to drag via the mouse', () => { const spy = jasmine.createSpy('pointerMove spy'); const subscription = registry.pointerMove.subscribe(spy); - const item = new DragItem(true); + const item = new DragItem(true) as unknown as DragRef; registry.startDragging(item, createMouseEvent('mousedown')); dispatchMouseEvent(document, 'mousemove'); @@ -70,7 +71,7 @@ describe('DragDropRegistry', () => { it('should dispatch `touchmove` events after starting to drag via touch', () => { const spy = jasmine.createSpy('pointerMove spy'); const subscription = registry.pointerMove.subscribe(spy); - const item = new DragItem(true); + const item = new DragItem(true) as unknown as DragRef; registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent); dispatchTouchEvent(document, 'touchmove'); @@ -82,7 +83,7 @@ describe('DragDropRegistry', () => { it('should dispatch pointer move events if event propagation is stopped', () => { const spy = jasmine.createSpy('pointerMove spy'); const subscription = registry.pointerMove.subscribe(spy); - const item = new DragItem(true); + const item = new DragItem(true) as unknown as DragRef; fixture.nativeElement.addEventListener('mousemove', (e: MouseEvent) => e.stopPropagation()); registry.startDragging(item, createMouseEvent('mousedown')); dispatchMouseEvent(fixture.nativeElement, 'mousemove'); @@ -95,7 +96,7 @@ describe('DragDropRegistry', () => { it('should dispatch `mouseup` events after ending the drag via the mouse', () => { const spy = jasmine.createSpy('pointerUp spy'); const subscription = registry.pointerUp.subscribe(spy); - const item = new DragItem(); + const item = new DragItem() as unknown as DragRef; registry.startDragging(item, createMouseEvent('mousedown')); dispatchMouseEvent(document, 'mouseup'); @@ -108,7 +109,7 @@ describe('DragDropRegistry', () => { it('should dispatch `touchend` events after ending the drag via touch', () => { const spy = jasmine.createSpy('pointerUp spy'); const subscription = registry.pointerUp.subscribe(spy); - const item = new DragItem(); + const item = new DragItem() as unknown as DragRef; registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent); dispatchTouchEvent(document, 'touchend'); @@ -121,7 +122,7 @@ describe('DragDropRegistry', () => { it('should dispatch pointer up events if event propagation is stopped', () => { const spy = jasmine.createSpy('pointerUp spy'); const subscription = registry.pointerUp.subscribe(spy); - const item = new DragItem(); + const item = new DragItem() as unknown as DragRef; fixture.nativeElement.addEventListener('mouseup', (e: MouseEvent) => e.stopPropagation()); registry.startDragging(item, createMouseEvent('mousedown')); @@ -148,7 +149,7 @@ describe('DragDropRegistry', () => { }); it('should not emit pointer events when dragging is over (multi touch)', () => { - const item = new DragItem(); + const item = new DragItem() as unknown as DragRef; // First finger down registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent); @@ -183,7 +184,7 @@ describe('DragDropRegistry', () => { }); it('should prevent the default `touchmove` action when an item is being dragged', () => { - const item = new DragItem(true); + const item = new DragItem(true) as unknown as DragRef; registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent); expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(true); }); @@ -192,7 +193,7 @@ describe('DragDropRegistry', () => { 'should prevent the default `touchmove` if the item does not consider itself as being ' + 'dragged yet', () => { - const item = new DragItem(false); + const item = new DragItem(false) as unknown as DragRef & DragItem; registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent); expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(false); @@ -202,7 +203,7 @@ describe('DragDropRegistry', () => { ); it('should prevent the default `touchmove` if event propagation is stopped', () => { - const item = new DragItem(true); + const item = new DragItem(true) as unknown as DragRef; registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent); fixture.nativeElement.addEventListener('touchmove', (e: TouchEvent) => e.stopPropagation()); @@ -215,7 +216,7 @@ describe('DragDropRegistry', () => { }); it('should prevent the default `selectstart` action when an item is being dragged', () => { - const item = new DragItem(true); + const item = new DragItem(true) as unknown as DragRef; registry.startDragging(item, createMouseEvent('mousedown')); expect(dispatchFakeEvent(document, 'selectstart').defaultPrevented).toBe(true); }); @@ -223,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.scrolled().subscribe(spy); - const item = new DragItem(); + const item = new DragItem() as unknown as DragRef; registry.startDragging(item, createMouseEvent('mousedown')); dispatchFakeEvent(document, 'scroll'); @@ -247,13 +248,7 @@ describe('DragDropRegistry', () => { return this.shouldBeDragging; } constructor(public shouldBeDragging = false) { - registry.registerDragItem(this); - } - } - - class DragList { - constructor() { - registry.registerDropContainer(this); + registry.registerDragItem(this as unknown as DragRef); } } diff --git a/src/cdk/drag-drop/drag-drop-registry.ts b/src/cdk/drag-drop/drag-drop-registry.ts index 53655ef1dcea..4ae40617f2a0 100644 --- a/src/cdk/drag-drop/drag-drop-registry.ts +++ b/src/cdk/drag-drop/drag-drop-registry.ts @@ -24,6 +24,8 @@ import { signal, } from '@angular/core'; import {Observable, Observer, Subject, merge} from 'rxjs'; +import type {DropListRef} from './drop-list-ref'; +import type {DragRef} from './drag-ref'; /** Event options that can be used to bind an active, capturing event. */ const activeCapturingEventOptions = normalizePassiveListenerOptions({ @@ -48,28 +50,26 @@ const activeApps = new Set(); }) export class _ResetsLoader {} +// TODO(crisbeto): remove generics when making breaking changes. /** * Service that keeps track of all the drag item and drop container * instances, and manages global event listeners on the `document`. * @docs-private */ -// Note: this class is generic, rather than referencing CdkDrag and CdkDropList directly, in order -// to avoid circular imports. If we were to reference them here, importing the registry into the -// classes that are registering themselves will introduce a circular import. @Injectable({providedIn: 'root'}) -export class DragDropRegistry implements OnDestroy { +export class DragDropRegistry<_ = unknown, __ = unknown> implements OnDestroy { private _document: Document; private _appRef = inject(ApplicationRef); private _environmentInjector = inject(EnvironmentInjector); /** Registered drop container instances. */ - private _dropInstances = new Set(); + private _dropInstances = new Set(); /** Registered drag item instances. */ - private _dragInstances = new Set(); + private _dragInstances = new Set(); /** Drag item instances that are currently being dragged. */ - private _activeDragInstances: WritableSignal = signal([]); + private _activeDragInstances: WritableSignal = signal([]); /** Keeps track of the event listeners that we've bound to the `document`. */ private _globalListeners = new Map< @@ -84,7 +84,7 @@ export class DragDropRegistry implements O * Predicate function to check if an item is being dragged. Moved out into a property, * because it'll be called a lot and we don't want to create a new function every time. */ - private _draggingPredicate = (item: I) => item.isDragging(); + private _draggingPredicate = (item: DragRef) => item.isDragging(); /** * Emits the `touchmove` or `mousemove` events that are dispatched @@ -113,14 +113,14 @@ export class DragDropRegistry implements O } /** Adds a drop container to the registry. */ - registerDropContainer(drop: C) { + registerDropContainer(drop: DropListRef) { if (!this._dropInstances.has(drop)) { this._dropInstances.add(drop); } } /** Adds a drag item instance to the registry. */ - registerDragItem(drag: I) { + registerDragItem(drag: DragRef) { this._dragInstances.add(drag); // The `touchmove` event gets bound once, ahead of time, because WebKit @@ -140,12 +140,12 @@ export class DragDropRegistry implements O } /** Removes a drop container from the registry. */ - removeDropContainer(drop: C) { + removeDropContainer(drop: DropListRef) { this._dropInstances.delete(drop); } /** Removes a drag item instance from the registry. */ - removeDragItem(drag: I) { + removeDragItem(drag: DragRef) { this._dragInstances.delete(drag); this.stopDragging(drag); @@ -163,7 +163,7 @@ export class DragDropRegistry implements O * @param drag Drag instance which is being dragged. * @param event Event that initiated the dragging. */ - startDragging(drag: I, event: TouchEvent | MouseEvent) { + startDragging(drag: DragRef, event: TouchEvent | MouseEvent) { // Do not process the same drag twice to avoid memory leaks and redundant listeners if (this._activeDragInstances().indexOf(drag) > -1) { return; @@ -216,7 +216,7 @@ export class DragDropRegistry implements O } /** Stops dragging a drag item instance. */ - stopDragging(drag: I) { + stopDragging(drag: DragRef) { this._activeDragInstances.update(instances => { const index = instances.indexOf(drag); if (index > -1) { @@ -232,7 +232,7 @@ export class DragDropRegistry implements O } /** Gets whether a drag item instance is currently being dragged. */ - isDragging(drag: I) { + isDragging(drag: DragRef) { return this._activeDragInstances().indexOf(drag) > -1; } diff --git a/src/cdk/drag-drop/drag-drop.ts b/src/cdk/drag-drop/drag-drop.ts index c83c219a59e9..9492bc01c0de 100644 --- a/src/cdk/drag-drop/drag-drop.ts +++ b/src/cdk/drag-drop/drag-drop.ts @@ -28,7 +28,7 @@ export class DragDrop { @Inject(DOCUMENT) private _document: any, private _ngZone: NgZone, private _viewportRuler: ViewportRuler, - private _dragDropRegistry: DragDropRegistry, + private _dragDropRegistry: DragDropRegistry, ) {} /** diff --git a/src/cdk/drag-drop/drag-ref.ts b/src/cdk/drag-drop/drag-ref.ts index 376b826b7743..3d79211636d4 100644 --- a/src/cdk/drag-drop/drag-ref.ts +++ b/src/cdk/drag-drop/drag-ref.ts @@ -377,7 +377,7 @@ export class DragRef { private _document: Document, private _ngZone: NgZone, private _viewportRuler: ViewportRuler, - private _dragDropRegistry: DragDropRegistry, + private _dragDropRegistry: DragDropRegistry, ) { this.withRootElement(element).withParent(_config.parentDragRef || null); this._parentPositions = new ParentPositionTracker(_document); diff --git a/src/cdk/drag-drop/drop-list-ref.ts b/src/cdk/drag-drop/drop-list-ref.ts index 983a04943fb3..1338be3cdee1 100644 --- a/src/cdk/drag-drop/drop-list-ref.ts +++ b/src/cdk/drag-drop/drop-list-ref.ts @@ -190,7 +190,7 @@ export class DropListRef { constructor( element: ElementRef | HTMLElement, - private _dragDropRegistry: DragDropRegistry, + private _dragDropRegistry: DragDropRegistry, _document: any, private _ngZone: NgZone, private _viewportRuler: ViewportRuler, diff --git a/src/cdk/drag-drop/sorting/mixed-sort-strategy.ts b/src/cdk/drag-drop/sorting/mixed-sort-strategy.ts index 2bcbf4264180..00d7fcf145e7 100644 --- a/src/cdk/drag-drop/sorting/mixed-sort-strategy.ts +++ b/src/cdk/drag-drop/sorting/mixed-sort-strategy.ts @@ -54,7 +54,7 @@ export class MixedSortStrategy implements DropListSortStrategy { constructor( private _document: Document, - private _dragDropRegistry: DragDropRegistry, + private _dragDropRegistry: DragDropRegistry, ) {} /** diff --git a/src/cdk/drag-drop/sorting/single-axis-sort-strategy.ts b/src/cdk/drag-drop/sorting/single-axis-sort-strategy.ts index 9d3ad3a99702..ed2681216780 100644 --- a/src/cdk/drag-drop/sorting/single-axis-sort-strategy.ts +++ b/src/cdk/drag-drop/sorting/single-axis-sort-strategy.ts @@ -57,7 +57,7 @@ export class SingleAxisSortStrategy implements DropListSortStrategy { /** Layout direction of the drop list. */ direction: Direction; - constructor(private _dragDropRegistry: DragDropRegistry) {} + constructor(private _dragDropRegistry: DragDropRegistry) {} /** * Keeps track of the item that was last swapped with the dragged item, as well as what direction diff --git a/tools/public_api_guard/cdk/drag-drop.md b/tools/public_api_guard/cdk/drag-drop.md index 6611f665f4f7..eac49304fa19 100644 --- a/tools/public_api_guard/cdk/drag-drop.md +++ b/tools/public_api_guard/cdk/drag-drop.md @@ -305,7 +305,7 @@ export type DragConstrainPosition = (point: Point, dragRef: DragRef) => Point; // @public export class DragDrop { - constructor(_document: any, _ngZone: NgZone, _viewportRuler: ViewportRuler, _dragDropRegistry: DragDropRegistry); + constructor(_document: any, _ngZone: NgZone, _viewportRuler: ViewportRuler, _dragDropRegistry: DragDropRegistry); createDrag(element: ElementRef | HTMLElement, config?: DragRefConfig): DragRef; createDropList(element: ElementRef | HTMLElement): DropListRef; // (undocumented) @@ -353,24 +353,22 @@ export class DragDropModule { } // @public -export class DragDropRegistry implements OnDestroy { +export class DragDropRegistry<_ = unknown, __ = unknown> implements OnDestroy { constructor(_ngZone: NgZone, _document: any); - isDragging(drag: I): boolean; + isDragging(drag: DragRef): boolean; // (undocumented) ngOnDestroy(): void; readonly pointerMove: Subject; readonly pointerUp: Subject; - registerDragItem(drag: I): void; - registerDropContainer(drop: C): void; - removeDragItem(drag: I): void; - removeDropContainer(drop: C): void; + registerDragItem(drag: DragRef): void; + registerDropContainer(drop: DropListRef): void; + removeDragItem(drag: DragRef): void; + removeDropContainer(drop: DropListRef): void; // @deprecated readonly scroll: Subject; scrolled(shadowRoot?: DocumentOrShadowRoot | null): Observable; - startDragging(drag: I, event: TouchEvent | MouseEvent): void; - stopDragging(drag: I): void; + startDragging(drag: DragRef, event: TouchEvent | MouseEvent): void; + stopDragging(drag: DragRef): void; // (undocumented) static ɵfac: i0.ɵɵFactoryDeclaration, never>; // (undocumented) @@ -379,7 +377,7 @@ export class DragDropRegistry { - constructor(element: ElementRef | HTMLElement, _config: DragRefConfig, _document: Document, _ngZone: NgZone, _viewportRuler: ViewportRuler, _dragDropRegistry: DragDropRegistry); + constructor(element: ElementRef | HTMLElement, _config: DragRefConfig, _document: Document, _ngZone: NgZone, _viewportRuler: ViewportRuler, _dragDropRegistry: DragDropRegistry); readonly beforeStarted: Subject; constrainPosition?: (userPointerPosition: Point, dragRef: DragRef, dimensions: DOMRect, pickupPositionInElement: Point) => Point; data: T; @@ -480,7 +478,7 @@ export type DropListOrientation = 'horizontal' | 'vertical' | 'mixed'; // @public export class DropListRef { - constructor(element: ElementRef | HTMLElement, _dragDropRegistry: DragDropRegistry, _document: any, _ngZone: NgZone, _viewportRuler: ViewportRuler); + constructor(element: ElementRef | HTMLElement, _dragDropRegistry: DragDropRegistry, _document: any, _ngZone: NgZone, _viewportRuler: ViewportRuler); autoScrollDisabled: boolean; autoScrollStep: number; readonly beforeStarted: Subject;