Skip to content

Commit

Permalink
refactor(cdk/drag-drop): remove unnecessary generics (#29542)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
crisbeto authored Aug 6, 2024
1 parent ef21b82 commit 78c6941
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 57 deletions.
43 changes: 19 additions & 24 deletions src/cdk/drag-drop/drag-drop-registry.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<BlankComponent>;
let registry: DragDropRegistry<DragItem, DragList>;
let registry: DragDropRegistry;

beforeEach(fakeAsync(() => {
TestBed.configureTestingModule({
Expand All @@ -22,21 +23,21 @@ describe('DragDropRegistry', () => {
fixture = TestBed.createComponent(BlankComponent);
fixture.detectChanges();

inject([DragDropRegistry], (c: DragDropRegistry<DragItem, DragList>) => {
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'));
expect(registry.isDragging(item)).toBe(true);
});

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);
Expand All @@ -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);
Expand All @@ -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');

Expand All @@ -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');

Expand All @@ -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');
Expand All @@ -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');
Expand All @@ -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');
Expand All @@ -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'));
Expand All @@ -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);
Expand Down Expand Up @@ -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);
});
Expand All @@ -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);

Expand All @@ -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());

Expand All @@ -215,15 +216,15 @@ 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);
});

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');
Expand All @@ -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);
}
}

Expand Down
30 changes: 15 additions & 15 deletions src/cdk/drag-drop/drag-drop-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -48,28 +50,26 @@ const activeApps = new Set<ApplicationRef>();
})
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<I extends {isDragging(): boolean}, C> 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<C>();
private _dropInstances = new Set<DropListRef>();

/** Registered drag item instances. */
private _dragInstances = new Set<I>();
private _dragInstances = new Set<DragRef>();

/** Drag item instances that are currently being dragged. */
private _activeDragInstances: WritableSignal<I[]> = signal([]);
private _activeDragInstances: WritableSignal<DragRef[]> = signal([]);

/** Keeps track of the event listeners that we've bound to the `document`. */
private _globalListeners = new Map<
Expand All @@ -84,7 +84,7 @@ export class DragDropRegistry<I extends {isDragging(): boolean}, C> 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
Expand Down Expand Up @@ -113,14 +113,14 @@ export class DragDropRegistry<I extends {isDragging(): boolean}, C> 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
Expand All @@ -140,12 +140,12 @@ export class DragDropRegistry<I extends {isDragging(): boolean}, C> 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);

Expand All @@ -163,7 +163,7 @@ export class DragDropRegistry<I extends {isDragging(): boolean}, C> 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;
Expand Down Expand Up @@ -216,7 +216,7 @@ export class DragDropRegistry<I extends {isDragging(): boolean}, C> 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) {
Expand All @@ -232,7 +232,7 @@ export class DragDropRegistry<I extends {isDragging(): boolean}, C> implements O
}

