diff --git a/packages/app/e2e/browser/sidebar-reorder.spec.ts b/packages/app/e2e/browser/sidebar-reorder.spec.ts index 56cdd3dc5a..cf885a1d14 100644 --- a/packages/app/e2e/browser/sidebar-reorder.spec.ts +++ b/packages/app/e2e/browser/sidebar-reorder.spec.ts @@ -426,3 +426,81 @@ test("Manual restores pre-beta order and keeps drag order across sorting and rel await seed.cleanup(); } }); + +function workspaceRows(page: Page, ids: string[]) { + return page.locator( + ids.map((id) => `[data-testid="sidebar-workspace-row-${getServerId()}:${id}"]`).join(", "), + ); +} + +async function openWorkspaceRow(page: Page, workspaceId: string) { + await page.getByTestId(`sidebar-workspace-row-${getServerId()}:${workspaceId}`).click(); + await expect(page).toHaveURL(new RegExp(`/workspace/${workspaceId}`)); +} + +test("collapsed groups lead with the open workspace instead of hiding it behind Show more", async ({ + page, +}) => { + test.setTimeout(120_000); + const { seed, ids } = await seedCompactProject(); + try { + await gotoAppShell(page); + await waitForSidebarHydration(page); + await selectWorkspaceSort(page, "Created"); + const { rows, group } = compactProjectRows(page, seed); + await expectWorkspaceOrder(rows, ids.slice(0, 7)); + await revealWorkspaces(group, "Show more"); + await openWorkspaceRow(page, ids[9]!); + await expectWorkspaceOrder(rows, ids); + await revealWorkspaces(group, "Show less"); + await expectWorkspaceOrder(rows, [ids[9]!, ...ids.slice(0, 7)]); + await expect( + page.getByTestId(`sidebar-workspace-row-${getServerId()}:${ids[9]!}`), + ).toHaveAttribute("aria-selected", "true"); + await reconnectSidebar(page); + await expectWorkspaceOrder(rows, [ids[9]!, ...ids.slice(0, 7)]); + await expect( + page.getByTestId(`sidebar-workspace-row-${getServerId()}:${ids[9]!}`), + ).toHaveAttribute("aria-selected", "true"); + await openWorkspaceRow(page, ids[2]!); + await expectWorkspaceOrder(rows, ids.slice(0, 7)); + await expect( + page.getByTestId(`sidebar-workspace-row-${getServerId()}:${ids[2]!}`), + ).toHaveAttribute("aria-selected", "true"); + } finally { + await seed.cleanup(); + } +}); + +test("a Manual drag in a collapsed group keeps the open workspace's saved place", async ({ + page, +}) => { + test.setTimeout(120_000); + const { seed, ids } = await seedCompactProject(); + try { + await gotoAppShell(page); + await waitForSidebarHydration(page); + const projectKey = projectEquivalenceViewKey(seed.projectKey); + await restorePreBetaOrder( + page, + projectKey, + ids.map((id) => `${getServerId()}:${id}`), + ); + await selectWorkspaceSort(page, "Manual"); + const rows = workspaceRows(page, ids); + const toggle = page.getByTestId(`sidebar-project-show-more-${projectKey}`); + await expectWorkspaceOrder(rows, ids.slice(0, 7)); + await toggle.click(); + await openWorkspaceRow(page, ids[9]!); + await toggle.click(); + await expectWorkspaceOrder(rows, [ids[9]!, ...ids.slice(0, 7)]); + await quickDragFirstRowAfterSecond(workspaceRows(page, [ids[0]!, ids[1]!]), pressWorkspaceRow); + await expectWorkspaceOrder(rows, [ids[9]!, ids[1]!, ids[0]!, ...ids.slice(2, 7)]); + await reconnectSidebar(page); + await expectWorkspaceOrder(rows, [ids[9]!, ids[1]!, ids[0]!, ...ids.slice(2, 7)]); + await toggle.click(); + await expectWorkspaceOrder(rows, [ids[1]!, ids[0]!, ...ids.slice(2)]); + } finally { + await seed.cleanup(); + } +}); diff --git a/packages/app/e2e/browser/workspace-relative-navigation.spec.ts b/packages/app/e2e/browser/workspace-relative-navigation.spec.ts index 909672c489..5066411c78 100644 --- a/packages/app/e2e/browser/workspace-relative-navigation.spec.ts +++ b/packages/app/e2e/browser/workspace-relative-navigation.spec.ts @@ -70,25 +70,31 @@ test("previous and next traverse every visible workspace while digit shortcuts s workspaceId: order[8], }); }); - await test.step("hidden Done rows remain shortcut targets without expanding the project", async () => { - await expectHiddenWorkspaceShortcuts(page, order); + await test.step("collapsed groups keep shortcut targets selected without expanding", async () => { + await expectCollapsedWorkspaceShortcuts(page, order); }); } finally { await project.cleanup(); } }); -async function expectHiddenWorkspaceShortcuts(page: Page, order: string[]) { +async function expectCollapsedWorkspaceShortcuts(page: Page, order: string[]) { await page.getByLabel("Show less", { exact: true }).click(); const rows = page.getByRole("button", { name: /^Workspace \d{2}$/ }); - await expect(rows).toHaveCount(7); + await expect(rows).toHaveCount(8); + await expectSidebarWorkspaceSelected({ page, serverId: getServerId(), workspaceId: order[8] }); await switchWorkspaceViaSidebar({ page, serverId: getServerId(), workspaceId: order[0] }); + await expect(rows).toHaveCount(7); await page.keyboard.press("Alt+9"); await expect(page).toHaveURL(new RegExp(`/workspace/${order[8]}$`)); + await expectSidebarWorkspaceSelected({ page, serverId: getServerId(), workspaceId: order[8] }); + await expect(rows).toHaveCount(8); + await page.keyboard.press("Alt+]"); + await expect(page).toHaveURL(new RegExp(`/workspace/${order[9]}$`)); + await expectSidebarWorkspaceSelected({ page, serverId: getServerId(), workspaceId: order[9] }); + await expect(rows).toHaveCount(8); await expect(page.getByTestId(`sidebar-workspace-row-${getServerId()}:${order[8]}`)).toHaveCount( 0, ); - await page.keyboard.press("Alt+]"); - await expect(page).toHaveURL(new RegExp(`/workspace/${order[9]}$`)); - await expect(rows).toHaveCount(7); + await expect(page.getByLabel("Show more", { exact: true })).toBeVisible(); } diff --git a/packages/app/src/components/sidebar-workspace-list.test.tsx b/packages/app/src/components/sidebar-workspace-list.test.tsx index fc57dc2dce..9532c422dd 100644 --- a/packages/app/src/components/sidebar-workspace-list.test.tsx +++ b/packages/app/src/components/sidebar-workspace-list.test.tsx @@ -462,7 +462,7 @@ describe("sidebar group visibility", () => { it.each([0, 7, 8])("limits %i Done rows to seven", (count) => { const items = entries(Array(count).fill("done")); - const { result } = renderHook(() => useLimitedSidebarGroup(items, policy)); + const { result } = renderHook(() => useLimitedSidebarGroup({ items, selection: null, policy })); expect(result.current.visibleItems).toEqual(items.slice(0, 7)); expect(result.current.canToggle).toBe(count > 7); }); @@ -471,7 +471,9 @@ describe("sidebar group visibility", () => { "keeps row twelve visible for %s without filling gaps", (status) => { const items = entries([...Array(11).fill("done"), status]); - const { result } = renderHook(() => useLimitedSidebarGroup(items, policy)); + const { result } = renderHook(() => + useLimitedSidebarGroup({ items, selection: null, policy }), + ); expect(names(result.current.visibleItems)).toEqual([ "row-1", "row-2", @@ -492,7 +494,11 @@ describe("sidebar group visibility", () => { const hydrated = new Map(items.slice(0, 9).map((item) => [item.workspaceKey, item])); for (const entriesByKey of [new Map(), hydrated]) { const { result } = renderHook(() => - useLimitedSidebarGroup(items, createWorkspaceVisibilityPolicy(entriesByKey)), + useLimitedSidebarGroup({ + items, + selection: null, + policy: createWorkspaceVisibilityPolicy(entriesByKey), + }), ); expect(result.current.canToggle).toBe(true); expect(names(result.current.visibleItems)).toEqual( @@ -512,7 +518,7 @@ describe("sidebar group visibility", () => { "needs_input", "failed", ]); - const { result } = renderHook(() => useLimitedSidebarGroup(items, policy)); + const { result } = renderHook(() => useLimitedSidebarGroup({ items, selection: null, policy })); expect(result.current.visibleItems).toEqual(items); expect(result.current.canToggle).toBe(false); }); @@ -527,7 +533,7 @@ describe("sidebar group visibility", () => { "attention", ]); const { result, rerender, unmount } = renderHook( - ({ rows }) => useLimitedSidebarGroup(rows, policy), + ({ rows }) => useLimitedSidebarGroup({ items: rows, selection: null, policy }), { initialProps: { rows: items } }, ); expect(names(result.current.visibleItems)).toEqual([ @@ -568,7 +574,7 @@ describe("sidebar group visibility", () => { expect(result.current.expanded).toBe(false); act(() => result.current.toggleExpanded()); unmount(); - const remounted = renderHook(() => useLimitedSidebarGroup(items, policy)); + const remounted = renderHook(() => useLimitedSidebarGroup({ items, selection: null, policy })); expect(remounted.result.current.expanded).toBe(false); expect(remounted.result.current.visibleItems).toHaveLength(10); }); @@ -592,7 +598,12 @@ describe("sidebar group visibility", () => { lifecycle: "active" as const, }; const { result, rerender } = renderHook( - ({ last }) => useLimitedSidebarGroup([...items.slice(0, 11), last], policy), + ({ last }) => + useLimitedSidebarGroup({ + items: [...items.slice(0, 11), last], + selection: null, + policy, + }), { initialProps: { last: items[11] } }, ); expect(result.current.visibleItems).toHaveLength(7); @@ -630,7 +641,7 @@ describe("sidebar group visibility", () => { it("preserves the twenty-row pinned policy regardless of status", () => { const items = entries(Array(21).fill("running")); - const { result } = renderHook(() => useLimitedSidebarGroup(items)); + const { result } = renderHook(() => useLimitedSidebarGroup({ items, selection: null })); expect(result.current.visibleItems).toEqual(items.slice(0, 20)); expect(result.current.canToggle).toBe(true); act(() => result.current.toggleExpanded()); diff --git a/packages/app/src/components/sidebar-workspace-list.tsx b/packages/app/src/components/sidebar-workspace-list.tsx index 333eb68221..8697a4b27b 100644 --- a/packages/app/src/components/sidebar-workspace-list.tsx +++ b/packages/app/src/components/sidebar-workspace-list.tsx @@ -1598,11 +1598,16 @@ function ProjectBlock({ [workspaceEntriesByKey], ); const { + leadingItem: leadingWorkspace, visibleItems: visibleWorkspaces, expanded: workspacesExpanded, canToggle: canToggleWorkspaces, toggleExpanded: toggleWorkspacesExpanded, - } = useLimitedSidebarGroup(project.workspaces, visibilityPolicy); + } = useLimitedSidebarGroup({ + items: project.workspaces, + selection: selectionEnabled ? activeWorkspaceSelection : null, + policy: visibilityPolicy, + }); const activeServerId = activeWorkspaceSelection?.serverId ?? null; const activeHostStatus = useHostRuntimeConnectionStatus(activeServerId ?? ""); const preferredServerId = activeHostStatus === "online" ? activeServerId : null; @@ -1761,28 +1766,33 @@ function ProjectBlock({ let projectChildren = null; if (!collapsed) { if (project.workspaces.length > 0) { + const leadingWorkspaceRow = leadingWorkspace ? renderWorkspaceRow(leadingWorkspace) : null; projectChildren = ( <> {manualSort ? ( - + <> + {leadingWorkspaceRow} + + ) : ( + {leadingWorkspaceRow} {visibleWorkspaces.map((item) => renderWorkspaceRow(item))} )} @@ -2188,11 +2198,15 @@ function ProjectModeList({ const activeWorkspaceSelection = useActiveWorkspaceSelection(); const { pinnedChats, unpinnedProjects } = pinnedGroups; const { + leadingItem: leadingPinnedChat, visibleItems: visiblePinnedChats, expanded: pinnedChatsExpanded, canToggle: canTogglePinnedChats, toggleExpanded: togglePinnedChatsExpanded, - } = useLimitedSidebarGroup(pinnedChats); + } = useLimitedSidebarGroup({ + items: pinnedChats, + selection: selectionEnabled ? activeWorkspaceSelection : null, + }); const nativeScrollGestureProps = useMemo( () => parentGestureRef @@ -2395,37 +2409,38 @@ function ProjectModeList({ [renderProjectBlock], ); - const renderPinnedChat = useCallback( - ({ - item: workspace, - drag, - isActive, - dragHandleProps, - }: DraggableRenderItemInfo) => { - return ( - - ); - }, + const renderPinnedChatRow = useCallback( + ( + workspace: SidebarWorkspacePlacement, + input?: { + drag?: () => void; + isDragging?: boolean; + dragHandleProps?: DraggableListDragHandleProps; + }, + ) => ( + + ), [ activeWorkspaceSelection, creatingWorkspaceIds, @@ -2441,6 +2456,22 @@ function ProjectModeList({ ], ); + const renderPinnedChat = useCallback( + ({ + item: workspace, + drag, + isActive, + dragHandleProps, + }: DraggableRenderItemInfo) => { + return renderPinnedChatRow(workspace, { + drag, + isDragging: isActive, + dragHandleProps, + }); + }, + [renderPinnedChatRow], + ); + const projectBody = projects.length === 0 ? ( @@ -2473,6 +2504,7 @@ function ProjectModeList({ /> {pinnedCollapsed ? null : ( <> + {leadingPinnedChat ? renderPinnedChatRow(leadingPinnedChat) : null} ({ + id: index + 1, + serverId: rowServerId, + workspaceId: `workspace-${index + 1}`, + statusBucket, + })); +} + +const workspacePolicy: SidebarGroupVisibilityPolicy = { + kind: "workspaces", + getStatusBucket: (item) => item.statusBucket, +}; + +function selectionFor(item: SidebarGroupItem): ActiveWorkspaceSelection { + return { serverId: item.serverId, workspaceId: item.workspaceId }; +} + +describe("selectSidebarGroupVisibility", () => { + it("leads with a hidden open row while keeping the baseline order", () => { + const items = rows(Array(9).fill("done")); + const result = selectSidebarGroupVisibility({ + items, + expanded: false, + policy: workspacePolicy, + selection: selectionFor(items[8]!), + }); + + expect(result.leadingItem).toBe(items[8]); + expect(result.visibleItems).toEqual(items.slice(0, 7)); + expect(result.canToggle).toBe(true); + }); + + it("leaves selected rows in place when they are already in the baseline", () => { + const items = rows([...Array(8).fill("done"), "running", "done"]); + const expected = [...items.slice(0, 7), items[8]]; + + for (const selectedIndex of [2, 8]) { + const result = selectSidebarGroupVisibility({ + items, + expanded: false, + policy: workspacePolicy, + selection: selectionFor(items[selectedIndex]!), + }); + + expect(result.leadingItem).toBeNull(); + expect(result.visibleItems).toEqual(expected); + expect(result.canToggle).toBe(true); + } + }); + + it("shows the whole group without a toggle when the open row is the only hidden row", () => { + const items = rows(Array(8).fill("done")); + const result = selectSidebarGroupVisibility({ + items, + expanded: false, + policy: workspacePolicy, + selection: selectionFor(items[7]!), + }); + + expect(result.leadingItem).toBeNull(); + expect(result.visibleItems).toEqual(items); + expect(result.canToggle).toBe(false); + }); + + it("keeps expanded groups in their original order", () => { + const items = rows(Array(9).fill("done")); + const result = selectSidebarGroupVisibility({ + items, + expanded: true, + policy: workspacePolicy, + selection: selectionFor(items[8]!), + }); + + expect(result.leadingItem).toBeNull(); + expect(result.visibleItems).toEqual(items); + expect(result.canToggle).toBe(true); + }); + + it("applies the twenty-row limit to pinned groups", () => { + const items = rows(Array(22).fill("done")); + const result = selectSidebarGroupVisibility({ + items, + expanded: false, + policy: { kind: "pinned" }, + selection: selectionFor(items[21]!), + }); + + expect(result.leadingItem).toBe(items[21]); + expect(result.visibleItems).toEqual(items.slice(0, 20)); + expect(result.canToggle).toBe(true); + }); + + it("preserves the current limit when selection is null", () => { + const items = rows(Array(8).fill("done")); + const result = selectSidebarGroupVisibility({ + items, + expanded: false, + policy: workspacePolicy, + selection: null, + }); + + expect(result.leadingItem).toBeNull(); + expect(result.visibleItems).toEqual(items.slice(0, 7)); + expect(result.canToggle).toBe(true); + }); + + it("matches the open row by server and workspace identity", () => { + const items = rows(Array(9).fill("done"), "host-b"); + const result = selectSidebarGroupVisibility({ + items, + expanded: false, + policy: workspacePolicy, + selection: { serverId: serverId, workspaceId: items[8]!.workspaceId }, + }); + + expect(result.leadingItem).toBeNull(); + expect(result.visibleItems).toEqual(items.slice(0, 7)); + expect(result.canToggle).toBe(true); + }); +}); diff --git a/packages/app/src/components/sidebar/sidebar-group-visibility.ts b/packages/app/src/components/sidebar/sidebar-group-visibility.ts index 1bc3018e9a..f109f0114a 100644 --- a/packages/app/src/components/sidebar/sidebar-group-visibility.ts +++ b/packages/app/src/components/sidebar/sidebar-group-visibility.ts @@ -1,4 +1,28 @@ import type { SidebarStateBucket } from "@/hooks/sidebar-workspaces-view-model"; +import type { ActiveWorkspaceSelection } from "@/stores/last-workspace-selection"; + +export interface SidebarGroupItem { + serverId: string; + workspaceId: string; +} + +export interface SidebarGroupVisibilityInput { + items: readonly T[]; + expanded: boolean; + policy: SidebarGroupVisibilityPolicy; + /** The workspace the group's rows highlight as selected. Null when no row can be selected. */ + selection: ActiveWorkspaceSelection | null; +} + +export interface SidebarGroupVisibility { + /** + * The open workspace when the collapsed limit would hide it. Rendered before `visibleItems`, + * out of group order, outside any drag list. Never also present in `visibleItems`. + */ + leadingItem: T | null; + visibleItems: T[]; + canToggle: boolean; +} export type SidebarGroupVisibilityPolicy = | { kind: "pinned" } @@ -16,18 +40,45 @@ export function createWorkspaceVisibilityPolicy( - items: readonly T[], - expanded: boolean, - policy: SidebarGroupVisibilityPolicy, -) { - const limit = policy.kind === "pinned" ? 20 : 7; - const baseline = items.filter( - (item, index) => - index < limit || (policy.kind === "workspaces" && policy.getStatusBucket(item) !== "done"), +function isOpenWorkspace( + item: SidebarGroupItem, + selection: ActiveWorkspaceSelection | null, +): boolean { + return ( + selection !== null && + item.serverId === selection.serverId && + item.workspaceId === selection.workspaceId ); +} + +export function selectSidebarGroupVisibility( + input: SidebarGroupVisibilityInput, +): SidebarGroupVisibility { + const { items, expanded, policy, selection } = input; + const limit = policy.kind === "pinned" ? 20 : 7; + const baseline: T[] = []; + let leadingItem: T | null = null; + let canToggle = false; + + items.forEach((item, index) => { + const inBaseline = + index < limit || (policy.kind === "workspaces" && policy.getStatusBucket(item) !== "done"); + if (inBaseline) { + baseline.push(item); + } else if (leadingItem === null && isOpenWorkspace(item, selection)) { + leadingItem = item; + } else { + canToggle = true; + } + }); + + if (expanded || !canToggle) { + return { leadingItem: null, visibleItems: items.slice(), canToggle }; + } + return { - visibleItems: expanded ? items.slice() : baseline, - canToggle: baseline.length < items.length, + leadingItem, + visibleItems: baseline, + canToggle, }; } diff --git a/packages/app/src/components/sidebar/sidebar-status-list.tsx b/packages/app/src/components/sidebar/sidebar-status-list.tsx index 44622385a4..ce8ad535c4 100644 --- a/packages/app/src/components/sidebar/sidebar-status-list.tsx +++ b/packages/app/src/components/sidebar/sidebar-status-list.tsx @@ -147,6 +147,7 @@ export function SidebarStatusWorkspaceList({ dragGestureHostActive, }: StatusWorkspaceListProps) { const { t } = useTranslation(); + const activeWorkspaceSelection = useActiveWorkspaceSelection(); const collapsedWorkspaceGroupKeys = useSidebarCollapsedSectionsStore( (state) => state.collapsedWorkspaceGroupKeys, ); @@ -155,23 +156,27 @@ export function SidebarStatusWorkspaceList({ (state) => state.togglePinnedCollapsed, ); const { + leadingItem: leadingPinnedWorkspace, visibleItems: visiblePinnedWorkspaces, expanded: pinnedWorkspacesExpanded, canToggle: canTogglePinnedWorkspaces, toggleExpanded: togglePinnedWorkspacesExpanded, - } = useLimitedSidebarGroup(pinnedWorkspaces); + } = useLimitedSidebarGroup({ items: pinnedWorkspaces, selection: activeWorkspaceSelection }); const statusShortcutIndex = showShortcutBadges ? shortcutIndexByWorkspaceKey : EMPTY_SHORTCUT_INDEX; - const renderPinnedWorkspace = useCallback( - ({ - item: workspace, - drag, - isActive, - dragHandleProps, - }: DraggableRenderItemInfo) => ( + const renderPinnedWorkspaceRow = useCallback( + ( + workspace: SidebarWorkspaceEntry, + input?: { + drag?: () => void; + isDragging?: boolean; + dragHandleProps?: DraggableListDragHandleProps; + }, + ) => ( ), [ @@ -199,6 +204,20 @@ export function SidebarStatusWorkspaceList({ supportsPinningByServerId, ], ); + const renderPinnedWorkspace = useCallback( + ({ + item: workspace, + drag, + isActive, + dragHandleProps, + }: DraggableRenderItemInfo) => + renderPinnedWorkspaceRow(workspace, { + drag, + isDragging: isActive, + dragHandleProps, + }), + [renderPinnedWorkspaceRow], + ); const content = ( <> {pinnedWorkspaces.length > 0 ? ( @@ -211,6 +230,7 @@ export function SidebarStatusWorkspaceList({ /> {pinnedCollapsed ? null : ( <> + {leadingPinnedWorkspace ? renderPinnedWorkspaceRow(leadingPinnedWorkspace) : null} ; onToggleWorkspacePin: ToggleSidebarWorkspacePin; }) { + const activeWorkspaceSelection = useActiveWorkspaceSelection(); const { + leadingItem: leadingWorkspace, visibleItems: visibleWorkspaces, expanded: workspacesExpanded, canToggle: canToggleWorkspaces, toggleExpanded: toggleWorkspacesExpanded, - } = useLimitedSidebarGroup(group.rows, STATUS_GROUP_VISIBILITY); + } = useLimitedSidebarGroup({ + items: group.rows, + selection: activeWorkspaceSelection, + policy: STATUS_GROUP_VISIBILITY, + }); + + function renderWorkspaceRow(workspace: SidebarWorkspaceEntry) { + return ( + + ); + } return ( @@ -360,22 +405,8 @@ function StatusGroupRows({ style={styles.statusWorkspaceListContainer} testID={`sidebar-status-group-rows-${group.key}`} > - {visibleWorkspaces.map((workspace) => ( - - ))} + {leadingWorkspace ? renderWorkspaceRow(leadingWorkspace) : null} + {visibleWorkspaces.map(renderWorkspaceRow)} {canToggleWorkspaces ? ( ( - items: readonly T[], - policy: SidebarGroupVisibilityPolicy = PINNED_POLICY, -) { +interface LimitedSidebarGroupInput { + items: readonly T[]; + selection: ActiveWorkspaceSelection | null; + policy?: SidebarGroupVisibilityPolicy; +} + +export function useLimitedSidebarGroup({ + items, + selection, + policy = PINNED_POLICY, +}: LimitedSidebarGroupInput): SidebarGroupVisibility & { + expanded: boolean; + toggleExpanded: () => void; +} { const [expanded, setExpanded] = useState(false); const visibility = useMemo( - () => selectSidebarGroupVisibility(items, expanded, policy), - [expanded, items, policy], + () => selectSidebarGroupVisibility({ items, expanded, policy, selection }), + [expanded, items, policy, selection], ); const toggleExpanded = useCallback(() => setExpanded((current) => !current), []);