Skip to content
Merged
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
42 changes: 42 additions & 0 deletions packages/panes/src/core/store/store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,48 @@ describe("movePaneToSplit", () => {
});
});

describe("movePaneToNewTab", () => {
it("moves a pane into a new tab at the requested index", () => {
const store = makeStore();
store.getState().addTab({
id: "t1",
panes: [tp("p1"), tp("p2")],
activePaneId: "p1",
});
store.getState().addTab({ id: "t2", panes: [tp("p3")] });

store.getState().movePaneToNewTab({ paneId: "p2", toIndex: 1 });

const tabs = store.getState().tabs;
const newTab = tabs[1];
if (!newTab) throw new Error("Expected new tab at index 1");

expect(tabs.map((t) => t.id)).toEqual(["t1", newTab.id, "t2"]);
expect(newTab.panes.p2).toBeDefined();
expect(newTab.activePaneId).toBe("p2");
expect(newTab.layout).toEqual({ type: "pane", paneId: "p2" });
expect(tabs[0]?.panes.p2).toBeUndefined();
expect(tabs[0]?.layout).toEqual({ type: "pane", paneId: "p1" });
expect(store.getState().activeTabId).toBe(newTab.id);
});

it("keeps insertion position stable when the source tab is removed", () => {
const store = makeStore();
store.getState().addTab({ id: "t1", panes: [tp("p1")] });
store.getState().addTab({ id: "t2", panes: [tp("p2")] });

store.getState().movePaneToNewTab({ paneId: "p1", toIndex: 1 });

const tabs = store.getState().tabs;
const newTab = tabs[0];
if (!newTab) throw new Error("Expected new tab at index 0");

expect(tabs.map((t) => t.id)).toEqual([newTab.id, "t2"]);
expect(newTab.panes.p1).toBeDefined();
expect(store.getState().activeTabId).toBe(newTab.id);
});
});

