From 158f372be8fea43031a46ccc37993882792c8e27 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 07:46:43 +0000 Subject: [PATCH 1/7] feat(types,layout,app-shell): a navigation entry with no label renders its target's current label at render time (objectui#9868) @objectstack/spec 17.5.0 made NavigationItemSchema.label optional with a declared semantic: absent means the entry shows the current label of what it opens (view, else object / dashboard), present means verbatim. - types: NavigationEntryItem.label is optional; the zod mirror accepts a label-less entry for every type the spec does, fills nothing in, still requires id, and still refuses an empty label. menuItemToNavigationItem stops writing ''. - layout: resolveNavItemLabel resolves an absent label through a new resolveTargetLabel resolver (view -> object / dashboard -> machine name); action entries, search and the AppSchemaRenderer tab bar go through it. - app-shell: useNavTargetLabel reads the metadata cache (object schema, merged list views, dashboards); UnifiedSidebar and nav:menu both wire it. Navigation sync no longer materialises label || pageName / dashboardName. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_011p7ikEivgXefNDaE5S5Uec --- .changeset/9868-nav-label-inherits-target.md | 37 +++ ...NavigationSync.labelInherits-9868.test.tsx | 128 +++++++++++ packages/app-shell/src/hooks/useNavPins.ts | 8 +- .../app-shell/src/hooks/useNavTargetLabel.ts | 104 +++++++++ .../app-shell/src/hooks/useNavigationSync.ts | 53 +++-- .../app-shell/src/layout/UnifiedSidebar.tsx | 7 + ...debar.navLabelInheritsTarget-9868.test.tsx | 144 ++++++++++++ ...label-inherits-target-9868.render.test.tsx | 167 ++++++++++++++ .../app-shell/src/views/nav-menu-renderer.tsx | 13 +- packages/layout/src/AppSchemaRenderer.tsx | 6 +- packages/layout/src/NavigationRenderer.tsx | 156 ++++++++++++- ...Renderer.labelInheritsTarget-9868.test.tsx | 213 ++++++++++++++++++ .../app-wizard-separator-layout-10867.test.ts | 12 +- .../__tests__/nav-label-optional-9868.test.ts | 114 ++++++++++ .../__tests__/navigation-spec-parity.test.ts | 13 +- packages/types/src/app.ts | 34 ++- packages/types/src/zod/app.zod.ts | 44 +++- 17 files changed, 1200 insertions(+), 53 deletions(-) create mode 100644 .changeset/9868-nav-label-inherits-target.md create mode 100644 packages/app-shell/src/hooks/__tests__/useNavigationSync.labelInherits-9868.test.tsx create mode 100644 packages/app-shell/src/hooks/useNavTargetLabel.ts create mode 100644 packages/app-shell/src/layout/__tests__/UnifiedSidebar.navLabelInheritsTarget-9868.test.tsx create mode 100644 packages/app-shell/src/views/__tests__/nav-label-inherits-target-9868.render.test.tsx create mode 100644 packages/layout/src/__tests__/NavigationRenderer.labelInheritsTarget-9868.test.tsx create mode 100644 packages/types/src/__tests__/nav-label-optional-9868.test.ts diff --git a/.changeset/9868-nav-label-inherits-target.md b/.changeset/9868-nav-label-inherits-target.md new file mode 100644 index 0000000000..0aeab6b21f --- /dev/null +++ b/.changeset/9868-nav-label-inherits-target.md @@ -0,0 +1,37 @@ +--- +'@object-ui/types': minor +'@object-ui/layout': patch +'@object-ui/app-shell': patch +--- + +feat(types,layout,app-shell): a navigation entry with no `label` shows its target's current label, resolved at render time (objectui#9868) + +`@objectstack/spec` 17.5.0 made `NavigationItemSchema.label` optional on every +navigation entry, with a declared semantic (the cloud#2021 letter-A ruling): +absent ⇒ the entry shows the CURRENT label of what it opens — the view's label +when it names a labelled view, else the object's / dashboard's label; present ⇒ +rendered verbatim. Nothing is stored for the absent case, so a renamed target +shows its new name on the next render. + +- `@object-ui/types` (widening): `NavigationEntryItem.label` is optional, and + `NavigationItemSchema` accepts a label-less entry of every type the spec does, + filling nothing in at parse time. `id` is still required. An EMPTY label is + still refused — `''` is a present label that would render empty text — and the + message says to omit the key instead. `menuItemToNavigationItem` no longer + turns a missing legacy label into `''`. +- `@object-ui/layout`: `resolveNavItemLabel` resolves an absent label through a + new `resolveTargetLabel` resolver (`NavigationRenderer` prop, types + `NavLabelTarget` / `NavTargetLabelResolver`), walking view → object / + dashboard, and falls back to the target's machine name (`viewName`, + `objectName`, `dashboardName`, `pageName`, `reportName`, `url`, + `componentRef`, the action name; a group shows its `id`). An authored label is + never replaced by the target's label, including one spelled like the machine + name. Action entries, the search filter and `AppSchemaRenderer`'s mobile tab + bar read the label through the same function, so an unlabelled entry no longer + throws there. +- `@object-ui/app-shell`: the sidebar and the `nav:menu` block both resolve + targets through one hook over the metadata cache the shell already holds + (object schema, merged list views, dashboards) — no request per nav entry. + Navigation sync stops writing `label: pageName` / `label: dashboardName` for an + unnamed page or dashboard; an author label is still written, and stored labels + are kept. diff --git a/packages/app-shell/src/hooks/__tests__/useNavigationSync.labelInherits-9868.test.tsx b/packages/app-shell/src/hooks/__tests__/useNavigationSync.labelInherits-9868.test.tsx new file mode 100644 index 0000000000..5c8916e89a --- /dev/null +++ b/packages/app-shell/src/hooks/__tests__/useNavigationSync.labelInherits-9868.test.tsx @@ -0,0 +1,128 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#9868 — navigation sync stops MATERIALISING a label for an unnamed + * page / dashboard entry. + * + * It used to write `label: label || pageName` / `label: label || dashboardName`, + * which froze the machine name into the app's stored navigation. Under + * `@objectstack/spec` 17.5.0 (the cloud#2021 letter-A ruling) an ABSENT label + * means "resolve at render time", so an unnamed entry is written with NO + * `label` key; an author label is still written, verbatim. + * + * Asserted on the real writer — `NavigationSyncEffect`, the one caller in the + * console, which never passes a label — and, for the author-label control, on + * `useNavigationSync().syncPageCreated` / `syncDashboardCreated` with one. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import React, { useEffect } from 'react'; +import { render, waitFor } from '@testing-library/react'; +import { MetadataCtx, AdapterCtx, type MetadataContextValue } from '@object-ui/react'; +import { NavigationSyncEffect, useNavigationSync } from '../useNavigationSync'; + +vi.mock('sonner', () => ({ + toast: { success: vi.fn(), error: vi.fn(), info: vi.fn() }, +})); + +interface World { + pages: any[]; + dashboards: any[]; +} + +const crm = { name: 'crm', label: 'CRM', navigation: [] }; + +function metaValue(world: World): MetadataContextValue { + return { + apps: [crm], + objects: [], + dashboards: world.dashboards, + reports: [], + pages: world.pages, + loading: false, + error: null, + refresh: async () => {}, + invalidate: () => {}, + ensureType: async () => [], + getItem: async () => null, + getItemsByType: () => [], + getTypeStatus: () => 'ready', + }; +} + +const adapterFor = (saveItem: ReturnType) => ({ getClient: () => ({ meta: { saveItem } }) }) as any; + +/** Every navigation array the writer PUT, flattened to its entries. */ +const writtenEntries = (saveItem: ReturnType) => + saveItem.mock.calls.flatMap(([, , schema]) => schema.navigation as Record[]); + +const flush = () => new Promise((r) => setTimeout(r, 0)); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('objectui#9868 — NavigationSyncEffect writes unnamed entries with NO label', () => { + it('a new page and a new dashboard land in navigation without a `label` key', async () => { + const saveItem = vi.fn().mockResolvedValue({}); + const tree = (world: World) => ( + + + + + + ); + const { rerender } = render(tree({ pages: [], dashboards: [] })); + await flush(); + + rerender(tree({ pages: [{ name: 'my_page' }], dashboards: [{ name: 'my_dash', label: 'My Dashboard' }] })); + await waitFor(() => expect(saveItem).toHaveBeenCalledTimes(2)); + + const [page, dash] = [ + writtenEntries(saveItem).find((e) => e.type === 'page'), + writtenEntries(saveItem).find((e) => e.type === 'dashboard'), + ]; + expect(page).toMatchObject({ type: 'page', pageName: 'my_page' }); + expect(dash).toMatchObject({ type: 'dashboard', dashboardName: 'my_dash' }); + // The subject: nothing materialised — not the machine name, not `''`. + expect(page).not.toHaveProperty('label'); + expect(dash).not.toHaveProperty('label'); + }); +}); + +describe('objectui#9868 — an author label is still written (control)', () => { + it('syncPageCreated / syncDashboardCreated with a label store it verbatim', async () => { + const saveItem = vi.fn().mockResolvedValue({}); + function Caller() { + const { syncPageCreated, syncDashboardCreated } = useNavigationSync(); + useEffect(() => { + void (async () => { + await syncPageCreated('crm', 'my_page', 'Team Home'); + await syncDashboardCreated('crm', 'my_dash', 'Pipeline Health'); + })(); + }, [syncPageCreated, syncDashboardCreated]); + return null; + } + render( + + + + + , + ); + await waitFor(() => expect(saveItem).toHaveBeenCalledTimes(2)); + + const entries = writtenEntries(saveItem); + expect(entries.find((e) => e.type === 'page')).toMatchObject({ pageName: 'my_page', label: 'Team Home' }); + expect(entries.find((e) => e.type === 'dashboard')).toMatchObject({ + dashboardName: 'my_dash', + label: 'Pipeline Health', + }); + }); +}); diff --git a/packages/app-shell/src/hooks/useNavPins.ts b/packages/app-shell/src/hooks/useNavPins.ts index 17f45dcc19..0e7f2dbd30 100644 --- a/packages/app-shell/src/hooks/useNavPins.ts +++ b/packages/app-shell/src/hooks/useNavPins.ts @@ -15,7 +15,7 @@ import { useCallback, useMemo } from 'react'; import type { NavigationItem } from '@object-ui/types'; -import { resolveHref } from '@object-ui/layout'; +import { resolveHref, resolveNavItemLabel } from '@object-ui/layout'; import { useFavorites } from '../context/FavoritesProvider.js'; const MAX_PINS = 20; @@ -65,7 +65,11 @@ export function useNavPins() { if (item && item.type !== 'separator') { addFavorite({ id: favId, - label: item.label, + // An entry's `label` may be absent (objectui#9868: it inherits its + // target's label at render time). A `type: 'nav'` favorite's label + // is a placeholder — the sidebar renders the live nav tree's label — + // so the machine-name text the renderer falls back to is enough here. + label: resolveNavItemLabel(item), href: deriveHref(item, basePath ?? ''), type: 'nav', navId: itemId, diff --git a/packages/app-shell/src/hooks/useNavTargetLabel.ts b/packages/app-shell/src/hooks/useNavTargetLabel.ts new file mode 100644 index 0000000000..3a60fe843a --- /dev/null +++ b/packages/app-shell/src/hooks/useNavTargetLabel.ts @@ -0,0 +1,104 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * useNavTargetLabel — the console's answer to "what is this navigation target + * called NOW?", for navigation entries that carry no `label` (objectui#9868). + * + * `@objectstack/spec` 17.5.0 made `NavigationItemSchema.label` optional with a + * declared semantic (the cloud#2021 letter-A ruling): an ABSENT label means the + * entry shows, at render time, the CURRENT label of what it opens — the view's + * label when it names a labelled view, else the object's / dashboard's label. + * `@object-ui/layout`'s `resolveNavItemLabel` walks that ladder; this hook is + * the metadata it asks, as a {@link NavTargetLabelResolver}. + * + * ## What it reads — the metadata the shell already holds, no fetch of its own + * + * `useMetadata()` — `MetadataProvider`'s per-type cache, read lazily per + * target kind: + * + * - an object's `label` from `objects` (the object schema — what a rename in + * the object designer changes). The sidebar already reads `objects` for its + * runtime-capability gate, so the type is loaded either way; + * - a view's `label` from that object's `listViews`, which `MetadataProvider` + * merges in from the eagerly-loaded `view` type. The nav entry's `viewName` + * is matched against the view ids by `resolveViewId` — the one matcher the + * object page itself uses — so an entry names the same view it opens; + * - a dashboard's `label` from `dashboards`, read ONLY when a dashboard target + * is asked about. One list request for the whole type (the cache's), never + * one per nav entry; the console's `NavigationSyncEffect` loads it anyway. + * + * Nothing is written back and nothing is memoised across metadata versions, so + * a renamed target shows its new name on the next render (the ruling accepts a + * reload). A metadata label that is an inline locale map resolves through the + * spec's own `resolveI18nLabel` in the active language, and the result passes + * through the same convention-based bundle lookup (`useObjectLabel`) the rest + * of the console applies to object / view / dashboard names, so the nav shows + * the name the target's own page shows. + * + * A target with no label answers `undefined` — the renderer then falls through + * the spec's ladder and, at the bottom, to the target's machine name. + */ + +import { useMemo } from 'react'; +import { resolveI18nLabel as resolveInlineI18nLabel } from '@objectstack/spec/ui'; +import { resolveViewId } from '@object-ui/core'; +import { useObjectLabel, useObjectTranslation } from '@object-ui/i18n'; +import type { NavLabelTarget, NavTargetLabelResolver } from '@object-ui/layout'; +import { useMetadata } from '../providers/MetadataProvider.js'; + +/** A metadata label (plain string or inline locale map) as display text, or `undefined` when there is none. */ +function metadataText(label: unknown, language: string | undefined): string | undefined { + const text = resolveInlineI18nLabel(label as Parameters[0], language); + return typeof text === 'string' && text.trim() !== '' ? text : undefined; +} + +/** `''` from a bundle lookup that found nothing and had no metadata to fall back to. */ +function nonEmpty(text: string | undefined): string | undefined { + return typeof text === 'string' && text.trim() !== '' ? text : undefined; +} + +function findByName(items: readonly unknown[] | undefined, name: string): Record | undefined { + return (items ?? []).find( + (item): item is Record => + !!item && typeof item === 'object' && (item as { name?: unknown }).name === name, + ); +} + +export function useNavTargetLabel(): NavTargetLabelResolver { + const metadata = useMetadata(); + const { objectLabel, viewLabel, dashboardLabel } = useObjectLabel(); + const { language } = useObjectTranslation(); + + return useMemo( + () => (target: NavLabelTarget) => { + switch (target.kind) { + case 'object': { + const def = findByName(metadata.objects, target.objectName); + const text = metadataText(def?.label, language); + return nonEmpty(objectLabel({ name: target.objectName, label: text ?? '' })); + } + case 'view': { + const def = findByName(metadata.objects, target.objectName); + const views: Record = def?.listViews ?? {}; + const id = resolveViewId(target.viewName, Object.keys(views), target.objectName); + const text = metadataText(id ? views[id]?.label : undefined, language); + return nonEmpty(viewLabel(target.objectName, target.viewName, text ?? '')); + } + case 'dashboard': { + const def = findByName(metadata.dashboards, target.dashboardName); + const text = metadataText(def?.label, language); + return nonEmpty(dashboardLabel({ name: target.dashboardName, label: text ?? '' })); + } + default: + return undefined; + } + }, + [metadata, objectLabel, viewLabel, dashboardLabel, language], + ); +} diff --git a/packages/app-shell/src/hooks/useNavigationSync.ts b/packages/app-shell/src/hooks/useNavigationSync.ts index 7b00d40fff..8b7c368a1b 100644 --- a/packages/app-shell/src/hooks/useNavigationSync.ts +++ b/packages/app-shell/src/hooks/useNavigationSync.ts @@ -38,6 +38,43 @@ export function addNavigationItem( return [...navigation, item]; } +/** + * The navigation entry written when a page is added to an app's navigation. + * + * With no author label it carries NO `label` key (objectui#9868): an absent + * label is `@objectstack/spec` 17.5.0's "resolve at render time", so nothing is + * frozen into the app's metadata the way `label || pageName` used to freeze the + * machine name. (The ruling names label inheritance for views, objects and + * dashboards; a label-less page entry renders its `pageName` — + * `resolveNavItemLabel` in `@object-ui/layout`.) An author label is still + * written, and renders verbatim. ⛔ Never `''` — that is a present label. + */ +export function newPageNavigationItem(pageName: string, label?: string): NavigationItem { + return { + id: generateNavId('nav_page'), + type: 'page', + ...(label ? { label } : {}), + pageName, + icon: 'FileText', + }; +} + +/** + * The navigation entry written when a dashboard is added to an app's + * navigation — the same rule as {@link newPageNavigationItem}: no author label, + * no `label` key, so the entry shows the dashboard's CURRENT label at render + * time (objectui#9868) instead of a stored copy of `dashboardName`. + */ +export function newDashboardNavigationItem(dashboardName: string, label?: string): NavigationItem { + return { + id: generateNavId('nav_dash'), + type: 'dashboard', + ...(label ? { label } : {}), + dashboardName, + icon: 'LayoutDashboard', + }; +} + /** * Recursively remove all navigation items that match a given type and name. * Returns a new array (immutable). @@ -237,13 +274,7 @@ export function useNavigationSync(): UseNavigationSyncReturn { if (!app) return; const prev = app.navigation ?? []; - const newItem: NavigationItem = { - id: generateNavId('nav_page'), - type: 'page', - label: label || pageName, - pageName, - icon: 'FileText', - }; + const newItem = newPageNavigationItem(pageName, label); const updated = addNavigationItem(prev, newItem); const updatedApp: AppComponentSchema = { ...app, navigation: updated }; @@ -275,13 +306,7 @@ export function useNavigationSync(): UseNavigationSyncReturn { if (!app) return; const prev = app.navigation ?? []; - const newItem: NavigationItem = { - id: generateNavId('nav_dash'), - type: 'dashboard', - label: label || dashboardName, - dashboardName, - icon: 'LayoutDashboard', - }; + const newItem = newDashboardNavigationItem(dashboardName, label); const updated = addNavigationItem(prev, newItem); const updatedApp: AppComponentSchema = { ...app, navigation: updated }; diff --git a/packages/app-shell/src/layout/UnifiedSidebar.tsx b/packages/app-shell/src/layout/UnifiedSidebar.tsx index 57360c3eea..87cbc4b436 100644 --- a/packages/app-shell/src/layout/UnifiedSidebar.tsx +++ b/packages/app-shell/src/layout/UnifiedSidebar.tsx @@ -48,6 +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 { useNavTargetLabel } from '../hooks/useNavTargetLabel.js'; import { matchAppBySegment, appRouteSegment, resolveKeyedI18nLabel } from '../utils/index.js'; import { useHomePath } from '../hooks/useHomePath.js'; // Aliased for symmetry with objectui's own `resolveKeyedI18nLabel` above (the @@ -168,6 +169,10 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { // resolves to the fully-wired console runner — confirm/param/result dialogs // included — with no provider of its own. const dispatchNavAction = useNavActionDispatch(); + // An entry with no `label` shows its target's CURRENT label, read from the + // metadata cache at render time (objectui#9868). `nav:menu` wires the same + // hook, so the two surfaces cannot name one entry two ways. + const resolveNavTargetLabel = useNavTargetLabel(); // Swipe-from-left-edge gesture to open sidebar on mobile React.useEffect(() => { @@ -568,6 +573,7 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { resolveObjectLabel={(objectName, fallback) => resolveNavObjectLabel({ name: objectName, label: fallback })} resolveDashboardLabel={(dashboardName, fallback) => resolveNavDashboardLabel({ name: dashboardName, label: fallback })} resolveViewLabel={(objectName, viewName, fallback) => resolveNavViewLabel(objectName, viewName, fallback)} + resolveTargetLabel={resolveNavTargetLabel} onAction={dispatchNavAction} t={t} templateContext={{ currentUserId: user?.id ?? null, currentOrgId: activeOrganization?.id ?? null, contextValues }} @@ -694,6 +700,7 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { resolveObjectLabel={(objectName, fallback) => resolveNavObjectLabel({ name: objectName, label: fallback })} resolveDashboardLabel={(dashboardName, fallback) => resolveNavDashboardLabel({ name: dashboardName, label: fallback })} resolveViewLabel={(objectName, viewName, fallback) => resolveNavViewLabel(objectName, viewName, fallback)} + resolveTargetLabel={resolveNavTargetLabel} onAction={dispatchNavAction} t={t} templateContext={{ currentUserId: user?.id ?? null, currentOrgId: activeOrganization?.id ?? null, contextValues }} diff --git a/packages/app-shell/src/layout/__tests__/UnifiedSidebar.navLabelInheritsTarget-9868.test.tsx b/packages/app-shell/src/layout/__tests__/UnifiedSidebar.navLabelInheritsTarget-9868.test.tsx new file mode 100644 index 0000000000..d704224a68 --- /dev/null +++ b/packages/app-shell/src/layout/__tests__/UnifiedSidebar.navLabelInheritsTarget-9868.test.tsx @@ -0,0 +1,144 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * UnifiedSidebar — an app navigation entry with NO `label` shows its target's + * CURRENT label from the metadata cache (objectui#9868, the cloud#2021 + * letter-A ruling). + * + * This pins the WIRING in the real sidebar: `UnifiedSidebar` hands + * `NavigationRenderer` the `useNavTargetLabel` resolver, which reads the + * object schemas `useMetadata()` publishes. The rename case re-renders the same + * navigation over a new metadata value and expects the new name — nothing is + * stored on the nav entry. + * + * Harness: the provider / chrome mocks of + * `UnifiedSidebar.derivedAreaVisibility.test.tsx`; `@object-ui/layout` and + * `@object-ui/components` stay REAL. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import type { NavigationItem } from '@object-ui/types'; + +vi.mock('@object-ui/i18n', async (importOriginal) => ({ + ...(await importOriginal>()), + useObjectTranslation: () => ({ + t: (key: string, options?: Record) => String(options?.defaultValue ?? key), + language: 'en', + }), + // No translation bundle: every convention lookup answers its fallback. + useObjectLabel: () => ({ + objectLabel: ({ label }: { label?: string }) => label, + viewLabel: (_o: string, _v: string, fallback?: string) => fallback, + dashboardLabel: ({ label }: { label?: string }) => label, + appLabel: ({ label }: { label?: string }) => label, + }), +})); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: null, activeOrganization: null }), + useWorkspaceAdminStatus: () => ({ isAdmin: false, isResolved: true }), +})); + +vi.mock('@object-ui/permissions', async (importOriginal) => ({ + ...(await importOriginal()), + usePermissions: () => ({ can: () => true, hasCapabilities: () => true }), +})); + +let metadataState: { apps: unknown[]; objects: unknown[] }; +vi.mock('../../providers/MetadataProvider', () => ({ + useMetadata: () => metadataState, +})); + +vi.mock('../../providers/ExpressionProvider', () => ({ + useExpressionContext: () => ({ evaluator: null }), + 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/getIcon', () => ({ getIcon: () => () => null })); +vi.mock('../../hooks/useRecentItems', () => ({ useRecentItems: () => ({ recentItems: [] }) })); +vi.mock('../../hooks/useFavorites', () => ({ + useFavorites: () => ({ favorites: [], removeFavorite: vi.fn() }), +})); +vi.mock('../../hooks/useNavPins', () => ({ + useNavPins: () => ({ togglePin: vi.fn(), applyPins: (items: unknown) => items }), +})); +vi.mock('../../hooks/useNavActionDispatch', () => ({ + useNavActionDispatch: () => vi.fn(), +})); +vi.mock('../../context/NavigationContext', () => ({ + useNavigationContext: () => ({ context: 'app', currentAppName: 'crm' }), +})); +vi.mock('../ContextSelectors', () => ({ + useAppContextSelectors: () => ({ contextValues: {}, element: null }), + contextSelectorQueryKey: (id: string) => (id === 'active_package' ? 'package' : id), + STUDIO_PACKAGE_SELECTOR_ID: 'active_package', +})); +vi.mock('../LocalizedSidebarTrigger', () => ({ + LocalizedSidebarTrigger: () => null, +})); + +import { SidebarProvider } from '@object-ui/components'; +import { UnifiedSidebar } from '../UnifiedSidebar'; + +const navigation: NavigationItem[] = [ + { id: 'nav_board', type: 'object', objectName: 'customer', viewName: 'board' }, + { id: 'nav_accounts', type: 'object', objectName: 'account', label: 'account' }, +]; + +function sidebarUi(viewLabel: string) { + metadataState = { + apps: [{ name: 'crm', label: 'CRM', active: true, navigation }], + objects: [ + { + name: 'customer', + label: 'Customers', + listViews: { 'customer.board': { name: 'customer.board', label: viewLabel } }, + }, + { name: 'account', label: 'Accounts' }, + ], + }; + return ( + + + + + + ); +} + +beforeEach(() => { + localStorage.clear(); +}); + +describe('UnifiedSidebar — an unlabelled entry inherits its target’s label (objectui#9868)', () => { + it('shows the view’s label, and the renamed one on the next render', () => { + const { rerender } = render(sidebarUi('客户管理仪表盘')); + expect(screen.getByRole('link', { name: '客户管理仪表盘' })).toHaveAttribute( + 'href', + '/apps/crm/customer/view/board', + ); + + rerender(sidebarUi('Customer Board')); + expect(screen.getByRole('link', { name: 'Customer Board' })).toBeInTheDocument(); + expect(screen.queryByText('客户管理仪表盘')).not.toBeInTheDocument(); + expect(navigation[0]).not.toHaveProperty('label'); + }); + + it('control: an authored label equal to the machine name stays verbatim', () => { + render(sidebarUi('Board')); + expect(screen.getByRole('link', { name: 'account' })).toHaveAttribute('href', '/apps/crm/account'); + expect(screen.queryByText('Accounts')).not.toBeInTheDocument(); + }); +}); diff --git a/packages/app-shell/src/views/__tests__/nav-label-inherits-target-9868.render.test.tsx b/packages/app-shell/src/views/__tests__/nav-label-inherits-target-9868.render.test.tsx new file mode 100644 index 0000000000..dbc38be940 --- /dev/null +++ b/packages/app-shell/src/views/__tests__/nav-label-inherits-target-9868.render.test.tsx @@ -0,0 +1,167 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#9868 — an app navigation entry with NO `label` shows its target's + * CURRENT label, on both surfaces that render an app's navigation, from the + * metadata the shell already holds (the cloud#2021 letter-A ruling; + * `@objectstack/spec` 17.5.0 made `label` optional with that semantic). + * + * The two surfaces are the sidebar's `NavigationRenderer` and the `nav:menu` + * page block. Both are wired here exactly as the shell wires them — the same + * `useNavTargetLabel` hook reading the same `MetadataCtx` — and every case + * asserts both, so "the two surfaces agree" is a measurement, not a docstring. + * + * "Rename" is a new metadata value in the context (a new view `label` in the + * `listViews` `MetadataProvider` merges into the object) with the SAME app and + * the SAME navigation: nothing is written to the nav entry, and the new name + * shows on the next render. + */ +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect } from 'vitest'; +import { render, screen, within } from '@testing-library/react'; +import React from 'react'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; + +// Module scope, never a `beforeAll` (AGENTS.md §测试纪律, objectui#3010). +import '@object-ui/components'; +import { SidebarProvider } from '@object-ui/components'; +import { SchemaRenderer, MetadataCtx } from '@object-ui/react'; +import { NavigationRenderer } from '@object-ui/layout'; +import '../nav-menu-renderer'; +import { useNavTargetLabel } from '../../hooks/useNavTargetLabel'; + +const NAVIGATION = [ + // The subject: no label, names a view. + { id: 'nav_board', type: 'object', objectName: 'customer', viewName: 'board' }, + // No label, names an object whose schema is labelled. + { id: 'nav_orders', type: 'object', objectName: 'order' }, + // No label, names a dashboard. + { id: 'nav_dash', type: 'dashboard', dashboardName: 'sales_overview' }, + // No label, names an object with NO label at all → machine-name backstop. + { id: 'nav_bare', type: 'object', objectName: 'bare_thing' }, + // Controls: authored labels, one of them spelled like the machine name. + { id: 'nav_accounts', type: 'object', objectName: 'account', label: 'account' }, + { id: 'nav_pipeline', type: 'object', objectName: 'customer', viewName: 'board', label: 'Pipeline' }, +]; + +/** The metadata cache as `MetadataProvider` publishes it: views merged into `listViews`, keyed by `.`. */ +function metadata(viewLabel: string) { + return { + apps: [{ name: 'crm', label: 'CRM', navigation: NAVIGATION }], + objects: [ + { + name: 'customer', + label: 'Customers', + listViews: { 'customer.board': { name: 'customer.board', label: viewLabel, type: 'kanban' } }, + }, + { name: 'order', label: { en: 'Orders', 'zh-CN': '订单' } }, + { name: 'account', label: 'Accounts' }, + { name: 'bare_thing' }, + ], + dashboards: [{ name: 'sales_overview', label: 'Sales Overview' }], + reports: [], + pages: [], + loading: false, + error: null, + refresh: async () => {}, + invalidate: () => {}, + ensureType: async () => [], + getItem: async () => null, + getItemsByType: () => [], + getTypeStatus: () => 'ready' as const, + }; +} + +/** The sidebar surface, wired the way `UnifiedSidebar` wires it. */ +function SidebarSurface() { + const resolveTargetLabel = useNavTargetLabel(); + return ( + + + + ); +} + +function tree(value: ReturnType) { + return ( + + + + + + + + } + /> + + + + ); +} + +const surfaces = () => [ + screen.getByRole('navigation', { name: 'Sidebar' }), + screen.getByRole('navigation', { name: 'App navigation' }), +]; + +describe('objectui#9868 — an unlabelled nav entry shows its target’s current label, on both surfaces', () => { + it('shows the view’s label, then the renamed one on the next render — nav unchanged, nothing stored', () => { + const utils = render(tree(metadata('客户管理仪表盘'))); + + for (const surface of surfaces()) { + const link = within(surface).getByRole('link', { name: '客户管理仪表盘' }); + expect(link).toHaveAttribute('href', '/apps/crm/customer/view/board'); + } + + utils.rerender(tree(metadata('Customer Board'))); + + for (const surface of surfaces()) { + expect(within(surface).getByRole('link', { name: 'Customer Board' })).toHaveAttribute( + 'href', + '/apps/crm/customer/view/board', + ); + expect(within(surface).queryByText('客户管理仪表盘')).toBeNull(); + } + // Nothing was written back into the navigation. + expect(NAVIGATION[0]).not.toHaveProperty('label'); + }); + + it('inherits the object’s and the dashboard’s label (an inline locale map resolves in the active language)', () => { + render(tree(metadata('Board'))); + for (const surface of surfaces()) { + expect(within(surface).getByRole('link', { name: 'Orders' })).toHaveAttribute('href', '/apps/crm/order'); + expect(within(surface).getByRole('link', { name: 'Sales Overview' })).toBeInTheDocument(); + } + }); + + it('renders the machine name when the target has no label at all', () => { + render(tree(metadata('Board'))); + for (const surface of surfaces()) { + expect(within(surface).getByRole('link', { name: 'bare_thing' })).toHaveAttribute('href', '/apps/crm/bare_thing'); + } + }); + + it('control: an authored label renders verbatim, even one equal to the machine name', () => { + render(tree(metadata('Board'))); + for (const surface of surfaces()) { + // 'account' is NOT replaced by the object's 'Accounts' (route B refused)… + expect(within(surface).getByRole('link', { name: 'account' })).toHaveAttribute('href', '/apps/crm/account'); + expect(within(surface).queryByText('Accounts')).toBeNull(); + // …and 'Pipeline' is NOT replaced by the view's 'Board' it also names. + expect(within(surface).getByRole('link', { name: 'Pipeline' })).toBeInTheDocument(); + } + }); +}); diff --git a/packages/app-shell/src/views/nav-menu-renderer.tsx b/packages/app-shell/src/views/nav-menu-renderer.tsx index 8e8c8b36ed..86b836bc84 100644 --- a/packages/app-shell/src/views/nav-menu-renderer.tsx +++ b/packages/app-shell/src/views/nav-menu-renderer.tsx @@ -34,8 +34,10 @@ * nav → URL, so `recordId` / `filters` / `viewName` / `runAction` * precedence cannot drift between the sidebar and an authored menu; * - labels from `resolveNavItemLabel` — the same convention-based object / - * view / dashboard i18n resolution the sidebar gets, so the two surfaces - * cannot show one entry under two names; + * view / dashboard i18n resolution the sidebar gets, and for an entry with + * NO `label` the same inherited target label (`useNavTargetLabel`, + * objectui#9868), so the two surfaces cannot show one entry under two + * names; * - the active row from `resolveActiveNavItem`, the round-trip inverse of * `resolveHref`; * - the item-level guards in the same ORDER `NavigationItemRenderer` applies @@ -123,6 +125,7 @@ import { import type { NavigationItem } from '@object-ui/types'; import { useExpressionContext, evaluateVisibility } from '../providers/ExpressionProvider.js'; import { useNavActionDispatch } from '../hooks/useNavActionDispatch.js'; +import { useNavTargetLabel } from '../hooks/useNavTargetLabel.js'; import { useNavigationContext } from '../context/NavigationContext.js'; import { getIcon } from '../utils/getIcon.js'; import { appRouteSegment, matchAppBySegment } from '../utils/index.js'; @@ -157,6 +160,9 @@ export const NavMenuRenderer: React.FC = ({ const { pathname, search } = useLocation(); const { user, activeOrganization } = useAuth(); const dispatchNavAction = useNavActionDispatch(); + // The same resolver `UnifiedSidebar` hands `NavigationRenderer`: an entry with + // no `label` shows its target's current label (objectui#9868). + const targetLabel = useNavTargetLabel(); /* ── The three guards, wired to the same providers `UnifiedSidebar` uses ── */ @@ -250,8 +256,9 @@ export const NavMenuRenderer: React.FC = ({ t, (dashboardName, fallback) => dashboardLabel({ name: dashboardName, label: fallback }), (objectName, viewName, fallback) => viewLabel(objectName, viewName, fallback), + targetLabel, ), - [objectLabel, dashboardLabel, viewLabel, t], + [objectLabel, dashboardLabel, viewLabel, t, targetLabel], ); // A plain (hoisted) function declaration, not a `useCallback`: it recurses diff --git a/packages/layout/src/AppSchemaRenderer.tsx b/packages/layout/src/AppSchemaRenderer.tsx index f27c662530..50a6f97934 100644 --- a/packages/layout/src/AppSchemaRenderer.tsx +++ b/packages/layout/src/AppSchemaRenderer.tsx @@ -47,6 +47,7 @@ import { hasVisibleNavigationItems, resolveIcon, resolveLabel, + resolveNavItemLabel, type VisibilityEvaluator, type PermissionChecker, type CapabilityChecker, @@ -331,7 +332,10 @@ function MobileBottomNav({ }`} > - {resolveLabel(item.label)} + {/* `resolveNavItemLabel`, not a raw `item.label` read: an entry's label may be + absent since objectui#9868, and this host has no metadata, so it shows the + same machine-name backstop its `NavigationRenderer` does. */} + {resolveNavItemLabel(item)} ); })} diff --git a/packages/layout/src/NavigationRenderer.tsx b/packages/layout/src/NavigationRenderer.tsx index a3a59d3eb8..4cdbe971da 100644 --- a/packages/layout/src/NavigationRenderer.tsx +++ b/packages/layout/src/NavigationRenderer.tsx @@ -103,6 +103,34 @@ export type PermissionChecker = (permissions: string[]) => boolean; */ export type CapabilityChecker = (kind: 'object' | 'service', name: string) => boolean; +/** + * What an UNLABELLED navigation entry inherits its text from (objectui#9868). + * + * `@objectstack/spec` 17.5.0 made a nav entry's `label` optional with a + * declared semantic: absent ⇒ the entry shows, at RENDER time, the CURRENT label + * of what it opens — the view's label when it names a view and that view is + * labelled, else the object's / dashboard's label. These are the three targets + * that sentence names; `resolveNavItemLabel` walks them in that order and asks a + * {@link NavTargetLabelResolver} about each one. + */ +export type NavLabelTarget = + | { kind: 'view'; objectName: string; viewName: string } + | { kind: 'object'; objectName: string } + | { kind: 'dashboard'; dashboardName: string }; + +/** + * Answers the CURRENT display label of a nav target from the host's metadata — + * the object schema's `label`, the named view's `label`, the dashboard's + * `label` — or `undefined` when the target carries none (or is not loaded). + * + * This layer holds no metadata, so the host supplies it: the console shell + * reads the metadata cache it already loads (`useNavTargetLabel` in + * `@object-ui/app-shell`). Because it is asked on every render and nothing is + * stored, a renamed target shows its new name on the next render. Without it, + * an unlabelled entry falls through to its target's machine name. + */ +export type NavTargetLabelResolver = (target: NavLabelTarget) => string | undefined; + export interface NavigationRendererProps { /** Navigation items to render */ items: NavigationItem[]; @@ -197,6 +225,16 @@ export interface NavigationRendererProps { */ resolveViewLabel?: (objectName: string, viewName: string, fallbackLabel: string) => string; + /** + * Resolver for the text an entry with NO `label` inherits (objectui#9868): + * the current label of its view / object / dashboard, read from the host's + * metadata at render time. Consulted ONLY for an absent `label` — an authored + * label is never replaced by the target's metadata label, not even one + * spelled like the target's machine name (the ruling refused that sentinel). + * See {@link NavTargetLabelResolver} and {@link resolveNavItemLabel}. + */ + resolveTargetLabel?: NavTargetLabelResolver; + // RETIRED (`9c60144b5`): `resolveGroupLabel` / `resolveItemLabel`, the two // id-keyed label resolvers. They were unreachable by construction — see the // note on `resolveNavItemLabel` below — and app-navigation localization is @@ -300,6 +338,13 @@ export function resolveLabel( * under two names on two surfaces of one app, which is exactly the drift the * "single source of truth" note on `resolveHref` exists to prevent. Nothing * about the behaviour changed with the keyword. + * + * ABSENT `label` (objectui#9868 — `@objectstack/spec` 17.5.0 made it optional, + * cloud#2021 letter-A ruling): the entry inherits its target's CURRENT label, + * resolved here at render time and never written back — see + * {@link inheritedNavItemLabel} for the ladder. That arm is keyed on ABSENCE + * alone: the steps above never consult `targetLabel`, so an authored label is + * never swapped for the target's metadata label, however it is spelled. */ export function resolveNavItemLabel( item: NavigationItem, @@ -307,9 +352,13 @@ export function resolveNavItemLabel( t?: (key: string, options?: any) => string, dashboardResolver?: (dashboardName: string, fallbackLabel: string) => string, viewResolver?: (objectName: string, viewName: string, fallbackLabel: string) => string, + targetLabel?: NavTargetLabelResolver, ): string { // A separator carries no `label` (objectui#10867): there is nothing to name. if (item.type === 'separator') return ''; + // Absent ⇒ inherit (objectui#9868). Resolved BEFORE `resolveLabel`, which + // takes only a present label. + if (item.label === undefined) return inheritedNavItemLabel(item, targetLabel); const base = resolveLabel(item.label, t); // Only apply convention-based resolution for items with plain string labels. // I18nLabel objects (with explicit key/defaultValue) already have their own translation keys. @@ -344,6 +393,72 @@ export function resolveNavItemLabel( return base; } +/** + * The text of a navigation ENTRY whose `label` is absent (objectui#9868). + * + * The spec's declared default, walked in the spec's order and resolved on every + * render (nothing is stored, so a rename shows on the next render): + * + * - `object` naming a view → the VIEW's label, else the OBJECT's label, else + * the `viewName` it names; + * - `object` → the object's label, else `objectName`; + * - `dashboard` → the dashboard's label, else `dashboardName`. + * + * The labels come from `targetLabel` — the host's metadata; this layer holds + * none. The final rung is the MACHINE-NAME backstop: an entry always shows text + * (the spec's own rule: identity is the target, text is inherited), including + * before the host's metadata has loaded and in a host that supplies no + * resolver. + * + * The ruling names inheritance for those three targets only. Every other entry + * type shows its target's machine name — `pageName`, `reportName`, `url`, + * `componentRef`, `actionDef.actionName` — and an entry with no target of its + * own (a `group`), or with its target missing, shows its `id`, which the + * validator requires of every entry. + */ +function inheritedNavItemLabel( + item: Exclude, + targetLabel: NavTargetLabelResolver | undefined, +): string { + const ask = (target: NavLabelTarget): string | undefined => { + const text = targetLabel?.(target); + return typeof text === 'string' && text.trim() !== '' ? text : undefined; + }; + switch (item.type) { + case 'object': + if (!item.objectName) break; + if (item.viewName) { + return ( + ask({ kind: 'view', objectName: item.objectName, viewName: item.viewName }) + ?? ask({ kind: 'object', objectName: item.objectName }) + ?? item.viewName + ); + } + return ask({ kind: 'object', objectName: item.objectName }) ?? item.objectName; + case 'dashboard': + if (!item.dashboardName) break; + return ask({ kind: 'dashboard', dashboardName: item.dashboardName }) ?? item.dashboardName; + case 'page': + if (item.pageName) return item.pageName; + break; + case 'report': + if (item.reportName) return item.reportName; + break; + case 'url': + if (item.url) return item.url; + break; + case 'component': + if (item.componentRef) return item.componentRef; + break; + case 'action': + if (item.actionDef?.actionName) return item.actionDef.actionName; + break; + default: + break; + } + return item.id; +} + // --------------------------------------------------------------------------- // Default evaluators (always-visible, always-permitted) // --------------------------------------------------------------------------- @@ -879,10 +994,16 @@ const ActiveNavIdContext = React.createContext(null); /** * Recursively filter navigation items by search query (case-insensitive label match). * Groups are kept if any child matches, with non-matching children pruned. + * + * Matches the text the row SHOWS: `labelOf` defaults to + * {@link resolveNavItemLabel} with no resolvers, and `NavigationRenderer` passes + * its own resolvers so an unlabelled entry is found by the label it inherits + * (objectui#9868), not by a `label` it does not have. */ export function filterNavigationItems( items: NavigationItem[], query: string, + labelOf: (item: NavigationItem) => string = (item) => resolveNavItemLabel(item), ): NavigationItem[] { if (!query.trim()) return items; const lowerQuery = query.toLowerCase().trim(); @@ -893,15 +1014,15 @@ export function filterNavigationItems( // Groups: recursively filter children if (item.type === 'group' && item.children?.length) { - const filteredChildren = filterNavigationItems(item.children, query); + const filteredChildren = filterNavigationItems(item.children, query, labelOf); if (filteredChildren.length > 0) { acc.push({ ...item, children: filteredChildren }); } return acc; } - // Leaf items: match label - if (resolveLabel(item.label).toLowerCase().includes(lowerQuery)) { + // Leaf items: match the label the row shows + if (labelOf(item).toLowerCase().includes(lowerQuery)) { acc.push(item); } return acc; @@ -928,6 +1049,7 @@ function SortableNavigationItem({ resolveObjectLabel, resolveDashboardLabel, resolveViewLabel, + resolveTargetLabel, t: tProp, templateContext, }: { @@ -943,6 +1065,7 @@ function SortableNavigationItem({ resolveObjectLabel?: (objectName: string, fallbackLabel: string) => string; resolveDashboardLabel?: (dashboardName: string, fallbackLabel: string) => string; resolveViewLabel?: (objectName: string, viewName: string, fallbackLabel: string) => string; + resolveTargetLabel?: NavTargetLabelResolver; t?: (key: string, options?: any) => string; templateContext?: NavTemplateContext; }) { @@ -977,6 +1100,7 @@ function SortableNavigationItem({ resolveObjectLabel={resolveObjectLabel} resolveDashboardLabel={resolveDashboardLabel} resolveViewLabel={resolveViewLabel} + resolveTargetLabel={resolveTargetLabel} t={tProp} templateContext={templateContext} /> @@ -1001,6 +1125,7 @@ function NavigationItemRenderer({ resolveObjectLabel, resolveDashboardLabel, resolveViewLabel, + resolveTargetLabel, t: tProp, templateContext, }: { @@ -1016,6 +1141,7 @@ function NavigationItemRenderer({ resolveObjectLabel?: (objectName: string, fallbackLabel: string) => string; resolveDashboardLabel?: (dashboardName: string, fallbackLabel: string) => string; resolveViewLabel?: (objectName: string, viewName: string, fallbackLabel: string) => string; + resolveTargetLabel?: NavTargetLabelResolver; t?: (key: string, options?: any) => string; templateContext?: NavTemplateContext; }) { @@ -1095,7 +1221,7 @@ function NavigationItemRenderer({ // does for area election. if (!hasVisibleNavigationItems(children, guardOptions)) return null; - const groupLabel = resolveNavItemLabel(item, resolveObjectLabel, tProp, resolveDashboardLabel, resolveViewLabel); + const groupLabel = resolveNavItemLabel(item, resolveObjectLabel, tProp, resolveDashboardLabel, resolveViewLabel, resolveTargetLabel); return ( @@ -1125,6 +1251,7 @@ function NavigationItemRenderer({ resolveObjectLabel={resolveObjectLabel} resolveDashboardLabel={resolveDashboardLabel} resolveViewLabel={resolveViewLabel} + resolveTargetLabel={resolveTargetLabel} t={tProp} templateContext={templateContext} /> @@ -1147,7 +1274,10 @@ function NavigationItemRenderer({ // three releases led nowhere. if (!onAction) return null; const Icon = resolveIcon(item.icon); - const actionLabel = resolveLabel(item.label, tProp); + // Through `resolveNavItemLabel`, not `resolveLabel`: an action entry's + // `label` may be absent too (objectui#9868), and that is where the absent + // arm lives. For a present label the two answer identically on this type. + const actionLabel = resolveNavItemLabel(item, resolveObjectLabel, tProp, resolveDashboardLabel, resolveViewLabel, resolveTargetLabel); return ( {dragListeners && ( @@ -1201,7 +1331,7 @@ function NavigationItemRenderer({ const Icon = resolveIcon(item.icon); const { href, external } = resolveHref(item, basePath, templateContext); const isActive = activeNavId !== null && item.id === activeNavId; - const itemLabel = resolveNavItemLabel(item, resolveObjectLabel, tProp, resolveDashboardLabel, resolveViewLabel); + const itemLabel = resolveNavItemLabel(item, resolveObjectLabel, tProp, resolveDashboardLabel, resolveViewLabel, resolveTargetLabel); const content = ( <> @@ -1310,6 +1440,7 @@ export function NavigationRenderer({ resolveObjectLabel, resolveDashboardLabel, resolveViewLabel, + resolveTargetLabel, t: tProp, templateContext, }: NavigationRendererProps) { @@ -1324,10 +1455,16 @@ export function NavigationRenderer({ [items, location.pathname, location.search, basePath, templateContext], ); - // --- Search filtering --- + // --- Search filtering --- against the label each row SHOWS, so an + // unlabelled entry is found by the text it inherits (objectui#9868). const filteredItems = useMemo( - () => (searchQuery ? filterNavigationItems(items, searchQuery) : items), - [items, searchQuery], + () => + searchQuery + ? filterNavigationItems(items, searchQuery, (item) => + resolveNavItemLabel(item, resolveObjectLabel, tProp, resolveDashboardLabel, resolveViewLabel, resolveTargetLabel), + ) + : items, + [items, searchQuery, resolveObjectLabel, tProp, resolveDashboardLabel, resolveViewLabel, resolveTargetLabel], ); // --- Pinned items (favorites section) --- @@ -1376,6 +1513,7 @@ export function NavigationRenderer({ resolveObjectLabel, resolveDashboardLabel, resolveViewLabel, + resolveTargetLabel, t: tProp, templateContext, }; diff --git a/packages/layout/src/__tests__/NavigationRenderer.labelInheritsTarget-9868.test.tsx b/packages/layout/src/__tests__/NavigationRenderer.labelInheritsTarget-9868.test.tsx new file mode 100644 index 0000000000..b705c35226 --- /dev/null +++ b/packages/layout/src/__tests__/NavigationRenderer.labelInheritsTarget-9868.test.tsx @@ -0,0 +1,213 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#9868 — a navigation entry with NO `label` shows its target's CURRENT + * label, resolved at render time (`@objectstack/spec` 17.5.0; the cloud#2021 + * letter-A ruling). + * + * The ladder is the spec's own sentence, walked in its order: the view's label + * when the entry names a labelled view, else the object's / dashboard's label, + * else the target's machine name. This layer holds no metadata — the host + * answers each rung through `resolveTargetLabel` — so these pins drive the + * resolver directly and assert what the renderer does with each answer. + * + * The controls are the point as much as the subject: + * - an AUTHORED label is never swapped for the target's label — including one + * spelled exactly like the machine name (the refused "sentinel" route B), + * and the target resolver is not even consulted for it; + * - "rename" is a new resolver answer on the SAME nav tree, with nothing + * written anywhere, and the new text shows. + */ + +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { render, screen, within } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import type { NavigationItem } from '@object-ui/types'; +import { SidebarProvider } from '@object-ui/components'; +import { + NavigationRenderer, + resolveNavItemLabel, + filterNavigationItems, + type NavLabelTarget, + type NavTargetLabelResolver, +} from '../NavigationRenderer'; + +/** A metadata world: target labels by kind, as a host's cache would answer them. */ +function world(labels: { + views?: Record; + objects?: Record; + dashboards?: Record; +}): NavTargetLabelResolver { + return (target: NavLabelTarget) => { + switch (target.kind) { + case 'view': + return labels.views?.[`${target.objectName}.${target.viewName}`]; + case 'object': + return labels.objects?.[target.objectName]; + case 'dashboard': + return labels.dashboards?.[target.dashboardName]; + } + }; +} + +const VIEW_ENTRY: NavigationItem = { id: 'nav_board', type: 'object', objectName: 'customer', viewName: 'board' }; +const LABELLED = world({ + views: { 'customer.board': '客户管理仪表盘' }, + objects: { customer: 'Customers', account: 'Accounts' }, + dashboards: { sales_overview: 'Sales Overview' }, +}); + +const label = (item: NavigationItem, target?: NavTargetLabelResolver) => + resolveNavItemLabel(item, undefined, undefined, undefined, undefined, target); + +describe('objectui#9868 — resolveNavItemLabel: an absent label inherits its target', () => { + it('a view entry shows the VIEW’s label, and after a rename the new one — same nav tree, nothing stored', () => { + expect(label(VIEW_ENTRY, LABELLED)).toBe('客户管理仪表盘'); + + // Rename = a new answer from the host's metadata; the nav entry is the + // very same object and carries no label before or after. + const renamed = world({ views: { 'customer.board': 'Customer Board' }, objects: { customer: 'Customers' } }); + expect(label(VIEW_ENTRY, renamed)).toBe('Customer Board'); + expect(VIEW_ENTRY).not.toHaveProperty('label'); + }); + + it('walks the spec’s ladder: unlabelled view → the object’s label → the viewName', () => { + expect(label(VIEW_ENTRY, world({ objects: { customer: 'Customers' } }))).toBe('Customers'); + expect(label(VIEW_ENTRY, world({}))).toBe('board'); + }); + + it('an object entry shows the object’s label, else its objectName', () => { + const entry: NavigationItem = { id: 'nav_acc', type: 'object', objectName: 'account' }; + expect(label(entry, LABELLED)).toBe('Accounts'); + expect(label(entry, world({}))).toBe('account'); + }); + + it('a dashboard entry shows the dashboard’s label, else its dashboardName', () => { + const entry: NavigationItem = { id: 'nav_dash', type: 'dashboard', dashboardName: 'sales_overview' }; + expect(label(entry, LABELLED)).toBe('Sales Overview'); + expect(label(entry, world({}))).toBe('sales_overview'); + }); + + it('a blank answer is no label: the ladder moves on rather than showing empty text', () => { + const blank: NavTargetLabelResolver = (t) => (t.kind === 'view' ? ' ' : t.kind === 'object' ? '' : undefined); + expect(label(VIEW_ENTRY, blank)).toBe('board'); + }); + + it('with NO resolver at all (a host without metadata) the machine name renders', () => { + expect(label(VIEW_ENTRY)).toBe('board'); + expect(label({ id: 'nav_acc', type: 'object', objectName: 'account' })).toBe('account'); + }); + + it.each<[string, NavigationItem, string]>([ + ['page', { id: 'nav_page', type: 'page', pageName: 'home' }, 'home'], + ['report', { id: 'nav_rep', type: 'report', reportName: 'win_rate' }, 'win_rate'], + ['url', { id: 'nav_url', type: 'url', url: 'https://example.com/handbook' }, 'https://example.com/handbook'], + ['component', { id: 'nav_cmp', type: 'component', componentRef: 'metadata:directory' }, 'metadata:directory'], + ['action', { id: 'nav_act', type: 'action', actionDef: { actionName: 'sync_now' } }, 'sync_now'], + ['group', { id: 'grp_sales', type: 'group', children: [] }, 'grp_sales'], + ['object with no objectName', { id: 'nav_broken', type: 'object' }, 'nav_broken'], + ])('%s: not an inheriting target — shows its machine name / id, and the resolver is never asked', (_n, item, want) => { + const spy = vi.fn(LABELLED); + expect(label(item, spy)).toBe(want); + expect(spy).not.toHaveBeenCalled(); + }); +}); + +describe('objectui#9868 — the controls: a PRESENT label is never replaced by the target’s', () => { + it('an explicit label renders verbatim although the target’s label differs, and the resolver is not asked', () => { + const spy = vi.fn(LABELLED); + expect(label({ ...VIEW_ENTRY, label: 'Board' }, spy)).toBe('Board'); + expect(spy).not.toHaveBeenCalled(); + }); + + it('an explicit label EQUAL to the machine name stays verbatim — the sentinel route (B) was refused', () => { + const spy = vi.fn(LABELLED); + // The i18n convention resolvers answer "no translation" (their fallback), + // so the only thing that could change this text is the target resolver. + const echo = (_name: string, fallback: string) => fallback; + const echoView = (_o: string, _v: string, fallback: string) => fallback; + const item: NavigationItem = { id: 'nav_acc', type: 'object', objectName: 'account', label: 'account' }; + expect(resolveNavItemLabel(item, echo, undefined, echo, echoView, spy)).toBe('account'); + expect(resolveNavItemLabel({ ...VIEW_ENTRY, label: 'board' }, echo, undefined, echo, echoView, spy)).toBe('board'); + expect(spy).not.toHaveBeenCalled(); + }); +}); + +function renderNav(items: NavigationItem[], props: Record = {}) { + const tree = (p: Record) => ( + + + + + + ); + const utils = render(tree(props)); + return { ...utils, rerenderWith: (p: Record) => utils.rerender(tree(p)) }; +} + +describe('objectui#9868 — NavigationRenderer renders the inherited label', () => { + const items: NavigationItem[] = [ + VIEW_ENTRY, + { id: 'nav_acc', type: 'object', objectName: 'account', label: 'account' }, + { + id: 'grp_reports', + type: 'group', + children: [{ id: 'nav_dash', type: 'dashboard', dashboardName: 'sales_overview' }], + }, + ]; + + it('shows the target’s label, follows a rename on re-render, and keeps the authored one verbatim', () => { + const { rerenderWith } = renderNav(items, { resolveTargetLabel: LABELLED }); + + const board = screen.getByRole('link', { name: '客户管理仪表盘' }); + expect(board.getAttribute('href')).toBe('/apps/crm/customer/view/board'); + expect(screen.getByRole('link', { name: 'Sales Overview' })).toBeTruthy(); + // Control: the authored machine-name label is NOT replaced by 'Accounts'. + expect(screen.getByRole('link', { name: 'account' })).toBeTruthy(); + expect(screen.queryByText('Accounts')).toBeNull(); + // A label-less group shows its id — the one backstop a group has. + expect(screen.getByText('grp_reports')).toBeTruthy(); + + rerenderWith({ + resolveTargetLabel: world({ + views: { 'customer.board': 'Customer Board' }, + objects: { account: 'Accounts' }, + dashboards: { sales_overview: 'Revenue' }, + }), + }); + expect(screen.getByRole('link', { name: 'Customer Board' }).getAttribute('href')).toBe('/apps/crm/customer/view/board'); + expect(screen.queryByText('客户管理仪表盘')).toBeNull(); + expect(screen.getByRole('link', { name: 'Revenue' })).toBeTruthy(); + expect(screen.getByRole('link', { name: 'account' })).toBeTruthy(); + }); + + it('renders the machine name when no host metadata is wired', () => { + renderNav(items); + expect(screen.getByRole('link', { name: 'board' })).toBeTruthy(); + expect(screen.getByRole('link', { name: 'sales_overview' })).toBeTruthy(); + }); + + it('an unlabelled ACTION entry renders (it used to read `label.key` off undefined) with its action name', () => { + const onAction = vi.fn(); + renderNav([{ id: 'nav_act', type: 'action', actionDef: { actionName: 'sync_now' } }], { onAction }); + expect(screen.getByRole('button', { name: 'sync_now' })).toBeTruthy(); + }); + + it('search finds an unlabelled entry by the label it SHOWS', () => { + renderNav(items, { resolveTargetLabel: LABELLED, searchQuery: '仪表盘' }); + const found = screen.getByRole('link', { name: '客户管理仪表盘' }); + expect(within(found).getByText('客户管理仪表盘')).toBeTruthy(); + expect(screen.queryByRole('link', { name: 'account' })).toBeNull(); + }); + + it('filterNavigationItems without resolvers matches the machine-name backstop, and never throws on an absent label', () => { + expect(filterNavigationItems(items, 'board').map((i) => i.id)).toEqual(['nav_board']); + }); +}); diff --git a/packages/types/src/__tests__/app-wizard-separator-layout-10867.test.ts b/packages/types/src/__tests__/app-wizard-separator-layout-10867.test.ts index 49f8822159..91dca32eb7 100644 --- a/packages/types/src/__tests__/app-wizard-separator-layout-10867.test.ts +++ b/packages/types/src/__tests__/app-wizard-separator-layout-10867.test.ts @@ -76,10 +76,16 @@ describe('objectui#10867 — a separator carries only `type`, `id` and `order`', expect(labelled.type).toBe(bare.type); }); - it('CONTROL — every other item type still requires its `label`', () => { - // @ts-expect-error — an entry without a label. + it('CONTROL — an entry still requires its `id`; its `label` is optional since objectui#9868', () => { + // INVERTED by objectui#9868 — this line carried `@ts-expect-error — an entry + // without a label`. `@objectstack/spec` 17.5.0 made an entry's `label` + // optional (absent ⇒ inherited from its target at render time), so an + // unlabelled entry compiles now, while a labelled SEPARATOR above still + // does not. const unlabelled: NavigationItem = { id: 'account', type: 'object', objectName: 'account' }; - expect(unlabelled.type).toBe('object'); + // @ts-expect-error — an entry without an `id`: identity is still required. + const anonymous: NavigationItem = { type: 'object', objectName: 'account', label: 'Accounts' }; + expect(unlabelled.type).toBe(anonymous.type); }); }); diff --git a/packages/types/src/__tests__/nav-label-optional-9868.test.ts b/packages/types/src/__tests__/nav-label-optional-9868.test.ts new file mode 100644 index 0000000000..40a950ac79 --- /dev/null +++ b/packages/types/src/__tests__/nav-label-optional-9868.test.ts @@ -0,0 +1,114 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#9868 — a navigation entry's `label` is OPTIONAL, matching + * `@objectstack/spec` 17.5.0 (the cloud#2021 letter-A ruling). + * + * The spec's shared nav-item base declares `label: I18nLabelSchema.optional()` + * with the semantic "absent ⇒ inherit the target's CURRENT label at render + * time; present ⇒ verbatim". So this mirror accepts a label-less entry for + * EXACTLY the types the spec accepts one for, and — the half that keeps the + * ruling honest — fills nothing in: a parsed entry carries no `label` key, so + * the renderer (not the validator) answers what it shows. + * + * What did NOT relax, each asserted with its own path: + * - `id` is still required on every entry (identity is the target); + * - a separator is unchanged (a bare one passes, a labelled one is refused); + * - an EMPTY label is refused — `''` is a present label, so it would render + * empty text and opt the entry out of inheritance. + */ + +import { describe, it, expect } from 'vitest'; +import { NavigationItemSchema as SpecNavigationItemSchema } from '@objectstack/spec/ui'; +import { NavigationItemSchema, NavigationItemTypeSchema } from '../zod/app.zod.js'; +import { menuItemToNavigationItem } from '../app.js'; + +/** + * One minimal spec-valid entry per objectui entry type, WITHOUT a label. Keyed + * by the mirror's own type enum, so a type added there with no fixture here + * fails the coverage assertion below instead of going unmeasured. + */ +const UNLABELLED: Record> = { + object: { id: 'nav_obj', type: 'object', objectName: 'account' }, + dashboard: { id: 'nav_dash', type: 'dashboard', dashboardName: 'sales_overview' }, + page: { id: 'nav_page', type: 'page', pageName: 'home' }, + report: { id: 'nav_rep', type: 'report', reportName: 'win_rate' }, + url: { id: 'nav_url', type: 'url', url: 'https://example.com/handbook' }, + component: { id: 'nav_cmp', type: 'component', componentRef: 'metadata:directory' }, + group: { id: 'nav_grp', type: 'group', children: [] }, + action: { id: 'nav_act', type: 'action', actionDef: { actionName: 'sync_now' } }, +}; + +const ENTRY_TYPES = NavigationItemTypeSchema.options.filter((type) => type !== 'separator'); + +function issues(input: unknown) { + const r = NavigationItemSchema.safeParse(input); + return r.success ? [] : r.error.issues.map((i) => ({ path: i.path.join('.'), code: i.code })); +} + +describe('objectui#9868 — `label` is optional on every navigation entry, as in the spec', () => { + it('has an unlabelled fixture for every entry type the mirror declares', () => { + expect(Object.keys(UNLABELLED).sort()).toEqual([...ENTRY_TYPES].sort()); + }); + + it.each(ENTRY_TYPES)('%s: the spec accepts it without a label, and so does the mirror', (type) => { + const entry = UNLABELLED[type]; + // The spec is the contract: measured, not assumed, per type. + expect(SpecNavigationItemSchema.safeParse(entry).success).toBe(true); + // The mirror now agrees. + const r = NavigationItemSchema.safeParse(entry); + expect(r.success).toBe(true); + // ⭐ Nothing is filled in at parse time: absent stays absent, so the + // renderer resolves it on every render instead of reading a stored copy. + expect(r.success && r.data).not.toHaveProperty('label'); + }); + + it.each(ENTRY_TYPES)('%s: a present label is still accepted and kept verbatim (control)', (type) => { + const entry = { ...UNLABELLED[type], label: 'account' }; + expect(SpecNavigationItemSchema.safeParse(entry).success).toBe(true); + const r = NavigationItemSchema.safeParse(entry); + expect(r.success && r.data.label).toBe('account'); + }); + + it('still requires an id — identity is the target, and only the text is inherited', () => { + expect(issues({ type: 'object', objectName: 'account' })).toEqual([{ path: 'id', code: 'custom' }]); + expect(issues({ id: '', type: 'object', objectName: 'account' })).toEqual([{ path: 'id', code: 'custom' }]); + }); + + it('refuses an EMPTY label: it is present, so it would render empty text instead of inheriting', () => { + const r = NavigationItemSchema.safeParse({ id: 'nav_obj', type: 'object', objectName: 'account', label: '' }); + expect(r.success).toBe(false); + expect(issues({ id: 'nav_obj', type: 'object', objectName: 'account', label: '' })).toEqual([ + { path: 'label', code: 'custom' }, + ]); + // The refusal names the fix: omit the key. + expect(!r.success && r.error.issues[0].message).toMatch(/^`label` is empty on a navigation item of type 'object': omit the key/); + }); + + it('leaves the separator unchanged: a bare one passes, a labelled one is refused (objectui#10867)', () => { + expect(NavigationItemSchema.safeParse({ type: 'separator' }).success).toBe(true); + expect(issues({ type: 'separator', label: 'Section' })).toEqual([{ path: 'label', code: 'custom' }]); + }); +}); + +describe('objectui#9868 — `menuItemToNavigationItem` writes no empty label', () => { + it.each([ + ['a path item', { type: 'item' as const, path: '/leads' }], + ['an href item', { type: 'item' as const, href: 'https://example.com' }], + ['a group', { type: 'group' as const, children: [] }], + ])('%s with no label converts to an entry with NO `label` key, which validates', (_name, menuItem) => { + const item = menuItemToNavigationItem(menuItem, 0); + expect(item).not.toHaveProperty('label'); + expect(NavigationItemSchema.safeParse(item).success).toBe(true); + }); + + it('keeps an authored label verbatim (control)', () => { + expect(menuItemToNavigationItem({ type: 'item', label: 'Leads', path: '/leads' }, 0).label).toBe('Leads'); + }); +}); diff --git a/packages/types/src/__tests__/navigation-spec-parity.test.ts b/packages/types/src/__tests__/navigation-spec-parity.test.ts index e25296aae5..77e79138ba 100644 --- a/packages/types/src/__tests__/navigation-spec-parity.test.ts +++ b/packages/types/src/__tests__/navigation-spec-parity.test.ts @@ -91,12 +91,19 @@ describe('NavigationItemSchema keeps the spec vocabulary it used to drop', () => expect(keep({ type: 'separator' })).not.toBeNull(); }); - it('exempts ONLY the separator from needing id + label', () => { + it('exempts ONLY the separator from needing an id — and no entry needs a label since objectui#9868', () => { // The separator exemption is implemented by declaring `id`/`label` - // optional, so without the refinement that re-imposes them this would + // optional, so without the refinement that re-imposes `id` this would // also wave through an unidentifiable destination. expect(keep({ type: 'object', label: 'No id', objectName: 'contact' })).toBeNull(); - expect(keep({ id: 'no_label', type: 'object', objectName: 'contact' })).toBeNull(); + // INVERTED by objectui#9868 (was `toBeNull()`): `@objectstack/spec` 17.5.0 + // made `label` optional on every entry — absent ⇒ inherit the target's + // label at render time — so a label-less entry is valid here too, and it + // parses with NO `label` key (nothing is filled in at parse time). + const unlabelled = keep({ id: 'no_label', type: 'object', objectName: 'contact' }); + expect(unlabelled).not.toBeNull(); + expect(unlabelled).not.toHaveProperty('label'); + expect(SpecNavigationItemSchema.safeParse({ id: 'no_label', type: 'object', objectName: 'contact' }).success).toBe(true); expect(keep({ id: 'ok', type: 'object', label: 'Ok', objectName: 'contact' })).not.toBeNull(); }); diff --git a/packages/types/src/app.ts b/packages/types/src/app.ts index 0b89cbc52c..231d7ee584 100644 --- a/packages/types/src/app.ts +++ b/packages/types/src/app.ts @@ -98,8 +98,22 @@ export interface NavigationEntryItem { /** Navigation item type — any spec nav type except `'separator'`. */ type: Exclude; - /** Display label (plain string per @objectstack/spec v4 protocol) */ - label: string; + /** + * Display label (plain string per @objectstack/spec v4 protocol). + * + * OPTIONAL since `@objectstack/spec` 17.5.0 (the cloud#2021 / objectui#9868 + * letter-A ruling), with the spec's declared semantic: **absent** ⇒ the entry + * inherits, at RENDER time, the current label of what it opens — the view's + * label when it names a labelled view, else the object's / dashboard's label, + * else the target's machine name (`resolveNavItemLabel` in + * `@object-ui/layout`); **present** ⇒ rendered verbatim. Nothing is stored for + * the absent case, so a renamed target shows its new name on the next render. + * + * ⛔ Do not write `''` for "no label": an empty string is PRESENT, so it would + * render empty text instead of inheriting — omit the key. `objectui validate` + * refuses an empty label for that reason. + */ + label?: string; /** Icon name (Lucide) */ icon?: string; @@ -349,8 +363,11 @@ export type NavigationSeparatorItem = Pick * * A union of two arms, discriminated by `type`: a {@link NavigationEntryItem} * (every spec nav type but `'separator'`) and a {@link NavigationSeparatorItem} - * (objectui#10867). Narrow on `item.type === 'separator'` before relying on an - * entry's `label`. + * (objectui#10867). A separator has no `label`, and since objectui#9868 an + * entry's `label` may be absent too (inherited from its target at render time — + * see {@link NavigationEntryItem.label}), so display text comes from + * `resolveNavItemLabel` in `@object-ui/layout`, never from a raw `item.label` + * read. */ export type NavigationItem = NavigationEntryItem | NavigationSeparatorItem; @@ -758,6 +775,9 @@ export interface AppMenuItem { * - `hidden` → `visible` (inverted) * - `path` → `pageName` (last segment) or kept as-is for url * - `href` → `url` with `target: '_blank'` + * - a missing (or empty) `label` stays ABSENT rather than becoming `''` + * (objectui#9868): absent is the spec's "inherit at render time", while `''` + * is a present label that renders empty text and fails `objectui validate` */ export function menuItemToNavigationItem( item: AppMenuItem, @@ -775,7 +795,7 @@ export function menuItemToNavigationItem( return { id, type: 'group', - label: item.label || '', + ...(item.label ? { label: item.label } : {}), icon: item.icon, children: (item.children || []).map((child, i) => menuItemToNavigationItem(child, index * 100 + i), @@ -791,7 +811,7 @@ export function menuItemToNavigationItem( return { id, type: 'url', - label: item.label || '', + ...(item.label ? { label: item.label } : {}), icon: item.icon, url: item.href, target: '_blank', @@ -804,7 +824,7 @@ export function menuItemToNavigationItem( return { id, type: 'page', - label: item.label || '', + ...(item.label ? { label: item.label } : {}), icon: item.icon, pageName: item.path || '', visible: item.hidden !== undefined ? !item.hidden : undefined, diff --git a/packages/types/src/zod/app.zod.ts b/packages/types/src/zod/app.zod.ts index 5b01210fc5..4b015bade9 100644 --- a/packages/types/src/zod/app.zod.ts +++ b/packages/types/src/zod/app.zod.ts @@ -140,12 +140,14 @@ function codeList(keys: readonly string[]): string { const NavigationItemObject = z.object({ // Declared optional so a bare `{ type: 'separator' }` — which the spec // accepts, and which carries no identity or text by definition — validates - // here too (objectstack#4115). Every OTHER type still requires both; that is + // here too (objectstack#4115). Every OTHER type still requires `id`; that is // re-imposed by the refinement below rather than by the field declarations, // because this is one flat shape and not the spec's discriminated union. + // `label` is optional for every type since objectui#9868 (inherited when + // absent); the refinement refuses only an EMPTY one. id: z.string().optional().describe('Unique identifier'), type: NavigationItemTypeSchema.describe('Navigation item type'), - label: z.string().optional().describe('Display label'), + label: z.string().optional().describe("Display label. Optional: absent ⇒ the entry inherits its target's current label at render time (view, else object / dashboard, else the target's machine name); present ⇒ rendered verbatim. Never an empty string"), icon: z.string().optional().describe('Icon name (Lucide)'), // Type-specific target fields @@ -199,7 +201,9 @@ const NavigationItemObject = z.object({ // Identity and text are required for every real destination; only the // separator — a rule, not an entry — is exempt. Declaring the fields // optional above is what lets `{ type: 'separator' }` through, so without - // this an id-less `type: 'object'` item would validate too. + // this an id-less `type: 'object'` item would validate too. Since + // objectui#9868 the TEXT half is met by inheritance when `label` is absent + // (see below); identity is still required here. // // The separator carries exactly what the spec's separator declares // (objectui#10867). This shape is flat, so it declares `label`, `icon` and @@ -218,14 +222,32 @@ const NavigationItemObject = z.object({ } return; } - for (const key of ['id', 'label'] as const) { - if (typeof item[key] !== 'string' || item[key] === '') { - ctx.addIssue({ - code: 'custom', - path: [key], - message: `\`${key}\` is required for navigation items of type '${item.type}'`, - }); - } + // Identity is required; TEXT may be inherited (objectui#9868, the cloud#2021 + // letter-A ruling, matching `@objectstack/spec` 17.5.0, whose shared nav-item + // base declares `label` optional for every entry type). An ABSENT `label` + // means "inherit the target's current label at render time" — + // `resolveNavItemLabel` in `@object-ui/layout` resolves it; this validator + // fills nothing in, so the parsed item keeps no `label` key. The rule above + // still holds: identity is the target, text is inherited. + if (typeof item.id !== 'string' || item.id === '') { + ctx.addIssue({ + code: 'custom', + path: ['id'], + message: `\`id\` is required for navigation items of type '${item.type}'`, + }); + } + // A PRESENT empty label is not "no label": it renders verbatim, i.e. as + // empty text, and silently opts the entry out of inheritance. Refused here as + // it was while `label` was required; the spec's base accepts `''`, so this is + // the one place the empty string is caught before it is stored. + if (item.label === '') { + ctx.addIssue({ + code: 'custom', + path: ['label'], + message: + `\`label\` is empty on a navigation item of type '${item.type}': omit the key to inherit ` + + "the target's label at render time, or write the text to show", + }); } // The spec's OWN target-exclusivity rule, CHAINED rather than restated From a2ebd10d83e59b6a418108ad073df9676e89c063 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 07:56:36 +0000 Subject: [PATCH 2/7] test(types),docs: invert the two pins that asserted a required nav label; the navigation guide states the optional label (objectui#9868) navigation-model's "should reject missing required label" is inverted to accept a label-less entry that parses with no label key. The objectui#7918 nested-superRefine pin keeps its subject with a missing id as the trigger, now with a positive control. The app-navigation guide no longer says label is mandatory, and says what an entry without one shows. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_011p7ikEivgXefNDaE5S5Uec --- content/docs/guide/designing-app-navigation.md | 10 +++++++++- .../types/src/__tests__/navigation-model.test.ts | 9 +++++++-- .../zod-lazy-getter-identity-7918.test.ts | 15 ++++++++++++--- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/content/docs/guide/designing-app-navigation.md b/content/docs/guide/designing-app-navigation.md index 833d249398..2b405c4353 100644 --- a/content/docs/guide/designing-app-navigation.md +++ b/content/docs/guide/designing-app-navigation.md @@ -97,7 +97,15 @@ field — there is no generic `path` and no `kind`: Requirements: -- `id` (snake_case), `type`, and `label` are mandatory on every item. +- `id` (snake_case) and `type` are mandatory on every item. `label` is + optional: an entry without one shows its target's **current** label, resolved + when the navigation renders — the view's label for an entry that names a + labelled view, else the object's or dashboard's label, else the target's + machine name. A renamed object, view or dashboard therefore shows its new + name on the next load without touching the navigation. Entries of other types + fall back to their target's name (`pageName`, `reportName`, the `url`) and a + group to its `id`, so give those a `label`. A `label` you do write renders + verbatim. Never write an empty `label` — omit the key instead. - The target field must match the type: `objectName`, `pageName`, `dashboardName`, `reportName`, or `url`. Keys like `path` or `kind` are ignored at runtime and rejected at save. diff --git a/packages/types/src/__tests__/navigation-model.test.ts b/packages/types/src/__tests__/navigation-model.test.ts index 2bb4ff27e7..f9942c3185 100644 --- a/packages/types/src/__tests__/navigation-model.test.ts +++ b/packages/types/src/__tests__/navigation-model.test.ts @@ -167,14 +167,19 @@ describe('NavigationItem Zod Schema', () => { expect(result.success).toBe(false); }); - it('should reject missing required label', () => { + // INVERTED by objectui#9868 (was 'should reject missing required label' → + // `success` false): `@objectstack/spec` 17.5.0 made an entry's `label` + // optional — absent ⇒ the entry inherits its target's label at render time — + // so a label-less entry is accepted, and nothing is filled in at parse time. + it('should accept a missing label, adding none (objectui#9868)', () => { const item = { id: 'nav_no_label', type: 'object', objectName: 'contact', }; const result = NavigationItemSchema.safeParse(item); - expect(result.success).toBe(false); + expect(result.success).toBe(true); + expect(result.success && result.data).not.toHaveProperty('label'); }); }); diff --git a/packages/types/src/__tests__/zod-lazy-getter-identity-7918.test.ts b/packages/types/src/__tests__/zod-lazy-getter-identity-7918.test.ts index 33a12346b6..3b42577be2 100644 --- a/packages/types/src/__tests__/zod-lazy-getter-identity-7918.test.ts +++ b/packages/types/src/__tests__/zod-lazy-getter-identity-7918.test.ts @@ -270,13 +270,22 @@ describe('objectui#7918 · z.lazy getter identity', () => { }); it('NavigationItemSchema still runs its superRefine on nested children', () => { - // `label` missing on a non-separator child — the refinement, not the - // field declarations, is what refuses this. + // `id` missing on a non-separator child — the refinement, not the + // field declarations, is what refuses this. (The trigger was a missing + // `label` until objectui#9868 made `label` optional to match the spec; + // `id` is the requirement the refinement still re-imposes.) const bad = { id: 'root', type: 'group', label: 'Root', - children: [{ id: 'acct', type: 'object' }], + children: [{ type: 'object', label: 'Accounts', objectName: 'account' }], }; expect(NavigationItemSchema.safeParse(bad).success).toBe(false); + // Control: the same child WITH its id is accepted, so the refusal above + // is the id rule and not something else about the fixture. + const good = { + id: 'root', type: 'group', label: 'Root', + children: [{ id: 'acct', type: 'object', label: 'Accounts', objectName: 'account' }], + }; + expect(NavigationItemSchema.safeParse(good).success).toBe(true); }); it('FilterBuilderConditionSchema still accepts a condition and refuses a bad operator', () => { From 3c8a2efb88b6fc20cf2df4aa902052a4a6dd48e5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 08:01:37 +0000 Subject: [PATCH 3/7] refactor(app-shell,layout): type the nav target-label lookup without any, drop the now-unused resolveLabel import (objectui#9868) Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_011p7ikEivgXefNDaE5S5Uec --- ...NavigationSync.labelInherits-9868.test.tsx | 7 +++--- .../app-shell/src/hooks/useNavTargetLabel.ts | 22 ++++++++++++++----- packages/layout/src/AppSchemaRenderer.tsx | 3 +-- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/packages/app-shell/src/hooks/__tests__/useNavigationSync.labelInherits-9868.test.tsx b/packages/app-shell/src/hooks/__tests__/useNavigationSync.labelInherits-9868.test.tsx index 5c8916e89a..62c526911a 100644 --- a/packages/app-shell/src/hooks/__tests__/useNavigationSync.labelInherits-9868.test.tsx +++ b/packages/app-shell/src/hooks/__tests__/useNavigationSync.labelInherits-9868.test.tsx @@ -32,8 +32,8 @@ vi.mock('sonner', () => ({ })); interface World { - pages: any[]; - dashboards: any[]; + pages: unknown[]; + dashboards: unknown[]; } const crm = { name: 'crm', label: 'CRM', navigation: [] }; @@ -56,7 +56,8 @@ function metaValue(world: World): MetadataContextValue { }; } -const adapterFor = (saveItem: ReturnType) => ({ getClient: () => ({ meta: { saveItem } }) }) as any; +const adapterFor = (saveItem: ReturnType) => + ({ getClient: () => ({ meta: { saveItem } }) }) as unknown as React.ComponentProps['value']; /** Every navigation array the writer PUT, flattened to its entries. */ const writtenEntries = (saveItem: ReturnType) => diff --git a/packages/app-shell/src/hooks/useNavTargetLabel.ts b/packages/app-shell/src/hooks/useNavTargetLabel.ts index 3a60fe843a..a2a58152a0 100644 --- a/packages/app-shell/src/hooks/useNavTargetLabel.ts +++ b/packages/app-shell/src/hooks/useNavTargetLabel.ts @@ -63,13 +63,26 @@ function nonEmpty(text: string | undefined): string | undefined { return typeof text === 'string' && text.trim() !== '' ? text : undefined; } -function findByName(items: readonly unknown[] | undefined, name: string): Record | undefined { +/** The two members this hook reads off a cached metadata record. */ +interface LabelledRecord { + name?: unknown; + label?: unknown; + listViews?: unknown; +} + +function findByName(items: readonly unknown[] | undefined, name: string): LabelledRecord | undefined { return (items ?? []).find( - (item): item is Record => - !!item && typeof item === 'object' && (item as { name?: unknown }).name === name, + (item): item is LabelledRecord => + !!item && typeof item === 'object' && (item as LabelledRecord).name === name, ); } +/** An object's merged `listViews` map (`.` → view), or `{}` when it has none. */ +function listViewsOf(def: LabelledRecord | undefined): Record { + const views = def?.listViews; + return views && typeof views === 'object' ? (views as Record) : {}; +} + export function useNavTargetLabel(): NavTargetLabelResolver { const metadata = useMetadata(); const { objectLabel, viewLabel, dashboardLabel } = useObjectLabel(); @@ -84,8 +97,7 @@ export function useNavTargetLabel(): NavTargetLabelResolver { return nonEmpty(objectLabel({ name: target.objectName, label: text ?? '' })); } case 'view': { - const def = findByName(metadata.objects, target.objectName); - const views: Record = def?.listViews ?? {}; + const views = listViewsOf(findByName(metadata.objects, target.objectName)); const id = resolveViewId(target.viewName, Object.keys(views), target.objectName); const text = metadataText(id ? views[id]?.label : undefined, language); return nonEmpty(viewLabel(target.objectName, target.viewName, text ?? '')); diff --git a/packages/layout/src/AppSchemaRenderer.tsx b/packages/layout/src/AppSchemaRenderer.tsx index 50a6f97934..6025b1c875 100644 --- a/packages/layout/src/AppSchemaRenderer.tsx +++ b/packages/layout/src/AppSchemaRenderer.tsx @@ -46,7 +46,6 @@ import { NavigationRenderer, hasVisibleNavigationItems, resolveIcon, - resolveLabel, resolveNavItemLabel, type VisibilityEvaluator, type PermissionChecker, @@ -172,7 +171,7 @@ export interface AppSchemaRendererProps { * (objectui#4167 renamed objectui's own resolver to keep them apart): * * - `NavigationItem.label` is objectui's KEYED ref — a translation key plus a - * default (`{ key, defaultValue, params }`) — resolved by {@link resolveLabel} + * default (`{ key, defaultValue, params }`) — resolved by `resolveLabel` * against an injected `t`; * - `NavigationArea.label` is `@objectstack/spec`'s `I18nLabel`, which * 17.0.0-rc.6 widened from `string` to `string | Record` — From 03f298272786a59c3aecb2724843c97166ed1106 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 08:04:29 +0000 Subject: [PATCH 4/7] docs(changeset,app-shell): cite the optional nav label by the entry arms that carry it, not as a spec member path (objectui#9868) check:spec-symbols reads a dotted spec citation as a key of the symbol's own shape; the nav-item union has none, so the passage now names the entry arms. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_011p7ikEivgXefNDaE5S5Uec --- .changeset/9868-nav-label-inherits-target.md | 13 +++++++------ packages/app-shell/src/hooks/useNavTargetLabel.ts | 5 +++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/.changeset/9868-nav-label-inherits-target.md b/.changeset/9868-nav-label-inherits-target.md index 0aeab6b21f..ddc1d39574 100644 --- a/.changeset/9868-nav-label-inherits-target.md +++ b/.changeset/9868-nav-label-inherits-target.md @@ -6,12 +6,13 @@ feat(types,layout,app-shell): a navigation entry with no `label` shows its target's current label, resolved at render time (objectui#9868) -`@objectstack/spec` 17.5.0 made `NavigationItemSchema.label` optional on every -navigation entry, with a declared semantic (the cloud#2021 letter-A ruling): -absent ⇒ the entry shows the CURRENT label of what it opens — the view's label -when it names a labelled view, else the object's / dashboard's label; present ⇒ -rendered verbatim. Nothing is stored for the absent case, so a renamed target -shows its new name on the next render. +`@objectstack/spec` 17.5.0 made a navigation entry's `label` optional on every +entry arm of its `NavigationItemSchema` (the separator still carries none), with +a declared semantic (the cloud#2021 letter-A ruling): absent ⇒ the entry +shows the CURRENT label of what it opens — the view's label when it names a +labelled view, else the object's / dashboard's label; present ⇒ rendered +verbatim. Nothing is stored for the absent case, so a renamed target shows its +new name on the next render. - `@object-ui/types` (widening): `NavigationEntryItem.label` is optional, and `NavigationItemSchema` accepts a label-less entry of every type the spec does, diff --git a/packages/app-shell/src/hooks/useNavTargetLabel.ts b/packages/app-shell/src/hooks/useNavTargetLabel.ts index a2a58152a0..8fb687bee0 100644 --- a/packages/app-shell/src/hooks/useNavTargetLabel.ts +++ b/packages/app-shell/src/hooks/useNavTargetLabel.ts @@ -10,8 +10,9 @@ * useNavTargetLabel — the console's answer to "what is this navigation target * called NOW?", for navigation entries that carry no `label` (objectui#9868). * - * `@objectstack/spec` 17.5.0 made `NavigationItemSchema.label` optional with a - * declared semantic (the cloud#2021 letter-A ruling): an ABSENT label means the + * `@objectstack/spec` 17.5.0 made a navigation entry's `label` optional (on the + * nav-item base every `NavigationItemSchema` entry arm shares) with a declared + * semantic (the cloud#2021 letter-A ruling): an ABSENT label means the * entry shows, at render time, the CURRENT label of what it opens — the view's * label when it names a labelled view, else the object's / dashboard's label. * `@object-ui/layout`'s `resolveNavItemLabel` walks that ladder; this hook is From 444bdb9b50c0087cea56de83ccda62215f6078db Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 08:04:59 +0000 Subject: [PATCH 5/7] docs(types): the empty-label refusal is a stated divergence from the spec, caught by objectui validate rather than the save door (objectui#9868) Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_011p7ikEivgXefNDaE5S5Uec --- packages/types/src/zod/app.zod.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/types/src/zod/app.zod.ts b/packages/types/src/zod/app.zod.ts index 4b015bade9..368b65447d 100644 --- a/packages/types/src/zod/app.zod.ts +++ b/packages/types/src/zod/app.zod.ts @@ -238,8 +238,9 @@ const NavigationItemObject = z.object({ } // A PRESENT empty label is not "no label": it renders verbatim, i.e. as // empty text, and silently opts the entry out of inheritance. Refused here as - // it was while `label` was required; the spec's base accepts `''`, so this is - // the one place the empty string is caught before it is stored. + // it was while `label` was required. ⚠️ A divergence from the spec, stated: + // the spec's base accepts `''`, so the platform's save door does not catch + // it — this mirror (what `objectui validate` answers) is where it is caught. if (item.label === '') { ctx.addIssue({ code: 'custom', From ebe8588c27698dc707ff23ebfc4010663c092e7a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 08:50:37 +0000 Subject: [PATCH 6/7] fix(app-shell): the command palette and the search page name a label-less nav entry through the shared resolver; layout's changeset level is minor (objectui#9868) Contract review 5907554475. The palette rendered resolveKeyedI18nLabel( item.label, t), which answers undefined for an absent label, so an entry NavigationSyncEffect now writes without a label drew a blank row whose cmdk value read "page undefined my_page". The search page listed such an entry by its machine name via item.label || item.objectName || ... Both now call resolveNavItemLabel with the useNavTargetLabel resolver the sidebar and nav:menu share; a present label resolves as before. @object-ui/layout goes patch -> minor in the changeset: the diff adds exported types and an optional prop. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_011p7ikEivgXefNDaE5S5Uec --- .changeset/9868-nav-label-inherits-target.md | 16 +- ...mandPalette.navLabelInherits-9868.test.tsx | 137 ++++++++++++++++++ .../app-shell/src/chrome/CommandPalette.tsx | 27 ++-- .../app-shell/src/views/SearchResultsPage.tsx | 14 +- ...ResultsPage.navLabelInherits-9868.test.tsx | 110 ++++++++++++++ 5 files changed, 286 insertions(+), 18 deletions(-) create mode 100644 packages/app-shell/src/chrome/CommandPalette.navLabelInherits-9868.test.tsx create mode 100644 packages/app-shell/src/views/__tests__/SearchResultsPage.navLabelInherits-9868.test.tsx diff --git a/.changeset/9868-nav-label-inherits-target.md b/.changeset/9868-nav-label-inherits-target.md index ddc1d39574..2d557af244 100644 --- a/.changeset/9868-nav-label-inherits-target.md +++ b/.changeset/9868-nav-label-inherits-target.md @@ -1,6 +1,6 @@ --- '@object-ui/types': minor -'@object-ui/layout': patch +'@object-ui/layout': minor '@object-ui/app-shell': patch --- @@ -20,8 +20,9 @@ new name on the next render. still refused — `''` is a present label that would render empty text — and the message says to omit the key instead. `menuItemToNavigationItem` no longer turns a missing legacy label into `''`. -- `@object-ui/layout`: `resolveNavItemLabel` resolves an absent label through a - new `resolveTargetLabel` resolver (`NavigationRenderer` prop, types +- `@object-ui/layout` (additive public surface): `resolveNavItemLabel` + resolves an absent label through a new `resolveTargetLabel` resolver + (`NavigationRenderer` prop, types `NavLabelTarget` / `NavTargetLabelResolver`), walking view → object / dashboard, and falls back to the target's machine name (`viewName`, `objectName`, `dashboardName`, `pageName`, `reportName`, `url`, @@ -30,9 +31,12 @@ new name on the next render. name. Action entries, the search filter and `AppSchemaRenderer`'s mobile tab bar read the label through the same function, so an unlabelled entry no longer throws there. -- `@object-ui/app-shell`: the sidebar and the `nav:menu` block both resolve - targets through one hook over the metadata cache the shell already holds - (object schema, merged list views, dashboards) — no request per nav entry. +- `@object-ui/app-shell`: the sidebar, the `nav:menu` block, the ⌘K command + palette and the full-page search all name a nav entry through + `resolveNavItemLabel` with one hook over the metadata cache the shell already + holds (object schema, merged list views, dashboards) — no request per nav + entry. Read raw, a label-less entry drew a blank palette row searched as + `undefined`, and the search page listed it by its machine name. Navigation sync stops writing `label: pageName` / `label: dashboardName` for an unnamed page or dashboard; an author label is still written, and stored labels are kept. diff --git a/packages/app-shell/src/chrome/CommandPalette.navLabelInherits-9868.test.tsx b/packages/app-shell/src/chrome/CommandPalette.navLabelInherits-9868.test.tsx new file mode 100644 index 0000000000..206f1e85f4 --- /dev/null +++ b/packages/app-shell/src/chrome/CommandPalette.navLabelInherits-9868.test.tsx @@ -0,0 +1,137 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * The ⌘K palette names an app navigation entry the way the sidebar does, + * including an entry with NO `label` (objectui#9868, contract review finding 1). + * + * Since objectui#9868 `NavigationSyncEffect` writes every page and dashboard + * created in the console WITHOUT a `label` — absent means "inherit the target's + * current label at render time". The palette read `resolveKeyedI18nLabel( + * item.label, t)`, which answers `undefined` for an absent label, so such an + * entry drew a BLANK row whose cmdk filter value read `page undefined my_page`. + * It now resolves through `resolveNavItemLabel` with the same + * `useNavTargetLabel` resolver the sidebar and `nav:menu` use. + * + * Pinned through the real `CommandPaletteProvider` (opened by its `?palette=1` + * deep link) under a real `MetadataCtx`, asserting both what the row SHOWS and + * the cmdk `data-value` it is searched by: + * + * - a label-less page shows its machine-name backstop (`pageName`), the text + * the old sync used to store; + * - a label-less dashboard and a label-less view entry show their targets' + * CURRENT metadata labels, and a rename shows on the next render; + * - CONTROL: an authored label renders verbatim. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import { MetadataCtx } from '@object-ui/react'; + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u1' }, activeOrganization: null }), +})); + +import { CommandPalette } from './CommandPalette'; +import { CommandPaletteProvider } from '../context/CommandPaletteProvider'; + +const APP = { + name: 'crm', + label: 'CRM', + navigation: [ + // What NavigationSyncEffect writes for a page / dashboard since objectui#9868. + { id: 'nav_page', type: 'page', pageName: 'my_page' }, + { id: 'nav_dash', type: 'dashboard', dashboardName: 'sales_overview' }, + { id: 'nav_board', type: 'object', objectName: 'customer', viewName: 'board' }, + // Control: an authored label. + { id: 'nav_home', type: 'page', pageName: 'home', label: 'Team Home' }, + ], +}; + +function metadata(viewLabel: string, dashboardLabel: string) { + return { + apps: [APP], + objects: [ + { + name: 'customer', + label: 'Customers', + listViews: { 'customer.board': { name: 'customer.board', label: viewLabel } }, + }, + ], + dashboards: [{ name: 'sales_overview', label: dashboardLabel }], + reports: [], + pages: [{ name: 'my_page' }, { name: 'home', label: 'Home' }], + loading: false, + error: null, + refresh: async () => {}, + invalidate: () => {}, + ensureType: async () => [], + getItem: async () => null, + getItemsByType: () => [], + getTypeStatus: () => 'ready' as const, + }; +} + +function tree(value: ReturnType) { + return ( + + + + {}} /> + + + + ); +} + +/** Every nav row of one kind: its visible text and its cmdk filter value. */ +function rows(kind: string): Array<{ text: string; value: string }> { + return Array.from(document.querySelectorAll(`[cmdk-item][data-value^="${kind} "]`)).map((el) => ({ + text: (el.textContent ?? '').trim(), + value: el.getAttribute('data-value') ?? '', + })); +} + +afterEach(() => cleanup()); + +describe('objectui#9868 — the ⌘K palette names a label-less nav entry like the sidebar does', () => { + it('a label-less page shows its pageName, and is searched by it — never blank, never "undefined"', () => { + render(tree(metadata('客户管理仪表盘', 'Sales Overview'))); + if (!document.querySelector('[cmdk-input]')) throw new Error('the palette did not open'); + + const page = rows('page').find((r) => r.value.includes('my_page')); + expect(page).toBeDefined(); + expect(page!.text).toBe('my_page'); + expect(page!.value).not.toContain('undefined'); + expect(page!.value).toBe('page my_page my_page'); + }); + + it('a label-less dashboard and view entry show their targets’ current labels, and follow a rename', () => { + const utils = render(tree(metadata('客户管理仪表盘', 'Sales Overview'))); + + expect(rows('dashboard').map((r) => r.text)).toEqual(['Sales Overview']); + expect(rows('dashboard')[0].value.toLowerCase()).toContain('sales overview'); + const board = rows('object').find((r) => r.value.includes('customer')); + expect(board?.text).toBe('客户管理仪表盘'); + expect(board?.value).toContain('客户管理仪表盘'); + + utils.rerender(tree(metadata('Customer Board', 'Revenue'))); + expect(rows('dashboard').map((r) => r.text)).toEqual(['Revenue']); + expect(rows('object').find((r) => r.value.includes('customer'))?.text).toBe('Customer Board'); + }); + + it('control: an authored label renders verbatim', () => { + render(tree(metadata('Board', 'Sales Overview'))); + const home = rows('page').find((r) => r.value.includes('home')); + expect(home?.text).toBe('Team Home'); + expect(home?.value.toLowerCase()).toContain('team home'); + }); +}); diff --git a/packages/app-shell/src/chrome/CommandPalette.tsx b/packages/app-shell/src/chrome/CommandPalette.tsx index e32303f05b..1560aafc1c 100644 --- a/packages/app-shell/src/chrome/CommandPalette.tsx +++ b/packages/app-shell/src/chrome/CommandPalette.tsx @@ -36,7 +36,9 @@ import { resolveKeyedI18nLabel, getRecordDisplayName, appRouteSegment } from '.. import { getIcon } from '../utils/getIcon.js'; import { useRecentItems } from '../context/RecentItemsProvider.js'; import { useCommandPalette } from '../context/CommandPaletteProvider.js'; -import { resolveHref } from '@object-ui/layout'; +import { resolveHref, resolveNavItemLabel } from '@object-ui/layout'; +import type { NavigationItem } from '@object-ui/types'; +import { useNavTargetLabel } from '../hooks/useNavTargetLabel.js'; import { useAuth } from '@object-ui/auth'; interface CommandPaletteProps { @@ -59,6 +61,13 @@ export function CommandPalette({ apps, activeApp, objects, onAppChange, dataSour const { setTheme } = useTheme(); const { evaluator } = useExpressionContext(); const { t } = useObjectTranslation(); + // A nav entry's text, the way the sidebar and `nav:menu` name it: an entry + // with NO `label` shows its target's current label, else its target's machine + // name (objectui#9868 — `NavigationSyncEffect` writes such entries). A present + // label resolves exactly as `resolveKeyedI18nLabel(item.label, t)` did here. + const targetLabel = useNavTargetLabel(); + const navLabel = (item: NavigationItem) => + resolveNavItemLabel(item, undefined, t, undefined, undefined, targetLabel); // The ⌘K / Ctrl+K accelerator and the open-state source of truth now live in // CommandPaletteProvider so the keyboard shortcut, the header button, and the @@ -266,11 +275,11 @@ export function CommandPalette({ apps, activeApp, objects, onAppChange, dataSour return ( runCommand(() => navigate(resolveHref(item, baseUrl, templateContext).href))} > - {resolveKeyedI18nLabel(item.label, t)} + {navLabel(item)} ); })} @@ -285,11 +294,11 @@ export function CommandPalette({ apps, activeApp, objects, onAppChange, dataSour .map(item => ( runCommand(() => navigate(resolveHref(item, baseUrl, templateContext).href))} > - {resolveKeyedI18nLabel(item.label, t)} + {navLabel(item)} ))} @@ -303,11 +312,11 @@ export function CommandPalette({ apps, activeApp, objects, onAppChange, dataSour .map(item => ( runCommand(() => navigate(resolveHref(item, baseUrl, templateContext).href))} > - {resolveKeyedI18nLabel(item.label, t)} + {navLabel(item)} ))} @@ -321,11 +330,11 @@ export function CommandPalette({ apps, activeApp, objects, onAppChange, dataSour .map(item => ( runCommand(() => navigate(resolveHref(item, baseUrl, templateContext).href))} > - {resolveKeyedI18nLabel(item.label, t)} + {navLabel(item)} ))} diff --git a/packages/app-shell/src/views/SearchResultsPage.tsx b/packages/app-shell/src/views/SearchResultsPage.tsx index 7232523b8b..669da790e0 100644 --- a/packages/app-shell/src/views/SearchResultsPage.tsx +++ b/packages/app-shell/src/views/SearchResultsPage.tsx @@ -31,7 +31,8 @@ import { useAdapter } from '../providers/AdapterProvider.js'; import { matchAppBySegment } from '../utils/appRoute.js'; import { resolveKeyedI18nLabel, getRecordDisplayName } from '../utils/index.js'; import { getIcon } from '../utils/getIcon.js'; -import { resolveHref } from '@object-ui/layout'; +import { resolveHref, resolveNavItemLabel } from '@object-ui/layout'; +import { useNavTargetLabel } from '../hooks/useNavTargetLabel.js'; import { useAuth } from '@object-ui/auth'; interface SearchResult { @@ -85,6 +86,10 @@ export function SearchResultsPage() { const baseUrl = `/apps/${appName}`; const { user, activeOrganization } = useAuth(); const dataSource = useAdapter(); + // The same resolver the sidebar, `nav:menu` and the ⌘K palette use: an entry + // with NO `label` is listed — and matched — by its target's current label, + // else its target's machine name (objectui#9868). + const targetLabel = useNavTargetLabel(); // Build searchable items from navigation const allItems = useMemo((): SearchResult[] => { @@ -96,13 +101,16 @@ export function SearchResultsPage() { return { id: item.id, - label: item.label || item.objectName || item.dashboardName || item.pageName || item.reportName || '', + // `resolveNavItemLabel`, not `item.label || item.objectName || …`: that + // chain listed an unlabelled entry by its machine name even when its + // target is labelled, and handed a keyed label object to `toLowerCase`. + label: resolveNavItemLabel(item, undefined, t, undefined, undefined, targetLabel), href, type: item.type, description: item.description, }; }).filter((item: SearchResult) => item.href !== '#'); - }, [activeApp, baseUrl, user?.id]); + }, [activeApp, baseUrl, user?.id, t, targetLabel]); // Filter results const results = useMemo(() => { diff --git a/packages/app-shell/src/views/__tests__/SearchResultsPage.navLabelInherits-9868.test.tsx b/packages/app-shell/src/views/__tests__/SearchResultsPage.navLabelInherits-9868.test.tsx new file mode 100644 index 0000000000..cd514f0c91 --- /dev/null +++ b/packages/app-shell/src/views/__tests__/SearchResultsPage.navLabelInherits-9868.test.tsx @@ -0,0 +1,110 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * The full-page search lists and MATCHES an app navigation entry by the text + * the sidebar shows for it, including an entry with NO `label` (objectui#9868, + * contract review finding 2). + * + * It built each result as `item.label || item.objectName || item.dashboardName + * || …`, so a label-less entry whose target IS labelled was listed — and could + * only be found — by its machine name. It now resolves through + * `resolveNavItemLabel` with the shared `useNavTargetLabel` resolver: + * + * - a label-less view / dashboard entry is listed by its target's CURRENT + * metadata label, and a query for that label finds it while a query for a + * text it no longer shows does not; + * - a label-less page falls back to its `pageName` (not an inheriting target); + * - CONTROL: an authored label is listed verbatim. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import React from 'react'; + +const url = vi.hoisted(() => ({ search: '' })); + +vi.mock('react-router-dom', () => ({ + useParams: () => ({ appName: 'crm' }), + useSearchParams: () => [new URLSearchParams(url.search), vi.fn()], + Link: ({ to, children, ...rest }: { to: unknown; children?: React.ReactNode }) => ( + + {children} + + ), +})); + +vi.mock('@object-ui/react', async (importOriginal) => ({ + ...(await importOriginal>()), + useRecordSearch: () => ({ results: [], isSearching: false, error: undefined }), +})); + +vi.mock('../../providers/MetadataProvider', () => ({ + useMetadata: () => ({ + apps: [ + { + name: 'crm', + label: 'CRM', + navigation: [ + { id: 'nav_board', type: 'object', objectName: 'customer', viewName: 'board' }, + { id: 'nav_dash', type: 'dashboard', dashboardName: 'sales_overview' }, + { id: 'nav_page', type: 'page', pageName: 'my_page' }, + { id: 'nav_home', type: 'page', pageName: 'home', label: 'Team Home' }, + ], + }, + ], + objects: [ + { + name: 'customer', + label: 'Customers', + listViews: { 'customer.board': { name: 'customer.board', label: '客户管理仪表盘' } }, + }, + ], + dashboards: [{ name: 'sales_overview', label: 'Sales Overview' }], + }), +})); + +vi.mock('../../providers/AdapterProvider', () => ({ + useAdapter: () => ({ find: vi.fn(), searchAll: vi.fn() }), +})); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u1' }, activeOrganization: null }), +})); + +import { SearchResultsPage } from '../SearchResultsPage'; + +/** The visible title of every nav result card, in render order. */ +const titles = () => Array.from(document.querySelectorAll('p.text-sm.font-medium')).map((p) => p.textContent); + +describe('objectui#9868 — search lists and matches a label-less nav entry by the label it shows', () => { + beforeEach(() => { + url.search = ''; + }); + + it('browse: inherited labels for the view and dashboard entries, pageName for the page, the authored label verbatim', () => { + render(); + expect(titles()).toEqual(expect.arrayContaining(['客户管理仪表盘', 'Sales Overview', 'my_page', 'Team Home'])); + // Not the machine names the old `item.label || item.objectName || …` chain listed. + expect(titles()).not.toContain('customer'); + expect(titles()).not.toContain('sales_overview'); + }); + + it('query: the inherited label finds the entry', () => { + url.search = 'q=仪表盘'; + render(); + expect(titles()).toEqual(['客户管理仪表盘']); + }); + + it('query: the dashboard is found by its label, not only by its machine name', () => { + url.search = 'q=overview'; + render(); + expect(screen.getByText('Sales Overview')).toBeTruthy(); + }); +}); From 26d6862a3e81e24e32e11e4164a340a44c967250 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 09:14:25 +0000 Subject: [PATCH 7/7] =?UTF-8?q?test(app-shell,layout):=20pin=20the=20two?= =?UTF-8?q?=20earlier=20raw=20nav-label=20readers=20=E2=80=94=20nav=20pins?= =?UTF-8?q?=20store=20text,=20the=20mobile=20tab=20bar=20shows=20the=20bac?= =?UTF-8?q?kstop=20(objectui#9868)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The patch-round sweep asks every FIXED reader to carry a pin. useNavPins and AppSchemaRenderer's mobile tab bar were routed through resolveNavItemLabel in the first round without one. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_011p7ikEivgXefNDaE5S5Uec --- .../useNavPins.labelInherits-9868.test.tsx | 62 +++++++++++++++++++ ...Renderer.labelInheritsTarget-9868.test.tsx | 29 +++++++++ 2 files changed, 91 insertions(+) create mode 100644 packages/app-shell/src/hooks/__tests__/useNavPins.labelInherits-9868.test.tsx diff --git a/packages/app-shell/src/hooks/__tests__/useNavPins.labelInherits-9868.test.tsx b/packages/app-shell/src/hooks/__tests__/useNavPins.labelInherits-9868.test.tsx new file mode 100644 index 0000000000..a3bb8edd91 --- /dev/null +++ b/packages/app-shell/src/hooks/__tests__/useNavPins.labelInherits-9868.test.tsx @@ -0,0 +1,62 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * Pinning a navigation entry with NO `label` stores text, not `undefined` + * (objectui#9868). + * + * `togglePin` writes a portable `type: 'nav'` favorite record. It used to copy + * `item.label` verbatim, which is `undefined` for an entry that inherits its + * target's label at render time — a record whose `label: string` member then + * held nothing. It now stores `resolveNavItemLabel(item)`: the machine-name + * text the renderer falls back to (the sidebar renders the live nav tree's + * label for a nav pin, so this stored text is a placeholder, like the one + * `migrateLegacyNavPins` writes). CONTROL: an authored label is stored verbatim. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { renderHook } from '@testing-library/react'; + +const favorites = vi.hoisted(() => ({ + addFavorite: vi.fn(), + removeFavorite: vi.fn(), + setPinned: vi.fn(), +})); + +vi.mock('../../context/FavoritesProvider', () => ({ + useFavorites: () => ({ favorites: [], pinnedNavIds: new Set(), ...favorites }), +})); + +import { useNavPins } from '../useNavPins'; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('objectui#9868 — pinning a label-less nav entry', () => { + it('stores the machine-name text the renderer falls back to, never undefined', () => { + const { result } = renderHook(() => useNavPins()); + result.current.togglePin('nav_page', true, { id: 'nav_page', type: 'page', pageName: 'my_page' }, '/apps/crm'); + + expect(favorites.addFavorite).toHaveBeenCalledTimes(1); + const record = favorites.addFavorite.mock.calls[0][0]; + expect(record).toMatchObject({ id: 'nav:nav_page', type: 'nav', navId: 'nav_page', label: 'my_page' }); + expect(typeof record.label).toBe('string'); + }); + + it('control: an authored label is stored verbatim', () => { + const { result } = renderHook(() => useNavPins()); + result.current.togglePin( + 'nav_home', + true, + { id: 'nav_home', type: 'page', pageName: 'home', label: 'Team Home' }, + '/apps/crm', + ); + expect(favorites.addFavorite.mock.calls[0][0]).toMatchObject({ label: 'Team Home' }); + }); +}); diff --git a/packages/layout/src/__tests__/NavigationRenderer.labelInheritsTarget-9868.test.tsx b/packages/layout/src/__tests__/NavigationRenderer.labelInheritsTarget-9868.test.tsx index b705c35226..c22d8ffe26 100644 --- a/packages/layout/src/__tests__/NavigationRenderer.labelInheritsTarget-9868.test.tsx +++ b/packages/layout/src/__tests__/NavigationRenderer.labelInheritsTarget-9868.test.tsx @@ -31,6 +31,7 @@ import { render, screen, within } from '@testing-library/react'; import { MemoryRouter } from 'react-router-dom'; import type { NavigationItem } from '@object-ui/types'; import { SidebarProvider } from '@object-ui/components'; +import { AppSchemaRenderer } from '../AppSchemaRenderer'; import { NavigationRenderer, resolveNavItemLabel, @@ -211,3 +212,31 @@ describe('objectui#9868 — NavigationRenderer renders the inherited label', () expect(filterNavigationItems(items, 'board').map((i) => i.id)).toEqual(['nav_board']); }); }); + +describe('objectui#9868 — AppSchemaRenderer’s mobile tab bar names an unlabelled entry', () => { + it('shows the machine-name backstop (it used to read `label.defaultValue` off undefined); control: an authored label', () => { + const { container } = render( + + +
+ + , + ); + const bar = container.querySelector('[role="navigation"][aria-label="Mobile navigation"]'); + expect(bar).not.toBeNull(); + const texts = Array.from(bar!.querySelectorAll('a span.truncate')).map((span) => span.textContent); + expect(texts).toEqual(['my_page', 'Accounts']); + }); +});