From 6e0bd90b86b9c4c9a2af513b11fcfc6f43b2b4de Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Thu, 29 Oct 2026 17:36:01 +0900 Subject: [PATCH] fix(navigation): preserve redirecting pages and descendants --- .changeset/tidy-guards-navigate.md | 6 + .../command-palette/command-palette.tsx | 2 +- .../sidebar/default-sidebar.test.tsx | 60 +++++- packages/core/src/routing/navigation.test.tsx | 200 +++++++++++++++++- packages/core/src/routing/navigation.tsx | 26 +-- 5 files changed, 279 insertions(+), 15 deletions(-) create mode 100644 .changeset/tidy-guards-navigate.md diff --git a/.changeset/tidy-guards-navigate.md b/.changeset/tidy-guards-navigate.md new file mode 100644 index 000000000..d9705ea24 --- /dev/null +++ b/.changeset/tidy-guards-navigate.md @@ -0,0 +1,6 @@ +--- +"@tailor-platform/app-shell": patch +--- + +Keep redirecting pages and their visible descendants in automatic sidebar navigation and Command Palette entries, including current-path-aware dynamic routes. +Include redirect-only pages as links to their original paths even without a component. Hidden guards continue to exclude their subtrees. diff --git a/packages/core/src/components/command-palette/command-palette.tsx b/packages/core/src/components/command-palette/command-palette.tsx index d69c52326..336ed9ed1 100644 --- a/packages/core/src/components/command-palette/command-palette.tsx +++ b/packages/core/src/components/command-palette/command-palette.tsx @@ -59,7 +59,7 @@ export function navItemsToRoutes(navItems: Array): Array { const breadcrumb = [...parentBreadcrumb, item.title]; - // Only add navigable routes (componentless resources have no url) + // Only add navigable routes if (item.url) { routes.push({ path: item.url, diff --git a/packages/core/src/components/sidebar/default-sidebar.test.tsx b/packages/core/src/components/sidebar/default-sidebar.test.tsx index aedd99811..29ef4783d 100644 --- a/packages/core/src/components/sidebar/default-sidebar.test.tsx +++ b/packages/core/src/components/sidebar/default-sidebar.test.tsx @@ -11,7 +11,7 @@ import { SidebarItem } from "./sidebar-item"; import { SidebarGroup } from "./sidebar-group"; import { SidebarSection } from "./sidebar-section"; import { SidebarSeparator } from "./sidebar-separator"; -import { defineModule, defineResource } from "@/resource"; +import { defineModule, defineResource, redirectTo } from "@/resource"; import { AppShell } from "@/components/appshell"; import { useURLCollectionVariables } from "@/lib/collection-url-state"; import { useEffect } from "react"; @@ -169,6 +169,64 @@ describe("DefaultSidebar", () => { expect(screen.getByRole("link", { name: /products/i })).toBeDefined(); }); + it.each([true, false])( + "keeps redirecting parents in auto navigation and their descendants in the palette (has component: %s)", + async (hasComponent) => { + const user = userEvent.setup(); + stubDesktopViewport(); + const modules = [ + defineModule({ + path: "workspace", + meta: { title: "Workspace" }, + component: hasComponent ? () =>
Workspace Root
: undefined, + guards: [() => redirectTo("/workspace/section/list")], + resources: [ + defineResource({ + path: "section", + meta: { title: "Section" }, + component: hasComponent ? () =>
Section Root
: undefined, + guards: [() => redirectTo("/workspace/section/list")], + subResources: [ + defineResource({ + path: "list", + meta: { title: "List" }, + component: () =>
List Page
, + }), + ], + }), + ], + }), + ]; + + window.history.pushState({}, "", "/workspace/section/list"); + render( + + + , + ); + + expect(await screen.findByText("List Page")).toBeDefined(); + const sidebar = document.querySelector('[data-slot="sidebar"]'); + assert(sidebar); + const moduleLink = await within(sidebar).findByRole("link", { name: "Workspace" }); + expect(moduleLink.getAttribute("href")).toBe("/workspace"); + const resourceLink = await within(sidebar).findByRole("link", { name: "Section" }); + expect(resourceLink.getAttribute("href")).toBe("/workspace/section"); + + await user.click(within(sidebar).getByRole("button", { name: /search/i })); + + const dialog = await screen.findByRole("dialog"); + const descendantEntry = await within(dialog).findByRole("button", { + name: "Workspace > Section > List /workspace/section/list", + }); + expect(descendantEntry).toBeDefined(); + expect(within(dialog).getByRole("button", { name: "Workspace /workspace" })).toBeDefined(); + expect( + within(dialog).getByRole("button", { name: "Workspace > Section /workspace/section" }), + ).toBeDefined(); + }, + ); + it("keeps explicit sidebar navigation working after a page writes search params", async () => { // Asserts on the link's text label, so the sidebar must be expanded rather // than an icon rail. diff --git a/packages/core/src/routing/navigation.test.tsx b/packages/core/src/routing/navigation.test.tsx index 5b2fd3054..ab8d9d89f 100644 --- a/packages/core/src/routing/navigation.test.tsx +++ b/packages/core/src/routing/navigation.test.tsx @@ -3,7 +3,7 @@ import { useMemo } from "react"; import { describe, it, expect, afterEach, vi } from "vitest"; import { useNavItems, buildCurrentPathAwareRoutes } from "./navigation"; import { useLocation, useNavigate } from "react-router"; -import { defineModule, defineResource, hidden, pass } from "@/resource"; +import { defineModule, defineResource, hidden, pass, redirectTo } from "@/resource"; import { AppShellConfigContext, AppShellDataContext, @@ -99,6 +99,112 @@ describe("useNavItems", () => { }); }); + it.each([true, false])( + "keeps redirecting parents and visible descendants (has component: %s)", + async (hasComponent) => { + const component = hasComponent ? () =>
Parent
: undefined; + const modules = [ + defineModule({ + path: "workspace", + meta: { title: "Workspace" }, + component, + guards: [() => redirectTo("/workspace/section/list")], + resources: [ + defineResource({ + path: "section", + component, + guards: [() => redirectTo("/workspace/section/list")], + subResources: [ + defineResource({ + path: "list", + component: () =>
List
, + }), + defineResource({ + path: "secret", + guards: [() => hidden()], + subResources: [ + defineResource({ + path: "details", + component: () =>
Secret Details
, + }), + ], + }), + ], + }), + ], + }), + ]; + + const { result } = renderNavItems(modules, "/workspace/section/list"); + + await waitFor(async () => { + expect(await result.current!).toEqual([ + { + title: "Workspace", + url: "workspace", + icon: expect.anything(), + items: [ + { + title: "Section", + url: "workspace/section", + items: [{ title: "List", url: "workspace/section/list" }], + }, + ], + }, + ]); + }); + }, + ); + + it.each([ + { result: "pass", guard: pass }, + { result: "redirect", guard: () => redirectTo("/app") }, + { result: "hidden", guard: hidden }, + ])( + "handles componentless leaf navigation when guards return $result", + async ({ result, guard }) => { + const modules = [ + defineModule({ + path: "", + meta: { title: "Home" }, + guards: [guard], + resources: [], + }), + defineModule({ + path: "legacy", + meta: { title: "Legacy" }, + guards: [guard], + resources: [], + }), + defineModule({ + path: "app", + meta: { title: "App" }, + component: () =>
App
, + resources: [defineResource({ path: "alias", guards: [guard] })], + }), + ]; + + const { result: navItems } = renderNavItems(modules, "/app"); + + await waitFor(async () => { + expect(await navItems.current!).toEqual([ + ...(result === "redirect" + ? [ + { title: "Home", url: "/", icon: expect.anything(), items: [] }, + { title: "Legacy", url: "legacy", icon: expect.anything(), items: [] }, + ] + : []), + { + title: "App", + url: "app", + icon: expect.anything(), + items: result === "redirect" ? [{ title: "Alias", url: "app/alias" }] : [], + }, + ]); + }); + }, + ); + it("filters out modules hidden by guards", async () => { const modules = [ defineModule({ @@ -650,6 +756,98 @@ describe("useCommandPaletteRoutes", () => { }); }); + it.each([true, false])( + "keeps descendants of redirecting dynamic and static parents (has component: %s)", + async (hasComponent) => { + const component = hasComponent ? () =>
Parent
: undefined; + const modules = [ + defineModule({ + path: "users", + meta: { title: "Users" }, + resources: [ + defineResource({ + path: ":id", + component, + guards: [() => redirectTo("/users/42/section/list")], + subResources: [ + defineResource({ + path: "section", + component, + guards: [() => redirectTo("/users/42/section/list")], + subResources: [ + defineResource({ + path: "list", + component: () =>
List
, + }), + defineResource({ + path: "secret", + guards: [() => hidden()], + subResources: [ + defineResource({ + path: "details", + component: () =>
Secret Details
, + }), + ], + }), + ], + }), + ], + }), + ], + }), + ]; + + const { result } = renderWithNavigationLoader( + () => useCommandPaletteRoutes(), + modules, + "/users/42/section/list", + ); + + await waitFor(async () => { + const routes = await result.current!; + expect(routes.map(({ path }) => path)).toEqual([ + "users/42", + "users/42/section", + "users/42/section/list", + ]); + }); + }, + ); + + it.each([ + { result: "pass", guard: pass }, + { result: "redirect", guard: () => redirectTo("/users/42/profile") }, + { result: "hidden", guard: hidden }, + ])( + "handles a componentless dynamic module when guards return $result", + async ({ result, guard }) => { + const modules = [ + defineModule({ + path: "users/:id", + meta: { title: "User" }, + guards: [guard], + resources: [ + defineResource({ path: "profile", component: () =>
Profile
}), + defineResource({ path: "alias", guards: [() => redirectTo("/users/42/profile")] }), + defineResource({ path: "namespace", guards: [pass] }), + ], + }), + ]; + + const routes = await buildCurrentPathAwareRoutes({ + modules, + locale: "en", + pathname: "/users/42/profile", + }); + + expect(routes.map(({ path }) => path)).toEqual( + result === "hidden" + ? [] + : [...(result === "redirect" ? ["users/42"] : []), "users/42/profile", "users/42/alias"], + ); + }, + ); + it("keeps traversing through a non-navigable dynamic namespace", async () => { const modules = [ defineModule({ diff --git a/packages/core/src/routing/navigation.tsx b/packages/core/src/routing/navigation.tsx index 824998292..e0fc1fabc 100644 --- a/packages/core/src/routing/navigation.tsx +++ b/packages/core/src/routing/navigation.tsx @@ -68,26 +68,27 @@ const buildNavItems = async (props: BuildNavItemsProps) => { if (parseDynamicSegment(module.path) !== null) return null; const guardResult = await runGuards(module.guards); - if (guardResult.type !== "pass") return null; + if (guardResult.type === "hidden") return null; const visibleResources = await filterVisibleResources( module.resources, module.path, resolveTitle, ); + const isNavigable = module.meta.menuItemClickable || guardResult.type === "redirect"; // Skip if no visible resources AND the module itself is not directly navigable - if (visibleResources.length === 0 && !module.meta.menuItemClickable) return null; + if (visibleResources.length === 0 && !isNavigable) return null; - return { module, resources: visibleResources }; + return { module, resources: visibleResources, isNavigable }; }), ); return resolvedModules .filter((entry) => entry !== null) - .map(({ module, resources }) => { + .map(({ module, resources, isNavigable }) => { return { title: resolveTitle(module.meta.title, module.path), - url: module.meta.menuItemClickable ? module.path || "/" : undefined, + url: isNavigable ? module.path || "/" : undefined, icon: module.meta.icon || , items: resources, }; @@ -105,11 +106,11 @@ const filterVisibleResources = async ( if (parseDynamicSegment(resource.path) !== null) return null; const guardResult = await runGuards(resource.guards); - if (guardResult.type !== "pass") return null; + if (guardResult.type === "hidden") return null; const resourcePath = `${basePath}/${resource.path}`; const resourceTitle = resolveTitle(resource.meta.title, resource.path); - const hasComponent = resource.component !== undefined; + const isNavigable = resource.component !== undefined || guardResult.type === "redirect"; // Recursively process subResources const subItems = resource.subResources @@ -118,12 +119,12 @@ const filterVisibleResources = async ( const hasVisibleSubItems = subItems && subItems.length > 0; - // Componentless resources without sub-resources are dead-end namespaces - if (!hasComponent && !hasVisibleSubItems) return null; + // Non-navigable resources without sub-resources are dead-end namespaces + if (!isNavigable && !hasVisibleSubItems) return null; return { title: resourceTitle, - url: hasComponent ? resourcePath : undefined, + url: isNavigable ? resourcePath : undefined, items: hasVisibleSubItems ? subItems : undefined, }; }), @@ -251,10 +252,11 @@ const collectCurrentPathAwareRoutes = async ({ routeMap: Map; }): Promise => { const guardResult = await runGuards(node.guards); - if (guardResult.type !== "pass") return; + if (guardResult.type === "hidden") return; const isNavigable = - "resources" in node ? node.meta.menuItemClickable : node.component !== undefined; + guardResult.type === "redirect" || + ("resources" in node ? node.meta.menuItemClickable : node.component !== undefined); if (isNavigable) { const path = routePathFromSegments(baseSegments); routeMap.set(path, {