fix(app): keep the agent pane mounted when opening the first split
Splitting a pane right from the default layout tore down the existing pane and rebuilt it, losing the composer, terminal, and scroll state in it. Both split reducers derived the parent group from `parentPath.length`, but `targetPath.slice(0, -1)` is empty for two different situations: the pane is the whole tree, and the pane is a direct child of the root group. The second case was read as parentless, so the same-direction fast path never ran at root level and every split wrapped the pane in a fresh group. That reparents the pane, and the renderer reconciles panes positionally, so React unmounted the subtree. The workspace already starts as a horizontal group holding the main pane and the hidden explorer pane — the group was there all along, ready to take another child. `findParentGroup` now asks `targetPath`, so a same-direction split appends a sibling into the group that is already running in that axis. Perpendicular splits still nest, because changing axis needs a group. The old test asserted the nesting as intended behaviour, which is how this survived. It now asserts flat insertion, alongside the invariant that matters: a same-direction split never changes an existing pane's render path. Node id sequences shifted in three tests because the first split no longer spends a group id, and four user splits now fit inside the depth cap where the fourth used to be refused.
This commit is contained in:
@@ -468,6 +468,21 @@ function findGroupPathById(
|
|||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The group holding the node at `targetPath`, or null when that node is the whole tree.
|
||||||
|
*
|
||||||
|
* Ask `targetPath`, never the parent path: `targetPath.slice(0, -1)` is empty both for "this node
|
||||||
|
* is the root" and for "this node is a direct child of the root group". Treating the second case as
|
||||||
|
* parentless wraps root-level panes in a redundant group on every split, which reparents the
|
||||||
|
* existing pane and remounts its whole subtree.
|
||||||
|
*/
|
||||||
|
function findParentGroup(root: SplitNodeInternal, targetPath: number[]): SplitNodeInternal | null {
|
||||||
|
if (targetPath.length === 0) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
return getNodeAtPath(root, targetPath.slice(0, -1));
|
||||||
|
}
|
||||||
|
|
||||||
function getNodeAtPath(node: SplitNodeInternal, path: number[]): SplitNodeInternal {
|
function getNodeAtPath(node: SplitNodeInternal, path: number[]): SplitNodeInternal {
|
||||||
let current = node;
|
let current = node;
|
||||||
for (const index of path) {
|
for (const index of path) {
|
||||||
@@ -869,7 +884,7 @@ function insertSplitInternal(input: InsertSplitInternalInput): InsertSplitIntern
|
|||||||
|
|
||||||
const parentPath = targetPath.slice(0, -1);
|
const parentPath = targetPath.slice(0, -1);
|
||||||
const targetIndex = targetPath[targetPath.length - 1] ?? 0;
|
const targetIndex = targetPath[targetPath.length - 1] ?? 0;
|
||||||
const parentNode = parentPath.length > 0 ? getNodeAtPath(detached.root, parentPath) : null;
|
const parentNode = findParentGroup(detached.root, targetPath);
|
||||||
|
|
||||||
if (parentNode?.kind === "group" && parentNode.group.direction === direction) {
|
if (parentNode?.kind === "group" && parentNode.group.direction === direction) {
|
||||||
const targetSize = parentNode.group.sizes[targetIndex] ?? 0;
|
const targetSize = parentNode.group.sizes[targetIndex] ?? 0;
|
||||||
@@ -1448,7 +1463,7 @@ export function splitPaneEmptyInLayout(
|
|||||||
|
|
||||||
const parentPath = targetPath.slice(0, -1);
|
const parentPath = targetPath.slice(0, -1);
|
||||||
const targetIndex = targetPath[targetPath.length - 1] ?? 0;
|
const targetIndex = targetPath[targetPath.length - 1] ?? 0;
|
||||||
const parentNode = parentPath.length > 0 ? getNodeAtPath(layout.root, parentPath) : null;
|
const parentNode = findParentGroup(layout.root, targetPath);
|
||||||
|
|
||||||
let nextRoot: SplitNodeInternal;
|
let nextRoot: SplitNodeInternal;
|
||||||
if (parentNode?.kind === "group" && parentNode.group.direction === direction) {
|
if (parentNode?.kind === "group" && parentNode.group.direction === direction) {
|
||||||
|
|||||||
@@ -218,11 +218,8 @@ describe("workspace-layout-store tree transforms", () => {
|
|||||||
workspaceLayoutIds.reset();
|
workspaceLayoutIds.reset();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("insertSplit wraps root-level same-direction splits in a nested group", () => {
|
it("insertSplit adds root-level same-direction splits as a flat sibling", () => {
|
||||||
useWorkspaceLayoutIds(
|
useWorkspaceLayoutIds("11111111-1111-1111-1111-111111111111");
|
||||||
"11111111-1111-1111-1111-111111111111",
|
|
||||||
"22222222-2222-2222-2222-222222222222",
|
|
||||||
);
|
|
||||||
|
|
||||||
const root: SplitNode = {
|
const root: SplitNode = {
|
||||||
kind: "group",
|
kind: "group",
|
||||||
@@ -239,14 +236,12 @@ describe("workspace-layout-store tree transforms", () => {
|
|||||||
|
|
||||||
const nextRoot = insertSplit(root, "right", "tab-c", "right", workspaceLayoutIds.createNodeId);
|
const nextRoot = insertSplit(root, "right", "tab-c", "right", workspaceLayoutIds.createNodeId);
|
||||||
const nextGroup = expectGroup(nextRoot);
|
const nextGroup = expectGroup(nextRoot);
|
||||||
const nestedGroup = expectGroup(nextGroup.group.children[1]);
|
|
||||||
|
|
||||||
|
expect(nextGroup.group.id).toBe("group-root");
|
||||||
expect(nextGroup.group.direction).toBe("horizontal");
|
expect(nextGroup.group.direction).toBe("horizontal");
|
||||||
expect(nextGroup.group.children).toHaveLength(2);
|
expect(nextGroup.group.children).toHaveLength(3);
|
||||||
expect(nextGroup.group.sizes).toEqual([0.25, 0.75]);
|
expect(nextGroup.group.sizes).toEqual([0.25, 0.375, 0.375]);
|
||||||
expect(nestedGroup.group.id).toBe("group_22222222-2222-2222-2222-222222222222");
|
expect(getTreeDepth(nextRoot)).toBe(getTreeDepth(root));
|
||||||
expect(nestedGroup.group.direction).toBe("horizontal");
|
|
||||||
expect(nestedGroup.group.sizes).toEqual([0.5, 0.5]);
|
|
||||||
expect(collectAllPanes(nextRoot).map((pane) => pane.id)).toEqual([
|
expect(collectAllPanes(nextRoot).map((pane) => pane.id)).toEqual([
|
||||||
"left",
|
"left",
|
||||||
"right",
|
"right",
|
||||||
@@ -258,6 +253,91 @@ describe("workspace-layout-store tree transforms", () => {
|
|||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The renderer keys panes by id and reconciles them positionally, so a pane that changes render
|
||||||
|
* path is unmounted and rebuilt — the composer, terminal, and scroll state in it are lost. A
|
||||||
|
* same-direction split has no reason to move anything: it appends a sibling into the group that
|
||||||
|
* is already running in that direction. Perpendicular splits are excluded because nesting is the
|
||||||
|
* only way to change axis.
|
||||||
|
*/
|
||||||
|
describe("same-direction splits never move an existing pane", () => {
|
||||||
|
function panePathsById(root: SplitNode, path: string[] = []): Map<string, string> {
|
||||||
|
if (root.kind === "pane") {
|
||||||
|
return new Map([[root.pane.id, path.join("/")]]);
|
||||||
|
}
|
||||||
|
const paths = new Map<string, string>();
|
||||||
|
for (const child of root.group.children) {
|
||||||
|
const childKey = child.kind === "pane" ? child.pane.id : child.group.id;
|
||||||
|
for (const [paneId, childPath] of panePathsById(child, [...path, childKey])) {
|
||||||
|
paths.set(paneId, childPath);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return paths;
|
||||||
|
}
|
||||||
|
|
||||||
|
function withPaneTabs(root: SplitNode, paneId: string, tabIds: string[]): SplitNode {
|
||||||
|
if (root.kind === "pane") {
|
||||||
|
return root.pane.id === paneId ? createPane({ id: paneId, tabIds }) : root;
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
kind: "group",
|
||||||
|
group: {
|
||||||
|
...root.group,
|
||||||
|
children: root.group.children.map((child) => withPaneTabs(child, paneId, tabIds)),
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function expectExistingPanesUnmoved(before: SplitNode, after: SplitNode) {
|
||||||
|
const beforePaths = panePathsById(before);
|
||||||
|
const afterPaths = panePathsById(after);
|
||||||
|
for (const [paneId, panePath] of beforePaths) {
|
||||||
|
expect(afterPaths.get(paneId)).toBe(panePath);
|
||||||
|
}
|
||||||
|
expect(getTreeDepth(after)).toBe(getTreeDepth(before));
|
||||||
|
}
|
||||||
|
|
||||||
|
for (const position of ["left", "right"] as const) {
|
||||||
|
it(`keeps every pane in place when a tab is dragged ${position} from the default layout`, () => {
|
||||||
|
workspaceLayoutIds.reset();
|
||||||
|
const mainPaneId = collectAllPanes(createWorkspaceLayoutWithExplorer().root)[0]?.id;
|
||||||
|
expect(mainPaneId).toBeTruthy();
|
||||||
|
const root = withPaneTabs(createWorkspaceLayoutWithExplorer().root, mainPaneId as string, [
|
||||||
|
"tab-a",
|
||||||
|
"tab-b",
|
||||||
|
]);
|
||||||
|
|
||||||
|
const nextRoot = insertSplit(
|
||||||
|
root,
|
||||||
|
mainPaneId as string,
|
||||||
|
"tab-a",
|
||||||
|
position,
|
||||||
|
workspaceLayoutIds.createNodeId,
|
||||||
|
);
|
||||||
|
|
||||||
|
expectExistingPanesUnmoved(root, nextRoot);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
it("keeps every pane in place when splitPaneEmpty opens the first split pane", () => {
|
||||||
|
workspaceLayoutIds.reset();
|
||||||
|
const workspaceKey = createWorkspaceKey();
|
||||||
|
const store = workspaceLayoutStore.getState();
|
||||||
|
const before = createWorkspaceLayoutWithExplorer();
|
||||||
|
const mainPaneId = collectAllPanes(before.root)[0]?.id;
|
||||||
|
expect(mainPaneId).toBeTruthy();
|
||||||
|
|
||||||
|
const newPaneId = store.splitPaneEmpty(workspaceKey, {
|
||||||
|
targetPaneId: mainPaneId as string,
|
||||||
|
position: "right",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(newPaneId).not.toBeNull();
|
||||||
|
const after = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey];
|
||||||
|
expectExistingPanesUnmoved(before.root, after.root);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
it("removePaneFromTree unwraps single-child groups and renormalizes siblings", () => {
|
it("removePaneFromTree unwraps single-child groups and renormalizes siblings", () => {
|
||||||
const root: SplitNode = {
|
const root: SplitNode = {
|
||||||
kind: "group",
|
kind: "group",
|
||||||
@@ -1357,10 +1437,12 @@ describe("workspace-layout-store actions", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey];
|
const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey];
|
||||||
|
// The first split joins the root group instead of nesting under it, so it spends a pane id and
|
||||||
|
// no group id, and the four user splits all fit inside the depth cap.
|
||||||
expect(pane1).toBe("pane_11111111-1111-1111-1111-111111111111");
|
expect(pane1).toBe("pane_11111111-1111-1111-1111-111111111111");
|
||||||
expect(pane2).toBe("pane_33333333-3333-3333-3333-333333333333");
|
expect(pane2).toBe("pane_22222222-2222-2222-2222-222222222222");
|
||||||
expect(pane3).toBe("pane_55555555-5555-5555-5555-555555555555");
|
expect(pane3).toBe("pane_44444444-4444-4444-4444-444444444444");
|
||||||
expect(pane4).toBeNull();
|
expect(pane4).toBe("pane_66666666-6666-6666-6666-666666666666");
|
||||||
expect(getTreeDepth(layout.root)).toBe(5);
|
expect(getTreeDepth(layout.root)).toBe(5);
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -1428,7 +1510,7 @@ describe("workspace-layout-store actions", () => {
|
|||||||
const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey];
|
const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey];
|
||||||
|
|
||||||
expect(paneBId).toBe("pane_78787878-7878-7878-7878-787878787878");
|
expect(paneBId).toBe("pane_78787878-7878-7878-7878-787878787878");
|
||||||
expect(paneCId).toBe("pane_9a9a9a9a-9a9a-9a9a-9a9a-9a9a9a9a9a9a");
|
expect(paneCId).toBe("pane_89898989-8989-8989-8989-898989898989");
|
||||||
expect(layout.focusedPaneId).toBe(paneCId);
|
expect(layout.focusedPaneId).toBe(paneCId);
|
||||||
expect(findPaneById(layout.root, "main")?.tabIds).toEqual(["file_/repo/worktree/a.ts"]);
|
expect(findPaneById(layout.root, "main")?.tabIds).toEqual(["file_/repo/worktree/a.ts"]);
|
||||||
expect(findPaneById(layout.root, paneCId)?.tabIds).toEqual(["file_/repo/worktree/c.ts"]);
|
expect(findPaneById(layout.root, paneCId)?.tabIds).toEqual(["file_/repo/worktree/c.ts"]);
|
||||||
@@ -1577,7 +1659,7 @@ describe("workspace-layout-store actions", () => {
|
|||||||
const total = resizedNestedGroup.group.sizes.reduce((sum, size) => sum + size, 0);
|
const total = resizedNestedGroup.group.sizes.reduce((sum, size) => sum + size, 0);
|
||||||
|
|
||||||
expect(rightPaneId).toBe("pane_eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee");
|
expect(rightPaneId).toBe("pane_eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee");
|
||||||
expect(farRightPaneId).toBe("pane_11111111-1111-1111-1111-111111111111");
|
expect(farRightPaneId).toBe("pane_ffffffff-ffff-ffff-ffff-ffffffffffff");
|
||||||
expect(resizedNestedGroup.group.sizes[0]).toBeGreaterThanOrEqual(0.1);
|
expect(resizedNestedGroup.group.sizes[0]).toBeGreaterThanOrEqual(0.1);
|
||||||
expect(resizedNestedGroup.group.sizes[1]).toBeGreaterThanOrEqual(0.1);
|
expect(resizedNestedGroup.group.sizes[1]).toBeGreaterThanOrEqual(0.1);
|
||||||
expect(total).toBeCloseTo(1, 10);
|
expect(total).toBeCloseTo(1, 10);
|
||||||
|
|||||||
Reference in New Issue
Block a user