Add support for dragging panel tabs docked into other panel tab bars#4006
Add support for dragging panel tabs docked into other panel tab bars#4006
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a dockable panel system, replacing fixed panel states with a flexible WorkspacePanelLayout. It enables reordering tabs within a panel area and dragging tabs between different areas (Properties, Layers, and Data). The backend now manages the layout state and synchronizes it with the frontend, ensuring that panel content is refreshed and diffing states are reset appropriately during transitions. Feedback includes minor optimizations for panel removal logic and code conciseness in layout utility functions.
There was a problem hiding this comment.
5 issues found across 14 files
Confidence score: 3/5
- There is moderate merge risk: two high-confidence issues in
editor/src/messages/portfolio/portfolio_message_handler.rscan cause visible stale/empty panel content when showing panels or moving tabs, which is a concrete UX regression risk. - The most severe issue is update ordering in
toggle_dockable_panel(editor/src/messages/portfolio/portfolio_message_handler.rs): refreshing beforeUpdateWorkspacePanelLayoutmeans newly shown panels may miss their initial data update. - Additional medium-risk correctness gaps include invalid layout states being representable in
editor/src/messages/portfolio/utility_types.rsand drag state not being cleared on unmount infrontend/src/components/window/Panel.svelte, which can leave lingering cross-panel drag behavior. - Pay close attention to
editor/src/messages/portfolio/portfolio_message_handler.rs,editor/src/messages/portfolio/utility_types.rs, andfrontend/src/components/window/Panel.svelte- panel refresh ordering/state validity and drag cleanup are the main sources of user-facing inconsistency.
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="editor/src/messages/portfolio/utility_types.rs">
<violation number="1" location="editor/src/messages/portfolio/utility_types.rs:143">
P2: `PanelAreaState` allows non-dockable `PanelType` values, so the workspace layout can represent invalid states. Restrict `tabs` to a dockable-only type to make those invalid states unrepresentable.
(Based on your team's feedback about making invalid states unrepresentable in data models.) [FEEDBACK_USED]</violation>
</file>
<file name="editor/src/messages/portfolio/portfolio_message_handler.rs">
<violation number="1" location="editor/src/messages/portfolio/portfolio_message_handler.rs:484">
P2: Moving a tab between areas does not refresh the source area’s newly active tab, which can leave that panel stale/empty after remount.</violation>
<violation number="2" location="editor/src/messages/portfolio/portfolio_message_handler.rs:495">
P1: `toggle_dockable_panel` refreshes panel content before sending `UpdateWorkspacePanelLayout`, so newly shown panels may miss their initial data updates.</violation>
<violation number="3" location="editor/src/messages/portfolio/portfolio_message_handler.rs:1649">
P3: Add `break` after removing the panel tab. A panel type can only exist in one area at a time, so continuing to iterate after it's found is unnecessary work.</violation>
</file>
<file name="frontend/src/components/window/Panel.svelte">
<violation number="1" location="frontend/src/components/window/Panel.svelte:107">
P2: Cross-panel drag store state is not cleaned up on component unmount, so an interrupted drag can leave stale global drag state.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
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="editor/src/messages/portfolio/utility_types.rs">
<violation number="1" location="editor/src/messages/portfolio/utility_types.rs:97">
P3: The doc comment is inconsistent with the implementation: `Document` is documented as unsupported but now returns `DocumentGroup`.
(Based on your team's feedback about using consistent, precise terminology in names and docs.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
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="editor/src/messages/portfolio/utility_types.rs">
<violation number="1" location="editor/src/messages/portfolio/utility_types.rs:97">
P3: The updated doc comment is now misleading because this function still panics for `PanelType::Welcome`; document the non-applicable variant (or panic behavior) explicitly.
(Based on your team's feedback about aligning behavior with comments and intended behavior.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Partly closes #195.