From 5b6a8339aa37d979bc14c410be1f024ea10a6a9c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 09:24:24 +0000 Subject: [PATCH 1/2] fix(components): page:header resolves declared action ids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `@objectstack/spec`'s `PageHeaderProps.actions` is `z.array(z.string())` — "Action IDs to show in header" — but the canonical `page:header` renderer read the array as ActionDef objects and resolved nothing, so metadata satisfying the published contract rendered zero header buttons. Resolve ids against the object's own metadata at the top of the actions pipeline, through the same `useMetadataItem` entry `record:quick_actions` uses, so the existing placement / capability / visible / order chain runs unchanged over uniformly-shaped defs. Inline objects keep working as renderer tolerance for the transition, per element, so a half-migrated array resolves. An id that resolves to nothing warns once and renders nothing, naming the object's declared action names. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM --- .../__tests__/page-header-action-ids.test.tsx | 304 ++++++++++++++++++ .../src/renderers/layout/containers.tsx | 164 +++++++++- vitest.config.mts | 1 + 3 files changed, 461 insertions(+), 8 deletions(-) create mode 100644 packages/components/src/__tests__/page-header-action-ids.test.tsx diff --git a/packages/components/src/__tests__/page-header-action-ids.test.tsx b/packages/components/src/__tests__/page-header-action-ids.test.tsx new file mode 100644 index 0000000000..7b6582a798 --- /dev/null +++ b/packages/components/src/__tests__/page-header-action-ids.test.tsx @@ -0,0 +1,304 @@ +/** + * 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. + */ + +/** + * `page:header` resolves `actions` as ACTION IDS (objectui#6252, implementing + * the objectstack#11592 ruling — maintainer, 2026-08-25, 「全部同意」 on + * recommendation B). + * + * `@objectstack/spec`'s `PageHeaderProps.actions` is + * `z.array(z.string()).describe('Action IDs to show in header')`. This renderer + * used to read that array as `ActionDef` OBJECTS and resolve nothing, so + * metadata satisfying the published contract rendered ZERO buttons — the defect + * the parent issue reports. + * + * The shape of the proof is deliberate: the same action metadata is authored + * TWICE — once as ids, once as the inline objects authors write today — and the + * two renders are compared. That is what makes "renders the same buttons" a + * measurement rather than a restatement, and the object-shape render is the + * LIVE CONTROL: every equivalence case asserts it is non-empty first, so an + * id-side green can never come from two empty headers agreeing. + * + * Population covers each filter the acceptance criterion names, so one + * equivalence assertion measures all of them at once: + * - `actionRendersAt` placement — `list_only` (list_item) must not render; + * `archive` (record_more) must land in the ⋯ menu, not inline + * - `requiredPermissions` — `gated` is denied for this user + * - `visible` — `closed_only` is a CEL predicate that is false for this record + * - `order` — `qualify` (order 1) renders BEFORE `convert` (order 2), which is + * the reverse of the order both authorings list them in + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { ComponentRegistry } from '@object-ui/core'; +import { ActionProvider, MetadataCtx, RecordContextProvider } from '@object-ui/react'; +import type { MetadataContextValue } from '@object-ui/react'; + +/** + * A script body on one of the resolved actions. The id path must never carry + * this into anything serialized — see the `body.source` case at the bottom. + */ +const BODY_MARKER = 'OS6252_HANDLER_BODY_MARKER'; + +const ACTIONS: Record = { + convert: { name: 'convert', label: 'Convert Lead', type: 'flow', locations: ['record_header'], order: 2 }, + qualify: { name: 'qualify', label: 'Qualify', type: 'api', locations: ['record_header'], order: 1 }, + scripted: { + name: 'scripted', + label: 'Run Script', + type: 'script', + locations: ['record_header'], + order: 3, + body: { language: 'js', source: `return { marker: '${BODY_MARKER}' };` }, + }, + archive: { name: 'archive', label: 'Archive', type: 'api', locations: ['record_more'] }, + list_only: { name: 'list_only', label: 'List Only', type: 'api', locations: ['list_item'] }, + gated: { + name: 'gated', + label: 'Gated Action', + type: 'api', + locations: ['record_header'], + requiredPermissions: ['nobody_holds_this'], + }, + closed_only: { + name: 'closed_only', + label: 'Closed Only', + type: 'api', + locations: ['record_header'], + visible: 'record.status == "closed"', + }, +}; + +/** Authoring order — deliberately NOT the rendered order (see `order` above). */ +const AUTHORED = ['convert', 'qualify', 'scripted', 'archive', 'list_only', 'gated', 'closed_only']; + +const OBJECT_META = { name: 'lead', label: 'Lead', actions: AUTHORED.map((n) => ACTIONS[n]) }; + +const RECORD = { id: 'rec-1', name: 'Ada', status: 'open' }; +const USER = { id: 'u1', systemPermissions: ['setup.access'] }; + +const getItem = vi.fn(async (type: string, name: string) => + type === 'object' && name === 'lead' ? OBJECT_META : null, +); + +/** + * Hand-rolled context value held at MODULE level on purpose: `getItem` is an + * effect dependency of `useMetadataItem`, so a value rebuilt per render spins + * that hook forever (the loop `NO_METADATA_PROVIDER` was frozen to fix). + */ +const METADATA: MetadataContextValue = { + apps: [], + objects: [OBJECT_META] as any, + dashboards: [], + reports: [], + pages: [], + loading: false, + error: null, + refresh: async () => {}, + invalidate: () => {}, + ensureType: async () => [], + getItem: getItem as unknown as MetadataContextValue['getItem'], + getItemsByType: () => [], + getTypeStatus: () => 'ready' as const, +}; + +function PageHeader({ schema }: { schema: any }) { + const Component = ComponentRegistry.get('page:header'); + if (!Component) throw new Error('page:header not registered'); + // eslint-disable-next-line react-hooks/static-components -- registry component is stable + return ; +} + +function mount(schema: any, metadata: MetadataContextValue = METADATA) { + return render( + + + + + + + , + ); +} + +/** + * Ordered accessible names of the buttons in the header's ACTION ROW. + * + * Scoped to `role="toolbar"` — the record chrome (the copy-id and follow-star + * buttons on the record chip) draws buttons of its own outside it, and those are + * not what any of this is about. An action row with nothing in it is not + * rendered at all, which reads here as `[]`. + */ +const buttonNames = (c: HTMLElement): string[] => { + const toolbar = c.querySelector('[role="toolbar"]'); + if (!toolbar) return []; + return Array.from(toolbar.querySelectorAll('button')).map( + (b) => (b.getAttribute('aria-label') || b.textContent || '').trim(), + ); +}; + +/** + * Structural projection of the rendered header, with the ids Radix mints per + * mount (`id` / `aria-controls` / `aria-labelledby` / `aria-describedby`, and + * the `:r0:`-style counters inside them) normalized away — those differ between + * ANY two mounts and say nothing about which buttons the header drew. + */ +const shape = (c: HTMLElement): string => + c.innerHTML + .replace(/\s(?:id|aria-controls|aria-labelledby|aria-describedby)="[^"]*"/g, '') + .replace(/:r[0-9a-z]+:/g, ':rN:'); + +const idAuthored = { type: 'page:header', title: 'Lead', actions: [...AUTHORED] }; +const objectAuthored = { type: 'page:header', title: 'Lead', actions: AUTHORED.map((n) => ACTIONS[n]) }; + +describe('page:header — declared action-id lookup (objectui#6252)', () => { + let warn: ReturnType; + + beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => { + warn.mockRestore(); + vi.clearAllMocks(); + }); + + it('renders the same buttons from ids as from the inline objects', async () => { + // Live control FIRST: the object-shape authoring must draw a real header. + // If this ever renders zero buttons the equivalence below measures nothing. + const objectRender = mount(objectAuthored); + const controlNames = buttonNames(objectRender.container); + expect(controlNames).toContain('Convert Lead'); + expect(controlNames.length).toBeGreaterThan(1); + const controlShape = shape(objectRender.container); + objectRender.unmount(); + + const idRender = mount(idAuthored); + // Resolution runs through `useMetadataItem`, which settles in an effect. + await screen.findByRole('button', { name: /Convert Lead/i }); + + expect(buttonNames(idRender.container)).toEqual(controlNames); + expect(shape(idRender.container)).toBe(controlShape); + }); + + it('honours order, placement, requiredPermissions and visible on the id path', async () => { + const { container } = mount(idAuthored); + await screen.findByRole('button', { name: /Convert Lead/i }); + const names = buttonNames(container); + + // `order`: qualify (1) before convert (2), the reverse of the authored order. + expect(names.indexOf('Qualify')).toBeLessThan(names.indexOf('Convert Lead')); + // `actionRendersAt`: list_item-only never renders on this surface. + expect(screen.queryByRole('button', { name: /List Only/i })).toBeNull(); + // `requiredPermissions`: denied for this user. + expect(screen.queryByRole('button', { name: /Gated Action/i })).toBeNull(); + // `visible`: the CEL predicate is false for a record with status "open". + expect(screen.queryByRole('button', { name: /Closed Only/i })).toBeNull(); + // record_more is routed to the ⋯ overflow menu, never an inline slot. + expect(screen.queryByRole('button', { name: /^Archive$/i })).toBeNull(); + }); + + it('routes a record_more id into the overflow menu, never an inline slot', async () => { + // Same evidence the sibling `record_more` pins in `page-header-actions.test.tsx` + // take: the ⋯ trigger exists and the action is NOT among the inline buttons. + // (The menu's own contents live behind a Radix portal that opens on + // pointerdown; no test in this repo drives it open, and the routing claim is + // fully carried by these two facts plus the equivalence case above.) + const { container } = mount(idAuthored); + await screen.findByRole('button', { name: /Convert Lead/i }); + expect(screen.getByRole('button', { name: /More actions/i })).toBeTruthy(); + expect(buttonNames(container)).not.toContain('Archive'); + }); + + it('renders nothing but says so once when an id resolves to no action', async () => { + const { container } = mount({ + type: 'page:header', + title: 'Lead', + actions: ['convert', 'covert_lead'], // second one is a typo of the first + }); + // The control: the sibling id still resolves, so "nothing rendered" is about + // the unresolvable id and not about a lookup that failed wholesale. + await screen.findByRole('button', { name: /Convert Lead/i }); + expect(buttonNames(container)).toEqual(['Convert Lead']); + + await waitFor(() => expect(warn).toHaveBeenCalled()); + const messages = warn.mock.calls.map((c) => String(c[0])); + const hits = messages.filter((m) => m.includes('covert_lead')); + expect(hits.length).toBe(1); + expect(hits[0]).toContain('[page:header]'); + // The message names what the object DOES declare — the fix is usually one + // of them. + expect(hits[0]).toContain('convert'); + }); + + it('does not warn while the metadata lookup is still in flight', async () => { + // A provider whose read never settles: `loading` stays true, so no id has + // been shown to be unresolvable yet. Warning here would fire on the first + // paint of every correctly-authored page. + const pending: MetadataContextValue = { + ...METADATA, + getItem: (() => new Promise(() => {})) as unknown as MetadataContextValue['getItem'], + }; + const { container } = mount({ type: 'page:header', title: 'Lead', actions: ['convert'] }, pending); + await Promise.resolve(); + await waitFor(() => expect(container.querySelector('h1, [data-page-actions-slot]')).toBeTruthy()); + expect(buttonNames(container)).toEqual([]); + expect(warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes('[page:header] action id'))).toEqual([]); + }); + + it('keeps the inline object shape working during the transition, including mixed arrays', async () => { + const { container } = mount({ + type: 'page:header', + title: 'Lead', + actions: ['convert', { name: 'adhoc', label: 'Ad Hoc', type: 'api', locations: ['record_header'] }], + }); + await screen.findByRole('button', { name: /Convert Lead/i }); + // Both render, and they are ordered by the SAME `order` rule the chain has + // always applied — the inline def declares none (0), `convert` declares 2 — + // which is the point: resolution happens above the filter chain, so a mixed + // array is one population, not two. + expect(buttonNames(container)).toEqual(['Ad Hoc', 'Convert Lead']); + }); + + it('resolves ids authored under the spec-bridge `properties.actions` spelling', async () => { + mount({ type: 'page:header', properties: { title: 'Lead', actions: ['convert'] } }); + expect(await screen.findByRole('button', { name: /Convert Lead/i })).toBeTruthy(); + }); + + /** + * Acceptance criterion 3 — "the id path carries no `body.source`". + * + * The authored node is what a page build serializes. Resolution must stay a + * READ: nothing may write the resolved def (and with it the action's script + * body) back onto the node. The object-shape authoring is the live control and + * it fails this by construction — that is the whole point of the ids ruling, + * and it is what proves the assertion below can fail at all. + */ + it('never writes a resolved def — and so no body.source — back onto the authored node', async () => { + const authored = { type: 'page:header', title: 'Lead', actions: ['convert', 'scripted'] }; + const before = JSON.stringify(authored); + expect(before).not.toContain(BODY_MARKER); + + const { container } = mount(authored); + await screen.findByRole('button', { name: /Run Script/i }); + + expect(JSON.stringify(authored)).toBe(before); + expect(container.innerHTML).not.toContain(BODY_MARKER); + + // Live control: the same action, authored inline, DOES carry its handler + // body into the serialized node. + expect(JSON.stringify(objectAuthored)).toContain(BODY_MARKER); + }); +}); diff --git a/packages/components/src/renderers/layout/containers.tsx b/packages/components/src/renderers/layout/containers.tsx index 6b68aa2a4f..3dafeb9442 100644 --- a/packages/components/src/renderers/layout/containers.tsx +++ b/packages/components/src/renderers/layout/containers.tsx @@ -22,7 +22,7 @@ import React from 'react'; import { ComponentRegistry, ExpressionEvaluator, evalRowPredicate, getRecordDisplayName, toPredicateRecord } from '@object-ui/core'; import type { ComponentInput } from '@object-ui/core'; import { actionRendersAt } from '@object-ui/types'; -import { useRecordContext, useAction, useCapabilityGate, usePredicateScope, usePageVariables, useInlineEdit, useActionTextLocalizer, reportUnresolvableVisibilityPredicate } from '@object-ui/react'; +import { useRecordContext, useAction, useCapabilityGate, usePredicateScope, usePageVariables, useInlineEdit, useActionTextLocalizer, useMetadataItem, reportUnresolvableVisibilityPredicate } from '@object-ui/react'; import { renderChildren, cn } from '../../lib/utils'; import { LazyIcon } from '../../lib/lazy-icon'; import { RelatedCountStore, useRelatedCountVersion } from '../../hooks/related-count-store'; @@ -995,8 +995,8 @@ ComponentRegistry.register('section', PageSectionRenderer, { // --------------------------------------------------------------------------- // page:header — title row + optional subtitle + breadcrumb/action slots. -// Action ids are intentionally not resolved here; that will land alongside -// the upcoming `record:quick_actions` renderer. +// `actions` entries are ACTION IDS, resolved against the object's own metadata +// (objectstack#11592 ruling, objectui#6252) — see `resolvedHeaderActions`. // --------------------------------------------------------------------------- /** @@ -1080,6 +1080,54 @@ function warnMissingRecordFields(name: unknown, source: string, record: unknown) ); } +/** + * Warn-once ledger for a declared header action id that resolved to nothing + * (objectui#6252). Keyed by `object::id::reason` so one page cannot spam the + * console across re-renders, mirroring `_warnedHeaderPredicates` above. + */ +const _warnedHeaderActionIds = new Set(); + +/** + * Report a `page:header.actions` id that the object's own metadata does not + * define. + * + * ⛔ Deliberately LOUD rather than a silent drop. A dropped id is invisible by + * construction — the header simply renders one button fewer, which is exactly + * what an author who mistyped `covert_lead` sees, and exactly what an author + * whose action is correctly gated by `visible` sees too. The silent-loss class + * this avoids is the one this repo keeps re-filing (objectui#7146/#7147/#7148); + * the resolution step is the only place that can still tell the two apart, so + * it says so here. + * + * The object's registered names are named in the message because the fix is + * almost always one of them. + */ +function warnUnresolvedHeaderActionId( + id: string, + objectName: string | undefined, + reason: 'no-object' | 'no-metadata' | 'not-found', + available: string[], +): void { + const key = `${objectName ?? ''}::${id}::${reason}`; + if (_warnedHeaderActionIds.has(key)) return; + _warnedHeaderActionIds.add(key); + const why = + reason === 'no-object' + ? 'this header is not bound to an object (no RecordContext `objectName`), so ids cannot be resolved at all' + : reason === 'no-metadata' + ? `no metadata could be read for object "${String(objectName)}" (no MetadataProvider in scope, or the object does not exist)` + : `object "${String(objectName)}" declares no action by that name. Declared: ${ + available.length > 0 ? available.join(', ') : '(none)' + }`; + console.warn( + `[page:header] action id "${id}" did not resolve — ${why}. ` + + 'Nothing is rendered for it. `PageHeaderProps.actions` is a list of ACTION IDS ' + + "(@objectstack/spec: `z.array(z.string())`, \"Action IDs to show in header\"), " + + "resolved against the object's own `actions` metadata the same way " + + '`record:quick_actions` resolves them.', + ); +} + const PageHeaderRenderer: React.FC = ({ schema, className, ...props }) => { const { designer } = splitDesignerProps(props); const ctx = useRecordContext(); @@ -1183,6 +1231,106 @@ const PageHeaderRenderer: React.FC = ({ schema, className, ...props }) => { // or by adding a name-clashing action of their own. const hostSystemActions = (ctx as any)?.headerSystemActions as any[] | undefined; + // ── Declared action IDS — the contract of record (objectstack#11592) ─────── + // + // `@objectstack/spec`'s `PageHeaderProps.actions` is + // `z.array(z.string()).describe('Action IDs to show in header')`, and has been + // for as long as the key has existed. This renderer read the array as + // `ActionDef` OBJECTS and resolved nothing, so metadata that satisfied the + // published contract rendered ZERO buttons here — the defect objectstack#11592 + // reports and its ruling (maintainer, 2026-08-25, 「全部同意」 on recommendation + // B) settles in favour of ids. Two sibling surfaces already implement that + // reading: `record:quick_actions` + // (`plugin-detail/src/renderers/record-quick-actions.tsx`) resolves a + // string-valued `actions` out of the object's own metadata, and + // `layout:page-header` reaches the same resolver by delegating its `actions` + // to a `record:quick_actions` node. + // + // Resolution happens HERE, at the top of the pipeline, so exactly ONE filter + // chain runs below: `actionRendersAt` placement, the capability gate, + // `visible` / `hidden` and `order` all see uniformly object-shaped defs and + // are untouched by this. That is also what makes the equivalence claim + // testable — an id-authored header and an object-authored header converge on + // the same array before a single filter runs. + // + // The object shape survives as RENDERER TOLERANCE for the transition (the + // card's licence: "keep the existing object-shape handling working during the + // transition if it is cheap"). It stays UNDECLARED — widening the spec's type + // to `string | ActionDef` would change what the contract accepts, which is a + // different (contract-review) change and is not this one. + // + // ⚠️ One deliberate difference from `record:quick_actions`: that renderer + // switches on the whole array (`rawActions.every(a => typeof a === 'string')`), + // this one normalizes PER ELEMENT, so a half-migrated `['convert', { … }]` + // resolves the id and passes the object through. Same mechanism (the object's + // `actions`, keyed by `name`), wider arity — a page mid-migration is exactly + // the state this card creates. + const headerActionIds = React.useMemo( + () => (Array.isArray(rawHeaderActions) + ? rawHeaderActions.filter((a: unknown): a is string => typeof a === 'string' && a.trim() !== '') + : []), + [rawHeaderActions], + ); + // `useMetadataItem` is the SAME entry `record:quick_actions` resolves through + // — not a second lookup path. Passing `null` for the name is its documented + // no-op, so a header with no ids (or no object bound) does not fetch. + const needsActionLookup = headerActionIds.length > 0 && !!headerObjectName; + const { item: headerActionsObjectMeta, loading: headerActionsMetaLoading } = useMetadataItem( + 'object', + needsActionLookup ? headerObjectName : null, + ); + const resolvedHeaderActions = React.useMemo(() => { + if (!Array.isArray(rawHeaderActions)) return []; + // No ids in the array — the object-shape path, byte-for-byte as before. + if (headerActionIds.length === 0) return rawHeaderActions; + // The lookup has not answered yet. Render the ids as nothing rather than as + // "unresolved" — the metadata read is in flight, and warning here would fire + // on every first paint. Same visible behaviour as `record:quick_actions`, + // which renders an empty `byName` map until its own `useMetadataItem` + // settles. + const settled = !needsActionLookup || !headerActionsMetaLoading; + const registered: any[] = Array.isArray(headerActionsObjectMeta?.actions) + ? (headerActionsObjectMeta as any).actions + : []; + // Keyed by `name`, the identity `record:quick_actions` keys on and the one + // the spec makes required — "id" in the spec's wording is the action's + // machine name, not a second key. One convention, not two. + const byName = new Map(); + for (const def of registered) { + const key = typeof def?.name === 'string' ? def.name : ''; + if (key && !byName.has(key)) byName.set(key, def); + } + const out: any[] = []; + for (const el of rawHeaderActions) { + if (typeof el !== 'string') { + out.push(el); // transition tolerance: an inline def passes through + continue; + } + const id = el.trim(); + if (id === '') continue; + const def = byName.get(id); + if (def) { + out.push(def); + continue; + } + if (!settled) continue; + warnUnresolvedHeaderActionId( + id, + headerObjectName, + !headerObjectName ? 'no-object' : (!headerActionsObjectMeta ? 'no-metadata' : 'not-found'), + [...byName.keys()], + ); + } + return out; + }, [ + rawHeaderActions, + headerActionIds, + headerActionsObjectMeta, + headerActionsMetaLoading, + needsActionLookup, + headerObjectName, + ]); + // ── Action predicates: ONE dialect, ONE entry (objectui#3521) ────────────── // // `visible` / `hidden` / `disabled` on a header action are evaluated by @@ -1351,9 +1499,9 @@ const PageHeaderRenderer: React.FC = ({ schema, className, ...props }) => { // (buildDefaultPageSchema.ts), so the leniency contradicted both. const placedOnHeader = (a: any): boolean => actionRendersAt(a, 'record_header') || actionRendersAt(a, 'record_more'); - const authored = Array.isArray(rawHeaderActions) - ? rawHeaderActions.filter(a => placedOnHeader(a) && filterAction(a)) - : []; + // `resolvedHeaderActions` — ids already resolved to defs, inline defs passed + // through — so this chain is shape-uniform and unchanged by objectui#6252. + const authored = resolvedHeaderActions.filter(a => placedOnHeader(a) && filterAction(a)); // Host-injected chrome (Edit / Share / Delete / Open-in-new-tab) is placed // by the HOST, not authored — so it is not location-filtered, matching // `action:bar`'s `systemActions` carve-out. Before #3142 this slot WAS @@ -1393,7 +1541,7 @@ const PageHeaderRenderer: React.FC = ({ schema, className, ...props }) => { // `evalHeaderPredicate` closes over `predicateScope` (via // `headerPredicateScope`) and the object's fields, so it replaces the raw // scope in this list rather than adding to it. - }, [rawHeaderActions, hostSystemActions, ctx?.data, evalHeaderPredicate]); + }, [resolvedHeaderActions, hostSystemActions, ctx?.data, evalHeaderPredicate]); /** * Localize the surviving actions ONCE, here, so the button text and the @@ -1828,7 +1976,7 @@ ComponentRegistry.register('header', PageHeaderRenderer, { // map form this description tells the author to write. { name: 'title', type: ['string', 'object'], label: 'Title', description: 'Supports {field} interpolation and inline translation maps; falls back to the record title' }, { name: 'subtitle', type: ['string', 'object'], label: 'Subtitle', description: 'Same interpolation as Title' }, - { name: 'actions', type: 'array', label: 'Actions', description: 'Action buttons rendered in the header, before any host-injected system actions' }, + { name: 'actions', type: 'array', label: 'Actions', description: "Action IDS — the names of actions declared on the object's own metadata — rendered in the header before any host-injected system actions. An id whose action declares neither record_header nor record_more in its locations renders nowhere." }, { name: 'breadcrumb', type: 'boolean', label: 'Breadcrumb', defaultValue: true }, { name: 'recordChrome', type: 'boolean', label: 'Record Chrome', defaultValue: true, description: 'Set false for the bare h1 header on non-record pages' }, { name: 'showStar', type: 'boolean', label: 'Show Follow Star', defaultValue: true }, diff --git a/vitest.config.mts b/vitest.config.mts index 96383af6bb..700423601d 100644 --- a/vitest.config.mts +++ b/vitest.config.mts @@ -108,6 +108,7 @@ const heavyDomTests = [ 'packages/components/src/__tests__/action-group.test.tsx', 'packages/components/src/__tests__/page-card-i18n-title.test.tsx', 'packages/components/src/__tests__/page-header-action-i18n.test.tsx', + 'packages/components/src/__tests__/page-header-action-ids.test.tsx', 'packages/components/src/__tests__/page-header-actions.test.tsx', 'packages/components/src/__tests__/page-header-capability-gate.test.tsx', 'packages/components/src/__tests__/page-header-lookup-predicate.test.tsx', From af40cf5c21f9b7c4775df41a6bc45f3ffbe3561f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 09:50:31 +0000 Subject: [PATCH 2/2] docs(slotted-pages): page:header.actions holds action ids Document the contract the renderer now honours: the header names actions by id and resolves them against the object's own metadata, with the inline-object form named as migration tolerance rather than a second declared shape. Adds the changeset and types the console-warning assertions in the new pin. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM --- .changeset/6252-page-header-action-ids.md | 35 ++++++++++++++ content/docs/guide/slotted-pages.md | 47 +++++++++++++++---- .../__tests__/page-header-action-ids.test.tsx | 8 ++-- 3 files changed, 79 insertions(+), 11 deletions(-) create mode 100644 .changeset/6252-page-header-action-ids.md diff --git a/.changeset/6252-page-header-action-ids.md b/.changeset/6252-page-header-action-ids.md new file mode 100644 index 0000000000..406c5c5422 --- /dev/null +++ b/.changeset/6252-page-header-action-ids.md @@ -0,0 +1,35 @@ +--- +'@object-ui/components': minor +--- + +`page:header` resolves its `actions` as declared ACTION IDS (objectui#6252, +implementing the objectstack#11592 ruling — maintainer, 2026-08-25, on +recommendation B). + +`@objectstack/spec`'s `PageHeaderProps.actions` is +`z.array(z.string()).describe('Action IDs to show in header')` and has been for +as long as the key has existed. The canonical renderer read that array as +`ActionDef` objects and resolved nothing, so a header authored the way the +published contract declares rendered **zero buttons** — satisfying the schema +deleted the header. Two sibling surfaces already read it as ids +(`record:quick_actions`, and `layout:page-header` by delegating to it), so one +authoring key meant two different things depending on which renderer drew the +header. + +Each id is now resolved against the object's own `actions` metadata through the +same `useMetadataItem` entry `record:quick_actions` uses. Resolution happens at +the top of the actions pipeline, so the existing chain — `record_header` / +`record_more` placement, the `requiredPermissions` capability gate, `visible` / +`hidden`, `order`, and the inline/overflow split — runs unchanged over +uniformly-shaped defs: an id-authored header and an object-authored one converge +before a single filter runs. + +- Inline `ActionDef` objects keep rendering, per element, so a half-migrated + `['convert', { … }]` array resolves the id and passes the object through. This + is renderer tolerance for the migration and stays undeclared — the contract is + ids. +- An id that resolves to no action renders nothing and warns **once**, naming + the object's declared action names. A mistyped id is no longer indistinguishable + from a correctly hidden one. +- Nothing is written back onto the authored node, so an id-authored page carries + no `ActionDef` — and no `body.source` handler body — into what it serializes. diff --git a/content/docs/guide/slotted-pages.md b/content/docs/guide/slotted-pages.md index f179735081..87c88046e9 100644 --- a/content/docs/guide/slotted-pages.md +++ b/content/docs/guide/slotted-pages.md @@ -103,9 +103,47 @@ Delete) as a button row. Up to **`maxVisible`** actions render inline, side by side (default **3** on desktop, **`mobileMaxVisible`**, default **1**, on mobile); the rest collapse into a `⋯` "More actions" menu. +### Naming the actions: `page:header.actions` holds IDS + +`PageHeaderProps.actions` is a list of **action ids** — the `name` of an +action declared on the object's own metadata. The header resolves each id +against that object, which is the same lookup `record:quick_actions` +performs, and keeps the definitions in exactly one place: change an action +once on the object and every page that names it follows. + +```ts +// on the object: the definitions +actions: [ + { name: 'convert_lead', label: 'Convert', locations: ['record_header'] }, + { name: 'export_pdf', label: 'Export', locations: ['record_more'] }, +] +``` + + +```ts +// on the page: the header names them +slots: { + header: { + type: 'page:header', + properties: { + title: '{name}', + actions: ['convert_lead', 'export_pdf'], + }, + }, +} +``` + +An id that names no action on the object renders nothing and says so once in +the console — a mistyped id is not a silently shorter header. + +> Inline `ActionDef` objects in this array still render, so pages written +> before the ids contract are not stranded. That is renderer tolerance for the +> migration, not a second declared shape: the contract is +> `z.array(z.string())`, and only ids are validated. + ### Declaring placement -An authored action renders here only if its **`locations`** declares +A named action renders here only if its **`locations`** declares `record_header` (inline) or `record_more` (straight into the `⋯` menu). There is no default: an action that declares no location renders in **no** located surface — not here, not the list toolbar, not the row menu. That @@ -114,13 +152,6 @@ one rule is shared by every surface that places actions by location metadata-admin toolbars and the action engine), so an action behaves the same wherever it is drawn. -```ts -actions: [ - { name: 'convert_lead', label: 'Convert', locations: ['record_header'] }, - { name: 'export_pdf', label: 'Export', locations: ['record_more'] }, -] -``` - Two placements come from somewhere other than `locations`, and neither needs an entry here: diff --git a/packages/components/src/__tests__/page-header-action-ids.test.tsx b/packages/components/src/__tests__/page-header-action-ids.test.tsx index 7b6582a798..d1715db6ac 100644 --- a/packages/components/src/__tests__/page-header-action-ids.test.tsx +++ b/packages/components/src/__tests__/page-header-action-ids.test.tsx @@ -166,6 +166,9 @@ const objectAuthored = { type: 'page:header', title: 'Lead', actions: AUTHORED.m describe('page:header — declared action-id lookup (objectui#6252)', () => { let warn: ReturnType; + /** Every string this render passed to `console.warn`, first argument only. */ + const warnMessages = (): string[] => + (warn.mock.calls as unknown[][]).map((c) => String(c[0])); beforeEach(() => { warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); @@ -234,8 +237,7 @@ describe('page:header — declared action-id lookup (objectui#6252)', () => { expect(buttonNames(container)).toEqual(['Convert Lead']); await waitFor(() => expect(warn).toHaveBeenCalled()); - const messages = warn.mock.calls.map((c) => String(c[0])); - const hits = messages.filter((m) => m.includes('covert_lead')); + const hits = warnMessages().filter((m) => m.includes('covert_lead')); expect(hits.length).toBe(1); expect(hits[0]).toContain('[page:header]'); // The message names what the object DOES declare — the fix is usually one @@ -255,7 +257,7 @@ describe('page:header — declared action-id lookup (objectui#6252)', () => { await Promise.resolve(); await waitFor(() => expect(container.querySelector('h1, [data-page-actions-slot]')).toBeTruthy()); expect(buttonNames(container)).toEqual([]); - expect(warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes('[page:header] action id'))).toEqual([]); + expect(warnMessages().filter((m) => m.includes('[page:header] action id'))).toEqual([]); }); it('keeps the inline object shape working during the transition, including mixed arrays', async () => {