From 62d9e656d749d47128392dc12baa3ef9c88903c5 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Fri, 12 Jun 2026 01:00:05 +0800 Subject: [PATCH] Bound retained workspace screens (#1472) * fix(app): prune retained workspace deck entries * fix(app): narrow workspace deck retention selectors * fix(app): preserve deck lru during route transitions * test(app): update startup reconnect expectation --- packages/app/e2e/helpers/startup-dsl.ts | 17 +-- packages/app/e2e/startup-loading.spec.ts | 6 +- .../workspace/[workspaceId]/index.tsx | 94 +++++++++++---- .../workspace-deck-retention.test.ts | 107 ++++++++++++++++++ .../workspace/workspace-deck-retention.ts | 89 +++++++++++++++ .../src/stores/session-store-hooks/index.ts | 18 +++ .../stores/session-store-hooks/selectors.ts | 20 +++- 7 files changed, 317 insertions(+), 34 deletions(-) create mode 100644 packages/app/src/screens/workspace/workspace-deck-retention.test.ts create mode 100644 packages/app/src/screens/workspace/workspace-deck-retention.ts diff --git a/packages/app/e2e/helpers/startup-dsl.ts b/packages/app/e2e/helpers/startup-dsl.ts index a3e771782..60e8893da 100644 --- a/packages/app/e2e/helpers/startup-dsl.ts +++ b/packages/app/e2e/helpers/startup-dsl.ts @@ -139,17 +139,18 @@ class StartupAssertions { this.page = page; } - async expectsReconnectWelcome(): Promise { - await expect(this.page.getByTestId("welcome-screen")).toBeVisible({ timeout: 15_000 }); - await expect(this.page.getByTestId("welcome-open-settings")).toBeVisible(); - await expect(this.page.getByTestId("welcome-direct-connection")).toBeVisible(); - await expect(this.page.getByTestId("welcome-paste-pairing-link")).toBeVisible(); - await expect(this.page.getByTestId("welcome-scan-qr")).toHaveCount(0); + async expectsSavedHostShell(input: { label: string }): Promise { + await expect(this.page.getByText(input.label, { exact: true })).toBeVisible({ + timeout: 15_000, + }); + await expect(this.page.getByRole("button", { name: "Add project" })).toBeVisible(); + await expect(this.page.getByRole("button", { name: "Home" })).toBeVisible(); + await expect(this.page.getByRole("button", { name: "Settings" })).toBeVisible(); + await expect(this.page.getByTestId("welcome-screen")).toHaveCount(0); return this; } - async expectsNoSavedHostStatus(input: { label: string }): Promise { - await expect(this.page.getByText(input.label, { exact: true })).toHaveCount(0); + async expectsNoSavedHostErrorStatus(): Promise { await expect(this.page.getByText("Connection error", { exact: true })).toHaveCount(0); await expect(this.page.getByText("Offline", { exact: true })).toHaveCount(0); return this; diff --git a/packages/app/e2e/startup-loading.spec.ts b/packages/app/e2e/startup-loading.spec.ts index 38fd29c3d..c79aef688 100644 --- a/packages/app/e2e/startup-loading.spec.ts +++ b/packages/app/e2e/startup-loading.spec.ts @@ -4,7 +4,7 @@ import { getServerId } from "./helpers/server-id"; import { startupScenario } from "./helpers/startup-dsl"; test.describe("Startup loading presentation", () => { - test("mobile reconnect keeps connection recovery actions visible", async ({ page }) => { + test("mobile reconnect preserves the saved host shell", async ({ page }) => { const startup = await startupScenario(page) .withMobileViewport() .withSavedHost({ @@ -14,8 +14,8 @@ test.describe("Startup loading presentation", () => { }) .openRoot(); - await startup.expectsReconnectWelcome(); - await startup.expectsNoSavedHostStatus({ label: "Dev" }); + await startup.expectsSavedHostShell({ label: "Dev" }); + await startup.expectsNoSavedHostErrorStatus(); await startup.expectsNoLocalServerStartupCopy(); }); diff --git a/packages/app/src/app/h/[serverId]/workspace/[workspaceId]/index.tsx b/packages/app/src/app/h/[serverId]/workspace/[workspaceId]/index.tsx index d05d95ee3..3d3bced9a 100644 --- a/packages/app/src/app/h/[serverId]/workspace/[workspaceId]/index.tsx +++ b/packages/app/src/app/h/[serverId]/workspace/[workspaceId]/index.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { useNavigation } from "@react-navigation/native"; import { StyleSheet, View } from "react-native"; import { useGlobalSearchParams, useLocalSearchParams, useRootNavigationState } from "expo-router"; @@ -7,9 +7,18 @@ import { type ActiveWorkspaceSelection, useActiveWorkspaceSelection, } from "@/stores/navigation-active-workspace-store"; +import { useHasHydratedWorkspaces, useWorkspaceExists } from "@/stores/session-store-hooks"; import type { WorkspaceTabTarget } from "@/stores/workspace-tabs-store"; import { WorkspaceScreen } from "@/screens/workspace/workspace-screen"; import { useWorkspaceLayoutStoreHydrated } from "@/stores/workspace-layout-store"; +import { + areWorkspaceSelectionListsEqual, + areWorkspaceSelectionsEqual, + getWorkspaceSelectionKey, + pruneMountedWorkspaceSelections, + shouldKeepWorkspaceDeckEntryMounted, + WORKSPACE_DECK_MAX_MOUNTED_WORKSPACES, +} from "@/screens/workspace/workspace-deck-retention"; import { decodeWorkspaceIdFromPathSegment, parseWorkspaceOpenIntent, @@ -152,28 +161,33 @@ function HostWorkspaceRouteContent() { return ; } -function areWorkspaceSelectionsEqual( - left: ActiveWorkspaceSelection | null, - right: ActiveWorkspaceSelection | null, -): boolean { - return left?.serverId === right?.serverId && left?.workspaceId === right?.workspaceId; -} - function WorkspaceDeck() { const activeSelection = useActiveWorkspaceSelection(); const [mountedSelections, setMountedSelections] = useState(() => activeSelection ? [activeSelection] : [], ); + const unmountWorkspaceSelection = useCallback((selection: ActiveWorkspaceSelection) => { + setMountedSelections((current) => + current.filter( + (mountedSelection) => !areWorkspaceSelectionsEqual(mountedSelection, selection), + ), + ); + }, []); useEffect(() => { if (!activeSelection) { return; } setMountedSelections((current) => { - if (current.some((selection) => areWorkspaceSelectionsEqual(selection, activeSelection))) { + const next = pruneMountedWorkspaceSelections({ + currentSelections: current, + activeSelection, + maxMountedWorkspaces: WORKSPACE_DECK_MAX_MOUNTED_WORKSPACES, + }); + if (areWorkspaceSelectionListsEqual(current, next)) { return current; } - return [...current, activeSelection]; + return next; }); }, [activeSelection]); @@ -184,25 +198,61 @@ function WorkspaceDeck() { return ( {mountedSelections.map((selection) => { - const isActive = areWorkspaceSelectionsEqual(selection, activeSelection); return ( - - - + ); })} ); } +function WorkspaceDeckEntry({ + selection, + activeSelection, + onUnmountInactive, +}: { + selection: ActiveWorkspaceSelection; + activeSelection: ActiveWorkspaceSelection; + onUnmountInactive: (selection: ActiveWorkspaceSelection) => void; +}) { + const isActive = areWorkspaceSelectionsEqual(selection, activeSelection); + const hasHydratedWorkspaces = useHasHydratedWorkspaces(selection.serverId); + const workspaceExists = useWorkspaceExists(selection.serverId, selection.workspaceId); + const shouldKeepMounted = shouldKeepWorkspaceDeckEntryMounted({ + isActive, + hasHydratedWorkspaces, + workspaceExists, + }); + + useEffect(() => { + if (!shouldKeepMounted) { + onUnmountInactive(selection); + } + }, [onUnmountInactive, selection, shouldKeepMounted]); + + if (!shouldKeepMounted) { + return null; + } + + return ( + + + + ); +} + const styles = StyleSheet.create({ deck: { flex: 1, diff --git a/packages/app/src/screens/workspace/workspace-deck-retention.test.ts b/packages/app/src/screens/workspace/workspace-deck-retention.test.ts new file mode 100644 index 000000000..92ff04965 --- /dev/null +++ b/packages/app/src/screens/workspace/workspace-deck-retention.test.ts @@ -0,0 +1,107 @@ +import { describe, expect, it } from "vitest"; +import type { ActiveWorkspaceSelection } from "@/stores/navigation-active-workspace-store"; +import { + pruneMountedWorkspaceSelections, + shouldKeepWorkspaceDeckEntryMounted, +} from "@/screens/workspace/workspace-deck-retention"; + +function workspace(workspaceId: string, serverId = "server"): ActiveWorkspaceSelection { + return { serverId, workspaceId }; +} + +function mountedWorkspaceIds(selections: ActiveWorkspaceSelection[]): string[] { + return selections.map((selection) => selection.workspaceId); +} + +describe("pruneMountedWorkspaceSelections", () => { + it("keeps the active workspace and the two most recent inactive workspaces", () => { + const mountedAfterA = pruneMountedWorkspaceSelections({ + currentSelections: [], + activeSelection: workspace("A"), + }); + const mountedAfterB = pruneMountedWorkspaceSelections({ + currentSelections: mountedAfterA, + activeSelection: workspace("B"), + }); + const mountedAfterC = pruneMountedWorkspaceSelections({ + currentSelections: mountedAfterB, + activeSelection: workspace("C"), + }); + const mountedAfterD = pruneMountedWorkspaceSelections({ + currentSelections: mountedAfterC, + activeSelection: workspace("D"), + }); + + expect(mountedWorkspaceIds(mountedAfterD)).toEqual(["D", "C", "B"]); + }); + + it("retains the active workspace", () => { + const mountedSelections = pruneMountedWorkspaceSelections({ + currentSelections: [workspace("A")], + activeSelection: workspace("A"), + }); + + expect(mountedWorkspaceIds(mountedSelections)).toEqual(["A"]); + }); + + it("deduplicates retained workspace selections", () => { + const mountedSelections = pruneMountedWorkspaceSelections({ + currentSelections: [workspace("B"), workspace("A"), workspace("B")], + activeSelection: workspace("A"), + }); + + expect(mountedWorkspaceIds(mountedSelections)).toEqual(["A", "B"]); + }); + + it("always allows at least the active workspace", () => { + const mountedSelections = pruneMountedWorkspaceSelections({ + currentSelections: [workspace("A"), workspace("B")], + activeSelection: workspace("C"), + maxMountedWorkspaces: 0, + }); + + expect(mountedWorkspaceIds(mountedSelections)).toEqual(["C"]); + }); +}); + +describe("shouldKeepWorkspaceDeckEntryMounted", () => { + it("keeps the active workspace mounted even when it is missing from hydrated workspaces", () => { + expect( + shouldKeepWorkspaceDeckEntryMounted({ + isActive: true, + hasHydratedWorkspaces: true, + workspaceExists: false, + }), + ).toBe(true); + }); + + it("keeps inactive workspaces mounted until workspace hydration finishes", () => { + expect( + shouldKeepWorkspaceDeckEntryMounted({ + isActive: false, + hasHydratedWorkspaces: false, + workspaceExists: false, + }), + ).toBe(true); + }); + + it("unmounts inactive workspaces that are gone after hydration", () => { + expect( + shouldKeepWorkspaceDeckEntryMounted({ + isActive: false, + hasHydratedWorkspaces: true, + workspaceExists: false, + }), + ).toBe(false); + }); + + it("keeps inactive workspaces that still exist after hydration", () => { + expect( + shouldKeepWorkspaceDeckEntryMounted({ + isActive: false, + hasHydratedWorkspaces: true, + workspaceExists: true, + }), + ).toBe(true); + }); +}); diff --git a/packages/app/src/screens/workspace/workspace-deck-retention.ts b/packages/app/src/screens/workspace/workspace-deck-retention.ts new file mode 100644 index 000000000..21b0c7a66 --- /dev/null +++ b/packages/app/src/screens/workspace/workspace-deck-retention.ts @@ -0,0 +1,89 @@ +import type { ActiveWorkspaceSelection } from "@/stores/navigation-active-workspace-store"; + +export const WORKSPACE_DECK_MAX_MOUNTED_WORKSPACES = 3; + +interface PruneMountedWorkspaceSelectionsInput { + currentSelections: ActiveWorkspaceSelection[]; + activeSelection: ActiveWorkspaceSelection | null; + maxMountedWorkspaces?: number; +} + +interface WorkspaceDeckEntryMountInput { + isActive: boolean; + hasHydratedWorkspaces: boolean; + workspaceExists: boolean; +} + +export function getWorkspaceSelectionKey(selection: ActiveWorkspaceSelection): string { + return `${selection.serverId}:${selection.workspaceId}`; +} + +export function areWorkspaceSelectionsEqual( + left: ActiveWorkspaceSelection | null, + right: ActiveWorkspaceSelection | null, +): boolean { + return left?.serverId === right?.serverId && left?.workspaceId === right?.workspaceId; +} + +export function areWorkspaceSelectionListsEqual( + left: ActiveWorkspaceSelection[], + right: ActiveWorkspaceSelection[], +): boolean { + if (left.length !== right.length) { + return false; + } + return left.every((selection, index) => + areWorkspaceSelectionsEqual(selection, right[index] ?? null), + ); +} + +export function pruneMountedWorkspaceSelections({ + currentSelections, + activeSelection, + maxMountedWorkspaces = WORKSPACE_DECK_MAX_MOUNTED_WORKSPACES, +}: PruneMountedWorkspaceSelectionsInput): ActiveWorkspaceSelection[] { + if (!activeSelection) { + return []; + } + + const maxSelections = Math.max(1, maxMountedWorkspaces); + const nextSelections: ActiveWorkspaceSelection[] = []; + const seenSelectionKeys = new Set(); + + function appendSelection(selection: ActiveWorkspaceSelection): void { + if (nextSelections.length >= maxSelections) { + return; + } + const selectionKey = getWorkspaceSelectionKey(selection); + if (seenSelectionKeys.has(selectionKey)) { + return; + } + seenSelectionKeys.add(selectionKey); + nextSelections.push(selection); + } + + appendSelection(activeSelection); + + for (const selection of currentSelections) { + if (areWorkspaceSelectionsEqual(selection, activeSelection)) { + continue; + } + appendSelection(selection); + } + + return nextSelections; +} + +export function shouldKeepWorkspaceDeckEntryMounted({ + isActive, + hasHydratedWorkspaces, + workspaceExists, +}: WorkspaceDeckEntryMountInput): boolean { + if (isActive) { + return true; + } + if (!hasHydratedWorkspaces) { + return true; + } + return workspaceExists; +} diff --git a/packages/app/src/stores/session-store-hooks/index.ts b/packages/app/src/stores/session-store-hooks/index.ts index cb2a72d20..a6cea46e3 100644 --- a/packages/app/src/stores/session-store-hooks/index.ts +++ b/packages/app/src/stores/session-store-hooks/index.ts @@ -3,11 +3,13 @@ import { useStoreWithEqualityFn } from "zustand/traditional"; import { useSidebarOrderStore } from "@/stores/sidebar-order-store"; import { composeWorkspaceStructure, + selectHasHydratedWorkspaces, selectHasWorkspaces, selectProjectOrder, selectRecommendedProjectPaths, selectResolveWorkspaceIdByCwd, selectWorkspace, + selectWorkspaceExists, selectWorkspaceExecutionAuthority, selectWorkspaceFields, selectWorkspaceKeys, @@ -54,6 +56,22 @@ export function useWorkspaceFields( ); } +export function useWorkspaceExists(serverId: string | null, workspaceId: string | null): boolean { + return useStoreWithEqualityFn( + useSessionStore, + (state) => selectWorkspaceExists(state, serverId, workspaceId), + workspaceEqualityFns.identity, + ); +} + +export function useHasHydratedWorkspaces(serverId: string | null): boolean { + return useStoreWithEqualityFn( + useSessionStore, + (state) => selectHasHydratedWorkspaces(state, serverId), + workspaceEqualityFns.identity, + ); +} + export function useWorkspaceExecutionAuthority( serverId: string | null, workspaceId: string | null, diff --git a/packages/app/src/stores/session-store-hooks/selectors.ts b/packages/app/src/stores/session-store-hooks/selectors.ts index 8dd402832..adb2bc0d4 100644 --- a/packages/app/src/stores/session-store-hooks/selectors.ts +++ b/packages/app/src/stores/session-store-hooks/selectors.ts @@ -17,7 +17,10 @@ export type { DesktopBadgeWorkspaceStatus } from "@/utils/desktop-badge-state"; export type { WorkspaceStructure, WorkspaceStructureProject } from "@/projects/workspace-structure"; export interface SessionsSnapshot { - sessions: Record }>; + sessions: Record< + string, + { hasHydratedWorkspaces?: boolean; workspaces: Map } + >; } export interface SidebarOrderSnapshot { @@ -110,6 +113,21 @@ export function selectWorkspaceFields( return workspace ? project(workspace) : null; } +export function selectWorkspaceExists( + state: SessionsSnapshot, + serverId: string | null, + workspaceId: string | null, +): boolean { + return selectWorkspace(state, serverId, workspaceId) !== null; +} + +export function selectHasHydratedWorkspaces( + state: SessionsSnapshot, + serverId: string | null, +): boolean { + return serverId ? (state.sessions[serverId]?.hasHydratedWorkspaces ?? false) : false; +} + export function selectWorkspaceExecutionAuthority( state: SessionsSnapshot, serverId: string | null,