From 268656d9cffefbfb003b61c38b36865e5224bdfe Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Sat, 20 Sep 2025 14:03:08 +0200 Subject: [PATCH] feat(projectFolders): better tree component using angular cdk drag and drop --- .../nav-list/nav-list-tree.component.html | 16 +- .../nav-list/nav-list-tree.component.ts | 3 - .../ui/tree-dnd/dnd-draggable.directive.ts | 129 ---- .../ui/tree-dnd/dnd-drop-target.directive.ts | 88 --- src/app/ui/tree-dnd/dnd.helpers.spec.ts | 29 - src/app/ui/tree-dnd/dnd.helpers.ts | 30 - src/app/ui/tree-dnd/tree-constants.ts | 5 + src/app/ui/tree-dnd/tree-drag.service.ts | 220 ++++++ .../ui/tree-dnd/tree-drop-zones.component.ts | 81 ++ src/app/ui/tree-dnd/tree-guards.ts | 23 + src/app/ui/tree-dnd/tree-indicator.service.ts | 108 +++ src/app/ui/tree-dnd/tree.component.html | 177 ++--- src/app/ui/tree-dnd/tree.component.scss | 92 +-- src/app/ui/tree-dnd/tree.component.ts | 719 +++++++++++------- src/app/ui/tree-dnd/tree.types.ts | 110 ++- src/app/ui/tree-dnd/tree.utils.ts | 49 +- 16 files changed, 1128 insertions(+), 751 deletions(-) delete mode 100644 src/app/ui/tree-dnd/dnd-draggable.directive.ts delete mode 100644 src/app/ui/tree-dnd/dnd-drop-target.directive.ts delete mode 100644 src/app/ui/tree-dnd/dnd.helpers.spec.ts delete mode 100644 src/app/ui/tree-dnd/dnd.helpers.ts create mode 100644 src/app/ui/tree-dnd/tree-constants.ts create mode 100644 src/app/ui/tree-dnd/tree-drag.service.ts create mode 100644 src/app/ui/tree-dnd/tree-drop-zones.component.ts create mode 100644 src/app/ui/tree-dnd/tree-guards.ts create mode 100644 src/app/ui/tree-dnd/tree-indicator.service.ts diff --git a/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.html b/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.html index 6551c3d60c..37afcff013 100644 --- a/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.html +++ b/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.html @@ -49,9 +49,9 @@ draggable="false" > {{ data.isFolder ? 'folder' : 'assignment' }} - {{ data.node.label }} + {{ + data.node.data?.kind === 'folder' + ? data.node.data.name + : data.node.data?.kind === 'project' + ? data.node.data.project.title + : data.node.data?.tag.title + }} diff --git a/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.ts b/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.ts index 6794db5820..c9fcbda931 100644 --- a/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.ts +++ b/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.ts @@ -133,7 +133,6 @@ export class NavListTreeComponent { if (node.kind === 'folder') { return { id: `folder-${node.id}`, - label: node.name, isFolder: true, expanded: node.isExpanded, data: node, @@ -143,14 +142,12 @@ export class NavListTreeComponent { if (node.kind === 'project') { return { id: `project-${node.project.id}`, - label: node.project.title, isFolder: false, data: node, } satisfies TreeNode; } return { id: `tag-${node.tag.id}`, - label: node.tag.title, isFolder: false, data: node, } satisfies TreeNode; diff --git a/src/app/ui/tree-dnd/dnd-draggable.directive.ts b/src/app/ui/tree-dnd/dnd-draggable.directive.ts deleted file mode 100644 index b4b0798fda..0000000000 --- a/src/app/ui/tree-dnd/dnd-draggable.directive.ts +++ /dev/null @@ -1,129 +0,0 @@ -import { - DestroyRef, - Directive, - effect, - ElementRef, - inject, - input, - TemplateRef, - ViewContainerRef, - Injector, -} from '@angular/core'; -import { draggable } from '@atlaskit/pragmatic-drag-and-drop/element/adapter'; -import { setCustomNativeDragPreview } from '@atlaskit/pragmatic-drag-and-drop/element/set-custom-native-drag-preview'; -import { pointerOutsideOfPreview } from '@atlaskit/pragmatic-drag-and-drop/element/pointer-outside-of-preview'; -import { makeDragData } from './dnd.helpers'; - -@Directive({ - selector: '[dndDraggable]', - standalone: true, -}) -export class DndDraggableDirective { - private el = inject(ElementRef); - private destroyRef = inject(DestroyRef); - private vcr = inject(ViewContainerRef); - - // Inputs (signal-based) - id = input.required({ alias: 'dndDraggable' }); - dndContext = input.required(); - dragPreviewTemplate = input | null>(null); - dragPreviewData = input(null); - - private cleanup: (() => void) | null = null; - - private _bindEffect = effect( - () => { - // Rebind when id/context changes - if (this.cleanup) { - this.cleanup(); - this.cleanup = null; - } - const id = this.id(); - const ctx = this.dndContext(); - this.cleanup = draggable({ - element: this.el.nativeElement, - getInitialData: () => makeDragData(ctx, id), - onGenerateDragPreview: ({ nativeSetDragImage }) => { - setCustomNativeDragPreview({ - getOffset: pointerOutsideOfPreview({ x: '8px', y: '8px' }), - nativeSetDragImage, - render: ({ container }) => { - const template = this.dragPreviewTemplate(); - if (template) { - return this.renderCustomTemplate(template, container); - } else { - return this.renderDefaultPreview(container); - } - }, - }); - }, - }); - }, - { injector: inject(Injector) }, - ); - - constructor() { - this.destroyRef.onDestroy(() => this.cleanup?.()); - } - - private renderCustomTemplate( - template: TemplateRef, - container: HTMLElement, - ): () => void { - // Create embedded view from the template - const context = { - $implicit: this.dragPreviewData(), - id: this.id(), - element: this.el.nativeElement, - }; - - const embeddedView = this.vcr.createEmbeddedView(template, context); - - // Add the rendered template nodes to the container - embeddedView.rootNodes.forEach((node) => { - if (node.nodeType === Node.ELEMENT_NODE) { - container.appendChild(node); - } - }); - - // Manually trigger change detection to ensure the view is properly rendered - embeddedView.detectChanges(); - - return () => { - embeddedView.destroy(); - }; - } - - private renderDefaultPreview(container: HTMLElement): () => void { - // Create a clean preview with just the item's text content - const preview = document.createElement('div'); - preview.style.cssText = ` - background: var(--background-color, #ffffff); - border: 1px solid var(--border-color, #ddd); - border-radius: 4px; - padding: 8px 12px; - font-family: inherit; - font-size: inherit; - color: var(--text-color, #333); - white-space: nowrap; - box-shadow: 0 2px 8px rgba(0,0,0,0.15); - max-width: 300px; - overflow: hidden; - text-overflow: ellipsis; - `; - - // Get the text content of the dragged item - const textElement = this.el.nativeElement.querySelector('.nav-label, .label, span'); - const text = - textElement?.textContent?.trim() || - this.el.nativeElement.textContent?.trim() || - 'Item'; - - preview.textContent = text; - container.appendChild(preview); - - return () => { - preview.remove(); - }; - } -} diff --git a/src/app/ui/tree-dnd/dnd-drop-target.directive.ts b/src/app/ui/tree-dnd/dnd-drop-target.directive.ts deleted file mode 100644 index 5a2b60854c..0000000000 --- a/src/app/ui/tree-dnd/dnd-drop-target.directive.ts +++ /dev/null @@ -1,88 +0,0 @@ -import { - DestroyRef, - Directive, - effect, - ElementRef, - inject, - input, - output, - Injector, -} from '@angular/core'; -import { - dropTargetForElements, - type ElementDropTargetEventBasePayload, -} from '@atlaskit/pragmatic-drag-and-drop/element/adapter'; -import type { DropData } from './tree.types'; -import { asDragData, makeDropData } from './dnd.helpers'; - -@Directive({ - selector: '[dndDropTarget]', - standalone: true, -}) -export class DndDropTargetDirective { - private el = inject(ElementRef); - private destroyRef = inject(DestroyRef); - - // Inputs (modern signal-based) - config = input<{ id: string; where: DropData['where'] } | null>(null, { - alias: 'dndDropTarget', - }); - dndContext = input.required(); - - // Outputs (modern signal-based) - activeChange = output(); - indicator = output<{ - active: boolean; - element: HTMLElement; - where: DropData['where']; - }>(); - - private cleanup: (() => void) | null = null; - - // Rebind drop target whenever config/context changes - private _bindEffect = effect( - () => { - // ensure cleanup of previous binding - this.cleanup?.(); - this.cleanup = null; - - const cfg = this.config(); - if (!cfg) { - return; // disabled target - } - - const where = cfg.where; - this.cleanup = dropTargetForElements({ - element: this.el.nativeElement, - canDrop: ({ source }) => - asDragData(source.data)?.uniqueContextId === this.dndContext(), - getData: () => makeDropData({ type: 'drop', id: cfg.id, where }), - onDragStart: (p) => this.onActive(p, where), - onDropTargetChange: (p) => this.onActive(p, where), - onDragLeave: () => { - this.activeChange.emit(false); - this.indicator.emit({ active: false, element: this.el.nativeElement, where }); - }, - onDrop: () => { - this.activeChange.emit(false); - this.indicator.emit({ active: false, element: this.el.nativeElement, where }); - }, - }); - }, - { injector: inject(Injector) }, - ); - constructor() { - this.destroyRef.onDestroy(() => this.cleanup?.()); - } - - private onActive( - { location, self }: ElementDropTargetEventBasePayload, - where: DropData['where'], - ): void { - const list = location.current.dropTargets; - const innerMost = list[list.length - 1]; - const isActive = innerMost?.element === self.element; - this.activeChange.emit(isActive); - this.indicator.emit({ active: isActive, element: this.el.nativeElement, where }); - } -} diff --git a/src/app/ui/tree-dnd/dnd.helpers.spec.ts b/src/app/ui/tree-dnd/dnd.helpers.spec.ts deleted file mode 100644 index bfe2e2a16b..0000000000 --- a/src/app/ui/tree-dnd/dnd.helpers.spec.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { asDragData, asDropData, makeDragData, makeDropData } from './dnd.helpers'; - -describe('dnd.helpers', () => { - it('creates and parses DragData safely', () => { - const ctx = Symbol('ctx'); - const anyData = makeDragData(ctx, 'X'); - const parsed = asDragData(anyData); - expect(parsed).toBeTruthy(); - expect(parsed!.id).toBe('X'); - expect(parsed!.uniqueContextId).toBe(ctx); - }); - - it('rejects invalid DragData', () => { - const parsed = asDragData({ foo: 'bar' }); - expect(parsed).toBeNull(); - }); - - it('creates and parses DropData safely', () => { - const anyDrop = makeDropData({ type: 'drop', id: 'A', where: 'before' }); - const parsed = asDropData(anyDrop); - expect(parsed).toEqual({ type: 'drop', id: 'A', where: 'before' }); - }); - - it('handles root drop', () => { - const anyDrop = makeDropData({ type: 'drop', id: '', where: 'root' }); - const parsed = asDropData(anyDrop); - expect(parsed).toEqual({ type: 'drop', id: '', where: 'root' }); - }); -}); diff --git a/src/app/ui/tree-dnd/dnd.helpers.ts b/src/app/ui/tree-dnd/dnd.helpers.ts deleted file mode 100644 index 32c96bf898..0000000000 --- a/src/app/ui/tree-dnd/dnd.helpers.ts +++ /dev/null @@ -1,30 +0,0 @@ -import type { DragData, DropData } from './tree.types'; - -type AnyData = Record; - -export const makeDragData = (ctx: symbol, id: string): AnyData => { - const data: DragData = { type: 'item', id, uniqueContextId: ctx }; - return data as unknown as AnyData; -}; - -export const makeDropData = (data: DropData): AnyData => data as unknown as AnyData; - -export const asDragData = (data: unknown): DragData | null => { - if (!data || typeof data !== 'object') return null; - const d = data as Partial; - return d.type === 'item' && - typeof d.id === 'string' && - typeof d.uniqueContextId === 'symbol' - ? ({ type: 'item', id: d.id, uniqueContextId: d.uniqueContextId } as DragData) - : null; -}; - -export const asDropData = (data: unknown): DropData | null => { - if (!data || typeof data !== 'object') return null; - const d = data as Partial; - if (d.type !== 'drop' || typeof d.where !== 'string') return null; - if (d.where === 'root') return { type: 'drop', id: '', where: 'root' }; - if (typeof d.id === 'string') - return { type: 'drop', id: d.id, where: d.where as DropData['where'] }; - return null; -}; diff --git a/src/app/ui/tree-dnd/tree-constants.ts b/src/app/ui/tree-dnd/tree-constants.ts new file mode 100644 index 0000000000..fae11fc3bf --- /dev/null +++ b/src/app/ui/tree-dnd/tree-constants.ts @@ -0,0 +1,5 @@ +export const TREE_CONSTANTS = { + DROP_FLASH_DURATION: 300, + ANIMATION_DURATION: 300, + DEFAULT_INDENT: 16, +} as const; diff --git a/src/app/ui/tree-dnd/tree-drag.service.ts b/src/app/ui/tree-dnd/tree-drag.service.ts new file mode 100644 index 0000000000..f051065eb7 --- /dev/null +++ b/src/app/ui/tree-dnd/tree-drag.service.ts @@ -0,0 +1,220 @@ +import { Injectable } from '@angular/core'; +import { CdkDragDrop } from '@angular/cdk/drag-drop'; +import { + DropWhere, + MoveInstruction, + TreeNode, + TreeId, + HoverTarget, + DropListContext, + CanDropPredicate, + PointerPosition, +} from './tree.types'; + +// Function type definitions for dependency injection +type NodeFinderFn = (id: TreeId) => TreeNode | null; +type NodesFn = () => TreeNode[]; +type SiblingsFn = (parentId: TreeId) => readonly TreeNode[]; +type IsFolderFn = (node: TreeNode) => boolean; +type IsAncestorFn = (ancestorId: TreeId, descendantId: TreeId) => boolean; + +// Drop zone calculation constants for folders +const FOLDER_THRESHOLD_RATIO = 0.2 as const; // 20% of item height for before/after zones (folders) +const FOLDER_MAX_THRESHOLD = 8 as const; // Maximum pixel threshold for folders + +/** + * Service responsible for converting drag/drop interactions into tree move instructions. + * + * This service handles the complex logic of determining where a dragged item should be moved + * based on different types of drop events. It provides two main approaches: + * + * 1. **Hover-based detection**: Uses mouse position to determine precise drop zones + * 2. **Drop-list-based detection**: Uses CDK's container and index information + * + * The service is stateless and requires callback functions to access tree data, + * making it reusable across different tree component instances. + */ +@Injectable() +export class TreeDragService { + /** + * Builds a move instruction based on precise mouse hover position. + * + * This method is used when the user hovers over specific drop zones during drag. + * It provides pixel-perfect feedback for where the item will be dropped. + * + * @param dragId - ID of the item being dragged + * @param hover - Contains the target element, drop zone (before/after/inside), and target ID + * @param findNode - Function to find a node by ID in the tree + * @param getNodes - Function to get all root nodes + * @param isFolder - Function to check if a node can contain children + * @param isNodeAncestor - Function to prevent dropping into descendants + * @param canDrop - Function to validate if the drop is allowed by business rules + * @returns MoveInstruction or null if the drop is invalid + */ + buildInstructionFromHover( + dragId: TreeId, + hover: HoverTarget, + findNode: NodeFinderFn, + getNodes: NodesFn, + isFolder: IsFolderFn, + isNodeAncestor: IsAncestorFn, + canDrop: CanDropPredicate, + ): MoveInstruction | null { + const dragNode = findNode(dragId); + if (!dragNode) return null; + + // Handle dropping to root level (empty area at bottom of tree) + if (hover.where === 'root') { + if (!canDrop({ drag: dragNode, drop: null, where: 'root' })) return null; + const roots = getNodes().filter((n) => n.id !== dragId); + // If no other root items, place as first root item + if (!roots.length) return { itemId: dragId, targetId: '', where: 'inside' }; + // Otherwise, place after the last root item + return { itemId: dragId, targetId: roots[roots.length - 1].id, where: 'after' }; + } + + // Can't drop on self + if (dragId === hover.id) return null; + const targetNode = findNode(hover.id); + // Can't drop on non-existent nodes or into descendants (would create cycles) + if (!targetNode || isNodeAncestor(dragId, hover.id)) return null; + + // Handle dropping inside a folder + if (hover.where === 'inside') { + if ( + !isFolder(targetNode) || + !canDrop({ drag: dragNode, drop: targetNode, where: 'inside' }) + ) + return null; + return { itemId: dragId, targetId: hover.id, where: 'inside' }; + } + + // Handle dropping before/after an item (sibling positioning) + if (!canDrop({ drag: dragNode, drop: targetNode, where: hover.where })) return null; + return { itemId: dragId, targetId: hover.id, where: hover.where }; + } + + /** + * Builds a move instruction based on Angular CDK drop list container and index. + * + * This method is used when items are dropped into CDK drop lists. It's more reliable + * for list ordering but less precise than hover-based detection for folder targeting. + * The CDK provides the target container and the index where the item was dropped. + * + * @param dragId - ID of the item being dragged + * @param event - CDK drop event containing container data and drop index + * @param findNode - Function to find a node by ID in the tree + * @param getSiblings - Function to get siblings of a parent node + * @param isFolder - Function to check if a node can contain children + * @param isNodeAncestor - Function to prevent dropping into descendants + * @param canDrop - Function to validate if the drop is allowed by business rules + * @returns MoveInstruction or null if the drop is invalid + */ + buildInstructionFromDropEvent( + dragId: TreeId, + event: CdkDragDrop>, + findNode: NodeFinderFn, + getSiblings: SiblingsFn, + isFolder: IsFolderFn, + isNodeAncestor: IsAncestorFn, + canDrop: CanDropPredicate, + ): MoveInstruction | null { + const dragNode = findNode(dragId); + if (!dragNode) return null; + + // Extract drop container information + const containerData = event.container.data; + const parentId = containerData?.parentId ?? ''; + // Prevent dropping into descendants (would create invalid tree structure) + if (parentId && isNodeAncestor(dragId, parentId)) return null; + + // Get siblings in the target container, excluding the dragged item + const siblings = (containerData?.items ?? getSiblings(parentId)).filter( + (n) => n.id !== dragId, + ) as TreeNode[]; + // Clamp the drop index to valid range + const index = Math.min(Math.max(event.currentIndex, 0), siblings.length); + + // Handle dropping into root level (top-level container) + if (!parentId) { + if (!canDrop({ drag: dragNode, drop: null, where: 'root' })) return null; + // If no siblings, this becomes the first root item + if (!siblings.length) return { itemId: dragId, targetId: '', where: 'inside' }; + // If dropped past the end, place after the last sibling + if (index >= siblings.length) + return { + itemId: dragId, + targetId: siblings[siblings.length - 1].id, + where: 'after', + }; + // Otherwise, place before the sibling at the drop index + return { itemId: dragId, targetId: siblings[index].id, where: 'before' }; + } + + // Handle dropping into a folder + const parentNode = findNode(parentId); + if (!parentNode || !isFolder(parentNode)) return null; + + // If folder is empty, place as first child + if (!siblings.length) { + if (!canDrop({ drag: dragNode, drop: parentNode, where: 'inside' })) return null; + return { itemId: dragId, targetId: parentId, where: 'inside' }; + } + + // Place relative to sibling at the drop index + const targetId = siblings[Math.min(index, siblings.length - 1)].id; + const targetNode = findNode(targetId); + if (!targetNode) return null; + + // If dropped past the end, place after the last sibling; otherwise before the target sibling + const where = index >= siblings.length ? 'after' : 'before'; + if (!canDrop({ drag: dragNode, drop: targetNode, where })) return null; + return { itemId: dragId, targetId, where }; + } + + /** + * Calculates which drop zone (before/after/inside) the pointer is in. + * This uses proportional thresholds - folders get smaller before/after zones + * to make the "inside" zone easier to target. + * + * Example for a 40px tall item: + * - Files: 10px before, 20px inside, 10px after (25% each side) + * - Folders: 8px before, 24px inside, 8px after (20% each side) + */ + calculateHoverZone( + pointer: PointerPosition, + itemRect: DOMRect, + isTargetFolder?: boolean, + ): DropWhere { + const offsetY = pointer.y - itemRect.top; + + // Different logic for folders vs regular items + if (isTargetFolder) { + // Folders get smaller before/after zones so "inside" is easier to target + const threshold = Math.min( + itemRect.height * FOLDER_THRESHOLD_RATIO, + FOLDER_MAX_THRESHOLD, + ); + if (offsetY < threshold) return 'before'; + if (offsetY > itemRect.height - threshold) return 'after'; + return 'inside'; + } else { + // Regular items: no inside zone, just before/after based on which half + const midpoint = itemRect.height / 2; + return offsetY < midpoint ? 'before' : 'after'; + } + } + + /** + * Simple utility to check if a point is within a rectangle. + * Used for boundary detection during drag operations. + */ + isPointInRect(point: PointerPosition, rect: DOMRect): boolean { + return ( + point.x >= rect.left && + point.x <= rect.right && + point.y >= rect.top && + point.y <= rect.bottom + ); + } +} diff --git a/src/app/ui/tree-dnd/tree-drop-zones.component.ts b/src/app/ui/tree-dnd/tree-drop-zones.component.ts new file mode 100644 index 0000000000..20228f51ae --- /dev/null +++ b/src/app/ui/tree-dnd/tree-drop-zones.component.ts @@ -0,0 +1,81 @@ +import { ChangeDetectionStrategy, Component, input } from '@angular/core'; + +@Component({ + selector: 'tree-drop-zones', + standalone: true, + template: ` +
+
+ `, + styles: [ + ` + .drop { + height: 50%; + pointer-events: none; + width: 100%; + background: transparent; + position: absolute; + z-index: 1; + + :host-context(.tree.is-dragging) & { + pointer-events: all; + background: rgba(0, 123, 255, 0.1); + z-index: 10; + } + + &:hover { + background: rgba(0, 123, 255, 0.15); + } + } + + .drop--before, + .drop--after { + } + + .drop--before { + top: 0px; + + :host-context(.tree.is-dragging) & { + background: rgba(255, 165, 0, 0.1); /* Light orange when dragging */ + } + + &.is-over { + background: rgba( + 255, + 165, + 0, + 0.25 + ) !important; /* Darker orange when hovering */ + } + } + + .drop--after { + bottom: 0px; + + :host-context(.tree.is-dragging) & { + background: rgba(34, 139, 34, 0.1); /* Light green when dragging */ + } + + &.is-over { + background: rgba(34, 139, 34, 0.25) !important; /* Darker green when hovering */ + } + } + `, + ], + changeDetection: ChangeDetectionStrategy.OnPush, +}) +export class TreeDropZonesComponent { + readonly nodeId = input.required(); + readonly overBefore = input(false); + readonly overAfter = input(false); +} diff --git a/src/app/ui/tree-dnd/tree-guards.ts b/src/app/ui/tree-dnd/tree-guards.ts new file mode 100644 index 0000000000..bf2f3d2f2e --- /dev/null +++ b/src/app/ui/tree-dnd/tree-guards.ts @@ -0,0 +1,23 @@ +import { TreeId, TreeNode } from './tree.types'; + +export const isTreeId = (value: unknown): value is TreeId => + typeof value === 'string' && value.length > 0; + +export const isTreeNode = (value: unknown): value is TreeNode => { + if (typeof value !== 'object' || value === null) return false; + + const obj = value as Record; + return 'id' in obj && isTreeId(obj['id']); +}; + +export const assertTreeId: ( + value: unknown, + context?: string, +) => asserts value is TreeId = ( + value: unknown, + context = 'value', +): asserts value is TreeId => { + if (!isTreeId(value)) { + throw new TypeError(`Expected ${context} to be a valid TreeId, got: ${typeof value}`); + } +}; diff --git a/src/app/ui/tree-dnd/tree-indicator.service.ts b/src/app/ui/tree-dnd/tree-indicator.service.ts new file mode 100644 index 0000000000..f39bb153f4 --- /dev/null +++ b/src/app/ui/tree-dnd/tree-indicator.service.ts @@ -0,0 +1,108 @@ +import { computed, Injectable, signal, Signal } from '@angular/core'; +import { DropWhere } from './tree.types'; + +interface IndicatorStyle { + readonly display: string; + readonly top: string; + readonly left: string; + readonly width: string; +} + +interface ElementLayout { + readonly rect: DOMRect; + readonly paddingLeft: number; +} + +interface LastTarget { + readonly element: HTMLElement; + readonly where: DropWhere; +} + +@Injectable() +export class TreeIndicatorService { + private readonly _indicatorTop = signal(0); + private readonly _indicatorLeft = signal(0); + private readonly _indicatorWidth = signal(0); + private readonly _indicatorVisible = signal(false); + + private _containerRect: DOMRect | null = null; + private _elementLayoutCache = new WeakMap(); + private _lastTarget: LastTarget | null = null; + + readonly indicatorStyle: Signal = computed( + () => + ({ + display: this._indicatorVisible() ? 'block' : 'none', + top: `${this._indicatorTop()}px`, + left: `${this._indicatorLeft()}px`, + width: `${this._indicatorWidth()}px`, + }) as const, + ); + + show( + element: HTMLElement, + where: DropWhere, + container: HTMLElement, + indent: number, + ): void { + const sameTarget = + this._lastTarget?.element === element && this._lastTarget?.where === where; + if (sameTarget && this._indicatorVisible()) return; + + this._lastTarget = { element, where }; + this._containerRect ??= container.getBoundingClientRect(); + const containerRect = this._containerRect; + + let elementLayout = this._elementLayoutCache.get(element); + if (!elementLayout) { + const rect = element.getBoundingClientRect(); + const itemEl = element.closest('.item') as HTMLElement | null; + const paddingLeft = itemEl ? this._parsePaddingLeft(itemEl) : 0; + elementLayout = { rect, paddingLeft } as const; + this._elementLayoutCache.set(element, elementLayout); + } + + const elRect = elementLayout.rect; + const isAfter = where === 'after'; + const y = + elRect.top - + containerRect.top + + (isAfter || where === 'inside' ? elRect.height : 0); + + let left = 0; + let width = containerRect.width; + + const itemEl = element.closest('.item') as HTMLElement | null; + if (itemEl) { + const itemRect = itemEl.getBoundingClientRect(); + const paddingLeft = elementLayout.paddingLeft; + const extraIndent = where === 'inside' ? indent : 0; + left = itemRect.left - containerRect.left + paddingLeft + extraIndent; + width = Math.max(0, containerRect.width - left); + } else if (where === 'root') { + left = 0; + width = containerRect.width; + } + + this._indicatorTop.set(Math.round(y)); + this._indicatorLeft.set(Math.max(0, Math.round(left))); + this._indicatorWidth.set(Math.max(0, Math.round(width))); + this._indicatorVisible.set(true); + } + + hide(): void { + this._indicatorVisible.set(false); + this._lastTarget = null; + this._containerRect = null; + } + + clear(): void { + this.hide(); + this._elementLayoutCache = new WeakMap(); + } + + private _parsePaddingLeft(element: HTMLElement): number { + const paddingLeftStr = getComputedStyle(element).paddingLeft || '0'; + return parseFloat(paddingLeftStr) || 0; + } +} diff --git a/src/app/ui/tree-dnd/tree.component.html b/src/app/ui/tree-dnd/tree.component.html index fd06dc3ec4..65e152fdf7 100644 --- a/src/app/ui/tree-dnd/tree.component.html +++ b/src/app/ui/tree-dnd/tree.component.html @@ -3,34 +3,37 @@ role="tree" aria-label="Tree" [class.is-dragging]="draggingId()" - #root + [class.is-drag-invalid]="isDragInvalid()" + cdkDropListGroup > - @for (node of nodes(); track node.id) { - - } -
- - -
- - + @for (node of nodes(); track node.id) { + + } +
+ + @if (draggingId()) { +
+ + +
+ } + @let folder = node.children !== undefined; + @let overInside = isOver(node.id, 'inside'); + @let overBefore = isOver(node.id, 'before'); + @let overAfter = isOver(node.id, 'after'); + @let indentPx = level * indent(); +
-
- -
- @if (isFolder(node)) { - @if (folderTpl(); as tpl) { - - } @else { - - {{ node.expanded ? '▾' : '▸' }} - - {{ node.label }} - } - } @else { - @if (itemTpl(); as tpl) { - - } @else { - {{ node.label }} - } + @if (folder) { + @if (folderTpl(); as tpl) { + } -
- -
+ } @else { + @if (itemTpl(); as tpl) { + + } + }
- @if (node.children?.length) { + @if (folder) { + @let childLevel = level + 1;
- @for (child of node.children; track child.id) { + @for (child of node.children ?? []; track child.id) { }
diff --git a/src/app/ui/tree-dnd/tree.component.scss b/src/app/ui/tree-dnd/tree.component.scss index be5caa3f00..ecf99b2d6d 100644 --- a/src/app/ui/tree-dnd/tree.component.scss +++ b/src/app/ui/tree-dnd/tree.component.scss @@ -5,32 +5,39 @@ .tree { display: block; position: relative; + + &.is-dragging { + cursor: grabbing; + + * { + cursor: grabbing !important; + } + + &.is-drag-invalid { + cursor: not-allowed; + + * { + cursor: not-allowed !important; + } + } + } } .item { position: relative; + padding-left: var(--tree-indent, 0); &.is-dragging { opacity: 0.4; + pointer-events: none; } &.is-over-inside { - background: greenyellow; + background: rgba(138, 43, 226, 0.2); /* Purple for "inside" folder */ } } -.row { - min-height: 28px; - user-select: none; - -webkit-user-select: none; - cursor: grab; - - &.is-over { - background: greenyellow !important; - } -} - -.item.just-dropped > .row { +.item.was-just-dropped { background: gold; } @@ -40,52 +47,20 @@ user-select: none; } -.drop { - height: 12px; - pointer-events: none; - width: 100%; - background: transparent; - position: absolute; - z-index: 1; - - .is-dragging & { - pointer-events: all; - background: rgba(0, 123, 255, 0.1); - } - - &:hover { - background: rgba(0, 123, 255, 0.15); - } - - .item:not(.is-folder) & { - height: 16px; - } -} - -.drop--before, -.drop--after { - border-top: 2px solid transparent; -} - -.drop--before { - top: 0; -} - -.drop--after { - bottom: 0; -} - -.group { +.folder { margin-left: 0; } .root-drop { - height: 10px; + height: 20px; border-top: 2px dashed transparent; + position: relative; + z-index: 5; - &.is-over { - border-top-color: #007bff; - background: rgba(0, 123, 255, 0.1); + &.is-root-over { + border-top-color: #dc3545; + background: rgba(220, 53, 69, 0.15); /* Red for root drop (end of list) */ + z-index: 15; } &:hover { @@ -93,11 +68,22 @@ } } +.drag-preview { + background: var(--tree-preview-bg, #ffffff); + border: 1px solid var(--tree-preview-border, #ddd); + border-radius: 4px; + padding: 6px 10px; + box-shadow: 0 2px 8px rgba(0, 0, 0, 0.15); + white-space: nowrap; + pointer-events: none; +} + /* indicator */ .indicator { position: absolute; height: 0; pointer-events: none; + display: none; } .indicator-dot { diff --git a/src/app/ui/tree-dnd/tree.component.ts b/src/app/ui/tree-dnd/tree.component.ts index 70ed62c57a..b59e03d2a9 100644 --- a/src/app/ui/tree-dnd/tree.component.ts +++ b/src/app/ui/tree-dnd/tree.component.ts @@ -1,38 +1,49 @@ import { - AfterViewInit, ChangeDetectionStrategy, Component, contentChild, DestroyRef, - effect, ElementRef, inject, input, + model, output, signal, TemplateRef, } from '@angular/core'; -import { trigger, state, style, transition, animate } from '@angular/animations'; -import { NgTemplateOutlet } from '@angular/common'; -import { monitorForElements } from '@atlaskit/pragmatic-drag-and-drop/element/adapter'; +import { animate, state, style, transition, trigger } from '@angular/animations'; +import { NgClass, NgStyle, NgTemplateOutlet } from '@angular/common'; import { - DragData, - DropData, + CdkDrag, + CdkDragDrop, + CdkDragEnd, + CdkDragMove, + CdkDropList, + DragDropModule, +} from '@angular/cdk/drag-drop'; +import { + CanDropPredicate, + DropListContext, DropWhere, FolderTplContext, + HoverTarget, ItemTplContext, MoveInstruction, + PointerPosition, + TreeId, TreeNode, } from './tree.types'; import { moveNode } from './tree.utils'; -import { DndDraggableDirective } from './dnd-draggable.directive'; -import { DndDropTargetDirective } from './dnd-drop-target.directive'; -import { asDragData, asDropData } from './dnd.helpers'; +import { TreeDragService } from './tree-drag.service'; +import { TreeIndicatorService } from './tree-indicator.service'; +import { TREE_CONSTANTS } from './tree-constants'; +import { assertTreeId } from './tree-guards'; @Component({ selector: 'tree-dnd', standalone: true, - imports: [NgTemplateOutlet, DndDraggableDirective, DndDropTargetDirective], + imports: [NgTemplateOutlet, NgStyle, NgClass, DragDropModule], + providers: [TreeIndicatorService, TreeDragService], templateUrl: './tree.component.html', styleUrls: ['./tree.component.scss'], changeDetection: ChangeDetectionStrategy.OnPush, @@ -56,57 +67,68 @@ import { asDragData, asDropData } from './dnd.helpers'; ), transition('collapsed <=> expanded', [ style({ overflow: 'hidden' }), - animate('300ms ease-in-out'), + animate(`${TREE_CONSTANTS.ANIMATION_DURATION}ms ease-in-out`), ]), ]), ], }) -export class TreeDndComponent implements AfterViewInit { - private host = inject(ElementRef); - private destroyRef = inject(DestroyRef); +export class TreeDndComponent { + // === INJECTED DEPENDENCIES === + private readonly _host = inject(ElementRef); + private readonly _destroyRef = inject(DestroyRef); + private readonly _dragService = inject(TreeDragService); + private readonly _indicatorService = inject(TreeIndicatorService); - // Unique context to keep interactions scoped to one tree - private ctx = Symbol('tree-dnd'); - // expose to template - contextId = this.ctx; + // === PUBLIC INPUTS/OUTPUTS/MODELS === + readonly nodes = model.required[]>(); + readonly indent = input(TREE_CONSTANTS.DEFAULT_INDENT); + readonly canDrop = input>(() => true); + readonly moved = output(); - // Inputs (signal-based) - readonly data = input.required(); - readonly indent = input(16); - // Allow users to control drop permission (default: allow all valid moves) - readonly canDrop = input< - (ctx: { drag: TreeNode; drop: TreeNode | null; where: DropWhere }) => boolean - >(() => true); + // === PUBLIC CONTENT CHILDREN === + readonly itemTpl = contentChild>>('treeItem'); + readonly folderTpl = contentChild>>('treeFolder'); - // Local state mirrored from input for internal reordering - readonly nodes = signal([]); - private nodeCache = signal>(new Map()); + // === PUBLIC SIGNALS === + readonly draggingId = signal(null); + readonly justDroppedId = signal(null); + readonly isDragInvalid = signal(false); + readonly isRootOver = signal(false); + readonly indicatorStyle = this._indicatorService.indicatorStyle; - private _syncEffect = effect(() => { - const newNodes = this.data() ?? []; - this.nodes.set(newNodes); - this.rebuildNodeCache(newNodes); - }); + // === PRIVATE STATE === + /** + * Tracks which drop zones are currently being hovered over. + * Structure: { nodeId: { before: true, after: false, inside: false } } + * This allows multiple drop zones to be highlighted simultaneously if needed. + */ + private readonly _overMap = signal>>>( + {}, + ); + private _hoverTarget: HoverTarget | null = null; + private _treeRootEl: HTMLElement | null = null; + private _rootDropEl: HTMLElement | null = null; + private _dropHandled = false; + private _dropFlashTimer?: number; - // Content templates (signal-based) - readonly itemTpl = contentChild>('treeItem'); - readonly folderTpl = contentChild>('treeFolder'); - readonly dragPreviewTpl = contentChild>('treeDragPreview'); + constructor() { + this._destroyRef.onDestroy(() => { + if (this._dropFlashTimer) clearTimeout(this._dropFlashTimer); + this._indicatorService.clear(); + }); + } - // Output: notify consumers of moves - moved = output(); - // Output: emit latest tree after internal changes - updated = output(); + // === PUBLIC METHODS === + readonly canEnterList = ( + drag: CdkDrag, + drop: CdkDropList>, + ): boolean => this._canEnterListPredicate(drag, drop); - // drag highlight state (ids mapped to where states) - private overMap = signal>>>({}); - - setOver(id: string, where: DropWhere, on: boolean): void { - // Track hover state - this.overMap.update((m) => { + setOver(id: TreeId, where: DropWhere, on: boolean): void { + this._overMap.update((m) => { const current = m[id]?.[where] ?? false; if (current === on) { - return m; // Atlaskit fires multiple hover events per frame; skip no-op updates to avoid needless change detection. + return m; // No change needed } const entry = { ...(m[id] ?? {}) }; entry[where] = on; @@ -114,281 +136,422 @@ export class TreeDndComponent implements AfterViewInit { }); } - isOver(id: string, where: DropWhere): boolean { - return !!this.overMap()[id]?.[where]; + isOver(id: TreeId, where: DropWhere): boolean { + return !!this._overMap()[id]?.[where]; } - rootOver = signal(false); - draggingId = signal(null); - // recent drop flash state - justDroppedId = signal(null); - private _dropFlashTimer?: number; - // trackBy not needed with @for track expressions + toggle(node: TreeNode): void { + if (!node.children?.length) return; + node.expanded = !node.expanded; + const updatedNodes = [...this.nodes()] as TreeNode[]; + this.nodes.set(updatedNodes); + } - // Indicator state - indicatorVisible = signal(false); - indicatorTop = signal(0); - indicatorLeft = signal(0); - indicatorWidth = signal(0); - private treeRootEl: HTMLElement | null = null; - private containerRect: DOMRect | null = null; - private elementLayoutCache: WeakMap< - HTMLElement, - { rect: DOMRect; paddingLeft: number } - > = new WeakMap(); - private lastIndicatorTarget: { element: HTMLElement; where: DropWhere } | null = null; + // === EVENT HANDLERS === + onDrop(event: CdkDragDrop>): void { + const dragId = this._extractDragId(event.item.data); + this.draggingId.set(null); + this._dropHandled = true; - onIndicator(evt: { active: boolean; element: HTMLElement; where: DropWhere }): void { - if (!evt.active) { - this.indicatorVisible.set(false); - this.lastIndicatorTarget = null; - this.containerRect = null; + // if we have no dragId it means we dragged an invalid element, so we just reset + if (!dragId) { + this._scheduleItemReset(event.item); + this._clearHoverStates(); return; } - // No special-casing: indicator follows active drop target - this.treeRootEl ??= this.host.nativeElement.querySelector('.tree') as HTMLElement; - const container = this.treeRootEl; - if (!container) { + + const instruction = this._getDropInstruction(dragId, event); + const wasMoved = instruction ? this._applyInstruction(instruction) : false; + + if (wasMoved) { + this._flashJustDropped(dragId); + } + + // Always schedule reset to ensure proper cleanup + this._scheduleItemReset(event.item); + this._clearHoverStates(); + } + + onDragStarted(id: TreeId): void { + try { + assertTreeId(id, 'drag ID'); + this.draggingId.set(id); + this.isDragInvalid.set(false); + this._hoverTarget = null; + this._dropHandled = false; + this.isRootOver.set(false); + this._treeRootEl = this._host.nativeElement.querySelector('.tree'); + this._rootDropEl = this._host.nativeElement.querySelector('.root-drop'); + this._indicatorService.clear(); + } catch (error) { + console.error('Invalid drag start:', error); + } + } + + onDragMoved(event: CdkDragMove): void { + const dragId = this._extractDragId(event.source.data); + if (!dragId) return; + + const pointer = event.pointerPosition; + if (!pointer) return; + + this._treeRootEl ??= this._host.nativeElement.querySelector('.tree'); + this._rootDropEl ??= this._host.nativeElement.querySelector('.root-drop'); + + const target = this._findHoverTarget(pointer); + this._updateHover(dragId, target); + + // Clear hover if dragged outside tree boundaries + if (!target && this._hoverTarget) { + this._setHoverTarget(null); + } + } + + onDragEnded(event: CdkDragEnd): void { + const dragId = this._extractDragId(event.source.data); + + if (!this._dropHandled && dragId) { + if (this._hoverTarget) { + const instruction = this._buildInstructionFromHover(dragId, this._hoverTarget); + if (instruction && this._applyInstruction(instruction)) { + this._flashJustDropped(dragId); + } + } + // Always reset when drag ends without a drop, regardless of hover target + this._scheduleItemReset(event.source); + } + + this.draggingId.set(null); + this.isDragInvalid.set(false); + this._dropHandled = false; + this._clearHoverStates(); + } + + // === PRIVATE METHODS === + private _canEnterListPredicate( + drag: CdkDrag, + drop: CdkDropList>, + ): boolean { + const data = drop.data; + const dragId = this._extractDragId(drag.data); + if (!dragId || !data) { + return false; + } + if (data.parentId && this._isNodeAncestor(dragId, data.parentId)) { + return false; + } + const dragNode = this._findNode(dragId); + if (!dragNode) { + return false; + } + if (!data.parentId) { + return this.canDrop()({ drag: dragNode, drop: null, where: 'root' }); + } + const parentNode = this._findNode(data.parentId); + if (!parentNode || parentNode.children === undefined) { + return false; + } + return this.canDrop()({ drag: dragNode, drop: parentNode, where: 'inside' }); + } + + /** + * Determines what element the user is hovering over during drag operations. + * This is complex because we need to: + * 1. Respect tree boundaries (don't allow drops outside the tree) + * 2. Handle the special "root drop" zone at the bottom + * 3. Calculate drop zones (before/after/inside) based on pointer position + * 4. Return the correct element for indicator positioning + */ + private _findHoverTarget(pointer: PointerPosition): HoverTarget | null { + // First check: Are we even within the tree boundaries? + if (this._treeRootEl) { + const treeRect = this._treeRootEl.getBoundingClientRect(); + if (!this._dragService.isPointInRect(pointer, treeRect)) { + return null; // Outside tree boundaries - show invalid cursor + } + } + + // Special case: Check if hovering over the root drop zone (bottom of tree) + if (this._rootDropEl) { + const rect = this._rootDropEl.getBoundingClientRect(); + if (this._dragService.isPointInRect(pointer, rect)) { + return { id: '', where: 'root', element: this._rootDropEl }; + } + } + + // Find the actual DOM element under the mouse pointer + const element = document.elementFromPoint(pointer.x, pointer.y) as HTMLElement | null; + if (!element) return null; + + // Navigate up the DOM to find the nearest tree item container + const itemEl = element.closest('.item') as HTMLElement | null; + if (!itemEl || !this._treeRootEl?.contains(itemEl)) { + return null; // Element not within this tree instance + } + + // Extract the node ID from the data attribute + const nodeId = itemEl.getAttribute('data-node-id') as TreeId | null; + if (!nodeId) return null; + + const itemRect = itemEl.getBoundingClientRect(); + + // Calculate drop zone: folders get larger "inside" zones for easier targeting + const targetNode = this._findNode(nodeId); + const isTargetFolder = targetNode ? targetNode.children !== undefined : false; + const where = this._dragService.calculateHoverZone(pointer, itemRect, isTargetFolder); + + return { id: nodeId, where, element: itemEl }; + } + + private _updateHover(dragId: string, target: HoverTarget | null): void { + const isValid = target && this._isHoverAllowed(dragId, target); + + // Show invalid cursor when hovering outside tree or over invalid targets + this.isDragInvalid.set(!isValid); + + if (!isValid) { + this._setHoverTarget(null); return; } - const sameTarget = - this.lastIndicatorTarget?.element === evt.element && - this.lastIndicatorTarget?.where === evt.where; - if (sameTarget && this.indicatorVisible()) { - // Skip recomputing layout for repeated pointer move events over the same drop zone + this._setHoverTarget(target); + } + + private _isHoverAllowed(dragId: string, target: HoverTarget): boolean { + return this._buildInstructionFromHover(dragId, target) !== null; + } + + /** + * Updates the current hover target and manages visual feedback. + * This handles the complex state transitions between different hover targets: + * - Clearing previous hover states (remove highlights) + * - Setting new hover states (add highlights) + * - Managing the drop indicator position + * - Handling special "root" hover state separately + */ + private _setHoverTarget(next: HoverTarget | null): void { + const prev = this._hoverTarget; + + // Optimization: if we're hovering over the same target, just update indicator + if (prev && next && prev.id === next.id && prev.where === next.where) { + this._hoverTarget = next; + this._showIndicator(next); return; } - this.lastIndicatorTarget = { element: evt.element, where: evt.where }; - this.containerRect ??= container.getBoundingClientRect(); - const containerRect = this.containerRect; - let elementLayout = this.elementLayoutCache.get(evt.element); - if (!elementLayout) { - const rect = evt.element.getBoundingClientRect(); - const itemEl = evt.element.closest('.item') as HTMLElement | null; - const paddingLeft = itemEl - ? parseFloat(getComputedStyle(itemEl).paddingLeft || '0') || 0 - : 0; - elementLayout = { rect, paddingLeft }; - this.elementLayoutCache.set(evt.element, elementLayout); - } - const elRect = elementLayout.rect; - - // Vertical position: before/after lines snap to top/bottom of the drop zone - // For inside: draw at the bottom of the row to suggest insertion as first child - const isAfter = evt.where === 'after'; - const y = - elRect.top - - containerRect.top + - (isAfter || evt.where === 'inside' ? elRect.height : 0); - - // Horizontal position: reflect the level the item will be dropped into - // - root: level 0 (no indent) - // - before/after: same level as the target item (its container padding-left) - // - inside: one level deeper than the target item - let left = 0; - let width = containerRect.width; - - const itemEl = evt.element.closest('.item') as HTMLElement | null; - if (itemEl) { - const itemRect = itemEl.getBoundingClientRect(); - const paddingLeft = this.elementLayoutCache.get(evt.element)?.paddingLeft ?? 0; - const extraIndent = evt.where === 'inside' ? this.indent() : 0; - left = itemRect.left - containerRect.left + paddingLeft + extraIndent; - width = Math.max(0, containerRect.width - left); - } else if (evt.where === 'root') { - left = 0; - width = containerRect.width; + // Clear previous hover visual feedback + if (prev) { + if (prev.where === 'root') { + this.isRootOver.set(false); + } else { + this.setOver(prev.id, prev.where, false); + } } - this.indicatorTop.set(Math.round(y)); - this.indicatorLeft.set(Math.max(0, Math.round(left))); - this.indicatorWidth.set(Math.max(0, Math.round(width))); - if (!this.indicatorVisible()) { - this.indicatorVisible.set(true); + // Handle clearing hover state + if (!next) { + this._hoverTarget = null; + this._indicatorService.hide(); + return; } + + // Set new hover state and visual feedback + this._hoverTarget = next; + if (next.where === 'root') { + this.isRootOver.set(true); + } else { + this.setOver(next.id, next.where, true); + } + this._showIndicator(next); } - onRootActive(active: boolean): void { - this.rootOver.set(active); - } + /** + * Determines the final move instruction when a drop occurs. + * This is complex because we need to handle two different types of drop detection: + * 1. Hover-based drops (precise positioning for folders) + * 2. Drop-list-based drops (reliable for list ordering) + * + * We prioritize hover instructions for "inside" folder drops because they provide + * better UX - users can see exactly where they're dropping via the indicator. + */ + private _getDropInstruction( + dragId: TreeId, + event: CdkDragDrop>, + ): MoveInstruction | null { + // Get hover-based instruction (more precise for folders) + let hoverInstruction: MoveInstruction | null = null; + if (this._hoverTarget) { + hoverInstruction = this._buildInstructionFromHover(dragId, this._hoverTarget); + } - isFolder(n: TreeNode): boolean { - return !!n.isFolder || n.children !== undefined; - } - - ngAfterViewInit(): void { - const cleanup: Array<() => void> = []; - - // Register monitor only (drop-targets + draggables are directives) - cleanup.push( - monitorForElements({ - canMonitor: ({ source }) => asDragData(source.data)?.uniqueContextId === this.ctx, - onDragStart: ({ source }) => { - const data = asDragData(source.data); - if (data) this.draggingId.set(data.id); - this.treeRootEl ??= this.host.nativeElement.querySelector( - '.tree', - ) as HTMLElement; - this.containerRect = this.treeRootEl?.getBoundingClientRect() ?? null; - this.elementLayoutCache = new WeakMap(); - }, - onDrop: ({ location, source }) => { - this.draggingId.set(null); - const dropTargets = location.current.dropTargets; - if (!dropTargets.length) { - this.clearOverStates(); - return; - } - const s = asDragData(source.data); - // Use innermost drop target (last in list for this adapter) - const target = asDropData(dropTargets[dropTargets.length - 1].data); - if (!s || !target) { - this.clearOverStates(); - return; - } - - // Validate drop operation - if (!this.isValidDrop(s, target)) { - this.clearOverStates(); - return; - } - if (target.where === 'root') { - // Drop at the very end of the root list - const currentRoots = this.nodes(); - const lastRoot = [...currentRoots].reverse().find((n) => n.id !== s.id); - const instr: MoveInstruction = lastRoot - ? { itemId: s.id, targetId: lastRoot.id, where: 'after' } - : { itemId: s.id, targetId: '', where: 'inside' }; // empty root: insert as only item - const updatedNodes = moveNode(this.nodes(), instr); - this.nodes.set(updatedNodes); - this.rebuildNodeCache(updatedNodes); - this.moved.emit(instr); - this.updated.emit(this.nodes()); - this.flashJustDropped(s.id); - this.clearOverStates(); - return; - } - if (target.where === 'inside') { - const targetNode = this.findNode(target.id); - if (!targetNode) { - console.warn(`Drop target node with id '${target.id}' not found`); - this.clearOverStates(); - return; - } - if (!this.isFolder(targetNode)) { - this.clearOverStates(); - return; // disallow dropping inside items - } - } - const instr: MoveInstruction = { - itemId: s.id, - targetId: target.id, - where: target.where, - } as MoveInstruction; - const updatedNodes = moveNode(this.nodes(), instr); - this.nodes.set(updatedNodes); - this.rebuildNodeCache(updatedNodes); - this.moved.emit(instr); - this.updated.emit(this.nodes()); - this.flashJustDropped(s.id); - this.clearOverStates(); - }, - }), + // Get drop-list-based instruction (more reliable for ordering) + const dropInstruction = this._dragService.buildInstructionFromDropEvent( + dragId, + event, + this._findNode.bind(this), + this._getSiblings.bind(this), + (node: TreeNode) => node.children !== undefined, + this._isNodeAncestor.bind(this), + this.canDrop(), ); - this.destroyRef.onDestroy(() => { - cleanup.forEach((fn) => fn()); - if (this._dropFlashTimer) clearTimeout(this._dropFlashTimer); - this.justDroppedId.set(null); - }); + // Prioritize hover instruction for "inside" folder drops + // This gives users better visual feedback and more precise control + if (hoverInstruction?.where === 'inside' && hoverInstruction.targetId) { + const targetNode = this._findNode(hoverInstruction.targetId); + if (targetNode && targetNode.children !== undefined) { + return hoverInstruction; + } + } + + // Fall back to drop instruction for ordering, or hover instruction as last resort + return dropInstruction ?? hoverInstruction; } - private clearOverStates(): void { - this.overMap.set({}); - this.rootOver.set(false); - this.indicatorVisible.set(false); - this.lastIndicatorTarget = null; - this.containerRect = null; - this.elementLayoutCache = new WeakMap(); + /** + * Resets a dragged item to its original position with proper timing. + * We use requestAnimationFrame to ensure the reset happens after Angular's + * change detection cycle, preventing visual glitches. + */ + private _scheduleItemReset(item: CdkDrag): void { + const reset = (): void => { + try { + item.reset(); + // Access CDK internal API to ensure complete reset + const dragRef = (item as { _dragRef?: { reset: () => void } })._dragRef; + dragRef?.reset(); + } catch (error) { + // Ignore reset errors - they can happen if the item is already destroyed + console.debug('Failed to reset drag item:', error); + } + }; + + if (typeof requestAnimationFrame === 'function') { + requestAnimationFrame(reset); + } else { + reset(); + } } - private rebuildNodeCache(nodes: TreeNode[]): void { - const cache = new Map(); + private _showIndicator(target: HoverTarget): void { + if (!this._treeRootEl) return; + this._indicatorService.show( + target.element, + target.where, + this._treeRootEl, + this.indent(), + ); + } + + private _clearHoverStates(): void { + this._hoverTarget = null; + this._overMap.set({}); + this.isRootOver.set(false); + this._indicatorService.hide(); + this._rootDropEl = null; + } + + private _traverseNodes( + nodes: TreeNode[], + callback: (node: TreeNode) => void, + ): void { const stack = [...nodes]; while (stack.length) { const node = stack.pop()!; - cache.set(node.id, node); - if (node.children) { + callback(node); + if (node.children?.length) { stack.push(...node.children); } } - this.nodeCache.set(cache); } - private findNode(id: string): TreeNode | null { - return this.nodeCache().get(id) ?? null; + private _getSiblings(parentId: TreeId): TreeNode[] { + if (!parentId) return this.nodes() as TreeNode[]; + return this._findNode(parentId)?.children ?? []; } - private isValidDrop(dragData: DragData, dropData: DropData): boolean { - const dragNode = this.findNode(dragData.id); - if (!dragNode) return false; - - if (dropData.where === 'root') { - return this.canDrop()({ drag: dragNode, drop: null, where: 'root' }); - } - - // Prevent dropping item onto itself - if (dragData.id === dropData.id) return false; - - const targetNode = this.findNode(dropData.id); - if (!targetNode) return false; - if (this.isNodeAncestor(dragData.id, dropData.id)) return false; - if (dropData.where === 'inside' && !this.isFolder(targetNode)) return false; - - return this.canDrop()({ drag: dragNode, drop: targetNode, where: dropData.where }); - } - - // removed disallowed hover evaluation - - private isNodeAncestor(ancestorId: string, possibleDescendantId: string): boolean { - const ancestorNode = this.findNode(ancestorId); - if (!ancestorNode?.children) return false; - - const stack = [...ancestorNode.children]; + private readonly _findNode = (id: TreeId): TreeNode | null => { + const stack = [...this.nodes()] as TreeNode[]; while (stack.length) { const node = stack.pop()!; - if (node.id === possibleDescendantId) return true; - if (node.children) stack.push(...node.children); + if (node.id === id) return node; + if (node.children?.length) { + stack.push(...node.children); + } } - return false; + return null; + }; + + private _buildInstructionFromHover( + dragId: TreeId, + hover: HoverTarget, + ): MoveInstruction | null { + return this._dragService.buildInstructionFromHover( + dragId, + hover, + this._findNode.bind(this), + () => this.nodes() as TreeNode[], + (node: TreeNode) => node.children !== undefined, + this._isNodeAncestor.bind(this), + this.canDrop(), + ); } - toggle(node: TreeNode): void { - if (!node.children?.length) return; - node.expanded = !node.expanded; - // trigger change and update cache - const updatedNodes = [...this.nodes()]; + private _applyInstruction(instr: MoveInstruction): boolean { + const updatedNodes = moveNode(this.nodes() as TreeNode[], instr); this.nodes.set(updatedNodes); - this.rebuildNodeCache(updatedNodes); - this.updated.emit(this.nodes()); + this.moved.emit(instr); + return true; } - // Imperative update: replace tree data and rebuild cache - update(nodes: TreeNode[]): void { - this.nodes.set(nodes ?? []); - this.rebuildNodeCache(this.nodes()); - this.updated.emit(this.nodes()); - } + /** + * Checks if one node is an ancestor of another. + * This prevents users from dropping a folder into itself or its descendants, + * which would create invalid tree structures. + * + * Example: Can't drag "Folder A" into "Folder A/Subfolder B" + */ + private readonly _isNodeAncestor = ( + ancestorId: TreeId, + possibleDescendantId: TreeId, + ): boolean => { + const ancestorNode = this._findNode(ancestorId); + if (!ancestorNode?.children) return false; - private flashJustDropped(id: string): void { - if (typeof window === 'undefined') { - return; + let found = false; + this._traverseNodes(ancestorNode.children as TreeNode[], (node) => { + if (node.id === possibleDescendantId) { + found = true; + } + }); + return found; + }; + + /** + * Extracts and validates drag data as a TreeId. + * + * CDK drag events type item.data as 'any', but we set [cdkDragData]="node.id" + * so we expect it to be a TreeId (string). This function provides runtime validation: + * 1. Checks if data is a non-empty string + * 2. Verifies the string is actually a valid node ID in our tree + * + * This prevents issues from malformed drag data or external drag sources. + */ + private _extractDragId(data: unknown): TreeId | null { + if (typeof data === 'string' && data.length > 0) { + // Verify this is actually a node ID that exists in our tree + const node = this._findNode(data); + return node ? data : null; } + return null; + } + + private _flashJustDropped(id: TreeId): void { + if (typeof window === 'undefined') return; + this.justDroppedId.set(id); if (this._dropFlashTimer) clearTimeout(this._dropFlashTimer); this._dropFlashTimer = window.setTimeout(() => { if (this.justDroppedId() === id) this.justDroppedId.set(null); - }, 300); + }, TREE_CONSTANTS.DROP_FLASH_DURATION); } } diff --git a/src/app/ui/tree-dnd/tree.types.ts b/src/app/ui/tree-dnd/tree.types.ts index b0d167d509..8528d28c44 100644 --- a/src/app/ui/tree-dnd/tree.types.ts +++ b/src/app/ui/tree-dnd/tree.types.ts @@ -1,43 +1,107 @@ export type TreeId = string; -export interface TreeNode { - id: TreeId; - label?: string; - children?: TreeNode[]; +export interface TreeNode { + readonly id: TreeId; + children?: TreeNode[]; expanded?: boolean; - // mark as a folder even if children is empty - isFolder?: boolean; - data?: T; + readonly isFolder?: boolean; + readonly data?: TData; +} + +export interface TreeNodeMutable extends TreeNode { + children?: TreeNodeMutable[]; } export type DropWhere = 'before' | 'after' | 'inside' | 'root'; +export const DROP_WHERE_VALUES = ['before', 'after', 'inside', 'root'] as const; + +export const isDropWhere = (value: unknown): value is DropWhere => + typeof value === 'string' && DROP_WHERE_VALUES.includes(value as DropWhere); + export interface DropData { - type: 'drop'; - id: TreeId; - where: DropWhere; + readonly type: 'drop'; + readonly id: TreeId; + readonly where: DropWhere; } export interface DragData { - type: 'item'; - id: TreeId; - uniqueContextId: symbol; + readonly type: 'item'; + readonly id: TreeId; + readonly uniqueContextId: symbol; } +export type MoveTargetWhere = Exclude | 'inside'; + export interface MoveInstruction { - itemId: TreeId; - targetId: TreeId | ''; - where: Exclude | 'inside'; + readonly itemId: TreeId; + readonly targetId: TreeId | ''; + readonly where: MoveTargetWhere; } // Template context types -export interface ItemTplContext { - $implicit: TreeNode; +export interface ItemTplContext { + readonly $implicit: TreeNode; } -export interface FolderTplContext { - $implicit: TreeNode; - expanded: boolean; - toggle: (node: TreeNode) => void; - dragOver: boolean; +export interface FolderTplContext { + readonly $implicit: TreeNode; + readonly expanded: boolean; + readonly toggle: (node: TreeNode) => void; + readonly dragOver: boolean; +} + +export interface DragPreviewTplContext { + readonly $implicit: { node: TreeNode; isFolder: boolean }; + readonly id: TreeId; + readonly element: HTMLElement; +} + +// Predicate function types +export type CanDropPredicate = (ctx: CanDropContext) => boolean; + +export interface CanDropContext { + readonly drag: TreeNode; + readonly drop: TreeNode | null; + readonly where: DropWhere; +} + +// Internal service interfaces +export interface HoverTarget { + readonly id: TreeId; + readonly where: DropWhere; + readonly element: HTMLElement; +} + +export interface DropListContext { + readonly parentId: TreeId; + readonly items: readonly TreeNode[]; +} + +// Utility types +export type TreeNodeWithChildren = TreeNode & { + children: readonly TreeNode[]; +}; + +export type TreeNodeFolder = TreeNode & { + readonly isFolder: true; +}; + +// Event types +export interface TreeNodeEvent { + readonly node: TreeNode; +} + +export interface TreeMoveEvent extends TreeNodeEvent { + readonly instruction: MoveInstruction; +} + +export interface TreeUpdateEvent { + readonly nodes: readonly TreeNode[]; +} + +// Shared interfaces +export interface PointerPosition { + readonly x: number; + readonly y: number; } diff --git a/src/app/ui/tree-dnd/tree.utils.ts b/src/app/ui/tree-dnd/tree.utils.ts index 6d0847123d..d23eb00874 100644 --- a/src/app/ui/tree-dnd/tree.utils.ts +++ b/src/app/ui/tree-dnd/tree.utils.ts @@ -1,14 +1,19 @@ import { MoveInstruction, TreeId, TreeNode } from './tree.types'; -export const cloneNodes = (nodes: TreeNode[]): TreeNode[] => +export const cloneNodes = ( + nodes: readonly TreeNode[], +): TreeNode[] => nodes.map((n) => ({ ...n, children: n.children ? cloneNodes(n.children) : undefined, })); -export const getPath = (nodes: TreeNode[], targetId: TreeId): TreeId[] | null => { +export const getPath = ( + nodes: readonly TreeNode[], + targetId: TreeId, +): TreeId[] | null => { const path: TreeId[] = []; - const dfs = (list: TreeNode[], id: TreeId, acc: TreeId[]): boolean => { + const dfs = (list: readonly TreeNode[], id: TreeId, acc: TreeId[]): boolean => { for (const node of list) { acc.push(node.id); if (node.id === id) return true; @@ -21,8 +26,8 @@ export const getPath = (nodes: TreeNode[], targetId: TreeId): TreeId[] | null => return found ? path : null; }; -export const isAncestor = ( - nodes: TreeNode[], +export const isAncestor = ( + nodes: readonly TreeNode[], ancestorId: TreeId, possibleDescendantId: TreeId, ): boolean => { @@ -30,11 +35,11 @@ export const isAncestor = ( return !!path?.includes(ancestorId); }; -export const findAndRemove = ( - nodes: TreeNode[], +export const findAndRemove = ( + nodes: TreeNode[], id: TreeId, -): { node: TreeNode | null } => { - const stack: { parent: TreeNode | null; list: TreeNode[] }[] = [ +): { node: TreeNode | null } => { + const stack: { parent: TreeNode | null; list: TreeNode[] }[] = [ { parent: null, list: nodes }, ]; while (stack.length) { @@ -51,7 +56,10 @@ export const findAndRemove = ( return { node: null }; }; -export const getChildren = (nodes: TreeNode[], id: '' | TreeId): TreeNode[] => { +export const getChildren = ( + nodes: TreeNode[], + id: '' | TreeId, +): TreeNode[] => { if (id === '') return nodes; const stack = [...nodes]; while (stack.length) { @@ -62,13 +70,17 @@ export const getChildren = (nodes: TreeNode[], id: '' | TreeId): TreeNode[] => { return nodes; }; -export const moveNode = (data: TreeNode[], instr: MoveInstruction): TreeNode[] => { - if (instr.targetId && instr.itemId === instr.targetId) return data; // no-op - if (instr.targetId && isAncestor(data, instr.itemId, instr.targetId)) return data; // prevent into own child +export const moveNode = ( + data: readonly TreeNode[], + instr: MoveInstruction, +): TreeNode[] => { + if (instr.targetId && instr.itemId === instr.targetId) return cloneNodes(data); // no-op + if (instr.targetId && isAncestor(data, instr.itemId, instr.targetId)) + return cloneNodes(data); // prevent into own child const nodes = cloneNodes(data); const { node } = findAndRemove(nodes, instr.itemId); - if (!node) return data; + if (!node) return cloneNodes(data); if (instr.where === 'inside') { const children = getChildren(nodes, (instr.targetId as TreeId) ?? ''); @@ -80,15 +92,18 @@ export const moveNode = (data: TreeNode[], instr: MoveInstruction): TreeNode[] = // before/after among siblings of target const parentChildren = getChildrenOfParent(nodes, instr.targetId); const index = parentChildren.findIndex((n) => n.id === instr.targetId); - if (index === -1) return data; + if (index === -1) return cloneNodes(data); const insertIndex = instr.where === 'before' ? index : index + 1; parentChildren.splice(insertIndex, 0, node); return nodes; }; -const getChildrenOfParent = (nodes: TreeNode[], id: TreeId): TreeNode[] => { +const getChildrenOfParent = ( + nodes: TreeNode[], + id: TreeId, +): TreeNode[] => { // returns the array that contains the node with id - const stack: { parent: TreeNode | null; list: TreeNode[] }[] = [ + const stack: { parent: TreeNode | null; list: TreeNode[] }[] = [ { parent: null, list: nodes }, ]; while (stack.length) {