Fix workspace navigation regression on web (#772)
Centralize workspace navigation behind a small route-derived API so the workspace shell reads active selection from the current Expo Router route instead of a separately synchronized store.
Remove the stale layout-driven active-workspace write that regressed web after 02e5b564.
This commit is contained in:
@@ -2,7 +2,7 @@ import { useCallback } from "react";
|
||||
import { router } from "expo-router";
|
||||
import { useKeyboardActionHandler } from "@/hooks/use-keyboard-action-handler";
|
||||
import type { KeyboardActionId } from "@/keyboard/keyboard-action-dispatcher";
|
||||
import { useNavigationActiveWorkspaceSelection } from "@/stores/navigation-active-workspace-store";
|
||||
import { useActiveWorkspaceSelection } from "@/stores/navigation-active-workspace-store";
|
||||
import { useSessionStore } from "@/stores/session-store";
|
||||
import { buildHostNewWorkspaceRoute } from "@/utils/host-routes";
|
||||
import { projectDisplayNameFromProjectId } from "@/utils/project-display-name";
|
||||
@@ -10,7 +10,7 @@ import { projectDisplayNameFromProjectId } from "@/utils/project-display-name";
|
||||
const WORKTREE_NEW_ACTIONS: readonly KeyboardActionId[] = ["worktree.new"];
|
||||
|
||||
export function useActiveWorktreeNewAction() {
|
||||
const selection = useNavigationActiveWorkspaceSelection();
|
||||
const selection = useActiveWorkspaceSelection();
|
||||
const serverId = selection?.serverId ?? null;
|
||||
const workspaceId = selection?.workspaceId ?? null;
|
||||
|
||||
|
||||
@@ -24,8 +24,8 @@ import { getDesktopHost, isElectronRuntime } from "@/desktop/host";
|
||||
import { isImeComposingKeyboardEvent } from "@/utils/keyboard-ime";
|
||||
import { useActiveServerId } from "@/hooks/use-active-server-id";
|
||||
import {
|
||||
getLastNavigationWorkspaceRouteSelection,
|
||||
getNavigationActiveWorkspaceSelection,
|
||||
navigateToLastWorkspace,
|
||||
useActiveWorkspaceSelection,
|
||||
} from "@/stores/navigation-active-workspace-store";
|
||||
|
||||
export function useKeyboardShortcuts({
|
||||
@@ -55,6 +55,7 @@ export function useKeyboardShortcuts({
|
||||
});
|
||||
const activeServerId = useActiveServerId();
|
||||
const openProjectPickerAction = useOpenProjectPicker(activeServerId);
|
||||
const activeWorkspaceSelection = useActiveWorkspaceSelection();
|
||||
|
||||
useEffect(() => {
|
||||
if (!enabled) return;
|
||||
@@ -96,6 +97,8 @@ export function useKeyboardShortcuts({
|
||||
case "navigate-workspace":
|
||||
navigateToWorkspace(action.serverId, action.workspaceId, { currentPathname: pathname });
|
||||
return true;
|
||||
case "navigate-last-workspace":
|
||||
return navigateToLastWorkspace();
|
||||
case "router-replace":
|
||||
router.replace(action.route as Parameters<typeof router.replace>[0]);
|
||||
return true;
|
||||
@@ -194,8 +197,7 @@ export function useKeyboardShortcuts({
|
||||
pathname,
|
||||
isMobile,
|
||||
sidebarShortcutTargets: store.sidebarShortcutWorkspaceTargets,
|
||||
navigationActiveWorkspace: getNavigationActiveWorkspaceSelection(),
|
||||
lastNavigationWorkspaceRoute: getLastNavigationWorkspaceRouteSelection(),
|
||||
navigationActiveWorkspace: activeWorkspaceSelection,
|
||||
commandCenterOpen: store.commandCenterOpen,
|
||||
shortcutsDialogOpen: store.shortcutsDialogOpen,
|
||||
},
|
||||
@@ -275,6 +277,7 @@ export function useKeyboardShortcuts({
|
||||
bindings,
|
||||
cycleTheme,
|
||||
enabled,
|
||||
activeWorkspaceSelection,
|
||||
isMobile,
|
||||
openProjectPickerAction,
|
||||
pathname,
|
||||
|
||||
@@ -1,59 +1,27 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
/**
|
||||
* @vitest-environment jsdom
|
||||
*/
|
||||
import { renderHook } from "@testing-library/react";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
|
||||
const { dismissToMock } = vi.hoisted(() => ({
|
||||
dismissToMock: vi.fn(),
|
||||
const navigateToWorkspaceMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("@/stores/navigation-active-workspace-store", () => ({
|
||||
navigateToWorkspace: navigateToWorkspaceMock,
|
||||
}));
|
||||
|
||||
vi.mock("expo-router", () => ({
|
||||
router: {
|
||||
dismissTo: dismissToMock,
|
||||
},
|
||||
}));
|
||||
import { navigateToWorkspace, useWorkspaceNavigation } from "@/hooks/use-workspace-navigation";
|
||||
|
||||
import { navigateToWorkspace } from "@/hooks/use-workspace-navigation";
|
||||
import {
|
||||
activateNavigationWorkspaceSelection,
|
||||
getNavigationActiveWorkspaceSelection,
|
||||
syncNavigationActiveWorkspace,
|
||||
} from "@/stores/navigation-active-workspace-store";
|
||||
|
||||
describe("navigateToWorkspace", () => {
|
||||
beforeEach(() => {
|
||||
dismissToMock.mockReset();
|
||||
syncNavigationActiveWorkspace({ current: null });
|
||||
describe("useWorkspaceNavigation", () => {
|
||||
it("re-exports the workspace navigation action", () => {
|
||||
expect(navigateToWorkspace).toBe(navigateToWorkspaceMock);
|
||||
});
|
||||
|
||||
it("dismisses to the workspace route from a non-workspace route even when active selection is stale", () => {
|
||||
activateNavigationWorkspaceSelection({
|
||||
serverId: "server-1",
|
||||
workspaceId: "workspace-a",
|
||||
});
|
||||
it("returns a stable callback wrapper", () => {
|
||||
const { result } = renderHook(() => useWorkspaceNavigation());
|
||||
|
||||
navigateToWorkspace("server-1", "workspace-b", {
|
||||
currentPathname: "/h/server-1/sessions",
|
||||
});
|
||||
result.current.navigateToWorkspace("server-1", "workspace-a");
|
||||
|
||||
expect(dismissToMock).toHaveBeenCalledWith("/h/server-1/workspace/workspace-b");
|
||||
expect(getNavigationActiveWorkspaceSelection()).toEqual({
|
||||
serverId: "server-1",
|
||||
workspaceId: "workspace-a",
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps retained workspace switching on a workspace route", () => {
|
||||
activateNavigationWorkspaceSelection({
|
||||
serverId: "server-1",
|
||||
workspaceId: "workspace-a",
|
||||
});
|
||||
|
||||
navigateToWorkspace("server-1", "workspace-b", {
|
||||
currentPathname: "/h/server-1/workspace/workspace-a",
|
||||
});
|
||||
|
||||
expect(dismissToMock).not.toHaveBeenCalled();
|
||||
expect(getNavigationActiveWorkspaceSelection()).toEqual({
|
||||
serverId: "server-1",
|
||||
workspaceId: "workspace-b",
|
||||
});
|
||||
expect(navigateToWorkspaceMock).toHaveBeenCalledWith("server-1", "workspace-a", undefined);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,61 +1,16 @@
|
||||
import { useCallback } from "react";
|
||||
import { router } from "expo-router";
|
||||
import {
|
||||
activateNavigationWorkspaceSelection,
|
||||
getNavigationActiveWorkspaceSelection,
|
||||
} from "@/stores/navigation-active-workspace-store";
|
||||
import { buildHostWorkspaceRoute, parseHostWorkspaceRouteFromPathname } from "@/utils/host-routes";
|
||||
import { navigateToWorkspace } from "@/stores/navigation-active-workspace-store";
|
||||
|
||||
interface NavigateToWorkspaceOptions {
|
||||
currentPathname?: string | null;
|
||||
}
|
||||
|
||||
function shouldUseRetainedWorkspaceSwitch(input: {
|
||||
hasActiveWorkspace: boolean;
|
||||
currentPathname?: string | null;
|
||||
}): boolean {
|
||||
if (!input.hasActiveWorkspace) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (input.currentPathname == null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return parseHostWorkspaceRouteFromPathname(input.currentPathname) !== null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Open a workspace. Once the workspace shell is mounted, switching workspaces
|
||||
* is app-level state so native-stack does not rebuild every retained screen.
|
||||
*/
|
||||
export function navigateToWorkspace(
|
||||
serverId: string,
|
||||
workspaceId: string,
|
||||
options: NavigateToWorkspaceOptions = {},
|
||||
) {
|
||||
const activeWorkspace = getNavigationActiveWorkspaceSelection();
|
||||
if (
|
||||
shouldUseRetainedWorkspaceSwitch({
|
||||
hasActiveWorkspace: activeWorkspace !== null,
|
||||
currentPathname: options.currentPathname,
|
||||
})
|
||||
) {
|
||||
activateNavigationWorkspaceSelection(
|
||||
{ serverId, workspaceId },
|
||||
{ updateBrowserHistory: true, historyMode: "push" },
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const href = buildHostWorkspaceRoute(serverId, workspaceId);
|
||||
router.dismissTo(href);
|
||||
}
|
||||
export { navigateToWorkspace };
|
||||
|
||||
export function useWorkspaceNavigation() {
|
||||
return {
|
||||
navigateToWorkspace: useCallback(
|
||||
(serverId: string, workspaceId: string, options?: NavigateToWorkspaceOptions) => {
|
||||
(
|
||||
serverId: string,
|
||||
workspaceId: string,
|
||||
options?: Parameters<typeof navigateToWorkspace>[2],
|
||||
) => {
|
||||
navigateToWorkspace(serverId, workspaceId, options);
|
||||
},
|
||||
[],
|
||||
|
||||
Reference in New Issue
Block a user