mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-05 13:29:44 +02:00
refactor(desktop): pin tab — hover button, full title, drop ⌘⇧P (MUL-2449)
Jiayuan's interactive review of PR #2914 surfaced three changes to the RFC's D1 (entry / visual) decisions: 1. Drop the ⌘⇧P global shortcut — it added a keybinding for a low-frequency action and crowded the shortcut namespace. 2. Reveal a Pin / Unpin button on tab hover instead of relying on the right-click menu as the primary entry; right-click remains as a fallback (and for Close). 3. Pinned tabs keep their full title and width. The only weak visual differences vs. unpinned tabs are the accent left border and the suppressed X close button. Removes the global keydown listener (no other doc / handler referenced it). Adds a hover-only Pin / Unpin span next to the existing close affordance, both gated by group-hover. Drops the icon-only width / hidden-title styling for pinned tabs. Tests: new tab-bar.test.tsx covers Pin / Unpin button rendering, click handlers (togglePin), the hidden-X invariant on pinned tabs, and the full-title rendering. 146 passed, typecheck clean. Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
134
apps/desktop/src/renderer/src/components/tab-bar.test.tsx
Normal file
134
apps/desktop/src/renderer/src/components/tab-bar.test.tsx
Normal file
@@ -0,0 +1,134 @@
|
||||
import { describe, expect, it, vi, beforeEach } from "vitest";
|
||||
import { render, fireEvent, within } from "@testing-library/react";
|
||||
|
||||
type MockTab = {
|
||||
id: string;
|
||||
path: string;
|
||||
title: string;
|
||||
icon: string;
|
||||
pinned: boolean;
|
||||
};
|
||||
|
||||
const state = vi.hoisted(() => ({
|
||||
activeWorkspaceSlug: "acme" as string | null,
|
||||
byWorkspace: {
|
||||
acme: {
|
||||
activeTabId: "tA",
|
||||
tabs: [
|
||||
{ id: "tA", path: "/acme/issues", title: "Issues", icon: "ListTodo", pinned: false },
|
||||
{ id: "tB", path: "/acme/projects", title: "Projects", icon: "ListTodo", pinned: false },
|
||||
] as MockTab[],
|
||||
},
|
||||
} as Record<string, { activeTabId: string; tabs: MockTab[] }>,
|
||||
togglePin: vi.fn<(tabId: string) => void>(),
|
||||
closeTab: vi.fn<(tabId: string) => void>(),
|
||||
setActiveTab: vi.fn<(tabId: string) => void>(),
|
||||
moveTab: vi.fn<(from: number, to: number) => void>(),
|
||||
addTab: vi.fn<(path: string, title: string, icon: string) => string>(),
|
||||
}));
|
||||
|
||||
vi.mock("@/stores/tab-store", () => {
|
||||
const store = {
|
||||
get activeWorkspaceSlug() {
|
||||
return state.activeWorkspaceSlug;
|
||||
},
|
||||
get byWorkspace() {
|
||||
return state.byWorkspace;
|
||||
},
|
||||
togglePin: state.togglePin,
|
||||
closeTab: state.closeTab,
|
||||
setActiveTab: state.setActiveTab,
|
||||
moveTab: state.moveTab,
|
||||
addTab: state.addTab,
|
||||
};
|
||||
const useTabStore = Object.assign(
|
||||
(selector?: (s: typeof store) => unknown) =>
|
||||
selector ? selector(store) : store,
|
||||
{ getState: () => store },
|
||||
);
|
||||
const useActiveGroup = () =>
|
||||
state.activeWorkspaceSlug
|
||||
? (state.byWorkspace[state.activeWorkspaceSlug] ?? null)
|
||||
: null;
|
||||
const resolveRouteIcon = () => "ListTodo";
|
||||
return { useTabStore, useActiveGroup, resolveRouteIcon };
|
||||
});
|
||||
|
||||
vi.mock("@multica/core/paths", () => ({
|
||||
paths: {
|
||||
workspace: (slug: string) => ({
|
||||
issues: () => `/${slug}/issues`,
|
||||
}),
|
||||
},
|
||||
}));
|
||||
|
||||
import { TabBar } from "./tab-bar";
|
||||
|
||||
function reset() {
|
||||
state.activeWorkspaceSlug = "acme";
|
||||
state.byWorkspace = {
|
||||
acme: {
|
||||
activeTabId: "tA",
|
||||
tabs: [
|
||||
{ id: "tA", path: "/acme/issues", title: "Issues", icon: "ListTodo", pinned: false },
|
||||
{ id: "tB", path: "/acme/projects", title: "Projects", icon: "ListTodo", pinned: false },
|
||||
],
|
||||
},
|
||||
};
|
||||
state.togglePin.mockReset();
|
||||
state.closeTab.mockReset();
|
||||
state.setActiveTab.mockReset();
|
||||
state.moveTab.mockReset();
|
||||
state.addTab.mockReset();
|
||||
}
|
||||
|
||||
beforeEach(reset);
|
||||
|
||||
describe("TabBar hover action buttons", () => {
|
||||
it("renders a Pin button on every unpinned tab and an Unpin button on every pinned tab", () => {
|
||||
state.byWorkspace.acme.tabs = [
|
||||
{ id: "tA", path: "/acme/issues", title: "Issues", icon: "ListTodo", pinned: true },
|
||||
{ id: "tB", path: "/acme/projects", title: "Projects", icon: "ListTodo", pinned: false },
|
||||
];
|
||||
const { getAllByLabelText } = render(<TabBar />);
|
||||
expect(getAllByLabelText("Unpin tab")).toHaveLength(1);
|
||||
expect(getAllByLabelText("Pin tab")).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("clicking the Pin button calls togglePin for the tab", () => {
|
||||
const { getAllByLabelText } = render(<TabBar />);
|
||||
const pinButtons = getAllByLabelText("Pin tab");
|
||||
fireEvent.click(pinButtons[1]); // click Pin on tB (Projects)
|
||||
expect(state.togglePin).toHaveBeenCalledWith("tB");
|
||||
});
|
||||
|
||||
it("clicking the Unpin button on a pinned tab calls togglePin", () => {
|
||||
state.byWorkspace.acme.tabs = [
|
||||
{ id: "tA", path: "/acme/issues", title: "Issues", icon: "ListTodo", pinned: true },
|
||||
{ id: "tB", path: "/acme/projects", title: "Projects", icon: "ListTodo", pinned: false },
|
||||
];
|
||||
const { getByLabelText } = render(<TabBar />);
|
||||
fireEvent.click(getByLabelText("Unpin tab"));
|
||||
expect(state.togglePin).toHaveBeenCalledWith("tA");
|
||||
});
|
||||
|
||||
it("hides the X close button on a pinned tab but keeps it on an unpinned tab", () => {
|
||||
state.byWorkspace.acme.tabs = [
|
||||
{ id: "tA", path: "/acme/issues", title: "Issues", icon: "ListTodo", pinned: true },
|
||||
{ id: "tB", path: "/acme/projects", title: "Projects", icon: "ListTodo", pinned: false },
|
||||
];
|
||||
const { queryAllByLabelText } = render(<TabBar />);
|
||||
// Only the unpinned tab exposes a Close affordance — pinned tab requires
|
||||
// explicit Unpin first (RFC §3 D3c FINAL).
|
||||
expect(queryAllByLabelText("Close tab")).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("keeps the full title visible on a pinned tab (no icon-only collapse)", () => {
|
||||
state.byWorkspace.acme.tabs = [
|
||||
{ id: "tA", path: "/acme/issues", title: "Issues", icon: "ListTodo", pinned: true },
|
||||
];
|
||||
const { getByLabelText } = render(<TabBar />);
|
||||
const pinnedTab = getByLabelText("Issues (pinned)");
|
||||
expect(within(pinnedTab).getByText("Issues")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import { Fragment, useEffect } from "react";
|
||||
import { Fragment } from "react";
|
||||
import {
|
||||
Inbox,
|
||||
CircleUser,
|
||||
@@ -36,7 +36,6 @@ import {
|
||||
ContextMenuContent,
|
||||
ContextMenuItem,
|
||||
ContextMenuSeparator,
|
||||
ContextMenuShortcut,
|
||||
ContextMenuTrigger,
|
||||
} from "@multica/ui/components/ui/context-menu";
|
||||
import { cn } from "@multica/ui/lib/utils";
|
||||
@@ -44,7 +43,6 @@ import {
|
||||
useTabStore,
|
||||
useActiveGroup,
|
||||
resolveRouteIcon,
|
||||
getActiveTab,
|
||||
type Tab,
|
||||
} from "@/stores/tab-store";
|
||||
import { paths } from "@multica/core/paths";
|
||||
@@ -69,7 +67,7 @@ function SortableTabItem({
|
||||
/**
|
||||
* True iff this is the only tab in the workspace. Hiding X on the last
|
||||
* tab matches existing behavior and avoids the surprise of the store's
|
||||
* last-tab reseed kicking in. Pinned tabs always hide X (D3c).
|
||||
* last-tab reseed kicking in. Pinned tabs always hide X (RFC §3 D3c).
|
||||
*/
|
||||
isOnly: boolean;
|
||||
}) {
|
||||
@@ -105,18 +103,20 @@ function SortableTabItem({
|
||||
closeTab(tab.id);
|
||||
};
|
||||
|
||||
const stopDragOnClose = (e: React.PointerEvent) => {
|
||||
const handleTogglePin = (e: React.MouseEvent) => {
|
||||
e.stopPropagation();
|
||||
togglePin(tab.id);
|
||||
};
|
||||
|
||||
const stopDragOnAction = (e: React.PointerEvent) => {
|
||||
e.stopPropagation();
|
||||
};
|
||||
|
||||
// Pinned tabs:
|
||||
// - icon-only (no title, no X) — Chrome style, RFC §3 D1v-i FINAL.
|
||||
// - narrow fixed width so they collapse to ~icon + padding.
|
||||
// - accent left border so they read as a distinct group even when the
|
||||
// bar is crowded and the inter-zone gap (rendered by TabBar) gets
|
||||
// hidden by horizontal scroll.
|
||||
// Pinned tabs keep their full title (RFC §3 D1v-ii FINAL). The only weak
|
||||
// visual differences vs. unpinned tabs are the accent left border and the
|
||||
// suppressed X (closing requires explicit Unpin). Pin/Unpin is reachable
|
||||
// via the hover action button below and the right-click menu fallback.
|
||||
const showCloseButton = !tab.pinned && !isOnly;
|
||||
const showTitle = !tab.pinned;
|
||||
|
||||
const tabButton = (
|
||||
<button
|
||||
@@ -128,9 +128,8 @@ function SortableTabItem({
|
||||
aria-label={tab.pinned ? `${tab.title} (pinned)` : tab.title}
|
||||
title={tab.pinned ? `${tab.title} (pinned)` : undefined}
|
||||
className={cn(
|
||||
"group flex h-7 items-center gap-1.5 rounded-md text-xs transition-colors",
|
||||
"group flex h-7 w-40 items-center gap-1.5 rounded-md px-2 text-xs transition-colors",
|
||||
"select-none cursor-default",
|
||||
tab.pinned ? "w-8 justify-center px-1.5" : "w-40 px-2",
|
||||
isActive
|
||||
? "bg-sidebar-accent font-medium text-sidebar-accent-foreground"
|
||||
: "bg-sidebar-accent/50 text-muted-foreground hover:bg-sidebar-accent hover:text-sidebar-accent-foreground",
|
||||
@@ -139,21 +138,31 @@ function SortableTabItem({
|
||||
)}
|
||||
>
|
||||
{Icon && <Icon className="size-3.5 shrink-0" />}
|
||||
{showTitle && (
|
||||
<span
|
||||
className="min-w-0 flex-1 overflow-hidden whitespace-nowrap text-left"
|
||||
style={{
|
||||
maskImage: "linear-gradient(to right, black calc(100% - 12px), transparent)",
|
||||
WebkitMaskImage: "linear-gradient(to right, black calc(100% - 12px), transparent)",
|
||||
}}
|
||||
>
|
||||
{tab.title}
|
||||
</span>
|
||||
)}
|
||||
<span
|
||||
className="min-w-0 flex-1 overflow-hidden whitespace-nowrap text-left"
|
||||
style={{
|
||||
maskImage: "linear-gradient(to right, black calc(100% - 12px), transparent)",
|
||||
WebkitMaskImage: "linear-gradient(to right, black calc(100% - 12px), transparent)",
|
||||
}}
|
||||
>
|
||||
{tab.title}
|
||||
</span>
|
||||
<span
|
||||
onClick={handleTogglePin}
|
||||
onPointerDown={stopDragOnAction}
|
||||
role="button"
|
||||
aria-label={tab.pinned ? "Unpin tab" : "Pin tab"}
|
||||
title={tab.pinned ? "Unpin tab" : "Pin tab"}
|
||||
className="hidden size-3.5 shrink-0 items-center justify-center rounded-sm text-muted-foreground transition-colors group-hover:flex hover:bg-muted-foreground/20 hover:text-foreground"
|
||||
>
|
||||
{tab.pinned ? <PinOff className="size-2.5" /> : <Pin className="size-2.5" />}
|
||||
</span>
|
||||
{showCloseButton && (
|
||||
<span
|
||||
onClick={handleClose}
|
||||
onPointerDown={stopDragOnClose}
|
||||
onPointerDown={stopDragOnAction}
|
||||
role="button"
|
||||
aria-label="Close tab"
|
||||
className="hidden size-3.5 shrink-0 items-center justify-center rounded-sm text-muted-foreground transition-colors group-hover:flex hover:bg-muted-foreground/20 hover:text-foreground"
|
||||
>
|
||||
<X className="size-2.5" />
|
||||
@@ -178,7 +187,6 @@ function SortableTabItem({
|
||||
Pin tab
|
||||
</>
|
||||
)}
|
||||
<ContextMenuShortcut>⌘⇧P</ContextMenuShortcut>
|
||||
</ContextMenuItem>
|
||||
<ContextMenuSeparator />
|
||||
<ContextMenuItem
|
||||
@@ -219,33 +227,9 @@ function NewTabButton() {
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Listens for Cmd/Ctrl+Shift+P at the window level and toggles pin on the
|
||||
* active tab. Mounted once by TabBar so it stays alive for the entire
|
||||
* lifetime of the dashboard shell; unmounting on workspace switch would
|
||||
* miss keypresses during the transition.
|
||||
*/
|
||||
function usePinShortcut() {
|
||||
const togglePin = useTabStore((s) => s.togglePin);
|
||||
useEffect(() => {
|
||||
const handler = (e: KeyboardEvent) => {
|
||||
if (!e.shiftKey) return;
|
||||
if (!(e.metaKey || e.ctrlKey)) return;
|
||||
if (e.key !== "P" && e.key !== "p") return;
|
||||
const active = getActiveTab(useTabStore.getState());
|
||||
if (!active) return;
|
||||
e.preventDefault();
|
||||
togglePin(active.id);
|
||||
};
|
||||
window.addEventListener("keydown", handler);
|
||||
return () => window.removeEventListener("keydown", handler);
|
||||
}, [togglePin]);
|
||||
}
|
||||
|
||||
export function TabBar() {
|
||||
const group = useActiveGroup();
|
||||
const moveTab = useTabStore((s) => s.moveTab);
|
||||
usePinShortcut();
|
||||
|
||||
const sensors = useSensors(
|
||||
useSensor(PointerSensor, {
|
||||
|
||||
Reference in New Issue
Block a user