From e020fbc39d61999bea07a4e5d0e2608a31ee364b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=87=91=E4=B8=80=E6=B6=9B?= Date: Tue, 15 Sep 2026 19:29:21 +0800 Subject: [PATCH 1/4] fix navigation context for direct app routes --- .changeset/calm-routes-remember-navigation.md | 6 ++ .../docs/guide/designing-app-navigation.md | 14 ++++ packages/app-shell/README.md | 3 + packages/app-shell/src/layout/AppHeader.tsx | 68 +++++++++++++++++-- .../app-shell/src/layout/UnifiedSidebar.tsx | 48 ++++++++----- ...fiedSidebar.derivedAreaVisibility.test.tsx | 26 ++++--- packages/app-shell/src/utils/index.ts | 2 + .../src/utils/navigationContext.test.ts | 62 +++++++++++++++++ .../app-shell/src/utils/navigationContext.ts | 29 ++++++++ packages/layout/README.md | 6 ++ packages/layout/src/NavigationRenderer.tsx | 33 +++++++++ .../layout/src/__tests__/resolveHref.test.ts | 12 +++- 12 files changed, 276 insertions(+), 33 deletions(-) create mode 100644 .changeset/calm-routes-remember-navigation.md create mode 100644 packages/app-shell/src/utils/navigationContext.test.ts create mode 100644 packages/app-shell/src/utils/navigationContext.ts diff --git a/.changeset/calm-routes-remember-navigation.md b/.changeset/calm-routes-remember-navigation.md new file mode 100644 index 0000000000..bf14b4c6f6 --- /dev/null +++ b/.changeset/calm-routes-remember-navigation.md @@ -0,0 +1,6 @@ +--- +"@object-ui/layout": patch +"@object-ui/app-shell": patch +--- + +Keep sidebar area selection and business breadcrumbs aligned with direct app routes. diff --git a/content/docs/guide/designing-app-navigation.md b/content/docs/guide/designing-app-navigation.md index 833d249398..4f4bd6c58b 100644 --- a/content/docs/guide/designing-app-navigation.md +++ b/content/docs/guide/designing-app-navigation.md @@ -108,6 +108,20 @@ Object entries also support record deep-links — `recordId` (with template variables like `{current_user_id}`) opens a specific record, which is how "My Profile"-style entries are built. +## Direct Links Preserve Navigation Context + +The console resolves the current route back through the same navigation tree +that generated its URL. Opening, refreshing, or returning to a page, +dashboard, report, object, named view, filtered object slice, or record link +therefore selects the area that owns the entry and renders its ancestor groups +in the breadcrumb. For example, a direct link to a page declared under +`Projects > Time Management` keeps the Projects area active and shows that +business hierarchy instead of a generic Pages category. + +This behavior depends on a declared navigation target. Routes that are not in +the app's navigation remain valid, but the shell cannot infer an owning area or +business hierarchy for them. + ## Quick Checklist Before publishing an app, scan the navigation for: diff --git a/packages/app-shell/README.md b/packages/app-shell/README.md index fe847122a2..5b13d4dba9 100644 --- a/packages/app-shell/README.md +++ b/packages/app-shell/README.md @@ -108,6 +108,9 @@ function MyDashboard() { constrained reading width for long conversations - **Notification Surfaces**: `ConsoleShell` mounts `NotificationProvider` and every spec `displayType` presents distinctly — no per-app wiring +- **Route-Aware Navigation**: Direct links, refreshes, and browser history + recover the owning area and navigation trail, keeping the sidebar selection + and business breadcrumb aligned with app metadata ## Notifications diff --git a/packages/app-shell/src/layout/AppHeader.tsx b/packages/app-shell/src/layout/AppHeader.tsx index 76eccb3d28..e3dd9225b6 100644 --- a/packages/app-shell/src/layout/AppHeader.tsx +++ b/packages/app-shell/src/layout/AppHeader.tsx @@ -69,7 +69,7 @@ import { useObjectTranslation, useObjectLabel } from '@object-ui/i18n'; import type { BreadcrumbItem as BreadcrumbItemType } from '@object-ui/types'; import { useAuth, getUserInitials, useWorkspaceAdminStatus } from '@object-ui/auth'; import { useMetadata } from '../providers/MetadataProvider.js'; -import { resolveKeyedI18nLabel, preferLocal, matchAppBySegment, appRouteSegment, appStudioRoutePath } from '../utils/index.js'; +import { resolveKeyedI18nLabel, preferLocal, matchAppBySegment, appRouteSegment, appStudioRoutePath, resolveAppNavigationContext } from '../utils/index.js'; import { getIcon } from '../utils/getIcon.js'; import { useMobileViewSwitcher } from './MobileViewSwitcherContext.js'; import { useNavigationContext } from '../context/NavigationContext.js'; @@ -88,6 +88,25 @@ function humanizeSlug(slug: string): string { return slug.replace(/[-_]/g, ' ').replace(/\b\w/g, (c) => c.toUpperCase()); } +type TranslationFn = ReturnType['t']; +type KeyedNavigationLabel = Exclude[0], string | undefined>; + +function isKeyedNavigationLabel(label: unknown): label is KeyedNavigationLabel { + return typeof label === 'object' && label !== null && 'key' in label && typeof label.key === 'string'; +} + +function navigationLabel(label: unknown, t: TranslationFn): string { + if (typeof label === 'string') return label; + if (isKeyedNavigationLabel(label)) { + return resolveKeyedI18nLabel(label, t) || label.key; + } + if (label && typeof label === 'object') { + const localized = Object.values(label).find((value) => typeof value === 'string'); + if (typeof localized === 'string') return localized; + } + return ''; +} + /** Muted `/` separator between path segments */ function PathSep() { return ( @@ -331,31 +350,66 @@ export function AppHeader({ const extraSegments: BreadcrumbItemType[] = []; + // Recover the business navigation context from the current route. The + // sidebar and header both rely on resolveActiveNavItem's canonical inverse + // mapping, so a deep link cannot select one area while naming a different + // hierarchy in the breadcrumb. + const { + area: activeNavigationArea, + trail: activeNavigationTrail, + } = resolveAppNavigationContext({ + areas: currentApp?.areas || [], + navigation: currentApp?.navigation || [], + pathname: location.pathname, + search: location.search, + basePath: baseHref, + }); + + if (activeNavigationArea) { + const label = navigationLabel(activeNavigationArea.label, t); + if (label) extraSegments.push({ label }); + } + for (const ancestor of activeNavigationTrail.slice(0, -1)) { + const label = navigationLabel(ancestor.label, t); + if (label) extraSegments.push({ label }); + } + const hasNavigationContext = activeNavigationTrail.length > 0; + const activeNavigationLabel = navigationLabel( + activeNavigationTrail[activeNavigationTrail.length - 1]?.label, + t, + ); + if (isApp) { if (routeType === 'dashboard') { - extraSegments.push({ label: t('console.breadcrumb.dashboards'), href: baseHref }); + if (!hasNavigationContext) extraSegments.push({ label: t('console.breadcrumb.dashboards'), href: baseHref }); if (pathParts[3]) { const dashboardName = pathParts[3]; // ADR-0048 Phase 2 — prefer the current app's package (container-scoped). const dashboardDef = preferLocal(metadataDashboards as any[], dashboardName, (currentApp as any)?._packageId); const fallback = dashboardDef?.label || humanizeSlug(dashboardName); - extraSegments.push({ label: dashboardLabel({ name: dashboardName, label: fallback }) }); + extraSegments.push({ + label: activeNavigationLabel || dashboardLabel({ name: dashboardName, label: fallback }), + }); } } else if (routeType === 'page') { - extraSegments.push({ label: t('console.breadcrumb.pages'), href: baseHref }); + if (!hasNavigationContext) extraSegments.push({ label: t('console.breadcrumb.pages'), href: baseHref }); if (pathParts[3]) { const pageName = pathParts[3]; const pageDef = preferLocal(metadataPages as any[], pageName, (currentApp as any)?._packageId); const fallback = pageDef?.label || humanizeSlug(pageName); - extraSegments.push({ label: pageLabel({ name: pageName, label: fallback }) }); + extraSegments.push({ + label: activeNavigationLabel || pageLabel({ name: pageName, label: fallback }), + }); } } else if (routeType === 'report') { - extraSegments.push({ label: t('console.breadcrumb.reports'), href: baseHref }); + if (!hasNavigationContext) extraSegments.push({ label: t('console.breadcrumb.reports'), href: baseHref }); if (pathParts[3]) { const reportName = pathParts[3]; const reportDef = preferLocal(metadataReports as any[], reportName, (currentApp as any)?._packageId); const fallback = reportDef?.label || humanizeSlug(reportName); - extraSegments.push({ label: reportLabel({ name: reportName, label: fallback }) }); + extraSegments.push({ + label: activeNavigationLabel || reportLabel({ name: reportName, label: fallback }), + }); } } else if (routeType === 'system') { extraSegments.push({ label: t('console.breadcrumb.system') }); diff --git a/packages/app-shell/src/layout/UnifiedSidebar.tsx b/packages/app-shell/src/layout/UnifiedSidebar.tsx index afe06b955d..31467d127f 100644 --- a/packages/app-shell/src/layout/UnifiedSidebar.tsx +++ b/packages/app-shell/src/layout/UnifiedSidebar.tsx @@ -48,7 +48,7 @@ import { useRecentItems } from '../hooks/useRecentItems.js'; import { useFavorites } from '../hooks/useFavorites.js'; import { useNavPins } from '../hooks/useNavPins.js'; import { useNavActionDispatch } from '../hooks/useNavActionDispatch.js'; -import { matchAppBySegment, appRouteSegment } from '../utils/index.js'; +import { matchAppBySegment, appRouteSegment, resolveAppNavigationContext } from '../utils/index.js'; import { useHomePath } from '../hooks/useHomePath.js'; // Aliased for symmetry with objectui's own `resolveKeyedI18nLabel` above (the // names stopped colliding in objectui#4167): this is the spec's resolver (new in @@ -205,6 +205,15 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { const activeApps = apps.filter((a: any) => a.active !== false && a.hidden !== true); // ADR-0048 (A) — route segment may be a package id; match by it (name fallback). const activeApp = matchAppBySegment(apps.filter((a: any) => a.active !== false), activeAppName || currentAppName) || activeApps[0]; + const appBasePath = context === 'app' && activeApp ? `/apps/${appRouteSegment(activeApp)}` : ''; + + // App-level context selectors (e.g. Studio's package scope). Their values + // participate in route matching as well as href generation. + const { contextValues, element: contextSelectorsUI } = useAppContextSelectors( + activeApp?.name || 'home', + activeApp?.contextSelectors, + t, + ); // Drag-reorder and pin persistence const { applyOrder, handleReorder } = useNavOrder(activeApp?.name || 'home'); @@ -285,17 +294,31 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { ); const visibleAreaIds = visibleAreas.map((a) => a.id).join(','); - - // Re-elect when the app changes or the visible-area set changes. Keeping - // `prev` whenever it is still visible means merely REVEALING a new area - // never steals the user's current selection. + const routeAreaId = appBasePath + ? resolveAppNavigationContext({ + areas: visibleAreas, + pathname: location.pathname, + search: location.search, + basePath: appBasePath, + templateContext: { + currentUserId: user?.id ?? null, + currentOrgId: activeOrganization?.id ?? null, + contextValues, + }, + }).area?.id ?? null + : null; + + // A deep link, refresh, or browser history transition elects the area that + // owns the active navigation item. Routes outside the navigation tree keep + // a still-visible manual choice, preserving area-switcher behaviour on app + // landing and auxiliary pages. React.useEffect(() => { if (visibleAreas.length > 0) { - setActiveAreaId(prev => visibleAreas.some((a) => a.id === prev) ? prev : visibleAreas[0].id); + setActiveAreaId(prev => routeAreaId ?? (visibleAreas.some((a) => a.id === prev) ? prev : visibleAreas[0].id)); } else { setActiveAreaId(null); } - }, [activeApp?.name, visibleAreaIds]); + }, [activeApp?.name, routeAreaId, visibleAreaIds]); // Resolve navigation items. The render-time `?? visibleAreas[0]` fallback // covers the frame between a gating change hiding the active area and the @@ -303,15 +326,6 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { const activeArea = visibleAreas.find((a) => a.id === activeAreaId) ?? visibleAreas[0]; const appNavigation: NavigationItem[] = activeArea?.navigation || activeApp?.navigation || []; - // App-level context selectors (e.g. Studio's package scope). Their - // values are injected into nav items as `{}` template vars so a - // single dropdown transparently scopes every secondary menu. - const { contextValues, element: contextSelectorsUI } = useAppContextSelectors( - activeApp?.name || 'home', - activeApp?.contextSelectors, - t, - ); - // Home navigation items. For workspace admins we surface the full system // ("Administration") nav right here on /home — previously the home context // showed ONLY a "Home" link, so a fresh env (no apps yet) rendered a bare @@ -386,7 +400,7 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { // Determine which navigation to show based on context const navigationItems = context === 'home' ? homeNavigation : appNavigation; - const basePath = context === 'app' && activeApp ? `/apps/${appRouteSegment(activeApp)}` : ''; + const basePath = appBasePath; const isStudioApp = context === 'app' && activeApp?.name === 'studio'; // Studio's home link carries the active package scope. Read (and re-emit) // it through the SAME per-selector key derivation the selector writes with, diff --git a/packages/app-shell/src/layout/__tests__/UnifiedSidebar.derivedAreaVisibility.test.tsx b/packages/app-shell/src/layout/__tests__/UnifiedSidebar.derivedAreaVisibility.test.tsx index af91b0461c..a26eb6ea6f 100644 --- a/packages/app-shell/src/layout/__tests__/UnifiedSidebar.derivedAreaVisibility.test.tsx +++ b/packages/app-shell/src/layout/__tests__/UnifiedSidebar.derivedAreaVisibility.test.tsx @@ -74,12 +74,16 @@ vi.mock('../../providers/ExpressionProvider', () => ({ evaluateVisibility: (expr: unknown) => expr !== false && expr !== 'false', })); -vi.mock('../../utils', () => ({ - resolveKeyedI18nLabel: (label: unknown) => (typeof label === 'string' ? label : ''), - matchAppBySegment: (apps: Array<{ name?: string }>, segment?: string) => - apps.find((a) => a?.name === segment), - appRouteSegment: (app: { name?: string }) => app?.name, -})); +vi.mock('../../utils', async () => { + const { resolveAppNavigationContext } = await import('../../utils/navigationContext'); + return { + resolveKeyedI18nLabel: (label: unknown) => (typeof label === 'string' ? label : ''), + matchAppBySegment: (apps: Array<{ name?: string }>, segment?: string) => + apps.find((a) => a?.name === segment), + appRouteSegment: (app: { name?: string }) => app?.name, + resolveAppNavigationContext, + }; +}); // Lazy lucide DynamicIcon would suspend mid-test; a null icon is enough here. vi.mock('../../utils/getIcon', () => ({ getIcon: () => () => null })); @@ -140,13 +144,13 @@ const gatedSales: NavigationArea = { navigation: [{ ...salesArea.navigation[0], visible: false }], }; -function sidebarUi(areas: NavigationArea[]) { +function sidebarUi(areas: NavigationArea[], initialEntry = '/apps/crm') { metadataState = { apps: [{ name: 'crm', label: 'CRM', active: true, areas }], objects: [], }; return ( - + @@ -179,6 +183,12 @@ describe('UnifiedSidebar derived area visibility (#3319)', () => { expect(screen.queryByText('Opportunities')).not.toBeInTheDocument(); }); + it('elects the area that owns a direct route instead of the first visible area', () => { + render(sidebarUi([salesArea, serviceArea], '/apps/crm/case')); + expect(screen.getByText('Cases')).toBeInTheDocument(); + expect(screen.queryByText('Opportunities')).not.toBeInTheDocument(); + }); + it('keeps a PARTIALLY gated area visible and active, hiding only the gated item', () => { const partialSales: NavigationArea = { ...salesArea, diff --git a/packages/app-shell/src/utils/index.ts b/packages/app-shell/src/utils/index.ts index 4108a56b77..4e7e145cae 100644 --- a/packages/app-shell/src/utils/index.ts +++ b/packages/app-shell/src/utils/index.ts @@ -32,6 +32,8 @@ export { deriveRelatedLists } from './deriveRelatedLists.js'; export type { DerivedRelatedList } from './deriveRelatedLists.js'; export { preferLocal } from './preferLocal.js'; +export { resolveAppNavigationContext } from './navigationContext.js'; +export type { AppNavigationContext } from './navigationContext.js'; // Admin-override affordance + audit marker (objectui#5178). Exported because // the Approval Center (`apps/console`) renders the second timeline and must ask diff --git a/packages/app-shell/src/utils/navigationContext.test.ts b/packages/app-shell/src/utils/navigationContext.test.ts new file mode 100644 index 0000000000..4735e04003 --- /dev/null +++ b/packages/app-shell/src/utils/navigationContext.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from 'vitest'; +import type { NavigationArea, NavigationItem } from '@object-ui/types'; +import { resolveAppNavigationContext } from './navigationContext'; + +const BASE = '/apps/forge'; +const areas: NavigationArea[] = [ + { + id: 'workspace', + label: 'Workspace', + navigation: [{ id: 'home', type: 'page', label: 'Workbench', pageName: 'workbench' }], + }, + { + id: 'project', + label: 'Project', + navigation: [{ + id: 'project_management', + type: 'group', + label: 'Project Management', + children: [{ id: 'timesheet', type: 'page', label: 'Timesheets', pageName: 'page_project_timesheet_cost' }], + }], + }, +]; + +describe('resolveAppNavigationContext', () => { + it('recovers an area, group, and page from a direct page route', () => { + const result = resolveAppNavigationContext({ + areas, + pathname: `${BASE}/page/page_project_timesheet_cost`, + search: '', + basePath: BASE, + }); + + expect(result.area?.id).toBe('project'); + expect(result.trail.map((item) => item.id)).toEqual(['project_management', 'timesheet']); + }); + + it('supports apps that use a flat navigation tree', () => { + const navigation: NavigationItem[] = [ + { id: 'reports', type: 'group', label: 'Reports', children: [ + { id: 'margin', type: 'report', label: 'Margin', reportName: 'margin' }, + ] }, + ]; + const result = resolveAppNavigationContext({ + navigation, + pathname: `${BASE}/report/margin`, + search: '', + basePath: BASE, + }); + + expect(result.area).toBeNull(); + expect(result.trail.map((item) => item.id)).toEqual(['reports', 'margin']); + }); + + it('leaves routes outside the declared navigation unresolved', () => { + expect(resolveAppNavigationContext({ + areas, + pathname: `${BASE}/search`, + search: '', + basePath: BASE, + })).toEqual({ area: null, trail: [] }); + }); +}); diff --git a/packages/app-shell/src/utils/navigationContext.ts b/packages/app-shell/src/utils/navigationContext.ts new file mode 100644 index 0000000000..b3030f1d29 --- /dev/null +++ b/packages/app-shell/src/utils/navigationContext.ts @@ -0,0 +1,29 @@ +import { resolveActiveNavTrail, type NavTemplateContext } from '@object-ui/layout'; +import type { NavigationArea, NavigationItem } from '@object-ui/types'; + +export interface AppNavigationContext { + area: NavigationArea | null; + trail: NavigationItem[]; +} + +/** Resolve an app route back to its owning area and navigation trail. */ +export function resolveAppNavigationContext(options: { + areas?: NavigationArea[]; + navigation?: NavigationItem[]; + pathname: string; + search: string; + basePath: string; + templateContext?: NavTemplateContext; +}): AppNavigationContext { + const { areas = [], navigation = [], pathname, search, basePath, templateContext } = options; + + for (const area of areas) { + const trail = resolveActiveNavTrail(area.navigation || [], pathname, search, basePath, templateContext); + if (trail.length > 0) return { area, trail }; + } + + return { + area: null, + trail: resolveActiveNavTrail(navigation, pathname, search, basePath, templateContext), + }; +} diff --git a/packages/layout/README.md b/packages/layout/README.md index cd39170bb8..07a9f30fd4 100644 --- a/packages/layout/README.md +++ b/packages/layout/README.md @@ -108,6 +108,12 @@ ADR-0087 D2 conversion `page-header-subtitle-alias`. Navigation sidebar component with React Router integration. +`NavigationRenderer` also exports `resolveActiveNavTrail`. Given the same +navigation items, route, base path, and template context used to create links, +it returns the active item together with every ancestor group. App shells use +this inverse mapping to keep deep-linked routes aligned with their declared +navigation hierarchy. + ```typescript import { SidebarNav, type NavItem } from '@object-ui/layout'; import { Home, Settings, Users } from 'lucide-react'; diff --git a/packages/layout/src/NavigationRenderer.tsx b/packages/layout/src/NavigationRenderer.tsx index b375afdca6..985e3080f6 100644 --- a/packages/layout/src/NavigationRenderer.tsx +++ b/packages/layout/src/NavigationRenderer.tsx @@ -835,6 +835,39 @@ export function resolveActiveNavItem( return best; } +/** + * Resolve the active item's full navigation trail, including ancestor groups. + * + * This is the structural inverse of {@link resolveHref}: shell surfaces use + * the same winning leaf as {@link resolveActiveNavItem}, then recover the + * groups that contain it. Keeping the lookup here prevents sidebars and + * breadcrumbs from inventing separate route-matching rules. + */ +export function resolveActiveNavTrail( + items: NavigationItem[], + pathname: string, + search: string, + basePath: string, + templateContext?: NavTemplateContext, +): NavigationItem[] { + const active = resolveActiveNavItem(items, pathname, search, basePath, templateContext); + if (!active) return []; + + const findTrail = (nodes: NavigationItem[] | undefined): NavigationItem[] | null => { + if (!nodes) return null; + for (const node of nodes) { + if (node === active) return [node]; + if (node.type === 'group') { + const childTrail = findTrail(node.children); + if (childTrail) return [node, ...childTrail]; + } + } + return null; + }; + + return findTrail(items) ?? []; +} + /** * The elected active item id, provided once at the tree root by * {@link NavigationRenderer} — per-item active state is a plain id diff --git a/packages/layout/src/__tests__/resolveHref.test.ts b/packages/layout/src/__tests__/resolveHref.test.ts index a96c18ccd1..c4fa081700 100644 --- a/packages/layout/src/__tests__/resolveHref.test.ts +++ b/packages/layout/src/__tests__/resolveHref.test.ts @@ -127,7 +127,7 @@ describe('resolveHref — non-object targets unchanged', () => { // resolveActiveNavItem — the inverse mapping (#2272) // --------------------------------------------------------------------------- -import { resolveActiveNavItem } from '../NavigationRenderer'; +import { resolveActiveNavItem, resolveActiveNavTrail } from '../NavigationRenderer'; /** Split an href into the (pathname, search) pair resolveActiveNavItem takes. */ function locOf(href: string): { pathname: string; search: string } { @@ -196,6 +196,16 @@ describe('resolveActiveNavItem — single winner across the tree', () => { expect(activeId(`${BASE}/page/home`)).toBe('nav_home'); }); + it('returns the ancestor groups and winning leaf as one route trail', () => { + expect( + resolveActiveNavTrail(NAV, `${BASE}/page/home`, '', BASE, CTX).map((item) => item.id), + ).toEqual(['grp', 'nav_home']); + }); + + it('returns an empty trail for an unrelated route', () => { + expect(resolveActiveNavTrail(NAV, `${BASE}/search`, '', BASE, CTX)).toEqual([]); + }); + it('unrelated route → no active item', () => { expect(activeId(`${BASE}/search`)).toBeNull(); }); From 161d40d6acd407ed132aac4c2578b2c8185c5fab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=87=91=E4=B8=80=E6=B6=9B?= Date: Tue, 15 Sep 2026 20:47:31 +0800 Subject: [PATCH 2/4] fix active navigation for parameterized pages --- .../docs/guide/designing-app-navigation.md | 5 ++++ packages/layout/README.md | 4 +++- packages/layout/src/NavigationRenderer.tsx | 24 +++++++++++++++---- .../layout/src/__tests__/resolveHref.test.ts | 10 ++++++++ 4 files changed, 37 insertions(+), 6 deletions(-) diff --git a/content/docs/guide/designing-app-navigation.md b/content/docs/guide/designing-app-navigation.md index 4f4bd6c58b..eded6d2a2c 100644 --- a/content/docs/guide/designing-app-navigation.md +++ b/content/docs/guide/designing-app-navigation.md @@ -118,6 +118,11 @@ in the breadcrumb. For example, a direct link to a page declared under `Projects > Time Management` keeps the Projects area active and shows that business hierarchy instead of a generic Pages category. +When two entries intentionally reuse one page, give each entry a distinct +`params` value. The generated query string then preserves which entry the user +opened, and the same value restores the correct area, group, and breadcrumb +after a refresh. + This behavior depends on a declared navigation target. Routes that are not in the app's navigation remain valid, but the shell cannot infer an owning area or business hierarchy for them. diff --git a/packages/layout/README.md b/packages/layout/README.md index 07a9f30fd4..2fb1fc3fd4 100644 --- a/packages/layout/README.md +++ b/packages/layout/README.md @@ -112,7 +112,9 @@ Navigation sidebar component with React Router integration. navigation items, route, base path, and template context used to create links, it returns the active item together with every ancestor group. App shells use this inverse mapping to keep deep-linked routes aligned with their declared -navigation hierarchy. +navigation hierarchy. Authored query parameters participate in the inverse +mapping, so entries that intentionally reuse one page can still identify their +own navigation trail. ```typescript import { SidebarNav, type NavItem } from '@object-ui/layout'; diff --git a/packages/layout/src/NavigationRenderer.tsx b/packages/layout/src/NavigationRenderer.tsx index 985e3080f6..f6373e0303 100644 --- a/packages/layout/src/NavigationRenderer.tsx +++ b/packages/layout/src/NavigationRenderer.tsx @@ -707,6 +707,7 @@ export function resolveHref( */ const MATCH_RECORD = 50; const MATCH_FILTERS = 40; +const MATCH_PARAMS = 35; const MATCH_VIEW = 30; const MATCH_EXACT = 25; const MATCH_OBJECT_SUBROUTE = 10; @@ -733,6 +734,7 @@ function stripViewQualifier(objectName: string, view: string): string { function itemMatchScore( item: NavigationItem, pathname: string, + searchParams: URLSearchParams, filterParams: Map, basePath: string, ctx: NavTemplateContext | undefined, @@ -786,10 +788,21 @@ function itemMatchScore( return segs.length === 0 ? MATCH_EXACT : MATCH_OBJECT_SUBROUTE; } - // Non-object types match against their canonical href (metadata component - // hrefs may carry a query string — compare pathnames only). - const hrefPath = href.split('?')[0]; - if (pathname === hrefPath) return MATCH_EXACT; + // Non-object targets may share a pathname and use authored params to name + // the exact navigation context. Compare those params when present so two + // menu entries that intentionally reuse one page still round-trip to the + // correct item, group, and area. Unrelated runtime params remain ignored. + const [hrefPath, hrefSearch = ''] = href.split('?'); + if (pathname === hrefPath) { + const expected = new URLSearchParams(hrefSearch); + if ([...expected].length > 0) { + for (const [key, value] of expected) { + if (searchParams.get(key) !== value) return 0; + } + return MATCH_PARAMS; + } + return MATCH_EXACT; + } // Directory/index components (e.g. `metadata:directory`) link to a parent // route that also hosts more-specific child items (`metadata:resource` @@ -814,6 +827,7 @@ export function resolveActiveNavItem( basePath: string, templateContext?: NavTemplateContext, ): NavigationItem | null { + const searchParams = new URLSearchParams(search); const filterParams = parseFilterParams(search); let best: NavigationItem | null = null; let bestScore = 0; @@ -824,7 +838,7 @@ export function resolveActiveNavItem( visit(node.children); continue; } - const score = itemMatchScore(node, pathname, filterParams, basePath, templateContext); + const score = itemMatchScore(node, pathname, searchParams, filterParams, basePath, templateContext); if (score > bestScore) { best = node; bestScore = score; diff --git a/packages/layout/src/__tests__/resolveHref.test.ts b/packages/layout/src/__tests__/resolveHref.test.ts index c4fa081700..15e516da52 100644 --- a/packages/layout/src/__tests__/resolveHref.test.ts +++ b/packages/layout/src/__tests__/resolveHref.test.ts @@ -196,6 +196,16 @@ describe('resolveActiveNavItem — single winner across the tree', () => { expect(activeId(`${BASE}/page/home`)).toBe('nav_home'); }); + it('uses authored page params to distinguish items that share a page', () => { + const sharedPageNav: NavigationItem[] = [ + { id: 'sales_report', type: 'page', label: 'Sales', pageName: 'reports', params: { nav: 'sales_report' } }, + { id: 'profit_report', type: 'page', label: 'Profit', pageName: 'reports', params: { nav: 'profit_report' } }, + ]; + expect( + resolveActiveNavItem(sharedPageNav, `${BASE}/page/reports`, '?nav=profit_report', BASE)?.id, + ).toBe('profit_report'); + }); + it('returns the ancestor groups and winning leaf as one route trail', () => { expect( resolveActiveNavTrail(NAV, `${BASE}/page/home`, '', BASE, CTX).map((item) => item.id), From 4ede37b53967f199a11d40f51e60479b4ff5ea63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=87=91=E4=B8=80=E6=B6=9B?= Date: Wed, 16 Sep 2026 10:22:36 +0800 Subject: [PATCH 3/4] fix direct navigation without route qualifiers --- packages/layout/src/NavigationRenderer.tsx | 19 ++++++++++++++++++- .../layout/src/__tests__/resolveHref.test.ts | 17 +++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/layout/src/NavigationRenderer.tsx b/packages/layout/src/NavigationRenderer.tsx index f6373e0303..7916ea632a 100644 --- a/packages/layout/src/NavigationRenderer.tsx +++ b/packages/layout/src/NavigationRenderer.tsx @@ -831,6 +831,7 @@ export function resolveActiveNavItem( const filterParams = parseFilterParams(search); let best: NavigationItem | null = null; let bestScore = 0; + const unqualifiedPathMatches: NavigationItem[] = []; const visit = (nodes: NavigationItem[] | undefined) => { if (!nodes) return; for (const node of nodes) { @@ -843,10 +844,26 @@ export function resolveActiveNavItem( best = node; bestScore = score; } + if (score === 0 && node.type !== 'object') { + const { href, external } = resolveHref(node, basePath, templateContext); + if (!external && href !== '#') { + const [hrefPath, hrefSearch = ''] = href.split('?'); + const expected = new URLSearchParams(hrefSearch); + const expectedEntries = [...expected]; + const hasAnyQualifier = expectedEntries.some(([key]) => searchParams.has(key)); + if (pathname === hrefPath && expectedEntries.length > 0 && !hasAnyQualifier) { + unqualifiedPathMatches.push(node); + } + } + } } }; visit(items); - return best; + // A direct URL or bookmark may omit navigation-only params. Infer its menu + // context when exactly one authored item owns the pathname; shared routes + // remain intentionally unresolved until a qualifier (for example `nav`) + // identifies the intended item. + return best ?? (unqualifiedPathMatches.length === 1 ? unqualifiedPathMatches[0] : null); } /** diff --git a/packages/layout/src/__tests__/resolveHref.test.ts b/packages/layout/src/__tests__/resolveHref.test.ts index 15e516da52..7add81471d 100644 --- a/packages/layout/src/__tests__/resolveHref.test.ts +++ b/packages/layout/src/__tests__/resolveHref.test.ts @@ -206,6 +206,23 @@ describe('resolveActiveNavItem — single winner across the tree', () => { ).toBe('profit_report'); }); + it('infers a unique parameterized page from a direct URL without nav params', () => { + const uniquePageNav: NavigationItem[] = [ + { id: 'output_invoices', type: 'page', label: 'Output invoices', pageName: 'output_invoices', params: { nav: 'output_invoices' } }, + ]; + expect( + resolveActiveNavItem(uniquePageNav, `${BASE}/page/output_invoices`, '?verify=browser', BASE)?.id, + ).toBe('output_invoices'); + }); + + it('does not guess when an unqualified pathname is shared by multiple items', () => { + const sharedPageNav: NavigationItem[] = [ + { id: 'sales_report', type: 'page', label: 'Sales', pageName: 'reports', params: { nav: 'sales_report' } }, + { id: 'profit_report', type: 'page', label: 'Profit', pageName: 'reports', params: { nav: 'profit_report' } }, + ]; + expect(resolveActiveNavItem(sharedPageNav, `${BASE}/page/reports`, '', BASE)).toBeNull(); + }); + it('returns the ancestor groups and winning leaf as one route trail', () => { expect( resolveActiveNavTrail(NAV, `${BASE}/page/home`, '', BASE, CTX).map((item) => item.id), From aa83fc1f10b93fbfd02cbc6209bd3fe94e38df72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=87=91=E4=B8=80=E6=B6=9B?= Date: Wed, 16 Sep 2026 10:25:20 +0800 Subject: [PATCH 4/4] test direct qualified page navigation context --- packages/app-shell/src/utils/navigationContext.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/app-shell/src/utils/navigationContext.test.ts b/packages/app-shell/src/utils/navigationContext.test.ts index 4735e04003..29a8a87339 100644 --- a/packages/app-shell/src/utils/navigationContext.test.ts +++ b/packages/app-shell/src/utils/navigationContext.test.ts @@ -16,7 +16,13 @@ const areas: NavigationArea[] = [ id: 'project_management', type: 'group', label: 'Project Management', - children: [{ id: 'timesheet', type: 'page', label: 'Timesheets', pageName: 'page_project_timesheet_cost' }], + children: [{ + id: 'timesheet', + type: 'page', + label: 'Timesheets', + pageName: 'page_project_timesheet_cost', + params: { nav: 'timesheet' }, + }], }], }, ]; @@ -26,7 +32,7 @@ describe('resolveAppNavigationContext', () => { const result = resolveAppNavigationContext({ areas, pathname: `${BASE}/page/page_project_timesheet_cost`, - search: '', + search: '?verify=direct-link', basePath: BASE, });