Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .changeset/11166-page-header-breadcrumb-slot-retired.md
Original file line number Diff line number Diff line change
@@ -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.
15 changes: 7 additions & 8 deletions content/docs/guide/layout.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -300,7 +300,6 @@ does not render.
title?: string | Record<string, string>, // {field.path} tokens interpolated; derived from the record when omitted on a record page
subtitle?: string | Record<string, string>, // 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
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down
6 changes: 2 additions & 4 deletions content/docs/layout/page-header.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand All @@ -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
Expand Down Expand Up @@ -146,7 +145,6 @@ The renderer picks one of two layouts when it renders:

```plaintext
┌──────────────────────────────────────────────┐
│ (breadcrumb slot) │
│ Title [Actions] [⋯] │
│ Subtitle │
└──────────────────────────────────────────────┘
Expand Down
Original file line number Diff line number Diff line change
@@ -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 `<h1>` 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<string, unknown>) {
const body = <SchemaRenderer schema={schema as never} />;
const utils = render(
<ActionProvider>
{layout === 'record' ? (
<RecordContextProvider
objectName="crm_account"
recordId="rec-1"
data={{ id: 'rec-1', name: 'Acme' }}
objectSchema={RECORD_OBJECT}
>
{body}
</RecordContextProvider>
) : (
body
)}
</ActionProvider>,
);
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<Layout, string> = { bare: 'Orders', record: 'Acme' };

/** The authored spellings, each rendered against the absent-key control. */
const AUTHORED: ReadonlyArray<{ name: string; node: (layout: Layout) => Record<string, unknown> }> = [
{
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));
});
}
});
}
});
27 changes: 15 additions & 12 deletions packages/components/src/renderers/layout/containers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -1505,7 +1505,12 @@ const PageHeaderRenderer: React.FC<any> = ({ 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).
Expand Down Expand Up @@ -2340,12 +2345,6 @@ const PageHeaderRenderer: React.FC<any> = ({ schema, className, ...props }) => {
{...headerAria}
>
<div className="flex flex-col min-w-0 sm:min-w-48 flex-1">
{breadcrumb && (
<div
className="text-xs text-muted-foreground mb-1"
data-page-breadcrumb-slot
/>
)}
<RecordTitleChip
title={resolvedTitle}
objectLabel={objectLabel}
Expand Down Expand Up @@ -2376,9 +2375,6 @@ const PageHeaderRenderer: React.FC<any> = ({ schema, className, ...props }) => {
{...designer}
{...headerAria}
>
{breadcrumb && (
<div className="text-xs text-muted-foreground" data-page-breadcrumb-slot />
)}
<div className="flex items-center justify-between gap-4">
<div className="flex flex-col">
{explicitTitle && (
Expand Down Expand Up @@ -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' },
Expand Down
Loading