From 40a0eb181ecd7c1ea8a9553ee3f45ab375955015 Mon Sep 17 00:00:00 2001 From: yinlianghui Date: Sun, 16 Aug 2026 22:51:53 +0000 Subject: [PATCH] fix(studio): the Interfaces rail opens `action` nav entries instead of disabling them (#4019) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Interfaces pillar's rail is the package's App `navigation` tree, and each leaf opens the design surface of whatever it binds to. `resolveSurface` bound five shapes — page/object/dashboard/report/view — and not `action`, so an action entry rendered `disabled`: visible in the designer, inert on click. The same entry works in the shipped product (`NavigationRenderer` + `useNavActionDispatch`, framework#4509), and `action` already had a registered preview AND a registered default inspector. Only the binding was missing, so the one nav variant naming an authorable metadata item was the one the designer could not author. - navSurface.ts: `resolveSurface` / `findSurfaceInTree` / `NavNode` / `Surface` extracted out of the 3.7k-line surface so the binding is unit-testable without mounting a pillar — the reason `packageSurfaces.ts` and `centerTab.ts` already live beside it. The new `action` case is the only behaviour change. - `actionDef.actionName` is read as the ONLY spelling. `action` / `name` / `args` / `input` are named REJECTIONS in the spec (objectstack#4001, measured on 17.0.0-rc.6: `unrecognized_keys` + missing `actionName`), not second spellings; a tolerant read would re-open the bug #4001 closed. - `url` / `separator` / `component` stay unresolvable and that is correct — an external link, a divider and a code-shipped UI have no metadata item to design. Object-scoped actions keep the object's Actions tab: `ActionNavItemSchema` is strict `{ actionName, params? }` with no `objectName`, so a nav action is global by construction and this path cannot reach an object-scoped one. - `InterfacesPillar` is exported, as `DataPillar` / `AccessPillar` already are, for the rail-level acceptance test. The `dataset` half of #4019 is NOT here: `NavigationItemSchema` has no `dataset` member, so the Interfaces rail cannot carry one without a decision the card does not make. Reported back to the PM rather than guessed at. Co-authored-by: Claude --- .../studio-interfaces-action-nav-surface.md | 34 +++++ ...dioDesignSurface.interfacesAction.test.tsx | 127 ++++++++++++++++++ .../studio-design/StudioDesignSurface.tsx | 75 +---------- .../views/studio-design/navSurface.test.ts | 101 ++++++++++++++ .../src/views/studio-design/navSurface.ts | 115 ++++++++++++++++ 5 files changed, 380 insertions(+), 72 deletions(-) create mode 100644 .changeset/studio-interfaces-action-nav-surface.md create mode 100644 packages/app-shell/src/views/studio-design/StudioDesignSurface.interfacesAction.test.tsx create mode 100644 packages/app-shell/src/views/studio-design/navSurface.test.ts create mode 100644 packages/app-shell/src/views/studio-design/navSurface.ts diff --git a/.changeset/studio-interfaces-action-nav-surface.md b/.changeset/studio-interfaces-action-nav-surface.md new file mode 100644 index 0000000000..7f6ee7c467 --- /dev/null +++ b/.changeset/studio-interfaces-action-nav-surface.md @@ -0,0 +1,34 @@ +--- +'@object-ui/app-shell': patch +--- + +The Studio Interfaces rail opens `action` nav entries instead of greying them out. + +The Interfaces pillar's rail is the current package's App `navigation` tree, and +each leaf opens the design surface of whatever it binds to. `resolveSurface` +bound five shapes — `page`, `object`, `dashboard`, `report`, `view` — and not +`action`, so an action entry rendered `disabled`: visible in the designer, 40% +opacity, inert on click. The same entry works in the shipped product, where +`NavigationRenderer` renders it and `useNavActionDispatch` resolves and executes +it (framework#4509), and `action` has carried both a registered preview +(`ActionPreview`) and a registered default inspector (`ActionDefaultInspector`) +the whole time. Only the binding was missing, so the one nav variant naming an +authorable metadata item was the one variant the designer could not author. + +Clicking such an entry now opens the action on the standard surface — the +`ActionPreview` canvas plus the action's inspector — and draft-saves through the +same generic path as the pillar's other leaves. + +Scope, stated because the neighbours are deliberately untouched: `url`, +`separator` and `component` leaves stay unresolvable, and are not a gap — an +external link, a divider, and a first-party UI shipped in code have no metadata +item to design. Object-scoped actions keep their existing home, the object's +Actions tab: `ActionNavItemSchema` is strict `{ actionName, params? }` with no +`objectName`, so a nav action is a global action by construction and this path +cannot reach an object-scoped one. + +`actionDef.actionName` is read as the only spelling. The spec answers `action` / +`name` / `args` / `input` there with a named rejection rather than accepting +them (objectstack#4001, measured on spec 17.0.0-rc.6), and a tolerant read here +would re-open exactly what that closed: an entry that dispatches an action its +author did not declare. diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.interfacesAction.test.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.interfacesAction.test.tsx new file mode 100644 index 0000000000..f24f89a6a1 --- /dev/null +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.interfacesAction.test.tsx @@ -0,0 +1,127 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Acceptance for objectui#4019's `action` half, at the pillar level: the + * Studio Interfaces rail must LIST an action nav entry and OPEN it on the + * standard design surface. + * + * Before this, the entry rendered — the rail walks every nav leaf — but with + * `disabled` set, because `resolveSurface` had no `action` case. So the one + * nav variant that names an authorable metadata item was the one the designer + * could not open, while the shipped sidebar rendered and dispatched it + * (framework#4509). The two assertions below are exactly those two verbs. + */ +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; + +const NAV = [ + { id: 'nav_home', type: 'page', label: 'Home', pageName: 'home' }, + { id: 'nav_run_sync', type: 'action', label: 'Run Sync', actionDef: { actionName: 'sync_now' } }, +]; + +/** Spec-valid (`ActionSchema`, 17.0.0-rc.6) — the item the rail must open. */ +const ACTION = { + name: 'sync_now', + label: 'Sync Now', + type: 'script', + target: 'sync', + locations: ['list_toolbar'], +}; + +const mockClient = { + list: vi.fn(async (type: string) => + type === 'app' ? [{ name: 'acme_app', label: 'Acme' }] : [], + ), + listDrafts: vi.fn(async () => []), + layered: vi.fn(async (type: string, name: string) => { + if (type === 'app') return { effective: { name: 'acme_app', label: 'Acme', navigation: NAV } }; + if (type === 'action' && name === 'sync_now') return { effective: ACTION }; + return { effective: { name, kind: 'blocks', blocks: [] } }; + }), + getDraft: vi.fn(async () => null), + save: vi.fn(async () => ({})), + get: vi.fn(async () => undefined), +}; + +vi.mock('../metadata-admin/useMetadata', async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + useMetadataClient: () => mockClient, + useMetadataTypes: () => ({ entries: [] }), + }; +}); + +vi.mock('./packages-io', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, fetchPackages: vi.fn(async () => []) }; +}); + +vi.mock('@object-ui/react', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, useAdapter: () => ({}) }; +}); + +import { InterfacesPillar } from './StudioDesignSurface'; +import { registerMetadataPreview } from '../metadata-admin/preview-registry'; +import { ActionPreview } from '../metadata-admin/previews/ActionPreview'; + +// Registered directly rather than via `registerBuiltinPreviews()` — the barrel +// pulls every preview module (flow canvas, dashboard, report…) into this file's +// graph for one canvas. Same trade-off, and same precedent, as +// `EmbeddedItemEditor.preview.test.tsx`. That the barrel itself still carries +// the `action` line is the pre-existing registration this card builds on +// (`previews/index.ts`), not something this test can drift from. +// +// The right-hand inspector is deliberately not asserted here: `action`'s +// default inspector was already registered and already has its own coverage +// (`inspectors/ActionDefaultInspector.celGate.test.tsx`); mounting it would +// drag the CEL condition-builder tree into a rail test. +registerMetadataPreview('action', ActionPreview); + +afterEach(cleanup); + +function renderPillar() { + return render( + + + , + ); +} + +describe('Interfaces pillar — action nav entries (objectui#4019)', () => { + it('lists the action entry as an ENABLED rail item', async () => { + renderPillar(); + + const entry = await screen.findByTitle('action · sync_now'); + expect(entry).toBeInTheDocument(); + // The verb that was broken: the rail renders every leaf, but an + // unresolvable one is `disabled` and cannot be opened. + expect(entry).toBeEnabled(); + }); + + it('opens it on the standard design surface when clicked', async () => { + renderPillar(); + + // The first resolvable leaf (`Home`) auto-opens, so this also proves the + // rail SWITCHES to the action rather than merely defaulting to it. + const entry = await screen.findByTitle('action · sync_now'); + fireEvent.click(entry); + + // Canvas breadcrumb — the surface the pillar is now editing. Matched on + // the element's whole text: the breadcrumb is `{type} · {name}` in JSX, so + // it reaches the DOM as three sibling text nodes and a plain string query + // would never match it. + await waitFor(() => + expect( + screen.getAllByText((_content, el) => el?.tagName === 'SPAN' && el.textContent === 'action · sync_now'), + ).not.toHaveLength(0), + ); + // ...rendered by the registered `ActionPreview`, which draws the action's + // own label as the faux button an author is designing. + await waitFor(() => expect(screen.getAllByText('Sync Now').length).toBeGreaterThan(0)); + }); +}); diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx index e98c54ee36..4a7cae7af7 100644 --- a/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx @@ -87,6 +87,7 @@ import { import { SourcePageEditor } from '../metadata-admin/previews/SourcePageEditor'; import { formatMetadataError, formatPublishFailures, type PublishFailure } from './metadataError'; import { loadPackageSurfaces } from './packageSurfaces'; +import { resolveSurface, findSurfaceInTree, type NavNode, type Surface } from './navSurface'; import { useSurfaceDeepLink, resolveSurfaceDeepLink } from './useSurfaceDeepLink'; import { buildObjectSkeleton, buildFlowSkeleton, buildAppSkeleton, buildPermissionSkeleton } from './skeletons'; import { t, tFormat, useMetadataLocale } from '../metadata-admin/i18n'; @@ -129,33 +130,6 @@ const PILLARS: ReadonlyArray<{ key: string; label: string; Icon: LucideIcon }> = { key: 'access', label: 'Access', Icon: Shield }, ]; -interface Surface { - type: string; - name: string; - label: string; - /** Lucide icon name from the object's metadata (`icon` field); falls back per getIcon. */ - icon?: string; -} - -interface NavNode { - id?: string; - label?: string; - type?: string; - icon?: string; - children?: NavNode[]; - pageName?: string; - page?: string; - objectName?: string; - object?: string; - dashboardName?: string; - dashboard?: string; - reportName?: string; - report?: string; - viewName?: string; - view?: string; - [k: string]: unknown; -} - const KIND_ICON: Record = { group: Folder, page: FileText, @@ -163,53 +137,10 @@ const KIND_ICON: Record = { dashboard: LayoutDashboard, report: BarChart3, view: Table2, + action: MousePointer2, }; const navIcon = (type?: string): LucideIcon => KIND_ICON[type ?? ''] ?? Compass; -/** Resolve a leaf nav node → the surface {type,name} it binds to. */ -function resolveSurface(node: NavNode): Surface | null { - const label = String(node.label ?? ''); - switch (node.type) { - case 'page': - return node.pageName || node.page ? { type: 'page', name: String(node.pageName || node.page), label } : null; - case 'object': - return node.objectName || node.object - ? { type: 'object', name: String(node.objectName || node.object), label } - : null; - case 'dashboard': - return node.dashboardName || node.dashboard - ? { type: 'dashboard', name: String(node.dashboardName || node.dashboard), label } - : null; - case 'report': - return node.reportName || node.report - ? { type: 'report', name: String(node.reportName || node.report), label } - : null; - case 'view': - return node.viewName || node.view ? { type: 'view', name: String(node.viewName || node.view), label } : null; - default: - return null; - } -} - -/** - * Walk the nav tree for the leaf that binds to `{type,name}`, returning its - * resolved Surface (carrying the node's label so the canvas title / highlight - * match). Backs the `?surface=` deep-link restore — a shared URL only names - * the target, so we re-derive the label from the live tree. - */ -function findSurfaceInTree(nodes: NavNode[], target: { type: string; name: string }): Surface | null { - for (const node of nodes) { - if (node.type === 'group' || node.children?.length) { - const hit = findSurfaceInTree(node.children ?? [], target); - if (hit) return hit; - } else { - const s = resolveSurface(node); - if (s && s.type === target.type && s.name === target.name) return s; - } - } - return null; -} - /** Normalize the framework draft envelope `{ type, name, item }` → body | null. */ function extractDraftBody(resp: unknown): Record | null { if (!resp || typeof resp !== 'object') return null; @@ -1027,7 +958,7 @@ function StudioNavItemInspector({ ); } -function InterfacesPillar({ +export function InterfacesPillar({ packageId, publishNonce = 0, draftNonce = 0, diff --git a/packages/app-shell/src/views/studio-design/navSurface.test.ts b/packages/app-shell/src/views/studio-design/navSurface.test.ts new file mode 100644 index 0000000000..937cee848e --- /dev/null +++ b/packages/app-shell/src/views/studio-design/navSurface.test.ts @@ -0,0 +1,101 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Interfaces-pillar nav-leaf binding (objectui#4019). + * + * The gap this pins: an `action` nav item is a LIVE runtime surface — the + * shipped sidebar renders it and `useNavActionDispatch` executes it + * (framework#4509) — but the Studio Interfaces rail could not resolve it to a + * design surface, so the very same entry rendered permanently DISABLED in the + * designer while it worked in the running app. `action` has had both a + * registered preview (`ActionPreview`) and a registered default inspector + * (`ActionDefaultInspector`) all along; only this binding was missing. + * + * The fixtures are parsed against the spec's own `NavigationItemSchema` rather + * than asserted by eye, so this file cannot drift into pinning a shape the + * schema rejects (the phantom-rule trap): the positive fixture must be + * spec-VALID for the designer to be required to open it, and the rejected + * alias spelling must stay unresolvable here because the schema refuses it by + * name — Commandment #0.1, no second dialect in the consumer. + */ +import { describe, expect, it } from 'vitest'; +import { NavigationItemSchema } from '@objectstack/spec/ui'; +import { resolveSurface, findSurfaceInTree, type NavNode } from './navSurface'; + +/** A spec-valid global-action nav item. */ +const ACTION_NODE: NavNode = { + id: 'nav_run_sync', + type: 'action', + label: 'Run Sync', + actionDef: { actionName: 'sync_now' }, +}; + +describe('resolveSurface — action nav items (objectui#4019)', () => { + it('the fixture is a real authoring surface: the spec accepts it whole', () => { + const parsed = NavigationItemSchema.safeParse(ACTION_NODE); + expect(parsed.success).toBe(true); + }); + + it('binds an action nav leaf to the `action` design surface', () => { + expect(resolveSurface(ACTION_NODE)).toEqual({ + type: 'action', + name: 'sync_now', + label: 'Run Sync', + }); + }); + + it('leaves an action item with no actionName unresolved (stays disabled)', () => { + expect(resolveSurface({ id: 'nav_x', type: 'action', label: 'Nothing' })).toBeNull(); + expect(resolveSurface({ id: 'nav_x', type: 'action', label: 'Nothing', actionDef: {} })).toBeNull(); + }); + + it('reads the canonical key ONLY — a spelling the schema rejects stays unresolved', () => { + // `action` / `name` inside `actionDef` are REJECTED aliases carrying a + // redirect (objectstack#4001), not second spellings. Measured on + // spec 17.0.0-rc.6: `unrecognized_keys` on `actionDef` plus a missing + // `actionName`. A tolerant `??` limb here would resurrect exactly the bug + // #4001 closed — an entry that dispatches an action the author did not + // declare — so the designer must refuse what the schema refuses. + const aliasNode = { + id: 'nav_run_sync', + type: 'action', + label: 'Run Sync', + actionDef: { action: 'sync_now' }, + }; + const parsed = NavigationItemSchema.safeParse(aliasNode); + expect(parsed.success).toBe(false); + expect(resolveSurface(aliasNode as NavNode)).toBeNull(); + }); + + it('reaches an action leaf nested in a group (the `?surface=` deep-link path)', () => { + const tree: NavNode[] = [ + { id: 'g1', type: 'group', label: 'Ops', children: [ACTION_NODE] }, + ]; + expect(findSurfaceInTree(tree, { type: 'action', name: 'sync_now' })).toEqual({ + type: 'action', + name: 'sync_now', + label: 'Run Sync', + }); + }); +}); + +describe('resolveSurface — the variants around the new one are unchanged', () => { + it('still binds the surface-bearing leaves', () => { + expect(resolveSurface({ type: 'page', pageName: 'home', label: 'Home' })?.type).toBe('page'); + expect(resolveSurface({ type: 'object', objectName: 'crm_lead', label: 'Leads' })?.name).toBe('crm_lead'); + expect(resolveSurface({ type: 'dashboard', dashboardName: 'sales', label: 'Sales' })?.name).toBe('sales'); + expect(resolveSurface({ type: 'report', reportName: 'pipeline', label: 'Pipeline' })?.name).toBe('pipeline'); + }); + + it('leaves the variants with no authorable target unresolved', () => { + // Deliberately NOT openable — `url` points out of the product, `separator` + // is a divider, and `component` names a first-party UI shipped in code, so + // none of the three has a metadata item to design. Only `action` was a + // metadata type sitting in this bucket by omission. + expect(resolveSurface({ id: 'nav_docs', type: 'url', label: 'Docs', url: 'https://example.com' })).toBeNull(); + expect(resolveSurface({ id: 'nav_sep', type: 'separator' })).toBeNull(); + expect( + resolveSurface({ id: 'nav_dir', type: 'component', label: 'Directory', componentRef: 'metadata:directory' }), + ).toBeNull(); + }); +}); diff --git a/packages/app-shell/src/views/studio-design/navSurface.ts b/packages/app-shell/src/views/studio-design/navSurface.ts new file mode 100644 index 0000000000..917a24eb0f --- /dev/null +++ b/packages/app-shell/src/views/studio-design/navSurface.ts @@ -0,0 +1,115 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Nav-leaf → design-surface resolution for the Studio Interfaces pillar. + * + * The Interfaces pillar's rail is NOT a per-type list (the Data / Automations / + * Access rails are — `object` / `flow` / `permission`). It is the current + * package's App `navigation` tree: every leaf is rendered, and the one that + * `resolveSurface` can bind to a `{type,name}` opens that item's design surface + * (`getMetadataPreview` canvas + `getMetadataInspector` / default inspector). + * A leaf that resolves to `null` renders DISABLED — correct for the nav + * variants whose target is not an authorable metadata item (`url` points out of + * the product, `separator` is a divider, `component` names a first-party UI + * shipped in code), and a dead entry for any variant that does have one. + * + * Extracted from `StudioDesignSurface.tsx` so the binding is unit-testable + * without mounting the pillar — the same reason `packageSurfaces.ts` and + * `centerTab.ts` live beside it. + */ + +/** One rail entry / canvas target. */ +export interface Surface { + type: string; + name: string; + label: string; + /** Lucide icon name from the object's metadata (`icon` field); falls back per getIcon. */ + icon?: string; +} + +export interface NavNode { + id?: string; + label?: string; + type?: string; + icon?: string; + children?: NavNode[]; + pageName?: string; + page?: string; + objectName?: string; + object?: string; + dashboardName?: string; + dashboard?: string; + reportName?: string; + report?: string; + viewName?: string; + view?: string; + /** + * `ActionNavItemSchema.actionDef` — a `.strict()` object of exactly + * `{ actionName, params? }`. The spec answers `action` / `name` / `args` / + * `input` here as REJECTED spellings with a redirect (objectstack#4001), so + * they are authoring errors, never second spellings to read (Commandment + * #0.1): this type declares the canonical key alone. + */ + actionDef?: { actionName?: string; params?: Record }; + [k: string]: unknown; +} + +/** Resolve a leaf nav node → the surface {type,name} it binds to. */ +export function resolveSurface(node: NavNode): Surface | null { + const label = String(node.label ?? ''); + switch (node.type) { + case 'page': + return node.pageName || node.page ? { type: 'page', name: String(node.pageName || node.page), label } : null; + case 'object': + return node.objectName || node.object + ? { type: 'object', name: String(node.objectName || node.object), label } + : null; + case 'dashboard': + return node.dashboardName || node.dashboard + ? { type: 'dashboard', name: String(node.dashboardName || node.dashboard), label } + : null; + case 'report': + return node.reportName || node.report + ? { type: 'report', name: String(node.reportName || node.report), label } + : null; + case 'view': + return node.viewName || node.view ? { type: 'view', name: String(node.viewName || node.view), label } : null; + // A nav action is a GLOBAL action by construction: `ActionNavItemSchema` is + // `.strict()` with exactly `{ actionName, params? }` and carries no + // `objectName`, so an object-scoped action is not addressable from the nav + // (see `useNavActionDispatch`, which resolves the name against `action` + // metadata at click time). That makes this leaf the design-time half of a + // surface the running app already dispatches (framework#4509) — before + // this case it rendered permanently disabled in the rail while the same + // entry worked in the shipped sidebar. + // + // Object-scoped actions keep their own home, the object's Actions tab + // (`ObjectActionsPanel`, objectui#2330) — this case cannot reach them and + // must not try to. + case 'action': + return node.actionDef?.actionName + ? { type: 'action', name: String(node.actionDef.actionName), label } + : null; + default: + return null; + } +} + +/** + * Walk the nav tree for the leaf that binds to `{type,name}`, returning its + * resolved Surface (carrying the node's label so the canvas title / highlight + * match). Backs the `?surface=` deep-link restore — a shared URL only names + * the target, so we re-derive the label from the live tree. + */ +export function findSurfaceInTree(nodes: NavNode[], target: { type: string; name: string }): Surface | null { + for (const node of nodes) { + if (node.type === 'group' || node.children?.length) { + const hit = findSurfaceInTree(node.children ?? [], target); + if (hit) return hit; + } else { + const s = resolveSurface(node); + if (s && s.type === target.type && s.name === target.name) return s; + } + } + return null; +}