Skip to content

Add Alt+Drag layer duplication to Layers Panel#3651

Open
afrdbaig7 wants to merge 1 commit intoGraphiteEditor:masterfrom
afrdbaig7:feat/layers-panel-alt-drag-duplicate
Open

Add Alt+Drag layer duplication to Layers Panel#3651
afrdbaig7 wants to merge 1 commit intoGraphiteEditor:masterfrom
afrdbaig7:feat/layers-panel-alt-drag-duplicate

Conversation

@afrdbaig7
Copy link
Contributor

Implements Alt+Drag duplication in Layers.svelte and exposes a WASM API for duplicating the current layer selection. Behavior matches the Select Tool, including modifier changes during drag and focus loss handling.

Fixes #2824

@timon-schelling
Copy link
Member

This contains commits from other PRs. Please only include the relevant commit.

@timon-schelling timon-schelling marked this pull request as draft January 17, 2026 16:34
@afrdbaig7
Copy link
Contributor Author

sorry about that i will fix it now

@afrdbaig7 afrdbaig7 force-pushed the feat/layers-panel-alt-drag-duplicate branch from bac869e to 3774516 Compare January 18, 2026 06:04
@afrdbaig7 afrdbaig7 marked this pull request as ready for review January 25, 2026 16:44
@Keavon Keavon force-pushed the feat/layers-panel-alt-drag-duplicate branch from 3774516 to 150c3c7 Compare February 14, 2026 06:41
@Keavon
Copy link
Member

Keavon commented Feb 14, 2026

Please give this a try, it seems to be catastrophically broken (maybe something changed with the surrounding code as a result of the rebase?).

@Keavon Keavon marked this pull request as draft February 14, 2026 06:44
@afrdbaig7 afrdbaig7 force-pushed the feat/layers-panel-alt-drag-duplicate branch from 150c3c7 to 21cb257 Compare February 20, 2026 18:24
@Keavon Keavon force-pushed the master branch 2 times, most recently from 5bb6104 to 52d2b38 Compare March 9, 2026 23:35
@Keavon
Copy link
Member

Keavon commented Mar 10, 2026

Hi, do you have any updates on this? Could you please resolve the merge conflict, confirm it works in the latest, and mark it as ready for review (and reply so I get an email about it)? I really do look forward to merging this feature, thank you!

@afrdbaig7
Copy link
Contributor Author

Hi, do you have any updates on this? Could you please resolve the merge conflict, confirm it works in the latest, and mark it as ready for review (and reply so I get an email about it)? I really do look forward to merging this feature, thank you!

ok keavon i will resolve the merge conflicts and will test against the latest changes and will push an update shortly

@afrdbaig7 afrdbaig7 force-pushed the feat/layers-panel-alt-drag-duplicate branch from 21cb257 to 147a0d1 Compare March 10, 2026 19:20
@afrdbaig7 afrdbaig7 force-pushed the feat/layers-panel-alt-drag-duplicate branch from 147a0d1 to e48c2a1 Compare March 10, 2026 19:31
@afrdbaig7 afrdbaig7 marked this pull request as ready for review March 10, 2026 19:44
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="frontend/src/components/panels/Layers.svelte">

<violation number="1" location="frontend/src/components/panels/Layers.svelte:109">
P2: The blur event listener uses an anonymous function and is never removed in `onDestroy`, causing a listener leak on each HMR cycle. Extract the callback into a named function (like the other handlers) and add the corresponding `removeEventListener` in `onDestroy`.</violation>

<violation number="2" location="frontend/src/components/panels/Layers.svelte:472">
P1: Race condition: `startDuplicates()` is async but not awaited. If Alt is released (or the drag is aborted) before `await tick()` resolves, `duplicatedLayerIds` is still `undefined`, so `stopDuplicates()` silently bails out — leaving orphaned duplicate layers in the document. Consider making the duplication fully synchronous, or guarding against this by tracking/awaiting the pending promise.</violation>
</file>

Since this is your first cubic review, here's how it works:

  • cubic automatically reviews your code and comments on bugs and improvements
  • Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
  • Add one-off context when rerunning by tagging @cubic-dev-ai with guidance or docs links (including llms.txt)
  • Ask questions if you need clarification on any suggestion

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

if (e.key === "Alt" && internalDragState?.active && !altKeyPressedDuringDrag) {
altKeyPressedDuringDrag = true;
startDuplicates();
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 10, 2026

Choose a reason for hiding this comment

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

P1: Race condition: startDuplicates() is async but not awaited. If Alt is released (or the drag is aborted) before await tick() resolves, duplicatedLayerIds is still undefined, so stopDuplicates() silently bails out — leaving orphaned duplicate layers in the document. Consider making the duplication fully synchronous, or guarding against this by tracking/awaiting the pending promise.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/panels/Layers.svelte, line 472:

<comment>Race condition: `startDuplicates()` is async but not awaited. If Alt is released (or the drag is aborted) before `await tick()` resolves, `duplicatedLayerIds` is still `undefined`, so `stopDuplicates()` silently bails out — leaving orphaned duplicate layers in the document. Consider making the duplication fully synchronous, or guarding against this by tracking/awaiting the pending promise.</comment>

<file context>
@@ -424,6 +466,18 @@
+
+		if (e.key === "Alt" && internalDragState?.active && !altKeyPressedDuringDrag) {
+			altKeyPressedDuringDrag = true;
+			startDuplicates();
+		}
+	}
</file context>
Fix with Cubic

addEventListener("keydown", draggingKeyDown);
addEventListener("keydown", handleLayerPanelKeyDown);
addEventListener("keyup", draggingKeyUp);
addEventListener("blur", () => internalDragState?.active && abortDrag());
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 10, 2026

Choose a reason for hiding this comment

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

P2: The blur event listener uses an anonymous function and is never removed in onDestroy, causing a listener leak on each HMR cycle. Extract the callback into a named function (like the other handlers) and add the corresponding removeEventListener in onDestroy.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/panels/Layers.svelte, line 109:

<comment>The blur event listener uses an anonymous function and is never removed in `onDestroy`, causing a listener leak on each HMR cycle. Extract the callback into a named function (like the other handlers) and add the corresponding `removeEventListener` in `onDestroy`.</comment>

<file context>
@@ -100,6 +105,8 @@
 		addEventListener("keydown", draggingKeyDown);
 		addEventListener("keydown", handleLayerPanelKeyDown);
+		addEventListener("keyup", draggingKeyUp);
+		addEventListener("blur", () => internalDragState?.active && abortDrag());
 
 		addEventListener("pointermove", clippingHover);
</file context>
Fix with Cubic

@Keavon Keavon force-pushed the master branch 2 times, most recently from 3697b7d to 90533e6 Compare March 11, 2026 12:15
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.

Duplicate in Layers Panel with Alt+Drag

3 participants