Skip to content

feat(tree-view): add copy-drag directive for non-destructive drag operations#2106

Closed
TaylorShane wants to merge 3 commits into
siemens:mainfrom
TaylorShane:feat/tree-view-drag-copy
Closed

feat(tree-view): add copy-drag directive for non-destructive drag operations#2106
TaylorShane wants to merge 3 commits into
siemens:mainfrom
TaylorShane:feat/tree-view-drag-copy

Conversation

@TaylorShane
Copy link
Copy Markdown

@TaylorShane TaylorShane commented May 28, 2026

Description

Adds a new SiCopyDragDirective that enables copy-drag for tree-view items. When applied alongside cdkDrag, the source item remains visible during the drag operation instead of being hidden by CDK's default placeholder behavior and remains in the sources tree to be dragged again to a new location.

This is useful for scenarios like dragging items from a catalog/palette into a working tree without removing them from the source. This could be used to assign devices to locations or creating an entire building hierarchy via drag and drop.

Usage

Pair with transferTreeItem(..., { removeFromSource: false }) in the drop handler to preserve the source data model.

Changes

New: SiCopyDragDirective — standalone directive that clones the CDK placeholder so the original item stays visible during drag
New: si-tree-view-drag-drop-copy example — demo with two side-by-side trees (building tree + equipment catalog) showing copy-drag in action
New: 7 unit tests covering the directive and transferTreeItem copy semantics (source preservation, deep clone independence, child/sibling insertion)
Export: Added SiCopyDragDirective to the tree-view public API

How it works

The directive subscribes to CDK Drag's started/ended events. On drag start, it clones the placeholder element (which CDK inserts to replace the dragged item), removes the cdk-drag-placeholder class so it isn't hidden by existing CSS, and inserts it after the placeholder. On drag end, the clone is removed.

Testing

16/16 tree-view tests pass (9 existing + 7 new)
No changes to existing APIs or behavior

tree-drag-and-drop-copy

@TaylorShane TaylorShane requested review from a team as code owners May 28, 2026 19:38
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Shane Taylor seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the SiCopyDragDirective to enable copy-drag semantics for tree-view items, keeping the source element visible during drag operations, along with corresponding unit tests and an example. The review feedback suggests registering a cleanup callback on DestroyRef to prevent a memory leak if the directive is destroyed during a drag. It also recommends using Vitest's semantic toMatchObject matcher for test assertions to align with the style guide, and defining the example's predicate method as an arrow function to preserve the lexical this context.

Comment thread projects/element-ng/tree-view/si-copy-drag.directive.ts
Comment thread src/app/examples/si-tree-view/si-tree-view-drag-drop-copy.ts Outdated

const items = fixture.debugElement.queryAll(By.css('si-tree-view-item'));
expect(items).toHaveLength(2);
expect(items[0].nativeElement.classList).toContain('si-copy-drag');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(items[0].nativeElement.classList).toContain('si-copy-drag');
expect(items[0].nativeElement).toHaveClass('si-copy-drag');

initTree(targetItems);

const targetFlat = flatten(targetItems);
const event = makeDrop(sourceItems, targetFlat, 0, 1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please make use of the userEvent API (see https://main.vitest.dev/api/browser/interactivity.html#userevent-draganddrop) for all the test cases

@@ -0,0 +1,65 @@
<h3>Copy between trees</h3>
<p
>Drag equipment from the catalog into a room. Items are copied (not moved) and inserted as
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The descriptions seems to be specfic for the buildings domain, can you come up with a more general use case?


equipmentItems: TreeItem[] = [
{
label: 'PXC5.E003',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to prevent building specific terms and attribute names. Please focus on a general use case and terminology.

class: 'si-copy-drag'
}
})
export class SiCopyDragDirective {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chintankavathia What do you think of the name? I would prefer to have tree in the name.

private clone: HTMLElement | null = null;

constructor() {
const drag = inject(CdkDrag);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without CDKDrag the directive will fail, I guess you have 3 options:

  • use hostDirective
  • combine the selector [siCopyDrag][cdkDrag]
  • or add a debug mode warning and make the CDK directive optional

@chintankavathia
Copy link
Copy Markdown
Member

@TaylorShane Thanks for the PR but this custom directive is not needed as CDK already has cdkDropListHasAnchor https://angular.dev/guide/drag-drop#copying-items-between-lists
currently our custom styles are not honoring it. I will adjust some styles so that cdkDropListHasAnchor can be used.

@TaylorShane
Copy link
Copy Markdown
Author

functionality provided in #2110

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants