Skip to content

refactor(cdk/drag-drop): remove unnecessary generics #29542

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 1 commit into from
Aug 6, 2024
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
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 @@ -197,7 +197,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
Loading