/** Gets whether a drag item instance is currently being dragged. */
isDragging(drag: I) {
isDragging(drag: DragRef) {
return this._activeDragInstances().indexOf(drag) > -1;
}

Expand Down
2 changes: 1 addition & 1 deletion src/cdk/drag-drop/drag-drop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class DragDrop {
@Inject(DOCUMENT) private _document: any,
private _ngZone: NgZone,
private _viewportRuler: ViewportRuler,
private _dragDropRegistry: DragDropRegistry<DragRef, DropListRef>,
private _dragDropRegistry: DragDropRegistry,
) {}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/cdk/drag-drop/drag-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ export class DragRef<T = any> {
private _document: Document,
private _ngZone: NgZone,
private _viewportRuler: ViewportRuler,
private _dragDropRegistry: DragDropRegistry<DragRef, DropListRef>,
private _dragDropRegistry: DragDropRegistry,
) {
this.withRootElement(element).withParent(_config.parentDragRef || null);
this._parentPositions = new ParentPositionTracker(_document);
Expand Down
2 changes: 1 addition & 1 deletion src/cdk/drag-drop/drop-list-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ export class DropListRef<T = any> {

constructor(
element: ElementRef<HTMLElement> | HTMLElement,
private _dragDropRegistry: DragDropRegistry<DragRef, DropListRef>,
private _dragDropRegistry: DragDropRegistry,
_document: any,
private _ngZone: NgZone,
private _viewportRuler: ViewportRuler,
Expand Down
2 changes: 1 addition & 1 deletion src/cdk/drag-drop/sorting/mixed-sort-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export class MixedSortStrategy implements DropListSortStrategy {

constructor(
private _document: Document,
private _dragDropRegistry: DragDropRegistry<DragRef, unknown>,
private _dragDropRegistry: DragDropRegistry,
) {}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/cdk/drag-drop/sorting/single-axis-sort-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class SingleAxisSortStrategy implements DropListSortStrategy {
/** Layout direction of the drop list. */
direction: Direction;

constructor(private _dragDropRegistry: DragDropRegistry<DragRef, unknown>) {}
constructor(private _dragDropRegistry: DragDropRegistry) {}

/**
* Keeps track of the item that was last swapped with the dragged item, as well as what direction
Expand Down
24 changes: 11 additions & 13 deletions tools/public_api_guard/cdk/drag-drop.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<DragRef, DropListRef>);
constructor(_document: any, _ngZone: NgZone, _viewportRuler: ViewportRuler, _dragDropRegistry: DragDropRegistry);
createDrag<T = any>(element: ElementRef<HTMLElement> | HTMLElement, config?: DragRefConfig): DragRef<T>;
createDropList<T = any>(element: ElementRef<HTMLElement> | HTMLElement): DropListRef<T>;
// (undocumented)
Expand Down Expand Up @@ -353,24 +353,22 @@ export class DragDropModule {
}

// @public
export class DragDropRegistry<I extends {
isDragging(): boolean;
}, C> 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<TouchEvent | MouseEvent>;
readonly pointerUp: Subject<TouchEvent | MouseEvent>;
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<Event>;
scrolled(shadowRoot?: DocumentOrShadowRoot | null): Observable<Event>;
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<DragDropRegistry<any, any>, never>;
// (undocumented)
Expand All @@ -379,7 +377,7 @@ export class DragDropRegistry<I extends {

// @public
export class DragRef<T = any> {
constructor(element: ElementRef<HTMLElement> | HTMLElement, _config: DragRefConfig, _document: Document, _ngZone: NgZone, _viewportRuler: ViewportRuler, _dragDropRegistry: DragDropRegistry<DragRef, DropListRef>);
constructor(element: ElementRef<HTMLElement> | HTMLElement, _config: DragRefConfig, _document: Document, _ngZone: NgZone, _viewportRuler: ViewportRuler, _dragDropRegistry: DragDropRegistry);
readonly beforeStarted: Subject<void>;
constrainPosition?: (userPointerPosition: Point, dragRef: DragRef, dimensions: DOMRect, pickupPositionInElement: Point) => Point;
data: T;
Expand Down Expand Up @@ -480,7 +478,7 @@ export type DropListOrientation = 'horizontal' | 'vertical' | 'mixed';

// @public
export class DropListRef<T = any> {
constructor(element: ElementRef<HTMLElement> | HTMLElement, _dragDropRegistry: DragDropRegistry<DragRef, DropListRef>, _document: any, _ngZone: NgZone, _viewportRuler: ViewportRuler);
constructor(element: ElementRef<HTMLElement> | HTMLElement, _dragDropRegistry: DragDropRegistry, _document: any, _ngZone: NgZone, _viewportRuler: ViewportRuler);
autoScrollDisabled: boolean;
autoScrollStep: number;
readonly beforeStarted: Subject<void>;
Expand Down

0 comments on commit 78c6941

Please sign in to comment.