diff --git a/frontend/src/lib/components/meltComponents/MenuItem.svelte b/frontend/src/lib/components/meltComponents/MenuItem.svelte index 282dc893da..18e41a15c8 100644 --- a/frontend/src/lib/components/meltComponents/MenuItem.svelte +++ b/frontend/src/lib/components/meltComponents/MenuItem.svelte @@ -35,6 +35,16 @@ let aRef: HTMLAnchorElement | undefined = $state(undefined) let buttonRef: HTMLButtonElement | undefined = $state(undefined) + + // Svelte delegates `onclick` to the app root, where SvelteKit's link handler runs first + // and follows `href` regardless. Listening on the anchor itself lets `preventDefault()` + // in `onClick` cancel the navigation. + function anchorClick(node: HTMLAnchorElement) { + const handler = onClick + if (!handler) return + node.addEventListener('click', handler) + return () => node.removeEventListener('click', handler) + } {#if href} @@ -57,7 +67,7 @@ {target} onfocusin={onFocusIn} onfocusout={onFocusOut} - onclick={onClick} + {@attach anchorClick} > {@render children?.()} diff --git a/frontend/src/lib/components/sidebar/WorkspacePickerBody.svelte b/frontend/src/lib/components/sidebar/WorkspacePickerBody.svelte index fa4468fa6a..8b898ca7c1 100644 --- a/frontend/src/lib/components/sidebar/WorkspacePickerBody.svelte +++ b/frontend/src/lib/components/sidebar/WorkspacePickerBody.svelte @@ -22,11 +22,10 @@ import { MenuItem, Tooltip } from '$lib/components/meltComponents' import { EXECUTIONS_HINT } from './executionsHint' import WorkspaceIcon from '$lib/components/workspace/WorkspaceIcon.svelte' - import { fixupUrlAfterWorkspaceSwitch } from './workspaceSwitchUrl' + import { switchWorkspaceAndPage } from './workspaceSwitchUrl' import { goto } from '$lib/navigation' import { base } from '$lib/base' import { page } from '$app/state' - import { switchWorkspace } from '$lib/storeUtils' import MultiplayerMenu from './MultiplayerMenu.svelte' import { isCloudHosted } from '$lib/cloud' import { workspaceAIClients } from '../copilot/lib' @@ -61,14 +60,13 @@ if ($workspaceStore === id) { return } - // Read before switchWorkspace: it swaps the stores this reads out from under us. + // Read before switching: the switch swaps the stores these read out from under us. const landOnHome = landsOnHome(id) + const href = workspaceHref(id) workspaceAIClients.init(id) - switchWorkspace(id) - // The sessions page needs no navigation here: the item's link navigation - // (workspaceHref) keeps the route, and the page's family reconcile swaps - // out a chat that doesn't belong to the new workspace's family. - await fixupUrlAfterWorkspaceSwitch(id, { landOnHome }) + // On the sessions page, the page's family reconcile swaps out a chat that + // doesn't belong to the new workspace's family. + await switchWorkspaceAndPage(id, { landOnHome, href }) } // Operator page access is granted per workspace, so a switch lands on home rather than @@ -345,11 +343,7 @@ {/if} {#if canForkHere} - (globalForkModal.val = { opened: true })} - {item} - > + (globalForkModal.val = { opened: true })} {item}> Workspace fork diff --git a/frontend/src/lib/components/sidebar/WorkspaceScopeHeader.svelte b/frontend/src/lib/components/sidebar/WorkspaceScopeHeader.svelte index 31762045f1..74f2003cef 100644 --- a/frontend/src/lib/components/sidebar/WorkspaceScopeHeader.svelte +++ b/frontend/src/lib/components/sidebar/WorkspaceScopeHeader.svelte @@ -6,8 +6,7 @@ userStore, superadmin } from '$lib/stores' - import { switchWorkspace } from '$lib/storeUtils' - import { fixupUrlAfterWorkspaceSwitch } from './workspaceSwitchUrl' + import { switchWorkspaceAndPage } from './workspaceSwitchUrl' import { base } from '$lib/base' import { workspaceAIClients } from '$lib/components/copilot/lib' import WorkspaceFamilyPicker from '$lib/components/sessions/WorkspaceFamilyPicker.svelte' @@ -53,8 +52,7 @@ function switchWorkspaceDirect(id: string) { if ($workspaceStore === id) return workspaceAIClients.init(id) - switchWorkspace(id) - void fixupUrlAfterWorkspaceSwitch(id) + void switchWorkspaceAndPage(id) } function openForkModal() { diff --git a/frontend/src/lib/components/sidebar/workspaceSwitchUrl.ts b/frontend/src/lib/components/sidebar/workspaceSwitchUrl.ts index 82130e3f95..aee2afc520 100644 --- a/frontend/src/lib/components/sidebar/workspaceSwitchUrl.ts +++ b/frontend/src/lib/components/sidebar/workspaceSwitchUrl.ts @@ -1,34 +1,90 @@ import { page } from '$app/state' import { replaceState } from '$app/navigation' import { goto } from '$lib/navigation' +import { AppService, FlowService, JobService, ScriptService } from '$lib/gen' +import { switchWorkspace } from '$lib/storeUtils' -// Post-workspace-switch URL fixup shared by the sidebar workspace pickers. -// Item-scoped pages would show a wrong-workspace (or missing) item after a -// switch — go home instead. Otherwise, really rewrite a ?workspace= param in -// the address bar (mutating page.url is a no-op there): left stale, it gets -// re-applied on reload or when exiting session mode restores the route, -// silently switching the workspace back. -const EDIT_PAGES = [ - '/scripts/edit/', - '/flows/edit/', - '/apps/edit/', - '/apps_raw/edit/', - '/scripts/get/', - '/flows/get/', - '/apps/get/', - '/apps_raw/get/' -] +type ItemExists = (workspace: string, param: string) => Promise -export async function fixupUrlAfterWorkspaceSwitch( +const appExists: ItemExists = (workspace, path) => AppService.existsApp({ workspace, path }) +const flowExists: ItemExists = (workspace, path) => + FlowService.existsFlowByPath({ workspace, path }) +const scriptExists: ItemExists = (workspace, path) => + ScriptService.existsScriptByPath({ workspace, path }) + +// The script page's param is a hash, a path, or a hub path. +const scriptHashOrPathExists: ItemExists = async (workspace, hashOrPath) => { + if (hashOrPath.startsWith('hub/')) return true + if (await scriptExists(workspace, hashOrPath)) return true + try { + await ScriptService.getScriptByHash({ workspace, hash: hashOrPath }) + return true + } catch { + return false + } +} + +// A one-row lookup, where getJob would download the job's args and result. +const jobExists: ItemExists = async (workspace, id) => { + try { + await JobService.getRootJobId({ workspace, id }) + return true + } catch { + return false + } +} + +// Pages showing one workspace item, keyed by route id, with the check that the item named +// by the route param exists in a given workspace. +const ITEM_PAGES: Record = { + '/(root)/(logged)/scripts/edit/[...path]': { param: 'path', exists: scriptExists }, + '/(root)/(logged)/scripts/get/[...hash]': { param: 'hash', exists: scriptHashOrPathExists }, + '/(root)/(logged)/flows/edit/[...path]': { param: 'path', exists: flowExists }, + '/(root)/(logged)/flows/get/[...path]': { param: 'path', exists: flowExists }, + '/(root)/(logged)/apps/edit/[...path]': { param: 'path', exists: appExists }, + '/(root)/(logged)/apps/get/[...path]': { param: 'path', exists: appExists }, + '/(root)/(logged)/apps_raw/edit/[...path]': { param: 'path', exists: appExists }, + '/(root)/(logged)/apps_raw/get/[...path]': { param: 'path', exists: appExists }, + '/(root)/(logged)/run/[...run]': { param: 'run', exists: jobExists } +} + +async function itemPageMissingIn(workspace: string): Promise { + const itemPage = page.route.id ? ITEM_PAGES[page.route.id] : undefined + const param = itemPage ? page.params[itemPage.param] : undefined + if (!itemPage || !param) return false + try { + return !(await itemPage.exists(workspace, param)) + } catch { + return true + } +} + +// Workspace switch shared by the sidebar workspace pickers. An item page stays open when +// its item also exists in `id`, and goes home otherwise. +export async function switchWorkspaceAndPage( id: string, - // Decided by the caller so the rule stays with the picker that has the context - // for it — a picker rendered outside the sidebar drives its own page and must - // not be navigated away from. - opts?: { landOnHome?: boolean } + opts?: { + // Decided by the caller so the rule stays with the picker that has the context + // for it — a picker rendered outside the sidebar drives its own page and must + // not be navigated away from. + landOnHome?: boolean + // Where to go when staying on the page, for a picker whose items link there. + href?: string + } ): Promise { - if (EDIT_PAGES.some((p) => page.route.id?.includes(p) ?? false) || opts?.landOnHome) { + if (opts?.landOnHome || (await itemPageMissingIn(id))) { + // Leave before switching: an item page still mounted when the store changes + // refetches its item in `id` and toasts the 404. await goto('/') + switchWorkspace(id) + return + } + switchWorkspace(id) + if (opts?.href) { + await goto(opts.href) } else if (page.url.searchParams.get('workspace')) { + // A stale ?workspace= param is re-applied on reload or when exiting session mode + // restores the route, silently switching the workspace back. const url = new URL(window.location.href) url.searchParams.set('workspace', id) replaceState(url, page.state)