From 3f2ed65c5c45fbfc527691d0b0cc2ffd6a028d4c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 18:30:13 +0000 Subject: [PATCH 1/2] fix(react): stop reading the retired `navigation.view` and substituting it for the navigation-MODE token (objectui#9874) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `view.list.navigation.view` was removed in `@objectstack/spec` 17.5.0 under ADR-0049 (enforce-or-remove). `useNavigationOverlay` was its one shipped consumer: it read `const view = navigation?.view` and spent it at two `onNavigate` call sites as `view ?? 'view'`, so an authored name landed in the argument that carries the navigation MODE token and matched no branch in a host reading it against `edit`/`view`. Both sites now pass the literal `'view'`. `NavigationOverlayState` loses its `view` member — it could only ever carry the retired key. Census before removing it: zero readers in this repository. Two pins that asserted the substitution are inverted rather than deleted, both keeping their authored `view` fixture because it is the input the old and new implementations disagree about. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq --- .../9874-navigation-view-retired-consumer.md | 46 +++++++++++++++ packages/app-shell/src/views/ObjectView.tsx | 17 ++++-- .../gridNavigationMembers-8071.test.tsx | 57 ++++++++++++------- .../useNavigationOverlay.modeDefault.test.tsx | 38 ++++++++++++- .../react/src/hooks/useNavigationOverlay.ts | 50 ++++++++++++---- 5 files changed, 172 insertions(+), 36 deletions(-) create mode 100644 .changeset/9874-navigation-view-retired-consumer.md diff --git a/.changeset/9874-navigation-view-retired-consumer.md b/.changeset/9874-navigation-view-retired-consumer.md new file mode 100644 index 0000000000..e8268a3e73 --- /dev/null +++ b/.changeset/9874-navigation-view-retired-consumer.md @@ -0,0 +1,46 @@ +--- +'@object-ui/react': minor +--- + +`useNavigationOverlay` stops reading the retired `navigation.view` key, and stops substituting an authored name for the navigation-MODE token + +`view.list.navigation.view` was removed in `@objectstack/spec` 17.5.0 under ADR-0049 +(enforce-or-remove). `useNavigationOverlay` was its one shipped consumer, and the spec's +tombstone describes that hook by name: the authored value "was passed straight into the +navigation-MODE argument of the console's `onNavigate`, where anything other than `edit` +or `view` matched no branch, so the key selected nothing and could silently deaden the +row click." + +The second argument of `onNavigate` is the navigation MODE token, not a view name. The +hook read `const view = navigation?.view` and spent it at two call sites as +`view ?? 'view'` — the no-config early return and the `page` branch — so an authored +`{ mode: 'page', view: 'summary_view' }` dispatched `'summary_view'` into a slot hosts +read against a closed `edit`/`view` vocabulary. An authored name did not SELECT a view, +it SUBSTITUTED for the mode. + +Both call sites now pass the literal `'view'`, which is what every config WITHOUT the key +already dispatched. No replacement route is wired into this hook, and no fallback belongs +here: per the same tombstone, choosing what opens for a record is page assignment — assign +a `record` page to the object and let `isDefault` pick the one that opens — while a list +view's `navigation` block only decides HOW the detail is surfaced (`mode`, `size`). + +**Breaking for anyone who authored the key, and for anyone reading the hook's `view` +member.** `NavigationOverlayState` no longer publishes `view`. It could only ever carry +the retired key, so keeping it would publish a field that is permanently `undefined` — +the same "declared, consumed, and wrong" state the retirement exists to end. Measured +before removing it: zero readers of that member anywhere in this repository, against a +lit control on its sibling members in the same command. Hosts outside this repository +were not measured. Per this repo's version policy a breaking change ships as `minor`; the +semantics are stated here rather than in the bump. + +Behaviour on the Console is unchanged, and that is the point rather than a caveat: +`ObjectView`'s `onNavigate` already routed every non-`new_window` action to the same +record-detail route, so the substitution was inaudible exactly where it was loudest in +the metadata. Its comment claimed a custom name was "resolved by RecordDetailView from +its own config"; no layer ever resolved a view by that name, and the comment is corrected +with the read it described. + +Two existing pins asserted the substitution as intended behaviour and are INVERTED rather +than deleted — `useNavigationOverlay.modeDefault` and `gridNavigationMembers-8071`, both +keeping their authored `view` fixture because it is the one input the old and new +implementations disagree about. diff --git a/packages/app-shell/src/views/ObjectView.tsx b/packages/app-shell/src/views/ObjectView.tsx index 28c951c927..6e2d1729cd 100644 --- a/packages/app-shell/src/views/ObjectView.tsx +++ b/packages/app-shell/src/views/ObjectView.tsx @@ -2170,10 +2170,19 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an return; } // Default: navigate to record detail page. - // `action` may be 'view' / 'page' / undefined, OR a custom view name - // forwarded from `navigation.view` (e.g. 'detail_form'). The view - // variant is resolved by RecordDetailView from its own config, so - // any non-`new_window` action lands on the record detail route. + // `action` is a navigation-MODE token — 'view' / 'page' / undefined + // — and every non-`new_window` value lands on the record detail + // route. + // + // ⛔ It is NOT a view name, and this comment used to say a custom + // one could arrive here "forwarded from `navigation.view` (e.g. + // 'detail_form')", resolved "by RecordDetailView from its own + // config". No layer ever resolved a view by that name — which is + // why `@objectstack/spec` 17.5.0 retired the key under ADR-0049, + // and why `useNavigationOverlay` stopped forwarding it + // (objectui#9874). Nothing here changes behaviour: this branch + // already treated the name and the token identically, which is + // exactly what made the substitution inaudible on this surface. const originState = { from: { pathname: location.pathname + (location.search || ''), diff --git a/packages/plugin-grid/src/__tests__/gridNavigationMembers-8071.test.tsx b/packages/plugin-grid/src/__tests__/gridNavigationMembers-8071.test.tsx index e7959f4111..8e008c146d 100644 --- a/packages/plugin-grid/src/__tests__/gridNavigationMembers-8071.test.tsx +++ b/packages/plugin-grid/src/__tests__/gridNavigationMembers-8071.test.tsx @@ -24,14 +24,23 @@ * * ⛔ What a declaration can never publish: * - * - **`view` is not a route — it is the second ARGUMENT `onNavigate` - * receives**, and the literal `'view'` is what stands in when the member is - * absent. The two are one character apart in the source and mean entirely - * different things. - * - **`openNewTab` OUTRANKS `mode` and DISCARDS `view` while doing it.** An - * authored `{ mode: 'page', view: 'summary', openNewTab: true }` dispatches - * `'new_window'`, not `'summary'`: the member the author wrote to choose a - * destination is dropped by the member they wrote to choose a window. + * - **`view` was not a route — it landed in the second ARGUMENT `onNavigate` + * receives**, which carries the navigation MODE token, and the literal + * `'view'` is what stands there. The two are one character apart in the + * source and mean entirely different things. ⭐ **objectui#9874 REVERSED + * the row that pinned this**: `@objectstack/spec` 17.5.0 retired + * `view.list.navigation.view` under ADR-0049 precisely because an authored + * name SUBSTITUTED for the mode and matched no branch, so the member no + * longer reaches `onNavigate` at all and every config dispatches `'view'`. + * The row below is kept, inverted, rather than deleted: the authored member + * is the input the two implementations disagree about, so it is the only + * input that can witness the change. + * - **`openNewTab` OUTRANKS `mode`.** An authored + * `{ mode: 'page', openNewTab: true }` dispatches `'new_window'`. Before + * objectui#9874 this row also read as "and DISCARDS `view` while doing it", + * because a sibling `view: 'summary'` was dropped here while riding through + * the `page` branch; now nothing rides through either branch and the + * precedence is the whole of what this row pins. * - **`preventNavigation` OUTRANKS every mode, including the overlay ones.** * `{ mode: 'drawer', preventNavigation: true }` draws no drawer and throws * nothing — a grid that looks clickable and is not. @@ -146,18 +155,27 @@ describe('object-grid `navigation` members decide the row click (objectui#8071)' expect(onNavigate).toHaveBeenCalledWith('7', 'view'); }); - it('`mode` defaults to `page`, and `view` is the action it dispatches', async () => { + it('`mode` defaults to `page`, and a RETIRED `view` member does not become the action (objectui#9874)', async () => { // No `mode` member at all — legal authored metadata, because the spec - // defaults it. `view` rides through as the second argument. + // defaults it. This row used to assert `('7', 'summary_view')` under the + // comment "`view` rides through as the second argument"; it did, into the + // slot that carries the navigation MODE token, where a host reading it + // against `edit`/`view` matched no branch and the row click went quiet. + // `@objectstack/spec` 17.5.0 retired the key for that (ADR-0049), so the + // authored name is now inert here and the dispatched action is the literal. const { onNavigate } = await clickRow({ view: 'summary_view' }); await waitFor(() => expect(onNavigate).toHaveBeenCalledTimes(1)); - expect(onNavigate).toHaveBeenCalledWith('7', 'summary_view'); + expect(onNavigate).toHaveBeenCalledWith('7', 'view'); + expect(onNavigate).not.toHaveBeenCalledWith('7', 'summary_view'); }); - it('an omitted `view` falls back to the literal `view`, not to undefined', async () => { - // The pair for the row above: same mode, member removed. A renderer passing - // `navigation.view` straight through would send `undefined` here and the - // host would open its default form — which looks identical until it is not. + it('an omitted `view` dispatches the literal `view`, not undefined', async () => { + // The pair for the row above: same mode, member removed. Since objectui#9874 + // the two rows agree by construction — which is the POINT of the retirement + // and not a reason to drop either. ⛔ Keeping both is what makes "the member + // is inert" a measurement rather than an assertion about one input: a + // renderer that resumed forwarding the member would turn the row above red + // and leave this one green. const { onNavigate } = await clickRow({ mode: 'page' }); await waitFor(() => expect(onNavigate).toHaveBeenCalledTimes(1)); expect(onNavigate).toHaveBeenCalledWith('7', 'view'); @@ -186,10 +204,11 @@ describe('object-grid `navigation` members decide the row click (objectui#8071)' await waitFor(() => expect(document.querySelector('[role="dialog"]')).not.toBeNull()); }); - it('`openNewTab` OUTRANKS `mode: "page"` and DISCARDS `view`', async () => { - // The author wrote a destination and a window preference. Only the window - // preference survives: the dispatched action is `new_window`, and - // `summary_view` is nowhere in the call. + it('`openNewTab` OUTRANKS `mode: "page"`, with a retired `view` still inert', async () => { + // The window preference decides: the dispatched action is `new_window`, and + // `summary_view` is nowhere in the call. Before objectui#9874 this row was + // the ONLY place the authored member was dropped; it is now dropped on + // every path, and the row survives as the precedence pin it also always was. const { onNavigate } = await clickRow({ mode: 'page', view: 'summary_view', diff --git a/packages/react/src/hooks/__tests__/useNavigationOverlay.modeDefault.test.tsx b/packages/react/src/hooks/__tests__/useNavigationOverlay.modeDefault.test.tsx index 3d9760b7cd..dd6c61b874 100644 --- a/packages/react/src/hooks/__tests__/useNavigationOverlay.modeDefault.test.tsx +++ b/packages/react/src/hooks/__tests__/useNavigationOverlay.modeDefault.test.tsx @@ -25,6 +25,14 @@ * makes a missing `mode` mean something other than `'page'`, this suite goes * red and the parity file stays green — which is exactly the split of duties * the two files are for. + * + * ⚠️ objectui#9874 rewrote one row here and added one. `AS_ALIAS` authors + * `view: 'summary_view'`, which used to ride through into `onNavigate`'s + * navigation-MODE argument; `@objectstack/spec` 17.5.0 retired that key under + * ADR-0049 and the read is gone, so the row now pins that the authored name + * does NOT reach the mode slot. ⛔ The fixture keeps its `view` member on + * purpose — it is the one input the old and new implementations disagree + * about, and it is also still what objectui#4550 reported the alias rejecting. */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; @@ -96,7 +104,7 @@ describe('useNavigationOverlay: a config without `mode` defaults to `page` (obje expect(result.current.isOpen).toBe(false); }); - it('routes a click through the `page` branch, carrying the declared view', () => { + it('routes a click through the `page` branch WITHOUT the authored `view` reaching the mode slot (objectui#9874)', () => { const onNavigate = vi.fn(); const { result } = renderHook(() => useNavigationOverlay({ navigation: AS_ALIAS, objectName: 'contacts', onNavigate }), @@ -106,11 +114,35 @@ describe('useNavigationOverlay: a config without `mode` defaults to `page` (obje result.current.handleClick(RECORD); }); - // `view ?? 'view'` — the authored `view` survives the default-mode path. - expect(onNavigate).toHaveBeenCalledWith('r1', 'summary_view'); + // ⭐ THE DISAGREEMENT ROW. `AS_ALIAS` authors `view: 'summary_view'`, and + // this line used to read `toHaveBeenCalledWith('r1', 'summary_view')` under + // the comment "`view ?? 'view'` — the authored `view` survives the + // default-mode path". It did survive, into the wrong slot: the second + // argument is the navigation MODE token, so the authored name SUBSTITUTED + // for the mode and matched no branch in a host reading it against + // `edit`/`view` — the silence `@objectstack/spec` 17.5.0 retired the key to + // end (ADR-0049). The old implementation and this one disagree on exactly + // this input, which is why the fixture keeps its `view` member. + expect(onNavigate).toHaveBeenCalledWith('r1', 'view'); + expect(onNavigate).not.toHaveBeenCalledWith('r1', 'summary_view'); expect(result.current.isOpen).toBe(false); }); + it('publishes no `view` member on its state — the key it mirrored is retired (objectui#9874)', () => { + const { result } = renderHook(() => + useNavigationOverlay({ navigation: AS_ALIAS, objectName: 'contacts' }), + ); + + // Asserted on the KEY, not on the value: `toBeUndefined()` passes just as + // happily against a member that is present and empty, which is the state + // this card removed. The lit control is the sibling members in the same + // expectation — without them an empty `state` object would pass row one. + expect(Object.keys(result.current)).not.toContain('view'); + expect(Object.keys(result.current)).toEqual( + expect.arrayContaining(['mode', 'isOverlay', 'width', 'handleClick']), + ); + }); + it('falls back to the `view` action when the config declares neither key', () => { const onNavigate = vi.fn(); // A present-but-empty config is a different input from NO config: it takes diff --git a/packages/react/src/hooks/useNavigationOverlay.ts b/packages/react/src/hooks/useNavigationOverlay.ts index 4535696198..68d8c20613 100644 --- a/packages/react/src/hooks/useNavigationOverlay.ts +++ b/packages/react/src/hooks/useNavigationOverlay.ts @@ -15,6 +15,38 @@ * * Used by plugin-grid, plugin-list, plugin-detail and any component that needs * NavigationConfig support. + * + * ## The RETIRED `view` key (objectui#9874) + * + * `view.list.navigation.view` was removed in `@objectstack/spec` 17.5.0 under + * ADR-0049 (enforce-or-remove). This hook was its one shipped consumer, and the + * spec's own tombstone describes THIS file: the authored value "was passed + * straight into the navigation-MODE argument of the console's `onNavigate`, + * where anything other than `edit` or `view` matched no branch, so the key + * selected nothing and could silently deaden the row click." + * + * ⭐ The point that is one character wide in the source and total in meaning: + * `onNavigate`'s second argument is the navigation MODE token, not a view name. + * An authored `view` did not SELECT a view, it SUBSTITUTED for the mode — so + * `{ mode: 'page', view: 'summary_view' }` dispatched `'summary_view'` into the + * slot a host reads against its closed `edit`/`view` vocabulary, matching + * neither branch. Two reads (`view ?? 'view'`, in the no-config and the `page` + * branch) are now the literal `'view'`, which is what every config without the + * key already dispatched and what every config dispatches now. + * + * The replacement route is NOT in this hook and no fallback belongs here. Per + * the same tombstone: to choose what opens for a record, assign a `record` page + * to the object and let `isDefault` pick the one that opens — page assignment + * is the machinery that resolves a detail layout, and a list view's + * `navigation` block only decides HOW the detail is surfaced (`mode`, `size`). + * + * ⚠️ `NavigationOverlayState` lost its `view` member with the read. It could + * only ever carry the retired key, so keeping it would publish a field that is + * permanently `undefined` — the same "declared, consumed, and wrong" state the + * retirement exists to end. Measured before removing it: zero readers of that + * member in this repository (`(navOverlay|nav|state).view` → no lines, against + * a lit control on its sibling members in the same command). Hosts outside this + * repository were NOT measured. */ import { useState, useCallback, useMemo } from 'react'; @@ -37,8 +69,10 @@ import type { SpecAuthoredInput } from '../spec-input.js'; * `mode` is OPTIONAL, because the spec says so and this hook agrees. * `packages/spec/src/ui/view.zod.ts` declares * `mode: NavigationModeSchema.default('page')`, and a `.default()` lands on the - * AUTHORING side as `| undefined` — so `navigation: { view: 'summary_view' }` - * is legal authored metadata that lets the mode default. + * AUTHORING side as `| undefined` — so `navigation: { size: 'lg' }` is legal + * authored metadata that lets the mode default. ⛔ This sentence used to make + * the same point with a `view` member; see "The RETIRED `view` key" below for + * why no example in this file may spell that key again. * * Until objectui#4550 this alias `Omit`ted `mode` and re-added it as * `NonNullable<…>`, on the stated reasoning that "this hook dispatches on @@ -208,8 +242,6 @@ export interface NavigationOverlayState { width: string | number | undefined; /** Whether navigation is an overlay mode (drawer/modal/split/popover) */ isOverlay: boolean; - /** The target view/form name from NavigationConfig */ - view: string | undefined; } /** @@ -272,7 +304,6 @@ export function useNavigationOverlay( const mode: NavigationMode = navigation?.mode ?? 'page'; const width = resolveOverlayWidth(navigation); - const view = navigation?.view; const isOverlay = mode === 'drawer' || mode === 'modal' || mode === 'split' || mode === 'popover'; const close = useCallback(() => { @@ -320,7 +351,7 @@ export function useNavigationOverlay( if (!navigation) { const recordId = record.id || record._id; if (onNavigate && recordId != null) { - onNavigate(recordId as string | number, view ?? 'view'); + onNavigate(recordId as string | number, 'view'); } return; } @@ -353,7 +384,7 @@ export function useNavigationOverlay( if (mode === 'page') { const recordId = record.id || record._id; if (onNavigate && recordId != null) { - onNavigate(recordId as string | number, view ?? 'view'); + onNavigate(recordId as string | number, 'view'); } return; } @@ -365,7 +396,7 @@ export function useNavigationOverlay( return; } }, - [onRowClick, navigation, mode, objectName, onNavigate, isOverlay, view] + [onRowClick, navigation, mode, objectName, onNavigate, isOverlay] ); return useMemo( @@ -378,9 +409,8 @@ export function useNavigationOverlay( setIsOpen, handleClick, width, - view, isOverlay, }), - [isOpen, selectedRecord, mode, close, open, handleClick, width, view, isOverlay] + [isOpen, selectedRecord, mode, close, open, handleClick, width, isOverlay] ); } From 2de140b9f289c998ac44febf698b467191de1103 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 19:10:14 +0000 Subject: [PATCH 2/2] fix(scripts): ledger the `navigation.view` retirement sentence as a release fact, not an installed-pin claim (objectui#9874) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check-installed-spec-pin-claims` flagged `gridNavigationMembers-8071.test.tsx:31` for naming `@objectstack/spec` 17.5.0 while the artifact resolves 17.4.0. The sentence is a "when it changed" claim — 17.5.0 is the release that retired `view.list.navigation.view` under ADR-0049 — so the gate's other remedy is not available to it: restamping to the resolved pin would assert that 17.4.0 retired the key, and 17.4.0 is precisely the version where the key still exists. That is the fresh false premise the gate's own docblock warns restamping plants. Recorded as class `historical`, the same class and the same shape as the existing `offline-nav-performance-spec-parity` entry ("retired that name in 17.0.0-rc.1"). The marker that puts the line in the population is the word `pinned` in "the row that pinned this", where it means a test row pinning a behaviour rather than a version pin. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq --- scripts/check-installed-spec-pin-claims.mjs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/scripts/check-installed-spec-pin-claims.mjs b/scripts/check-installed-spec-pin-claims.mjs index 90ba27f32c..ef9bcc691e 100644 --- a/scripts/check-installed-spec-pin-claims.mjs +++ b/scripts/check-installed-spec-pin-claims.mjs @@ -750,6 +750,14 @@ export const LEDGER = [ class: "historical", why: "\"`publicPicker` arrives in @objectstack/spec 17.0.0 GA\" \u2014 the arrival release. \u26a0\ufe0f The stale claim in the SAME docblock (\"this repo is pinned to `^17.0.0-rc.6`\") is a RANGE and therefore outside this gate's predicate by construction; it is recorded in objectui#8924 rather than silently covered here.", }, + { + file: "packages/plugin-grid/src/__tests__/gridNavigationMembers-8071.test.tsx", + package: "@objectstack/spec", + version: "17.5.0", + sites: 1, + class: "historical", + why: "\"`@objectstack/spec` 17.5.0 retired `view.list.navigation.view` under ADR-0049\" names the RELEASE that removed the key, not what is installed -- the same shape as the `offline-nav-performance-spec-parity` entry below (\"retired that name in 17.0.0-rc.1\"). The marker that puts it in the population is the word `pinned` in \"the row that pinned this\", where it means a TEST ROW pinning a behaviour rather than a version pin; the `pinned-recording-sense` cue does not reach that phrasing. This version is AHEAD of the resolved pin rather than behind it, which is why restamping is not merely unnecessary but false: 17.4.0 is exactly the version where the key still EXISTS, so stamping the sentence at the pin would assert that 17.4.0 retired it. That is the fresh false premise this gate\u0027s docblock warns restamping plants (objectui#9874).", + }, { file: "packages/react/src/hooks/__tests__/offline-nav-performance-spec-parity.test.ts", package: "@objectstack/spec",