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 new file mode 100644 index 0000000000..d1715db6ac --- /dev/null +++ b/packages/components/src/__tests__/page-header-action-ids.test.tsx @@ -0,0 +1,306 @@ +/** + * 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; + /** 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(() => {}); + }); + 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 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 + // 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(warnMessages().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',