diff --git a/.changeset/11166-page-header-breadcrumb-slot-retired.md b/.changeset/11166-page-header-breadcrumb-slot-retired.md new file mode 100644 index 0000000000..b3583c8fe8 --- /dev/null +++ b/.changeset/11166-page-header-breadcrumb-slot-retired.md @@ -0,0 +1,23 @@ +--- +'@object-ui/components': patch +--- + +`page:header` no longer draws an empty breadcrumb slot, and an authored `breadcrumb` is +ignored (objectui#11166). + +The renderer used to read `breadcrumb` (the spec's `PageHeaderProps.breadcrumb`, default +`true`) and draw an empty `data-page-breadcrumb-slot` div above the title, in both the bare +and the record-chip layout. Nothing ever filled it, and the console's app header already +draws the breadcrumb trail, so the key is being retired rather than wired up; the spec half is +objectstack#20758. Until that retirement reaches this package's `@objectstack/spec` pin the +contract still accepts the key, so an authored `breadcrumb` (`true` or `false`) renders +exactly like an absent one: no slot and no error. No trail is drawn in its place. + +**Visible change: the title moves up.** The empty div still took space. In the bare layout +it was an item of the root's `gap-2` column, so the title row sat one `gap-2` (0.5rem) below +the top of the header; in the record-chip layout it carried `mb-1` (0.25rem) above the chip. +Both are gone on every header that did not already set `breadcrumb: false`. No class string +on the header changed. + +The `page:header` registration keeps declaring the `breadcrumb` input while the contract +accepts the key, with a description that says it is ignored. diff --git a/content/docs/guide/layout.md b/content/docs/guide/layout.md index a5e09b0251..4089ed7d8f 100644 --- a/content/docs/guide/layout.md +++ b/content/docs/guide/layout.md @@ -13,7 +13,7 @@ The layout system provides: - **AppShell** - Full application container with a top navbar, sidebar, and content areas - **Page** - Individual page wrapper with header and body -- **PageHeader** - Consistent page headers with title, breadcrumbs, and actions +- **PageHeader** - Consistent page headers with title and actions - **SidebarNav** - Navigation sidebar with menu items ## Installation @@ -199,8 +199,8 @@ and `button.tsx`, which reads `schema.label`, renders a button with no text. > the array rather than refusing it — the same silent-accept shape as `actions`, retired > under the same ADR-0049 enforce-or-remove gate. The trail is a **node**, not a key: put > a `breadcrumb` node in `children`, as [Breadcrumbs for Deep Navigation](#2-breadcrumbs-for-deep-navigation) -> shows. ⛔ Not the `page:header` block's `breadcrumb` either — that one is singular and a -> **boolean** display toggle, not a list of links. +> shows. ⛔ Not the `page:header` block's `breadcrumb` either — that one is a singular +> **boolean** the renderer ignores, not a list of links. ### Schema API @@ -300,7 +300,6 @@ does not render. title?: string | Record, // {field.path} tokens interpolated; derived from the record when omitted on a record page subtitle?: string | Record, // secondary line; same interpolation actions?: string[], // action ids, resolved against the object's own actions - breadcrumb?: boolean, // default true — the breadcrumb slot above the title recordChrome?: boolean, // default true — the record chip on a record page showStar?: boolean, // default true showCopyId?: boolean, // default true @@ -318,9 +317,9 @@ does not render. > header's identity comes from the record chip (`recordChrome`), and each action carries its > own `icon`. -> **There is no `breadcrumbs` array.** The node's `breadcrumb` is singular and a -> **boolean** display toggle for the slot above the title, not a list of links. A trail of -> links is a `breadcrumb` node — see +> **There is no `breadcrumbs` array, and `breadcrumb` draws nothing.** The contract still +> accepts a singular **boolean** `breadcrumb` until objectstack#20758 retires it; the renderer +> ignores it. A trail of links is a `breadcrumb` node — see > [Breadcrumbs for Deep Navigation](#2-breadcrumbs-for-deep-navigation). > **`page-header` is an alias, not the author key.** `page-header` (and its namespaced form @@ -698,7 +697,7 @@ ellipsis while keeping the first crumb and the current page. See the ⛔ Not `"breadcrumbs"` on the `page` node — that key has no reader and is refused by name (objectui#8871), the same way `actions` is. ⛔ Nor the `page:header` block's `breadcrumb`, -which is a **boolean** display toggle rather than a list of links. +a **boolean** the renderer ignores rather than a list of links. ### 3. Action Buttons at the Top of the Body diff --git a/content/docs/layout/page-header.mdx b/content/docs/layout/page-header.mdx index 696fe2a641..1e3fd0db8e 100644 --- a/content/docs/layout/page-header.mdx +++ b/content/docs/layout/page-header.mdx @@ -45,7 +45,6 @@ absent. | `title` | string, or an inline locale map | derived from the record on a record page | The heading. `{field.path}` tokens are interpolated against the record in scope. | | `subtitle` | string, or an inline locale map | — | The secondary line under the title. Same interpolation as `title`. | | `actions` | array of action ids | — | Names of actions declared on the object's own metadata, drawn as the header's button row. | -| `breadcrumb` | boolean | `true` (*spec*) | Renders a breadcrumb slot above the title. `false` removes it. | | `recordChrome` | boolean | `true` (*spec*) | On a record page, renders the title as the record chip. `false` keeps the bare heading. | | `showStar` | boolean | `true` (*spec*) | The follow star beside the record title. Part of the record chip. | | `showCopyId` | boolean | `true` (*spec*) | The copy-record-id button beside the record title. Part of the record chip. | @@ -68,8 +67,8 @@ renders none of them and says so once per id in the console. That is why the dem carries no actions. Placement (`locations: ['record_header']` or `record_more`), ordering and the overflow menu are covered in [Slotted pages](/docs/guide/slotted-pages). -**`breadcrumb`** renders an empty slot element, marked `data-page-breadcrumb-slot`, above -the title. The renderer draws no trail into it itself. +**`breadcrumb` draws nothing:** the contract accepts it until objectstack#20758 retires it, +and the renderer ignores it, so `true`, `false` and an absent key render the same header. **`maxVisible` / `mobileMaxVisible`** must be positive integers: the contract rejects `0` and fractional values, and the renderer falls back to its default for a value the contract @@ -146,7 +145,6 @@ The renderer picks one of two layouts when it renders: ```plaintext ┌──────────────────────────────────────────────┐ -│ (breadcrumb slot) │ │ Title [Actions] [⋯] │ │ Subtitle │ └──────────────────────────────────────────────┘ diff --git a/packages/components/src/__tests__/page-header-breadcrumb-ignored-11166.test.tsx b/packages/components/src/__tests__/page-header-breadcrumb-ignored-11166.test.tsx new file mode 100644 index 0000000000..5453df65b7 --- /dev/null +++ b/packages/components/src/__tests__/page-header-breadcrumb-ignored-11166.test.tsx @@ -0,0 +1,154 @@ +/** + * 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` draws no breadcrumb slot, and an authored `breadcrumb` is + * ignored (objectui#11166, triage ruling RETIRE). + * + * Until this change the renderer read `breadcrumb` (spec default `true`) and + * drew an empty `data-page-breadcrumb-slot` div above the title in both + * layouts. Nothing ever filled it, and the console's app header already draws + * the trail, so the ruling retires the key rather than adding a second trail. + * The spec half is objectstack#20758. Until that retirement reaches the + * installed pin the contract still ACCEPTS the key, so an authored value has to + * keep rendering without error: it is ignored, which is the old behaviour + * minus the empty div. + * + * Pinned in both layouts, through the real `SchemaRenderer` and registry: + * + * - `breadcrumb: true`, in `properties` and on the node, renders no slot; + * - the key ABSENT renders no slot either, and is the control: every authored + * spelling, `false` included, must render the SAME header as the absent + * key. Before this change `true` and the absent key drew the slot and + * `false` did not, so each row here was red; + * - the header is really there: its `

` and the empty `actions` slot + * (`data-page-actions-slot`, which the `page-header-actions` suites own) + * render, so "no breadcrumb slot" cannot pass on a header that drew + * nothing. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import { ActionProvider, RecordContextProvider, SchemaRenderer } from '@object-ui/react'; +// Registers every renderer at module scope, not in a hook +// (object-ui/no-dynamic-import-in-test-hook, objectui#3010). +import '../renderers'; + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +const RECORD_OBJECT = { + name: 'crm_account', + label: 'Account', + nameField: 'name', + fields: { name: { type: 'text', label: 'Name' } }, +}; + +type Layout = 'bare' | 'record'; + +function renderHeader(layout: Layout, schema: Record) { + const body = ; + const utils = render( + + {layout === 'record' ? ( + + {body} + + ) : ( + body + )} + , + ); + const header = utils.container.querySelector('header'); + expect(header, `${layout}: the page:header root`).not.toBeNull(); + return { ...utils, header: header as HTMLElement }; +} + +/** + * The header's markup with React's generated ids blanked. Two separate mounts + * number `useId` differently, and those ids are the only bytes allowed to + * differ between an authored `breadcrumb` and an absent one. + */ +const ID_ATTRS = ['id', 'aria-controls', 'aria-labelledby', 'aria-describedby', 'for']; +function markupOf(header: HTMLElement): string { + const copy = header.cloneNode(true) as HTMLElement; + for (const el of [copy, ...Array.from(copy.querySelectorAll('*'))]) { + for (const name of ID_ATTRS) if (el.hasAttribute(name)) el.setAttribute(name, 'ID'); + } + return copy.outerHTML; +} + +const TITLE: Record = { bare: 'Orders', record: 'Acme' }; + +/** The authored spellings, each rendered against the absent-key control. */ +const AUTHORED: ReadonlyArray<{ name: string; node: (layout: Layout) => Record }> = [ + { + name: '`properties.breadcrumb: true`', + node: (layout) => ({ + type: 'page:header', + properties: layout === 'bare' ? { title: TITLE.bare, breadcrumb: true } : { breadcrumb: true }, + }), + }, + { + name: '`breadcrumb: true` on the node', + node: (layout) => ({ + type: 'page:header', + breadcrumb: true, + ...(layout === 'bare' ? { properties: { title: TITLE.bare } } : {}), + }), + }, + { + name: '`properties.breadcrumb: false`', + node: (layout) => ({ + type: 'page:header', + properties: layout === 'bare' ? { title: TITLE.bare, breadcrumb: false } : { breadcrumb: false }, + }), + }, +]; + +const ABSENT = (layout: Layout) => ({ + type: 'page:header', + ...(layout === 'bare' ? { properties: { title: TITLE.bare } } : {}), +}); + +describe('page:header draws no breadcrumb slot; an authored `breadcrumb` is ignored (objectui#11166)', () => { + for (const layout of ['bare', 'record'] as const) { + describe(`${layout} layout`, () => { + it('CONTROL — the key absent: the header renders, with no breadcrumb slot', () => { + const { header } = renderHeader(layout, ABSENT(layout)); + expect(header.querySelector('h1')?.textContent).toBe(TITLE[layout]); + expect(header.querySelectorAll('[data-page-actions-slot]')).toHaveLength(1); + expect(header.querySelector('[data-page-breadcrumb-slot]')).toBeNull(); + }); + + for (const row of AUTHORED) { + it(`${row.name} renders no slot and the same header as the absent key, without an error`, () => { + const errors = vi.spyOn(console, 'error'); + const { header } = renderHeader(layout, row.node(layout)); + expect(header.querySelector('h1')?.textContent).toBe(TITLE[layout]); + expect(header.querySelector('[data-page-breadcrumb-slot]')).toBeNull(); + // No slot, and no stray attribute either: the key reaches no element. + expect(header.outerHTML).not.toMatch(/breadcrumb/i); + expect(errors).not.toHaveBeenCalled(); + const authored = markupOf(header); + cleanup(); + + const { header: control } = renderHeader(layout, ABSENT(layout)); + expect(authored).toBe(markupOf(control)); + }); + } + }); + } +}); diff --git a/packages/components/src/renderers/layout/containers.tsx b/packages/components/src/renderers/layout/containers.tsx index de41fee223..c07608e4cc 100644 --- a/packages/components/src/renderers/layout/containers.tsx +++ b/packages/components/src/renderers/layout/containers.tsx @@ -1283,7 +1283,7 @@ ComponentRegistry.register('section', PageSectionRenderer, { }); // --------------------------------------------------------------------------- -// page:header — title row + optional subtitle + breadcrumb/action slots. +// page:header — title row + optional subtitle + action slot. // `actions` entries are ACTION IDS, resolved against the object's own metadata // (objectstack#11592 ruling, objectui#6252) — see `resolvedHeaderActions`. // --------------------------------------------------------------------------- @@ -1505,7 +1505,12 @@ const PageHeaderRenderer: React.FC = ({ schema, className, ...props }) => { fieldOptionLabel, headerObjectName, ); - const breadcrumb = (schema?.breadcrumb ?? schema?.properties?.breadcrumb) !== false; + // `breadcrumb` is deliberately NOT read (objectui#11166, ruling RETIRE). All + // it ever drew was an empty `data-page-breadcrumb-slot` div that nothing + // filled, and the console's app header already draws the trail. The spec + // still accepts `PageHeaderProps.breadcrumb` until objectstack#20758 retires + // it, so an authored value renders exactly as an absent one: ignored, no + // error. ⛔ Do not add a trail here; that is an ENFORCE ruling, not a fix. // Schema-level opt-outs let authors keep the historic "bare h1" header // when they don't want a record chip (e.g. a non-record landing page). @@ -2340,12 +2345,6 @@ const PageHeaderRenderer: React.FC = ({ schema, className, ...props }) => { {...headerAria} >
- {breadcrumb && ( -
- )} = ({ schema, className, ...props }) => { {...designer} {...headerAria} > - {breadcrumb && ( -
- )}
{explicitTitle && ( @@ -2413,7 +2409,14 @@ ComponentRegistry.register('header', PageHeaderRenderer, { { name: 'title', type: ['string', 'object'], description: 'Supports {field} interpolation and inline translation maps; falls back to the record title' }, { name: 'subtitle', type: ['string', 'object'], description: 'Same interpolation as Title' }, { name: 'actions', type: 'array', of: 'string', 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' }, + // Declared but NOT read (objectui#11166). It stays in `inputs` only while + // the installed spec still accepts the key: the reverse direction of + // `registry-inputs-spec-parity.test.ts` asks for every accepted spec key, + // and the manifest would otherwise warn `unknown-prop` on a value the + // contract accepts. It leaves with the spec retirement (objectstack#20758): + // once the pin carries the tombstone, this entry fails the forward + // direction. The description tells an author reading the manifest it is inert. + { name: 'breadcrumb', type: 'boolean', description: 'Ignored: the header draws no breadcrumb, and the key is being retired from the contract. Leave it out.' }, { name: 'recordChrome', type: 'boolean', description: 'Set false for the bare h1 header on non-record pages' }, { name: 'showStar', type: 'boolean' }, { name: 'showCopyId', type: 'boolean' },