describe("edge cases", () => {
it("invalid IDs are no-ops", () => {
const store = makeStore();
Expand Down
20 changes: 17 additions & 3 deletions packages/panes/src/core/store/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ export interface WorkspaceStore<TData> extends WorkspaceState<TData> {
}) => void;

movePaneToTab: (args: { paneId: string; targetTabId: string }) => void;
movePaneToNewTab: (args: { paneId: string }) => void;
movePaneToNewTab: (args: { paneId: string; toIndex?: number }) => void;

reorderTab: (args: { tabId: string; toIndex: number }) => void;

Expand Down Expand Up @@ -795,10 +795,12 @@ export function createWorkspaceStore<TData>(
set((s) => {
let sourceTab: Tab<TData> | undefined;
let pane: Pane<TData> | undefined;
for (const t of s.tabs) {
let sourceTabIndex = -1;
for (const [index, t] of s.tabs.entries()) {
if (t.panes[args.paneId]) {
sourceTab = t;
pane = t.panes[args.paneId];
sourceTabIndex = index;
break;
}
}
Expand Down Expand Up @@ -835,7 +837,19 @@ export function createWorkspaceStore<TData>(
})
.filter((t): t is Tab<TData> => t !== null);

nextTabs.push(newTab);
const requestedIndex = args.toIndex ?? nextTabs.length;
const adjustedIndex =
args.toIndex !== undefined &&
!nextSourceLayout &&
sourceTabIndex < args.toIndex
? args.toIndex - 1
: requestedIndex;
const insertIndex = Math.max(
0,
Math.min(adjustedIndex, nextTabs.length),
);

nextTabs.splice(insertIndex, 0, newTab);

return { tabs: nextTabs, activeTabId: newTab.id };
});
Expand Down
3 changes: 3 additions & 0 deletions packages/panes/src/react/components/Workspace/Workspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ export function Workspace<TData>({
onReorderTab={(tabId, toIndex) =>
store.getState().reorderTab({ tabId, toIndex })
}
onMovePaneToNewTab={(paneId, toIndex) =>
store.getState().movePaneToNewTab({ paneId, toIndex })
}
getTabTitle={(tab) => resolveTabTitle(tab, tabs, registry)}
renderTabIcon={renderTabIcon}
renderAddTabMenu={renderAddTabMenu}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
} from "react";
import { useDrop } from "react-dnd";
import type { Tab } from "../../../../../types";
import { PANE_DRAG_TYPE } from "../Tab/components/Pane/components/PaneHeader";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Cross-component coupling for PANE_DRAG_TYPE

TabBar now imports PANE_DRAG_TYPE from a deeply nested peer component's barrel (../Tab/components/Pane/components/PaneHeader). TabItem.tsx already does the same, but the PR adds a second consumer, making the coupling more visible. Consider hoisting this constant to a shared dnd-types.ts file (e.g., alongside TAB_DRAG_TYPE in TabBar/components/TabItem) so both TabBar and TabItem can import from a common location rather than from a UI component that happens to export it.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/panes/src/react/components/Workspace/components/TabBar/TabBar.tsx
Line: 18

Comment:
**Cross-component coupling for `PANE_DRAG_TYPE`**

`TabBar` now imports `PANE_DRAG_TYPE` from a deeply nested peer component's barrel (`../Tab/components/Pane/components/PaneHeader`). `TabItem.tsx` already does the same, but the PR adds a second consumer, making the coupling more visible. Consider hoisting this constant to a shared `dnd-types.ts` file (e.g., alongside `TAB_DRAG_TYPE` in `TabBar/components/TabItem`) so both `TabBar` and `TabItem` can import from a common location rather than from a UI component that happens to export it.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

import { TAB_DRAG_TYPE, TabItem } from "./components/TabItem";
import { computeInsertIndex, TAB_WIDTH } from "./utils";

Expand All @@ -27,12 +28,16 @@ interface TabBarProps<TData> {
onCloseAllTabs: () => void;
onRenameTab: (tabId: string, title: string | undefined) => void;
onReorderTab: (tabId: string, toIndex: number) => void;
onMovePaneToNewTab: (paneId: string, toIndex: number) => void;
getTabTitle: (tab: Tab<TData>) => string;
renderTabIcon?: (tab: Tab<TData>) => ReactNode;
renderAddTabMenu?: () => ReactNode;
renderTabAccessory?: (tab: Tab<TData>) => ReactNode;
}

type TabDragItem = { tabId: string };
type PaneDragItem = { paneId: string };

function AddTabButton<_TData>({
renderAddTabMenu,
}: {
Expand Down Expand Up @@ -72,6 +77,7 @@ export function TabBar<TData>({
onCloseAllTabs,
onRenameTab,
onReorderTab,
onMovePaneToNewTab,
getTabTitle,
renderTabIcon,
renderAddTabMenu,
Expand All @@ -85,8 +91,8 @@ export function TabBar<TData>({

const [{ isOver }, connectDrop] = useDrop(
() => ({
accept: TAB_DRAG_TYPE,
hover: (_item, monitor) => {
accept: [TAB_DRAG_TYPE, PANE_DRAG_TYPE],
hover: (_item: TabDragItem | PaneDragItem, monitor) => {
const track = tabsTrackRef.current;
const offset = monitor.getClientOffset();
if (!track || !offset) return;
Comment on lines +95 to 98

@cubic-dev-ai cubic-dev-ai Bot Apr 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Pane drop-to-new-tab is a no-op when the target workspace has zero tabs because insert index is never computed in the empty-tab-bar path.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/panes/src/react/components/Workspace/components/TabBar/TabBar.tsx, line 95:

<comment>Pane drop-to-new-tab is a no-op when the target workspace has zero tabs because insert index is never computed in the empty-tab-bar path.</comment>

<file context>
@@ -85,8 +91,8 @@ export function TabBar<TData>({
-			accept: TAB_DRAG_TYPE,
-			hover: (_item, monitor) => {
+			accept: [TAB_DRAG_TYPE, PANE_DRAG_TYPE],
+			hover: (_item: TabDragItem | PaneDragItem, monitor) => {
 				const track = tabsTrackRef.current;
 				const offset = monitor.getClientOffset();
</file context>
Fix with Cubic

Expand All @@ -101,26 +107,36 @@ export function TabBar<TData>({
setInsertIndex(idx);
}
},
drop: (item: { tabId: string }) => {
drop: (item: TabDragItem | PaneDragItem, monitor) => {
const idx = insertIndexRef.current;
if (idx === null) return;

insertIndexRef.current = null;
setInsertIndex(null);

if (monitor.getItemType() === PANE_DRAG_TYPE && "paneId" in item) {
onMovePaneToNewTab(item.paneId, idx);
return;
}

if (monitor.getItemType() !== TAB_DRAG_TYPE || !("tabId" in item)) {
return;
}

const dragIndex = tabs.findIndex((t) => t.id === item.tabId);
if (dragIndex === -1) return;

// Adjust for removal of dragged tab
let toIndex = idx;
if (dragIndex < toIndex) toIndex--;

insertIndexRef.current = null;
setInsertIndex(null);
onReorderTab(item.tabId, toIndex);
},
collect: (monitor) => ({
isOver: monitor.isOver(),
}),
}),
[tabs, onReorderTab],
[tabs, onReorderTab, onMovePaneToNewTab],
);

// Clear indicator when cursor leaves the tab bar
Expand All @@ -129,7 +145,7 @@ export function TabBar<TData>({
if (insertIndex !== null) setInsertIndex(null);
}

const setScrollContainerRef = useCallback(
const setRootRef = useCallback(
(node: HTMLDivElement | null) => {
connectDrop(node);
},
Expand All @@ -148,7 +164,10 @@ export function TabBar<TData>({

if (tabs.length === 0) {
return (
<div className="group/root-tabs flex h-10 min-w-0 shrink-0 items-stretch border-b border-border bg-background">
<div
ref={setRootRef}
className="group/root-tabs flex h-10 min-w-0 shrink-0 items-stretch border-b border-border bg-background"
>
<div className="flex h-full w-10 shrink-0 items-stretch bg-background">
<AddTabButton renderAddTabMenu={renderAddTabMenu} />
</div>
Expand All @@ -158,9 +177,11 @@ export function TabBar<TData>({
}

return (
<div className="group/root-tabs flex h-10 min-w-0 shrink-0 items-stretch border-b border-border bg-background">
<div
ref={setRootRef}
className="group/root-tabs flex h-10 min-w-0 shrink-0 items-stretch border-b border-border bg-background"
>
<OverflowFadeContainer
ref={setScrollContainerRef}
observeChildren
onOverflowChange={handleOverflowChange}
className="hide-scrollbar flex min-w-0 flex-1 items-stretch overflow-x-auto overflow-y-hidden"
Expand Down
Loading