From d0f3214b628ebe44d5e091da90817cd40c12e833 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Tue, 7 Apr 2026 04:23:30 -0700 Subject: [PATCH 1/5] Add interactive panel docking --- .../messages/portfolio/portfolio_message.rs | 12 +- .../portfolio/portfolio_message_handler.rs | 84 +++++++- .../src/messages/portfolio/utility_types.rs | 111 +++++++++-- frontend/src/components/window/Panel.svelte | 182 ++++++++++++++++-- .../components/window/PanelSubdivision.svelte | 12 ++ frontend/src/stores/panel-drag.ts | 42 +++- frontend/wrapper/src/editor_wrapper.rs | 24 ++- website/content/features.md | 12 +- 8 files changed, 440 insertions(+), 39 deletions(-) diff --git a/editor/src/messages/portfolio/portfolio_message.rs b/editor/src/messages/portfolio/portfolio_message.rs index 76c5cfc672..461b856209 100644 --- a/editor/src/messages/portfolio/portfolio_message.rs +++ b/editor/src/messages/portfolio/portfolio_message.rs @@ -1,5 +1,5 @@ use super::document::utility_types::document_metadata::LayerNodeIdentifier; -use super::utility_types::PanelGroupId; +use super::utility_types::{DockingSplitDirection, PanelGroupId, PanelType}; use crate::messages::frontend::utility_types::{ExportBounds, FileType}; use crate::messages::portfolio::document::utility_types::clipboards::Clipboard; use crate::messages::portfolio::utility_types::FontCatalog; @@ -61,6 +61,11 @@ pub enum PortfolioMessage { LoadDocumentResources { document_id: DocumentId, }, + MoveAllPanelTabs { + source_group: PanelGroupId, + target_group: PanelGroupId, + insert_index: usize, + }, MovePanelTab { source_group: PanelGroupId, target_group: PanelGroupId, @@ -146,6 +151,11 @@ pub enum PortfolioMessage { group: PanelGroupId, tab_index: usize, }, + SplitPanelGroup { + target_group: PanelGroupId, + direction: DockingSplitDirection, + tabs: Vec, + }, SelectDocument { document_id: DocumentId, }, diff --git a/editor/src/messages/portfolio/portfolio_message_handler.rs b/editor/src/messages/portfolio/portfolio_message_handler.rs index 685a0a79f1..a9ba7b655a 100644 --- a/editor/src/messages/portfolio/portfolio_message_handler.rs +++ b/editor/src/messages/portfolio/portfolio_message_handler.rs @@ -460,6 +460,54 @@ impl MessageHandler> for Portfolio self.load_document(new_document, document_id, responses, false); responses.add(PortfolioMessage::SelectDocument { document_id }); } + PortfolioMessage::MoveAllPanelTabs { + source_group, + target_group, + insert_index, + } => { + if source_group == target_group { + return; + } + + let Some(source_state) = self.workspace_panel_layout.panel_group(source_group) else { return }; + let tabs: Vec = source_state.tabs.clone(); + if tabs.is_empty() { + return; + } + + // Destroy layouts for all moved tabs and the displaced target tab + for &panel_type in &tabs { + Self::destroy_panel_layouts(panel_type, responses); + } + if let Some(old_target_panel) = self.workspace_panel_layout.panel_group(target_group).and_then(|g| g.active_panel_type()) { + Self::destroy_panel_layouts(old_target_panel, responses); + } + + // Clear the source group + if let Some(source) = self.workspace_panel_layout.panel_group_mut(source_group) { + source.tabs.clear(); + source.active_tab_index = 0; + } + + // Insert all tabs into the target group + if let Some(target) = self.workspace_panel_layout.panel_group_mut(target_group) { + let index = insert_index.min(target.tabs.len()); + for (i, panel_type) in tabs.iter().enumerate() { + target.tabs.insert(index + i, *panel_type); + } + target.active_tab_index = index; + } + + self.workspace_panel_layout.prune(); + + responses.add(MenuBarMessage::SendLayout); + responses.add(PortfolioMessage::UpdateWorkspacePanelLayout); + + // Refresh the new active tab + if let Some(panel_type) = self.workspace_panel_layout.panel_group(target_group).and_then(|g| g.active_panel_type()) { + self.refresh_panel_content(panel_type, responses); + } + } PortfolioMessage::MovePanelTab { source_group, target_group, @@ -1222,6 +1270,37 @@ impl MessageHandler> for Portfolio } } } + PortfolioMessage::SplitPanelGroup { target_group, direction, tabs } => { + // Destroy layouts for the dragged tabs and the target group's active panel (it may get remounted by the frontend) + for &panel_type in &tabs { + Self::destroy_panel_layouts(panel_type, responses); + } + if let Some(target_active) = self.workspace_panel_layout.panel_group(target_group).and_then(|g| g.active_panel_type()) { + Self::destroy_panel_layouts(target_active, responses); + } + + // Remove the dragged tabs from their current panel groups (without pruning, so the target group survives) + for &panel_type in &tabs { + self.remove_panel_from_layout(panel_type); + } + + // Create the new panel group adjacent to the target, then prune empty groups + let new_id = self.workspace_panel_layout.split_panel_group(target_group, direction, tabs.clone()); + self.workspace_panel_layout.prune(); + + responses.add(MenuBarMessage::SendLayout); + responses.add(PortfolioMessage::UpdateWorkspacePanelLayout); + + // Refresh the new panel group's active tab + if let Some(panel_type) = self.workspace_panel_layout.panel_group(new_id).and_then(|g| g.active_panel_type()) { + self.refresh_panel_content(panel_type, responses); + } + + // Refresh the target group's active panel since its component may have been remounted + if let Some(target_active) = self.workspace_panel_layout.panel_group(target_group).and_then(|g| g.active_panel_type()) { + self.refresh_panel_content(target_active, responses); + } + } PortfolioMessage::SelectDocument { document_id } => { // Auto-save the document we are leaving let mut node_graph_open = false; @@ -1667,7 +1746,7 @@ impl PortfolioMessageHandler { selected_nodes.first().copied() } - /// Remove a dockable panel type from whichever panel group currently contains it, then prune empty groups. + /// Remove a dockable panel type from whichever panel group currently contains it. Does not prune empty groups. fn remove_panel_from_layout(&mut self, panel_type: PanelType) { // Save the panel's current position so it can be restored there later self.workspace_panel_layout.save_panel_position(panel_type); @@ -1678,8 +1757,6 @@ impl PortfolioMessageHandler { group.tabs.retain(|&t| t != panel_type); group.active_tab_index = group.active_tab_index.min(group.tabs.len().saturating_sub(1)); } - - self.workspace_panel_layout.prune(); } /// Toggle a dockable panel on or off. When toggling off, refresh the newly active tab in its panel group (if any). @@ -1689,6 +1766,7 @@ impl PortfolioMessageHandler { let was_visible = self.workspace_panel_layout.panel_group(group_id).is_some_and(|g| g.is_visible(panel_type)); Self::destroy_panel_layouts(panel_type, responses); self.remove_panel_from_layout(panel_type); + self.workspace_panel_layout.prune(); // If the removed panel was the active tab, refresh whichever panel is now active in that panel group if was_visible && let Some(new_active) = self.workspace_panel_layout.panel_group(group_id).and_then(|g| g.active_panel_type()) { diff --git a/editor/src/messages/portfolio/utility_types.rs b/editor/src/messages/portfolio/utility_types.rs index a00a40b550..6d84b58a01 100644 --- a/editor/src/messages/portfolio/utility_types.rs +++ b/editor/src/messages/portfolio/utility_types.rs @@ -112,6 +112,16 @@ impl From for PanelType { #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)] pub struct PanelGroupId(pub u64); +/// Which edge of a panel group to split on when docking a dragged panel. +#[cfg_attr(feature = "wasm", derive(tsify::Tsify))] +#[derive(Clone, Copy, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub enum DockingSplitDirection { + Left, + Right, + Top, + Bottom, +} + /// State of a single panel group (leaf subdivision) in the workspace layout tree. #[cfg_attr(feature = "wasm", derive(tsify::Tsify), tsify(large_number_types_as_bigints))] #[derive(Clone, Debug, Default, PartialEq, serde::Serialize, serde::Deserialize)] @@ -207,6 +217,30 @@ impl WorkspacePanelLayout { self.root.prune(); } + /// Split a panel group by inserting a new panel group adjacent to it. + /// The direction determines where the new group goes relative to the target. + /// Left/Right creates a horizontal (row) split, Top/Bottom creates a vertical (column) split. + /// Returns the ID of the newly created panel group. + pub fn split_panel_group(&mut self, target_group_id: PanelGroupId, direction: DockingSplitDirection, tabs: Vec) -> PanelGroupId { + let new_id = self.next_id(); + let new_group = SplitChild { + subdivision: PanelLayoutSubdivision::PanelGroup { + id: new_id, + state: PanelGroupState { tabs, active_tab_index: 0 }, + }, + size: 50., + }; + + let insert_before = matches!(direction, DockingSplitDirection::Left | DockingSplitDirection::Top); + let needs_horizontal = matches!(direction, DockingSplitDirection::Left | DockingSplitDirection::Right); + + if !self.root.insert_split_adjacent(target_group_id, new_group, insert_before, needs_horizontal, 0) { + log::error!("Failed to insert split adjacent to panel group {target_group_id:?}: no matching-direction ancestor found"); + } + + new_id + } + /// Recalculate the default sizes for all splits in the tree based on document panel proximity. pub fn recalculate_default_sizes(&mut self) { self.root.recalculate_default_sizes(); @@ -409,23 +443,78 @@ impl PanelLayoutSubdivision { } } - /// Remove empty panel groups and collapse single-child splits. + /// Remove empty panel groups and collapse unnecessary nesting. + /// Does NOT collapse single-child splits into their child, as that would change subdivision depths + /// and break the direction-by-depth alternation system. pub fn prune(&mut self) { - if let PanelLayoutSubdivision::Split { children } = self { - // Recursively prune children first - children.iter_mut().for_each(|child| child.subdivision.prune()); + let PanelLayoutSubdivision::Split { children } = self else { return }; - // Remove empty panel groups - children.retain(|child| !matches!(&child.subdivision, PanelLayoutSubdivision::PanelGroup { state, .. } if state.tabs.is_empty())); + // Recursively prune children + children.iter_mut().for_each(|child| child.subdivision.prune()); - // Remove empty splits (splits that lost all their children after pruning) - children.retain(|child| !matches!(&child.subdivision, PanelLayoutSubdivision::Split { children } if children.is_empty())); + // Remove empty panel groups + children.retain(|child| !matches!(&child.subdivision, PanelLayoutSubdivision::PanelGroup { state, .. } if state.tabs.is_empty())); - // If a split has exactly one child, replace this subdivision with that child's subdivision - if children.len() == 1 { - *self = children.remove(0).subdivision; + // Remove empty splits (splits that lost all their children after pruning) + children.retain(|child| !matches!(&child.subdivision, PanelLayoutSubdivision::Split { children } if children.is_empty())); + } + + /// Check if this subtree contains a panel group with the given ID. + pub fn contains_group(&self, target_id: PanelGroupId) -> bool { + match self { + PanelLayoutSubdivision::PanelGroup { id, .. } => *id == target_id, + PanelLayoutSubdivision::Split { children } => children.iter().any(|child| child.subdivision.contains_group(target_id)), + } + } + + /// Inserts a new split child adjacent to a target panel group and returns whether the insertion was successful. + /// Recurses to the deepest split closest to the target that matches the requested split direction. + /// If the target is a direct child of a mismatched-direction split, this wraps it in a new sub-split. + pub fn insert_split_adjacent(&mut self, target_id: PanelGroupId, new_child: SplitChild, insert_before: bool, needs_horizontal: bool, depth: usize) -> bool { + let PanelLayoutSubdivision::Split { children } = self else { return false }; + + let is_horizontal = depth.is_multiple_of(2); + let direction_matches = is_horizontal == needs_horizontal; + + // Find which child subtree contains the target + let Some(containing_index) = children.iter().position(|child| child.subdivision.contains_group(target_id)) else { + return false; + }; + + // If the target is a direct child: we can certainly insert the new split, either as a sibling (if direction matches) or wrapping the target in a new split (if direction is mismatched) + let target_is_direct_child = matches!(&children[containing_index].subdivision, PanelLayoutSubdivision::PanelGroup { id, .. } if *id == target_id); + if target_is_direct_child { + // Direction matches and target is right here: insert as a sibling + if direction_matches { + let insert_index = if insert_before { containing_index } else { containing_index + 1 }; + children.insert(insert_index, new_child); } + // Direction mismatch: wrap the target in a new sub-split (at depth+1, which has the opposite direction of this and thus is the requested direction) + else { + let old_child_subdivision = std::mem::replace(&mut children[containing_index].subdivision, PanelLayoutSubdivision::Split { children: vec![] }); + let old_child = SplitChild { + subdivision: old_child_subdivision, + size: 50., + }; + + if let PanelLayoutSubdivision::Split { children: sub_children } = &mut children[containing_index].subdivision { + if insert_before { + sub_children.push(new_child); + sub_children.push(old_child); + } else { + sub_children.push(old_child); + sub_children.push(new_child); + } + } + } + + return true; } + + // The target is deeper, so recurse into the containing child's subtree and return its insertion outcome + children[containing_index] + .subdivision + .insert_split_adjacent(target_id, new_child.clone(), insert_before, needs_horizontal, depth + 1) } /// Check if this subtree contains the document panel. diff --git a/frontend/src/components/window/Panel.svelte b/frontend/src/components/window/Panel.svelte index cd69f4fda2..2725444cc7 100644 --- a/frontend/src/components/window/Panel.svelte +++ b/frontend/src/components/window/Panel.svelte @@ -9,7 +9,8 @@ import Welcome from "/src/components/panels/Welcome.svelte"; import IconButton from "/src/components/widgets/buttons/IconButton.svelte"; import TextLabel from "/src/components/widgets/labels/TextLabel.svelte"; - import { panelDrag, startCrossPanelDrag, endCrossPanelDrag, updateCrossPanelHover } from "/src/stores/panel-drag"; + import { panelDrag, startCrossPanelDrag, endCrossPanelDrag, updateCrossPanelHover, updateDockingHover } from "/src/stores/panel-drag"; + import type { DockingEdge } from "/src/stores/panel-drag"; import type { EditorWrapper, PanelType } from "/wrapper/pkg/graphite_wasm_wrapper"; const PANEL_COMPONENTS = { @@ -37,6 +38,8 @@ export let reorderAction: ((oldIndex: number, newIndex: number) => void) | undefined = undefined; export let emptySpaceAction: (() => void) | undefined = undefined; export let crossPanelDropAction: ((sourcePanelId: string, targetPanelId: string, insertIndex: number) => void) | undefined = undefined; + export let groupDropAction: ((sourcePanelId: string, targetPanelId: string, insertIndex: number) => void) | undefined = undefined; + export let splitDropAction: ((targetPanelId: string, direction: DockingEdge, tabs: PanelType[]) => void) | undefined = undefined; let className = ""; export { className as class }; @@ -48,7 +51,7 @@ let tabElements: (LayoutRow | undefined)[] = []; // Tab drag-and-drop state - let dragStartState: { tabIndex: number; pointerX: number; pointerY: number } | undefined = undefined; + let dragStartState: { tabIndex: number; pointerX: number; pointerY: number; isGroupDrag: boolean } | undefined = undefined; let dragging = false; let insertionIndex: number | undefined = undefined; let insertionMarkerLeft: number | undefined = undefined; @@ -64,6 +67,20 @@ if (e.button === BUTTON_MIDDLE || (e.button === BUTTON_LEFT && e.detail === 2)) emptySpaceAction?.(); } + function tabBarPointerDown(e: PointerEvent) { + // Only start a group drag from the tab bar background (not from a tab or button) + if (e.button !== BUTTON_LEFT) return; + if (e.target !== e.currentTarget) return; + if (!crossPanelDropAction) return; + + dragStartState = { tabIndex: tabActiveIndex, pointerX: e.clientX, pointerY: e.clientY, isGroupDrag: true }; + dragging = false; + insertionIndex = undefined; + insertionMarkerLeft = undefined; + + addDragListeners(); + } + export async function scrollTabIntoView(newIndex: number) { await tick(); tabElements[newIndex]?.div?.()?.scrollIntoView(); @@ -83,7 +100,7 @@ const canCrossPanelDrag = crossPanelDropAction !== undefined; if (!canReorder && !canCrossPanelDrag) return; - dragStartState = { tabIndex, pointerX: e.clientX, pointerY: e.clientY }; + dragStartState = { tabIndex, pointerX: e.clientX, pointerY: e.clientY, isGroupDrag: false }; dragging = false; insertionIndex = undefined; insertionMarkerLeft = undefined; @@ -103,8 +120,12 @@ dragging = true; if (crossPanelDropAction) { - // Notify the shared store that a cross-panel drag has started - startCrossPanelDrag(panelId, tabLabels[dragStartState.tabIndex].name, dragStartState.tabIndex); + if (dragStartState.isGroupDrag) { + startCrossPanelDrag(panelId, [...panelTypes], tabActiveIndex, true); + } else { + const draggedTab = panelTypes[dragStartState.tabIndex]; + startCrossPanelDrag(panelId, [draggedTab], dragStartState.tabIndex, false); + } } } @@ -123,7 +144,7 @@ // Check if the pointer is over any other dockable panel's tab bar if (crossPanelDropAction) { - const target = Array.from(document.querySelectorAll("[data-panel-tab-bar]")).find((element) => { + const tabBarTarget = Array.from(document.querySelectorAll("[data-panel-tab-bar]")).find((element) => { const targetPanelId = element.getAttribute("data-panel-tab-bar"); if (!targetPanelId || targetPanelId === panelId) return false; @@ -131,12 +152,39 @@ return e.clientX >= rect.left && e.clientX <= rect.right && e.clientY >= rect.top && e.clientY <= rect.bottom; }); - const targetPanelId = target?.getAttribute("data-panel-tab-bar"); - if (target instanceof HTMLDivElement && targetPanelId) { - calculateForeignInsertionIndex(e.clientX, targetPanelId, target); - } else { - updateCrossPanelHover(undefined, undefined, undefined); + const tabBarTargetId = tabBarTarget?.getAttribute("data-panel-tab-bar"); + if (tabBarTarget instanceof HTMLDivElement && tabBarTargetId) { + calculateForeignInsertionIndex(e.clientX, tabBarTargetId, tabBarTarget); + return; } + + // Check if the pointer is over any panel body's edge zone for split docking + const panelBody = Array.from(document.querySelectorAll("[data-panel-body]")).find((element) => { + const rect = element.getBoundingClientRect(); + return e.clientX >= rect.left && e.clientX <= rect.right && e.clientY >= rect.top && e.clientY <= rect.bottom; + }); + + const bodyPanelId = panelBody && panelBody.getAttribute("data-panel-body"); + if (bodyPanelId) { + const rect = panelBody.getBoundingClientRect(); + let edge: DockingEdge | undefined = detectDockingEdge(e.clientX, e.clientY, rect); + + // Block center drops between document and non-document panels + if (edge === "Center") { + const targetIsDockable = panelBody.hasAttribute("data-panel-dockable"); + const sourceIsDockable = crossPanelDropAction !== undefined; + if (targetIsDockable !== sourceIsDockable) edge = undefined; + } + + if (edge) { + updateDockingHover(bodyPanelId, edge); + return; + } + } + + // Not hovering any drop target + updateCrossPanelHover(undefined, undefined, undefined); + updateDockingHover(undefined, undefined); } } @@ -144,15 +192,25 @@ if (dragging && dragStartState) { const crossPanelState = $panelDrag; - // Cross-panel drop: the pointer is over a different panel's tab bar - if ( + // Center drop: append tabs to the target panel group + if (crossPanelState.active && crossPanelState.hoverDockingPanelId && crossPanelState.hoverDockingEdge === "Center") { + const dropAction = crossPanelState.draggingGroup ? groupDropAction : crossPanelDropAction; + dropAction?.(panelId, crossPanelState.hoverDockingPanelId, Number.MAX_SAFE_INTEGER); + } + // Edge docking drop: create a new split adjacent to the target panel + else if (crossPanelState.active && crossPanelState.hoverDockingPanelId && crossPanelState.hoverDockingEdge) { + splitDropAction?.(crossPanelState.hoverDockingPanelId, crossPanelState.hoverDockingEdge, crossPanelState.draggedTabs); + } + // Cross-panel tab bar drop: insert as a tab in the target panel group + else if ( crossPanelDropAction && crossPanelState.active && crossPanelState.hoverTargetPanelId && crossPanelState.hoverTargetPanelId !== panelId && crossPanelState.hoverInsertionIndex !== undefined ) { - crossPanelDropAction?.(panelId, crossPanelState.hoverTargetPanelId, crossPanelState.hoverInsertionIndex); + const dropAction = crossPanelState.draggingGroup ? groupDropAction : crossPanelDropAction; + dropAction?.(panelId, crossPanelState.hoverTargetPanelId, crossPanelState.hoverInsertionIndex); } // Within-panel reorder else if (insertionIndex !== undefined) { @@ -199,6 +257,27 @@ return e.clientX >= rect.left && e.clientX <= rect.right && e.clientY >= rect.top && e.clientY <= rect.bottom; } + /// Detect which zone the pointer is in: the nearest edge (by diagonal quadrant) if within the 25% border, or center if interior. + function detectDockingEdge(clientX: number, clientY: number, rect: DOMRect): DockingEdge { + const distLeft = clientX - rect.left; + const distRight = rect.right - clientX; + const distTop = clientY - rect.top; + const distBottom = rect.bottom - clientY; + const minDist = Math.min(distLeft, distRight, distTop, distBottom); + + // If the nearest edge is beyond the 25% threshold, it's the center zone + const THRESHOLD = 0.25; + const edgeThresholdX = rect.width * THRESHOLD; + const edgeThresholdY = rect.height * THRESHOLD; + if (distLeft > edgeThresholdX && distRight > edgeThresholdX && distTop > edgeThresholdY && distBottom > edgeThresholdY) return "Center"; + + // Return whichever edge is closest (diagonal dividing lines between quadrants) + if (minDist === distLeft) return "Left"; + if (minDist === distRight) return "Right"; + if (minDist === distTop) return "Top"; + return "Bottom"; + } + // Calculate the insertion position for a foreign panel's tab bar function calculateForeignInsertionIndex(pointerX: number, targetPanelId: string, tabBarDiv: HTMLDivElement) { const tabBarRect = tabBarDiv.getBoundingClientRect(); @@ -270,12 +349,21 @@ } - panelTypes[tabActiveIndex] && editor.setActivePanel(panelTypes[tabActiveIndex])} class={`panel ${className}`.trim()} {classes} style={styleName} {styles}> + panelTypes[tabActiveIndex] && editor.setActivePanel(panelTypes[tabActiveIndex])} + class={`panel ${className}`.trim()} + {classes} + style={styleName} + {styles} + data-panel-body={panelId} + data-panel-dockable={crossPanelDropAction ? "" : undefined} +> {/if} + {#if $panelDrag.active && $panelDrag.hoverDockingPanelId === panelId && $panelDrag.hoverDockingEdge} +
+ {/if} From 13b45c5459f7afcbb4dd89caaee0d83ee681a2d1 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Wed, 8 Apr 2026 05:39:34 -0700 Subject: [PATCH 4/5] Code review fixes --- .../messages/portfolio/portfolio_message_handler.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/editor/src/messages/portfolio/portfolio_message_handler.rs b/editor/src/messages/portfolio/portfolio_message_handler.rs index 9b8161ce88..21830dc635 100644 --- a/editor/src/messages/portfolio/portfolio_message_handler.rs +++ b/editor/src/messages/portfolio/portfolio_message_handler.rs @@ -476,6 +476,12 @@ impl MessageHandler> for Portfolio return; } + // Validate that the target group exists before modifying the source + if self.workspace_panel_layout.panel_group(target_group).is_none() { + log::error!("Target panel group {target_group:?} not found"); + return; + } + // Destroy layouts for all moved tabs and the displaced target tab for &panel_type in &tabs { Self::destroy_panel_layouts(panel_type, responses); @@ -493,9 +499,7 @@ impl MessageHandler> for Portfolio // Insert all tabs into the target group, preserving which tab was active in the source if let Some(target) = self.workspace_panel_layout.panel_group_mut(target_group) { let index = insert_index.min(target.tabs.len()); - for (i, panel_type) in tabs.iter().enumerate() { - target.tabs.insert(index + i, *panel_type); - } + target.tabs.splice(index..index, tabs.iter().copied()); target.active_tab_index = index + source_active_tab_index.min(tabs.len().saturating_sub(1)); } From 7f090f5d52bad92ce507b01fe08f32ff032c8e9d Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Wed, 8 Apr 2026 05:45:57 -0700 Subject: [PATCH 5/5] More code review --- .../messages/portfolio/portfolio_message_handler.rs | 5 ++++- editor/src/messages/portfolio/utility_types.rs | 10 +++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/editor/src/messages/portfolio/portfolio_message_handler.rs b/editor/src/messages/portfolio/portfolio_message_handler.rs index 21830dc635..7005941940 100644 --- a/editor/src/messages/portfolio/portfolio_message_handler.rs +++ b/editor/src/messages/portfolio/portfolio_message_handler.rs @@ -1295,7 +1295,10 @@ impl MessageHandler> for Portfolio } // Create the new panel group adjacent to the target, then prune empty groups - let new_id = self.workspace_panel_layout.split_panel_group(target_group, direction, tabs.clone(), active_tab_index); + let Some(new_id) = self.workspace_panel_layout.split_panel_group(target_group, direction, tabs.clone(), active_tab_index) else { + log::error!("Failed to insert split adjacent to panel group {target_group:?}"); + return; + }; self.workspace_panel_layout.prune(); responses.add(MenuBarMessage::SendLayout); diff --git a/editor/src/messages/portfolio/utility_types.rs b/editor/src/messages/portfolio/utility_types.rs index 8f06c2ddb2..63d96395ed 100644 --- a/editor/src/messages/portfolio/utility_types.rs +++ b/editor/src/messages/portfolio/utility_types.rs @@ -220,8 +220,8 @@ impl WorkspacePanelLayout { /// Split a panel group by inserting a new panel group adjacent to it. /// The direction determines where the new group goes relative to the target. /// Left/Right creates a horizontal (row) split, Top/Bottom creates a vertical (column) split. - /// Returns the ID of the newly created panel group. - pub fn split_panel_group(&mut self, target_group_id: PanelGroupId, direction: DockingSplitDirection, tabs: Vec, active_tab_index: usize) -> PanelGroupId { + /// Returns the ID of the newly created panel group, or `None` if insertion failed. + pub fn split_panel_group(&mut self, target_group_id: PanelGroupId, direction: DockingSplitDirection, tabs: Vec, active_tab_index: usize) -> Option { let new_id = self.next_id(); let new_group = SplitChild { subdivision: PanelLayoutSubdivision::PanelGroup { @@ -234,11 +234,7 @@ impl WorkspacePanelLayout { let insert_before = matches!(direction, DockingSplitDirection::Left | DockingSplitDirection::Top); let needs_horizontal = matches!(direction, DockingSplitDirection::Left | DockingSplitDirection::Right); - if !self.root.insert_split_adjacent(target_group_id, new_group, insert_before, needs_horizontal, 0) { - log::error!("Failed to insert split adjacent to panel group {target_group_id:?}: no matching-direction ancestor found"); - } - - new_id + self.root.insert_split_adjacent(target_group_id, new_group, insert_before, needs_horizontal, 0).then_some(new_id) } /// Recalculate the default sizes for all splits in the tree based on document panel proximity.