Skip to content
Draft
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
4 changes: 4 additions & 0 deletions editor/src/messages/portfolio/portfolio_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ pub enum PortfolioMessage {
layers: Vec<LayerNodeIdentifier>,
},
PrevDocument,
ReorderDocument {
document_id: DocumentId,
new_index: usize,
},
RequestWelcomeScreenButtonsLayout,
RequestStatusBarInfoLayout,
SetActivePanel {
Expand Down
8 changes: 8 additions & 0 deletions editor/src/messages/portfolio/portfolio_message_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1008,6 +1008,14 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageContext<'_>> for Portfolio
responses.add(PortfolioMessage::SelectDocument { document_id: prev_id });
}
}
PortfolioMessage::ReorderDocument { document_id, new_index } => {
if let Some(current_index) = self.document_ids.iter().position(|id| *id == document_id) {
self.document_ids.remove(current_index);
let clamped = new_index.min(self.document_ids.len());
self.document_ids.insert(clamped, document_id);
responses.add(PortfolioMessage::UpdateOpenDocumentsList);
}
}
PortfolioMessage::RequestWelcomeScreenButtonsLayout => {
let donate = "https://graphite.art/donate/";

Expand Down
164 changes: 161 additions & 3 deletions frontend/src/components/window/Panel.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
</script>

<script lang="ts">
import { getContext, tick } from "svelte";
import { getContext, onMount, onDestroy, tick } from "svelte";

import type { Editor } from "@graphite/editor";
import { isEventSupported } from "@graphite/utility-functions/platform";
Expand All @@ -26,8 +26,15 @@
import IconButton from "@graphite/components/widgets/buttons/IconButton.svelte";
import TextLabel from "@graphite/components/widgets/labels/TextLabel.svelte";

type DragState = {
startIndex: number;
startX: number;
startY: number;
};

const BUTTON_LEFT = 0;
const BUTTON_MIDDLE = 1;
const DRAG_THRESHOLD = 5;

const editor = getContext<Editor>("editor");

Expand All @@ -39,6 +46,7 @@
export let clickAction: ((index: number) => void) | undefined = undefined;
export let closeAction: ((index: number) => void) | undefined = undefined;
export let emptySpaceAction: (() => void) | undefined = undefined;
export let reorderAction: ((fromIndex: number, toIndex: number) => void) | undefined = undefined;

let className = "";
export { className as class };
Expand All @@ -48,6 +56,14 @@
export let styles: Record<string, string | number | undefined> = {};

let tabElements: (LayoutRow | undefined)[] = [];
let tabGroupElement: LayoutRow | undefined;

// Drag-and-drop state
let dragState: DragState | undefined = undefined;
let dragInPanel = false;
let dragDropIndex: number | undefined = undefined;
let dragIndicatorLeft: number | undefined = undefined;
let justFinishedDrag = false;

function onEmptySpaceAction(e: MouseEvent) {
if (e.target !== e.currentTarget) return;
Expand All @@ -58,19 +74,139 @@
await tick();
tabElements[newIndex]?.div?.()?.scrollIntoView();
}

// --- Drag-and-drop ---

function tabPointerDown(e: PointerEvent, tabIndex: number) {
if (e.button !== BUTTON_LEFT || !reorderAction) return;
dragState = { startIndex: tabIndex, startX: e.clientX, startY: e.clientY };
}

function calculateDropIndex(clientX: number): { index: number; left: number } | undefined {
const groupDiv = tabGroupElement?.div?.();
if (!groupDiv) return undefined;

const groupRect = groupDiv.getBoundingClientRect();
const scrollLeft = groupDiv.scrollLeft;

for (let i = 0; i < tabLabels.length; i++) {
const el = tabElements[i]?.div?.();
if (!el) continue;

const rect = el.getBoundingClientRect();
if (clientX < rect.left || clientX > rect.right) continue;

const pointerFraction = (clientX - rect.left) / rect.width;
if (pointerFraction < 0.5) {
return { index: i, left: rect.left - groupRect.left + scrollLeft };
} else {
return { index: i + 1, left: rect.right - groupRect.left + scrollLeft };
}
}

return undefined;
}

function draggingPointerMove(e: PointerEvent) {
if (!dragState || !tabGroupElement) return;

if (!dragInPanel) {
const distance = Math.hypot(e.clientX - dragState.startX, e.clientY - dragState.startY);
if (distance <= DRAG_THRESHOLD) return;
dragInPanel = true;
}

if (dragInPanel) {
const result = calculateDropIndex(e.clientX);
if (result) {
// Adjust index to account for the item being removed from its original position
let targetIndex = result.index;
if (targetIndex > dragState.startIndex) targetIndex -= 1;

if (targetIndex === dragState.startIndex) {
dragDropIndex = undefined;
dragIndicatorLeft = undefined;
} else {
dragDropIndex = targetIndex;
dragIndicatorLeft = result.left;
}
} else {
dragDropIndex = undefined;
dragIndicatorLeft = undefined;
}
}
}

function draggingPointerUp() {
if (dragInPanel && dragDropIndex !== undefined) {
reorderAction?.(dragState.startIndex, dragDropIndex);
Copy link

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

Choose a reason for hiding this comment

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

P2: Accessing dragState.startIndex without a null check. dragState is typed DragState | undefined and the enclosing if condition doesn't narrow it. Add an explicit guard to avoid a potential runtime TypeError.

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

<comment>Accessing `dragState.startIndex` without a null check. `dragState` is typed `DragState | undefined` and the enclosing `if` condition doesn't narrow it. Add an explicit guard to avoid a potential runtime `TypeError`.</comment>

<file context>
@@ -58,19 +74,139 @@
+
+	function draggingPointerUp() {
+		if (dragInPanel && dragDropIndex !== undefined) {
+			reorderAction?.(dragState.startIndex, dragDropIndex);
+			justFinishedDrag = true;
+			// Clear after the current tick so a same-tick click is still suppressed, but the next intentional click is not swallowed
</file context>
Suggested change
reorderAction?.(dragState.startIndex, dragDropIndex);
if (dragState) reorderAction?.(dragState.startIndex, dragDropIndex);
Fix with Cubic

justFinishedDrag = true;
// Clear after the current tick so a same-tick click is still suppressed, but the next intentional click is not swallowed
setTimeout(() => {
justFinishedDrag = false;
}, 0);
} else if (justFinishedDrag) {
// Avoid right-click abort getting stuck with `justFinishedDrag` set and blocking the first subsequent click
setTimeout(() => {
justFinishedDrag = false;
}, 0);
}

abortDrag();
}

function draggingMouseDown(e: MouseEvent) {
if (e.button === 2 && dragInPanel) {
justFinishedDrag = true;
abortDrag();
}
}

function draggingKeyDown(e: KeyboardEvent) {
if (e.key === "Escape" && dragInPanel) {
justFinishedDrag = true;
abortDrag();
}
}

function abortDrag() {
dragState = undefined;
dragInPanel = false;
dragDropIndex = undefined;
dragIndicatorLeft = undefined;
}

onMount(() => {
addEventListener("pointermove", draggingPointerMove);
addEventListener("pointerup", draggingPointerUp);
addEventListener("mousedown", draggingMouseDown);
addEventListener("keydown", draggingKeyDown);
});

onDestroy(() => {
removeEventListener("pointermove", draggingPointerMove);
removeEventListener("pointerup", draggingPointerUp);
removeEventListener("mousedown", draggingMouseDown);
removeEventListener("keydown", draggingKeyDown);
});
Comment on lines +179 to +191
Copy link
Contributor

Choose a reason for hiding this comment

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

high

Global event listeners for drag-and-drop are added in onMount and removed in onDestroy. This is inefficient as they are active throughout the component's lifecycle, even when no drag is occurring. This can lead to performance degradation and potential conflicts with other parts of the application.

A more performant and robust approach is to attach these listeners to the window object only when a drag operation begins (i.e., in tabPointerDown) and remove them once it's complete (in a cleanup function called from draggingPointerUp and other abort scenarios).

To implement this, you can:

  1. Remove the onMount and onDestroy lifecycle hooks for these listeners.
  2. Add window.addEventListener(...) for pointermove, pointerup, mousedown, and keydown inside tabPointerDown.
  3. Add window.removeEventListener(...) for the same events inside abortDrag.

</script>

<LayoutCol on:pointerdown={() => panelType && editor.handle.setActivePanel(panelType)} class={`panel ${className}`.trim()} {classes} style={styleName} {styles}>
<LayoutRow class="tab-bar" classes={{ "min-widths": tabMinWidths }}>
<LayoutRow class="tab-group" scrollableX={true} on:click={onEmptySpaceAction} on:auxclick={onEmptySpaceAction}>
<LayoutRow class="tab-group" classes={{ "drag-ongoing": dragInPanel }} scrollableX={true} on:click={onEmptySpaceAction} on:auxclick={onEmptySpaceAction} bind:this={tabGroupElement}>
{#each tabLabels as tabLabel, tabIndex}
<LayoutRow
class="tab"
classes={{ active: tabIndex === tabActiveIndex }}
classes={{ active: tabIndex === tabActiveIndex, dragging: dragInPanel && dragState?.startIndex === tabIndex }}
tooltipLabel={tabLabel.tooltipLabel}
tooltipDescription={tabLabel.tooltipDescription}
on:pointerdown={(e) => tabPointerDown(e, tabIndex)}
on:click={(e) => {
e.stopPropagation();
if (justFinishedDrag) {
justFinishedDrag = false;
return;
}
clickAction?.(tabIndex);
}}
on:auxclick={(e) => {
Expand Down Expand Up @@ -110,6 +246,9 @@
{/if}
</LayoutRow>
{/each}
{#if dragInPanel && dragDropIndex !== undefined && dragIndicatorLeft !== undefined}
<div class="drop-indicator" style:left={`${dragIndicatorLeft}px`} />
{/if}
</LayoutRow>
</LayoutRow>
<LayoutCol class="panel-body">
Expand Down Expand Up @@ -150,13 +289,21 @@
flex: 0 0 auto;
}

&.drag-ongoing .tab {
pointer-events: none;
}

.tab {
flex: 0 1 auto;
height: 28px;
padding: 0 8px;
align-items: center;
position: relative;

&.dragging {
opacity: 0.5;
}

&.active {
background: var(--color-3-darkgray);
border-radius: 6px 6px 0 0;
Expand Down Expand Up @@ -232,6 +379,17 @@
}
}
}

.drop-indicator {
position: absolute;
top: 4px;
bottom: 4px;
width: 2px;
background: var(--color-e-nearwhite);
pointer-events: none;
z-index: 1;
transform: translateX(-50%);
}
}
}

Expand Down
1 change: 1 addition & 0 deletions frontend/src/components/window/Workspace.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@
emptySpaceAction={() => editor.handle.newDocumentDialog()}
clickAction={(tabIndex) => editor.handle.selectDocument($portfolio.documents[tabIndex].id)}
closeAction={(tabIndex) => editor.handle.closeDocumentWithConfirmation($portfolio.documents[tabIndex].id)}
reorderAction={(fromIndex, toIndex) => editor.handle.reorderDocument($portfolio.documents[fromIndex].id, toIndex)}
tabActiveIndex={$portfolio.activeDocumentIndex}
bind:this={documentPanel}
/>
Expand Down
8 changes: 8 additions & 0 deletions frontend/wasm/src/editor_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,14 @@ impl EditorHandle {
self.dispatch(message);
}

/// Reorder a document tab to a new position index.
#[wasm_bindgen(js_name = reorderDocument)]
pub fn reorder_document(&self, document_id: u64, new_index: usize) {
let document_id = DocumentId(document_id);
let message = PortfolioMessage::ReorderDocument { document_id, new_index };
self.dispatch(message);
}

#[wasm_bindgen(js_name = newDocumentDialog)]
pub fn new_document_dialog(&self) {
let message = DialogMessage::RequestNewDocumentDialog;
Expand Down