fix(web): keep dockview tab close buttons reachable in a narrow group (#2918)
Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -67,12 +67,44 @@
|
||||
display: none !important;
|
||||
}
|
||||
|
||||
/* Give the tab strip back native horizontal scrolling.
|
||||
*
|
||||
* Dockview hides the strip's overflow and drives scrolling entirely from its
|
||||
* own `wheel` handler, which reads `deltaY` only: a two-finger horizontal
|
||||
* swipe on a touchpad produces `deltaX` and moved nothing. Handing the axis
|
||||
* back to the browser makes the gesture work. Dockview's overlay scrollbar
|
||||
* stays the visible affordance and follows along (it syncs from the element's
|
||||
* `scroll` event), so the native bar is hidden rather than squeezed into the
|
||||
* 40px strip. */
|
||||
.dockview-theme-kandev .dv-scrollable > .dv-tabs-container {
|
||||
overflow-x: auto;
|
||||
scrollbar-width: none;
|
||||
}
|
||||
|
||||
.dockview-theme-kandev .dv-scrollable > .dv-tabs-container::-webkit-scrollbar {
|
||||
display: none;
|
||||
}
|
||||
|
||||
/* Tabs shrink before the strip clips them.
|
||||
*
|
||||
* Dockview pins `.dv-tab` at `flex-shrink: 0`, so a group narrower than its tab
|
||||
* row kept every tab at full width and the strip clipped the row from the
|
||||
* right. The close button sits at a tab's right edge, so it was the first thing
|
||||
* to disappear and a tab could become impossible to close without widening the
|
||||
* group again. Shrinking down to a floor truncates the label instead and keeps
|
||||
* the close button in view; past the floor Dockview's own overflow dropdown
|
||||
* takes over.
|
||||
*
|
||||
* The floor is 4rem because that leaves a few characters of label beside the
|
||||
* close button, and because it sits just under the natural width of the
|
||||
* shortest panel titles ("Plan", "Files") so the common case is not widened. */
|
||||
.dockview-theme-kandev .dv-tab {
|
||||
padding: 0;
|
||||
margin: 0 1px;
|
||||
font-size: 12px;
|
||||
font-weight: 500;
|
||||
min-width: auto;
|
||||
flex-shrink: 1;
|
||||
min-width: 4rem;
|
||||
border-radius: 5px;
|
||||
border-bottom: none !important;
|
||||
outline: none;
|
||||
@@ -107,7 +139,26 @@
|
||||
line-height: 1;
|
||||
}
|
||||
|
||||
/* Every wrapper a tab component puts between `.dv-tab` and `.dv-default-tab`
|
||||
* (context-menu trigger, terminal ordinal row, session badge row, plugin icon
|
||||
* row) is a flex container, and a flex item's default `min-width: auto` would
|
||||
* stop the shrink above before it ever reaches the label. */
|
||||
.dockview-theme-kandev .dv-tab *:has(.dv-default-tab),
|
||||
.dockview-theme-kandev .dv-default-tab {
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
.dockview-theme-kandev .dv-default-tab .dv-default-tab-content {
|
||||
min-width: 0;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
/* `flex-shrink: 0` — the close control is the one part of a tab that must
|
||||
* never be shrunk away. */
|
||||
.dockview-theme-kandev .dv-default-tab .dv-default-tab-action {
|
||||
flex-shrink: 0;
|
||||
opacity: 0;
|
||||
transition:
|
||||
opacity 0.15s ease,
|
||||
@@ -144,6 +195,7 @@
|
||||
/* Session tabs own their delete control so the X can transition to an
|
||||
* in-place spinner while the delete request is in flight. */
|
||||
.dockview-theme-kandev .session-tab-close-action {
|
||||
flex-shrink: 0;
|
||||
opacity: 0;
|
||||
transition:
|
||||
opacity 0.15s ease,
|
||||
|
||||
@@ -13,6 +13,7 @@ import { cn } from "@/lib/utils";
|
||||
import { useDockviewStore } from "@/lib/state/dockview-store";
|
||||
import type { PreviewType } from "@/lib/state/dockview-panel-actions";
|
||||
import { useTabMaximizeOnDoubleClick } from "./use-tab-maximize";
|
||||
import { useTabContextActions } from "./use-tab-context-actions";
|
||||
import { useTranslation } from "react-i18next";
|
||||
|
||||
/**
|
||||
@@ -35,25 +36,6 @@ export function useMiddleClickClose(
|
||||
);
|
||||
}
|
||||
|
||||
function useTabContextActions(
|
||||
api: IDockviewPanelHeaderProps["api"],
|
||||
containerApi: IDockviewPanelHeaderProps["containerApi"],
|
||||
) {
|
||||
const handleClose = useCallback(() => {
|
||||
const panel = containerApi.getPanel(api.id);
|
||||
if (panel) containerApi.removePanel(panel);
|
||||
}, [api, containerApi]);
|
||||
|
||||
const handleCloseOthers = useCallback(() => {
|
||||
const toClose = api.group.panels.filter(
|
||||
(p) => p.id !== api.id && p.id !== "chat" && !p.id.startsWith("session:"),
|
||||
);
|
||||
for (const panel of toClose) containerApi.removePanel(panel);
|
||||
}, [api, containerApi]);
|
||||
|
||||
return { handleClose, handleCloseOthers };
|
||||
}
|
||||
|
||||
/**
|
||||
* Preview tab: italic title + double-click to pin + middle-click to close.
|
||||
* One per preview type (file-editor / file-diff / commit-detail).
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
import { describe, it, expect, vi, afterEach } from "vitest";
|
||||
import { render, screen, fireEvent, cleanup } from "@testing-library/react";
|
||||
|
||||
vi.mock("dockview-react", () => ({
|
||||
DockviewDefaultTab: () => null,
|
||||
}));
|
||||
|
||||
vi.mock("./use-tab-maximize", () => ({
|
||||
useTabMaximizeOnDoubleClick: () => () => {},
|
||||
}));
|
||||
|
||||
// Render the menu inline so its items are clickable without driving Radix's
|
||||
// pointer choreography.
|
||||
vi.mock("@kandev/ui/context-menu", () => ({
|
||||
ContextMenu: ({ children }: { children: React.ReactNode }) => <>{children}</>,
|
||||
ContextMenuTrigger: ({ children }: { children: React.ReactNode }) => <>{children}</>,
|
||||
ContextMenuContent: ({ children }: { children: React.ReactNode }) => <>{children}</>,
|
||||
ContextMenuItem: ({
|
||||
children,
|
||||
onSelect,
|
||||
disabled,
|
||||
}: {
|
||||
children: React.ReactNode;
|
||||
onSelect: () => void;
|
||||
disabled?: boolean;
|
||||
}) => (
|
||||
<button type="button" disabled={disabled} onClick={onSelect}>
|
||||
{children}
|
||||
</button>
|
||||
),
|
||||
}));
|
||||
|
||||
import { ContextMenuTab } from "./tab-context-menu";
|
||||
|
||||
type Panel = { id: string };
|
||||
|
||||
function makeProps(panelId: string, groupPanels: string[], params?: Record<string, unknown>) {
|
||||
const removePanel = vi.fn();
|
||||
const panels: Panel[] = groupPanels.map((id) => ({ id }));
|
||||
const api = { id: panelId, group: { panels } };
|
||||
const containerApi = {
|
||||
getPanel: (id: string) => panels.find((panel) => panel.id === id),
|
||||
removePanel,
|
||||
};
|
||||
return {
|
||||
removePanel,
|
||||
props: { api, containerApi, params } as unknown as React.ComponentProps<typeof ContextMenuTab>,
|
||||
};
|
||||
}
|
||||
|
||||
describe("ContextMenuTab", () => {
|
||||
afterEach(() => cleanup());
|
||||
|
||||
it("closes its own panel from the menu", () => {
|
||||
const { props, removePanel } = makeProps("browser", ["chat", "browser", "terminal"]);
|
||||
render(<ContextMenuTab {...props} />);
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "Close" }));
|
||||
|
||||
expect(removePanel).toHaveBeenCalledTimes(1);
|
||||
expect(removePanel).toHaveBeenCalledWith({ id: "browser" });
|
||||
});
|
||||
|
||||
it("closes siblings but spares the chat and session panels", () => {
|
||||
const { props, removePanel } = makeProps("browser", [
|
||||
"chat",
|
||||
"session:abc",
|
||||
"browser",
|
||||
"terminal",
|
||||
"files",
|
||||
]);
|
||||
render(<ContextMenuTab {...props} />);
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "Close Others" }));
|
||||
|
||||
expect(removePanel.mock.calls.map(([panel]) => (panel as Panel).id)).toEqual([
|
||||
"terminal",
|
||||
"files",
|
||||
]);
|
||||
});
|
||||
|
||||
it("renders panel-injected items alongside the close actions", () => {
|
||||
const onSelect = vi.fn();
|
||||
const { props } = makeProps("browser", ["browser"], {
|
||||
contextMenuItems: [{ label: "Reload", onSelect }],
|
||||
});
|
||||
render(<ContextMenuTab {...props} />);
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "Reload" }));
|
||||
|
||||
expect(onSelect).toHaveBeenCalledTimes(1);
|
||||
expect(screen.getByRole("button", { name: "Close" })).toBeTruthy();
|
||||
expect(screen.getByRole("button", { name: "Close Others" })).toBeTruthy();
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,5 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback } from "react";
|
||||
import { DockviewDefaultTab, type IDockviewPanelHeaderProps } from "dockview-react";
|
||||
import {
|
||||
ContextMenu,
|
||||
@@ -9,6 +8,7 @@ import {
|
||||
ContextMenuTrigger,
|
||||
} from "@kandev/ui/context-menu";
|
||||
import { useTabMaximizeOnDoubleClick } from "./use-tab-maximize";
|
||||
import { useTabContextActions } from "./use-tab-context-actions";
|
||||
import { useTranslation } from "react-i18next";
|
||||
|
||||
/** An item in the tab right-click context menu.
|
||||
@@ -25,19 +25,13 @@ export type TabContextMenuParams = {
|
||||
};
|
||||
|
||||
/** Default tab component — wraps DockviewDefaultTab with a right-click menu.
|
||||
* Always provides "Close Others". Panels may inject additional items via
|
||||
* props.params.contextMenuItems. */
|
||||
* Always provides "Close" and "Close Others". Panels may inject additional
|
||||
* items via props.params.contextMenuItems. */
|
||||
export function ContextMenuTab(props: IDockviewPanelHeaderProps) {
|
||||
const { t } = useTranslation();
|
||||
const { api, containerApi } = props;
|
||||
const onDoubleClick = useTabMaximizeOnDoubleClick(api);
|
||||
|
||||
const handleCloseOthers = useCallback(() => {
|
||||
const toClose = api.group.panels.filter(
|
||||
(p) => p.id !== api.id && p.id !== "chat" && !p.id.startsWith("session:"),
|
||||
);
|
||||
for (const panel of toClose) containerApi.removePanel(panel);
|
||||
}, [api, containerApi]);
|
||||
const { handleClose, handleCloseOthers } = useTabContextActions(api, containerApi);
|
||||
|
||||
const extraItems: TabContextMenuItem[] =
|
||||
(props.params as TabContextMenuParams | undefined)?.contextMenuItems ?? [];
|
||||
@@ -52,11 +46,21 @@ export function ContextMenuTab(props: IDockviewPanelHeaderProps) {
|
||||
</ContextMenuTrigger>
|
||||
<ContextMenuContent>
|
||||
{extraItems.map((item) => (
|
||||
<ContextMenuItem key={item.label} onSelect={item.onSelect} disabled={item.disabled}>
|
||||
<ContextMenuItem
|
||||
key={item.label}
|
||||
className="cursor-pointer"
|
||||
onSelect={item.onSelect}
|
||||
disabled={item.disabled}
|
||||
>
|
||||
{item.label}
|
||||
</ContextMenuItem>
|
||||
))}
|
||||
<ContextMenuItem onSelect={handleCloseOthers}>{t("task:closeOthers")}</ContextMenuItem>
|
||||
<ContextMenuItem className="cursor-pointer" onSelect={handleClose}>
|
||||
{t("task:close")}
|
||||
</ContextMenuItem>
|
||||
<ContextMenuItem className="cursor-pointer" onSelect={handleCloseOthers}>
|
||||
{t("task:closeOthers")}
|
||||
</ContextMenuItem>
|
||||
</ContextMenuContent>
|
||||
</ContextMenu>
|
||||
);
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback } from "react";
|
||||
import type { IDockviewPanelHeaderProps } from "dockview-react";
|
||||
|
||||
/**
|
||||
* The close actions every dockview tab context menu offers.
|
||||
*
|
||||
* `Close` matters beyond convenience: a group narrow enough to push a tab's
|
||||
* close button toward the edge of the strip leaves the context menu as the
|
||||
* reachable way to close it.
|
||||
*
|
||||
* `Close Others` deliberately spares the chat and session panels — they are
|
||||
* the task's permanent tabs and closing them is a session action, not a panel
|
||||
* one.
|
||||
*/
|
||||
export function useTabContextActions(
|
||||
api: IDockviewPanelHeaderProps["api"],
|
||||
containerApi: IDockviewPanelHeaderProps["containerApi"],
|
||||
) {
|
||||
const handleClose = useCallback(() => {
|
||||
const panel = containerApi.getPanel(api.id);
|
||||
if (panel) containerApi.removePanel(panel);
|
||||
}, [api, containerApi]);
|
||||
|
||||
const handleCloseOthers = useCallback(() => {
|
||||
const toClose = api.group.panels.filter(
|
||||
(p) => p.id !== api.id && p.id !== "chat" && !p.id.startsWith("session:"),
|
||||
);
|
||||
for (const panel of toClose) containerApi.removePanel(panel);
|
||||
}, [api, containerApi]);
|
||||
|
||||
return { handleClose, handleCloseOthers };
|
||||
}
|
||||
@@ -0,0 +1,129 @@
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
import type { Locator, Page } from "@playwright/test";
|
||||
import { openWideTask, resizeColumnViaSplitview } from "../../helpers/dockview-resize";
|
||||
|
||||
/**
|
||||
* A group narrow enough to squeeze its tab row has to take the space out of
|
||||
* the titles, not out of the close buttons.
|
||||
*
|
||||
* Dockview pins `.dv-tab` at `flex-shrink: 0`, so before the theme override in
|
||||
* `app/dockview-theme.css` the titles stayed at full width and the strip
|
||||
* clipped the row from the right instead. The close button sits at a tab's
|
||||
* right edge, so it was the first thing to go and a tab could become
|
||||
* impossible to close without widening the group again.
|
||||
*/
|
||||
|
||||
type TabProbe = {
|
||||
title: string;
|
||||
tabWidth: number;
|
||||
closeWidth: number;
|
||||
truncated: boolean;
|
||||
};
|
||||
|
||||
/**
|
||||
* Locate the Dockview group that owns the `Files` tab and hand back the two
|
||||
* elements every reading here comes from.
|
||||
*
|
||||
* Dockview can mount more than one group, so the lookup refuses to guess:
|
||||
* measuring one tab row while scrolling another would still look like a pass.
|
||||
* Returned via `data-*` markers because `page.evaluate` cannot return DOM
|
||||
* handles to the test process.
|
||||
*/
|
||||
async function markFilesGroup(page: Page): Promise<{ strip: Locator; row: Locator }> {
|
||||
await page.evaluate(() => {
|
||||
const strips = Array.from(document.querySelectorAll(".dv-tabs-and-actions-container"));
|
||||
const matches = strips.filter((strip) =>
|
||||
Array.from(strip.querySelectorAll(".dv-default-tab-content")).some(
|
||||
(label) => label.textContent?.trim() === "Files",
|
||||
),
|
||||
);
|
||||
if (matches.length !== 1) {
|
||||
throw new Error(
|
||||
`expected exactly one tab strip holding a Files tab, found ${matches.length}`,
|
||||
);
|
||||
}
|
||||
for (const marked of Array.from(document.querySelectorAll("[data-probe-strip]"))) {
|
||||
marked.removeAttribute("data-probe-strip");
|
||||
}
|
||||
matches[0].setAttribute("data-probe-strip", "files");
|
||||
});
|
||||
const strip = page.locator("[data-probe-strip='files']");
|
||||
return { strip, row: strip.locator(".dv-tabs-container") };
|
||||
}
|
||||
|
||||
/** Measure every tab of the marked group. */
|
||||
async function probeTabStrip(strip: Locator): Promise<TabProbe[]> {
|
||||
return strip.evaluate((element) =>
|
||||
Array.from(element.querySelectorAll(".dv-tab")).map((tab) => {
|
||||
const label = tab.querySelector(".dv-default-tab-content") as HTMLElement | null;
|
||||
const close = tab.querySelector(".dv-default-tab-action") as HTMLElement | null;
|
||||
if (!label || !close) {
|
||||
throw new Error(
|
||||
`tab "${label?.textContent?.trim() ?? tab.className}" has no label or close button`,
|
||||
);
|
||||
}
|
||||
return {
|
||||
title: label.textContent?.trim() ?? "",
|
||||
tabWidth: tab.getBoundingClientRect().width,
|
||||
closeWidth: close.getBoundingClientRect().width,
|
||||
truncated: label.scrollWidth > label.clientWidth,
|
||||
};
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
const rowWidth = (tabs: TabProbe[]): number => tabs.reduce((total, tab) => total + tab.tabWidth, 0);
|
||||
|
||||
const rowOverflow = (row: Locator): Promise<number> =>
|
||||
row.evaluate((element) => element.scrollWidth - element.clientWidth);
|
||||
|
||||
const rowScrollLeft = (row: Locator): Promise<number> =>
|
||||
row.evaluate((element) => element.scrollLeft);
|
||||
|
||||
test.describe("narrow tab strip", () => {
|
||||
test("squeezes tab titles rather than the close buttons", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
await openWideTask(testPage, apiClient, seedData, "Narrow right column tabs");
|
||||
const { strip } = await markFilesGroup(testPage);
|
||||
|
||||
const wide = await probeTabStrip(strip);
|
||||
expect(wide.length).toBeGreaterThan(1);
|
||||
expect(wide.map((tab) => tab.truncated)).not.toContain(true);
|
||||
expect(wide.every((tab) => tab.closeWidth > 0)).toBe(true);
|
||||
|
||||
await resizeColumnViaSplitview(testPage, "right", 240);
|
||||
const narrow = await probeTabStrip(strip);
|
||||
|
||||
expect(narrow.map((tab) => tab.title)).toEqual(wide.map((tab) => tab.title));
|
||||
// The tabs give up width...
|
||||
expect(rowWidth(narrow)).toBeLessThan(rowWidth(wide));
|
||||
// ...their titles ellipsize...
|
||||
expect(narrow.map((tab) => tab.truncated)).toContain(true);
|
||||
// ...and the close buttons keep every pixel they had.
|
||||
expect(narrow.map((tab) => tab.closeWidth)).toEqual(wide.map((tab) => tab.closeWidth));
|
||||
});
|
||||
|
||||
test("scrolls the tab row with a horizontal wheel gesture", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
await openWideTask(testPage, apiClient, seedData, "Horizontal tab scroll");
|
||||
await resizeColumnViaSplitview(testPage, "right", 240);
|
||||
const { row } = await markFilesGroup(testPage);
|
||||
|
||||
const filesTab = row.locator(".dv-default-tab").filter({ hasText: /^Files$/ });
|
||||
await expect(filesTab).toBeVisible();
|
||||
expect(await rowOverflow(row)).toBeGreaterThan(0);
|
||||
|
||||
// A two-finger horizontal touchpad swipe reaches the page as `deltaX`,
|
||||
// which Dockview's own wheel handler ignores.
|
||||
await filesTab.hover();
|
||||
await testPage.mouse.wheel(60, 0);
|
||||
|
||||
await expect.poll(() => rowScrollLeft(row)).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user