From d6b1493c7d5e8c49dc75ddef87677f0128aac319 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 09:28:59 +0000 Subject: [PATCH] fix(app-shell): drop the derived marker-title binding from interface-page maps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `defaultMapFromObject` bound a `titleField` alongside `locationField`, derived from the object's display field. It was added to route around a forge in `ObjectMap` that no longer exists (objectui#5953): `getMapConfig` used to fill an absent `titleField` with the literal `'name'` and the marker title was a plain `record[titleField]` read, so an object whose display field was not `name` titled every popup `undefined`. `ObjectMap` now resolves marker titles through `@object-ui/core#getRecordDisplayName`, the ADR-0079 resolver the kanban, calendar and gantt renderers already used — which is why none of them binds a derived title either. Once the forge was gone, the binding's only remaining effect was to INVERT precedence: it reaches the resolver as `options.titleField`, i.e. step 0 — ahead of `objectDef.titleField`, ahead of the declared `nameField` pointer, and ahead of the legacy `titleFormat` template at step 3. A field name derived by this page could therefore only ever change the answer by out-ranking something the object itself declared; in every other case it reproduced, at step 0, the string the resolver already computes further down. The deriver now binds `locationField` and nothing else, like every sibling deriver. An author's own `map.titleField` is untouched: it travels as the view-level `map` block, `ListView` merges it per key, and the resolver honours it at step 0 by design. The docblock is rewritten to describe what the code now does; the arms that pinned the derivation are retired and replaced with arms that pin its absence and the declared side's authority at the read site. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q --- ...listpage-drop-derived-marker-title-6343.md | 33 ++++ .../InterfaceListPage.mapConfig.test.tsx | 182 +++++++++--------- .../app-shell/src/views/InterfaceListPage.tsx | 98 ++++------ 3 files changed, 163 insertions(+), 150 deletions(-) create mode 100644 .changeset/interfacelistpage-drop-derived-marker-title-6343.md diff --git a/.changeset/interfacelistpage-drop-derived-marker-title-6343.md b/.changeset/interfacelistpage-drop-derived-marker-title-6343.md new file mode 100644 index 0000000000..02a60cf22f --- /dev/null +++ b/.changeset/interfacelistpage-drop-derived-marker-title-6343.md @@ -0,0 +1,33 @@ +--- +'@object-ui/app-shell': patch +--- + +Interface-page maps: drop the derived marker-title binding, restoring the +object's own declaration as the authority + +`defaultMapFromObject` bound a `titleField` alongside `locationField`, derived +from the object's display field. That binding was added to route around a forge +in `ObjectMap` — `getMapConfig` filled an absent `titleField` with the literal +`'name'`, and the marker title was a plain `record[titleField]` read, so an +object whose display field was not `name` titled every popup `undefined`. The +forge is gone: `ObjectMap` now resolves marker titles through +`@object-ui/core#getRecordDisplayName`, the same ADR-0079 resolver the kanban, +calendar and gantt renderers already used — which is why none of them binds a +derived title either. + +What the binding did once the forge was gone was invert precedence. It reaches +the resolver as `options.titleField`, i.e. step 0 — ahead of `titleField` on the +object, ahead of the declared `nameField` pointer, and ahead of the legacy +`titleFormat` template. A field name derived by the page could therefore only +ever change the answer by out-ranking something the object itself declared; in +every other case it reproduced, at step 0, the string the resolver already +computes further down its ladder. The deriver now binds `locationField` and +nothing else, exactly like its kanban / calendar / gallery / gantt siblings. + +No authoring surface changes. An author's own `map.titleField` is untouched — it +travels as the view-level `map` block, `ListView` merges it per key, and the +resolver honours it at step 0 by design. Objects that declare nothing resolve to +the same field as before, now via the resolver's own type-aware derivation +rather than a binding forced ahead of it; objects that declare a `titleFormat` +template (or a `titleField`) now have that declaration honoured on the map, as +it already was on every other visualization. diff --git a/packages/app-shell/src/views/InterfaceListPage.mapConfig.test.tsx b/packages/app-shell/src/views/InterfaceListPage.mapConfig.test.tsx index 16761acb64..6c7156b9c9 100644 --- a/packages/app-shell/src/views/InterfaceListPage.mapConfig.test.tsx +++ b/packages/app-shell/src/views/InterfaceListPage.mapConfig.test.tsx @@ -127,11 +127,12 @@ describe('InterfaceListPage forwards the view-level `map` block (objectui#5042)' const { map, optionsMap } = await renderWith({}); expect(map).toBeNull(); - // …and the ADR-0047 auto-derivation still fires. `titleField` joined the - // product in objectui#5909: this fixture object's display field is `title`, - // NOT `name`, so without it `ObjectMap` would title every marker off the - // literal `'name'` key and render `undefined`. - expect(optionsMap).toEqual({ locationField: 'location', titleField: 'title' }); + // …and the ADR-0047 auto-derivation still fires — with `locationField`, the + // map's own required binding, and nothing else. The derived `titleField` + // that objectui#5909 added to this product was removed in objectui#6343: + // it reached `getRecordDisplayName` as `options.titleField`, i.e. step 0, + // where a name this page guessed outranked the object's own declaration. + expect(optionsMap).toEqual({ locationField: 'location' }); }); it('keeps the auto-derived binding ALONGSIDE a partial authored block', async () => { @@ -139,10 +140,15 @@ describe('InterfaceListPage forwards the view-level `map` block (objectui#5042)' // author declared only a marker-title field, and the coordinate binding // still has to come from the derivation. `ListView` merges the two per // key; a `??` here would have discarded one of them. + // + // Sharper since objectui#6343 removed the derived title: the two sides no + // longer overlap at all. The AUTHOR owns `titleField` — declared, and + // honoured at the resolver's step 0 — and the derivation owns + // `locationField`. Each key has exactly one source. const { map, optionsMap } = await renderWith({ map: { titleField: 'title' } }); expect(map).toEqual({ titleField: 'title' }); - expect(optionsMap).toEqual({ locationField: 'location', titleField: 'title' }); + expect(optionsMap).toEqual({ locationField: 'location' }); }); it('CONTROL: the legacy `options.map` bag is still forwarded on its own path', async () => { @@ -156,109 +162,105 @@ describe('InterfaceListPage forwards the view-level `map` block (objectui#5042)' }); /** - * objectui#5909 — the derived marker-title binding. + * objectui#6343 — the derived marker-title binding is GONE, and the object's + * own declaration keeps its authority. * - * ## What the siblings actually do (measured, because the card asserted it) + * ## Why the binding was removed rather than re-narrated * - * The card's argument is that this deriver is the odd one out among "every - * sibling deriver". Measured on this file, it is not: NO deriver binds a title. - * Each binds its viz's own required field and stops — - * `defaultKanbanFromObject → { groupByField }`, - * `defaultCalendarFromObject → { startDateField }`, - * `defaultGalleryFromObject → { coverField }`, - * `defaultGanttFromObject → { startDateField, endDateField, progressField? }` — - * and `defaultMapFromObject` bound `{ locationField }`, its own required field. + * It was added (objectui#5909) to route around a forge that no longer exists: + * `getMapConfig` used to fill an absent `titleField` with the literal `'name'` + * and the marker title was a plain `record[titleField]` read, so an object + * whose display field was not `name` titled every popup `undefined`. + * objectui#5953 deleted that forge — `ObjectMap` now resolves marker titles + * through `@object-ui/core#getRecordDisplayName`, exactly like `ObjectKanban`, + * `ObjectCalendar` and `ObjectGantt` do, and exactly like no sibling deriver + * binds a title. * - * The real asymmetry is one layer down, at the renderers. `ObjectKanban`, - * `ObjectCalendar` and `ObjectGantt` resolve their item title through - * `@object-ui/core#getRecordDisplayName` (ADR-0079), so they need nothing - * derived. `ObjectMap` alone does not: `getMapConfig` fills an absent - * `titleField` with the literal `'name'` and the marker title is a plain - * `record[titleField]` read — `undefined` for every record of an object whose - * display field is not `name`. + * What was left behind was not a redundancy but an INVERSION. The binding + * arrives at the resolver as `options.titleField`, which is precedence **step + * 0** — ahead of `objectDef.titleField`, ahead of the declared `nameField` + * pointer, and ahead of the legacy `titleFormat` template at step 3. Measured + * against the resolver's ladder, a field name this page picked can only ever + * change the answer by OUT-RANKING something the object itself declared; in + * every other case it reproduces, at step 0, the string the resolver already + * computes at step 1/2/4. That is the whole of its effect, which is why it + * goes rather than gets described. * - * So these arms pin the binding against the SAME ADR-0079 field ranking the - * sibling renderers use, rather than against a rule invented here. + * These arms pin both halves: the product no longer carries a title binding + * (shape), and the declared side therefore decides at the read site + * (semantics). The arms that used to pin the derivation are retired with it. */ -describe('defaultMapFromObject derives the marker title (objectui#5909)', () => { +describe('defaultMapFromObject binds no marker title (objectui#6343)', () => { const loc = { type: 'location' }; - // THE DISCRIMINATING ARM. An object whose display field is `title`, not - // `name` — the exact case the card reports. Before the fix the product was - // `{ locationField: 'location' }` and `getMapConfig` fell through to `'name'`. - it('binds the display field when it is NOT `name`', () => { - expect(defaultMapFromObject({ fields: { title: { type: 'text' }, location: loc } })).toEqual({ - locationField: 'location', - titleField: 'title', - }); - }); - - // The declared pointer outranks the field scan. This arm is what makes the - // `nameField` step load-bearing rather than decorative: `deriveTitleField` - // alone ranks `title` above `headline` here, so an implementation that called - // only the scan would answer `title` and fail. - it('prefers the object’s declared `nameField` over the field scan', () => { - expect( - defaultMapFromObject({ - nameField: 'headline', - fields: { headline: { type: 'text' }, title: { type: 'text' }, location: loc }, - }), - ).toEqual({ locationField: 'location', titleField: 'headline' }); + // THE DISCRIMINATING ARM — the exact fixture objectui#5909 introduced the + // binding for. An object whose display field is `title`, not `name`: the + // deriver used to answer `{ locationField, titleField: 'title' }`. + it('binds only `locationField` when a display field IS derivable', () => { + const derived = defaultMapFromObject({ fields: { title: { type: 'text' }, location: loc } }); + expect(derived).toEqual({ locationField: 'location' }); + expect(derived && 'titleField' in derived).toBe(false); }); - it('accepts the deprecated `displayNameField` / `NAME_FIELD_KEY` aliases', () => { + // A DECLARED pointer is not a licence to bind either — the resolver reads + // `nameField` itself, at step 1, from the same object definition. + it('binds nothing from a declared `nameField` / `displayNameField` either', () => { const fields = { headline: { type: 'text' }, title: { type: 'text' }, location: loc }; - expect(defaultMapFromObject({ displayNameField: 'headline', fields })?.titleField).toBe('headline'); - expect(defaultMapFromObject({ NAME_FIELD_KEY: 'headline', fields })?.titleField).toBe('headline'); - }); - - it('picks up the `*_name` affix convention', () => { - expect(defaultMapFromObject({ fields: { site_name: { type: 'text' }, location: loc } })).toEqual({ + expect(defaultMapFromObject({ nameField: 'headline', fields })).toEqual({ locationField: 'location', - titleField: 'site_name', }); - }); - - // CONTROL — an object whose display field IS `name` keeps the binding it - // effectively had. This arm alone would pass on the defect, which is why it - // is not the only one. - it('CONTROL: still binds `name` when that IS the display field', () => { - expect(defaultMapFromObject({ fields: { name: { type: 'text' }, location: loc } })).toEqual({ + expect(defaultMapFromObject({ displayNameField: 'headline', fields })).toEqual({ locationField: 'location', - titleField: 'name', }); }); - // Omitted, not fabricated: every field here is title-INELIGIBLE (geo, date), - // so nothing resolves and the key stays absent rather than being invented. - it('omits `titleField` entirely when no field is title-eligible', () => { - const derived = defaultMapFromObject({ - fields: { location: { type: 'geolocation' }, due: { type: 'date' } }, - }); - expect(derived).toEqual({ locationField: 'location' }); - expect(derived && 'titleField' in derived).toBe(false); + // THE AUTHORITY ARM — semantics, not shape, and the reason this is a + // behaviour change rather than a comment repair. `titleFormat` is a DECLARED + // (deprecated, still live) object key that the resolver honours at step 3. + // A derived binding at step 0 outranked it; with no binding, the object's + // own declaration decides. + it('leaves a declared `titleFormat` its authority at the read site', () => { + const objectDef = { + titleFormat: '{code} · {city}', + fields: { + code: { type: 'text' }, + city: { type: 'text' }, + title: { type: 'text' }, + location: loc, + }, + }; + const record = { code: 'D4', city: 'Leeds', title: 'Raw title', location: 'x' }; + const product = defaultMapFromObject(objectDef); + + expect(product).toEqual({ locationField: 'location' }); + + // `ObjectMap`'s read site, verbatim: the product's `titleField` (now + // absent) is passed as `options.titleField`. + expect( + getRecordDisplayName(objectDef, record, { titleField: (product as any)?.titleField }), + ).toBe('D4 · Leeds'); + + // …and this is what the derived binding used to force instead: the field + // `deriveTitleField` ranks first, evaluated at step 0, silently beating the + // template the object declared. Pinned so the inversion cannot come back + // unnoticed. + expect(getRecordDisplayName(objectDef, record, { titleField: 'title' })).toBe('Raw title'); }); - it('CONTROL: no location field still derives nothing at all', () => { - expect(defaultMapFromObject({ fields: { title: { type: 'text' } } })).toBeUndefined(); + // POSITIVE CONTROL — removal loses no titles. With nothing declared, the + // resolver's own step 4 is the same scan the binding used to hoist to step 0, + // so the marker still reads the affix-convention display field. + it('CONTROL: the resolver alone still answers what the binding used to force', () => { + const objectDef = { fields: { site_name: { type: 'text' }, location: loc } }; + const record = { site_name: 'Depot 4', location: 'x' }; + + expect(defaultMapFromObject(objectDef)).toEqual({ locationField: 'location' }); + expect(getRecordDisplayName(objectDef, record)).toBe('Depot 4'); }); - // ANTI-DRIFT. The binding is a field NAME; `getRecordDisplayName` is the - // canonical per-record resolver every sibling renderer calls. Reading the - // record at the derived field must land on the same string the canonical - // resolver returns, or a map and a kanban over one object would disagree - // about what a record is called. Scoped to the steps a static binding can - // carry — the declared pointer and the type-aware field scan; `titleFormat` - // (a render-only template) and the record-key probe are out of reach by - // construction and are not asserted here. - it.each([ - ['display field is `title`', { fields: { title: { type: 'text' }, location: loc } }, { title: 'Fix the roof', location: 'x' }], - ['declared nameField', { nameField: 'headline', fields: { headline: { type: 'text' }, title: { type: 'text' }, location: loc } }, { headline: 'Roof', title: 'Ignored', location: 'x' }], - ['affix convention', { fields: { site_name: { type: 'text' }, location: loc } }, { site_name: 'Depot 4', location: 'x' }], - ['display field is `name`', { fields: { name: { type: 'text' }, location: loc } }, { name: 'HQ', location: 'x' }], - ])('agrees with getRecordDisplayName — %s', (_label, objectDef, record) => { - const titleField = defaultMapFromObject(objectDef)?.titleField; - expect(titleField).toBeTruthy(); - expect((record as any)[titleField as string]).toBe(getRecordDisplayName(objectDef, record)); + // CONTROL — the deriver's own required field is untouched by this change: + // no location field still derives nothing at all. + it('CONTROL: no location field still derives nothing at all', () => { + expect(defaultMapFromObject({ fields: { title: { type: 'text' } } })).toBeUndefined(); }); }); diff --git a/packages/app-shell/src/views/InterfaceListPage.tsx b/packages/app-shell/src/views/InterfaceListPage.tsx index 8f092398b4..a5247aae0a 100644 --- a/packages/app-shell/src/views/InterfaceListPage.tsx +++ b/packages/app-shell/src/views/InterfaceListPage.tsx @@ -23,7 +23,6 @@ import { Empty, EmptyTitle, EmptyDescription, NavigationOverlay } from '@object- import { Database } from 'lucide-react'; import { useObjectTranslation } from '@object-ui/i18n'; import { isSystemManagedField } from '@object-ui/types'; -import { deriveTitleField } from '@object-ui/core'; import type { ListViewSchema } from '@object-ui/types'; import { useMetadata } from '../providers/MetadataProvider.js'; import { useTenancyPosture } from '../hooks/useTenancyPosture.js'; @@ -182,77 +181,56 @@ export function defaultGanttFromObject(objectDef: any): { startDateField: string return { startDateField: start, endDateField: end, ...(progress ? { progressField: progress } : {}) }; } -/** - * The object's DISPLAY FIELD as a field *name*, for use as a static binding. - * - * This is the field-name half of ADR-0079's `getRecordDisplayName` precedence, - * which every sibling view renderer resolves per record. Steps kept, in order: - * - * 1+2. `objectDef.nameField` — the canonical record-title pointer — then its - * deprecated `displayNameField` / `NAME_FIELD_KEY` aliases. - * 4. `deriveTitleField(objectDef)` — the shared type-aware scan of - * `objectDef.fields` (name-ish exact → name-ish affix → declaration - * order), imported rather than reimplemented so this binding and the - * renderers can never rank fields differently. - * - * Steps deliberately NOT taken: step 0 (`objectDef.titleField`) is the caller's - * own explicit choice, which on this path is what we are computing; step 3 - * (`titleFormat`) is a render-only template, not a field name, so no static - * binding can carry it; and steps 4b/5 read a RECORD, which a binding derived - * from the object alone has none of. - * - * `deriveTitleField`'s own eligibility filter is used as-is — deliberately NOT - * additionally filtered through this file's `hidden`/system-managed screen. The - * point of this binding is to name the field the ADR-0079 renderers would name - * for the same object; screening it differently here would reintroduce exactly - * the per-view dialect ADR-0079 removed. - */ -function displayFieldOfObject(objectDef: any): string | undefined { - const declared = - objectDef?.nameField ?? objectDef?.displayNameField ?? objectDef?.NAME_FIELD_KEY; - if (typeof declared === 'string' && declared) return declared; - return deriveTitleField(objectDef); -} - /** * Map needs a location/geo field (or address). Auto-derive from a location-typed * field, else a field whose name looks geographic. * - * ## Why this one also binds a marker title (objectui#5909) + * Like every sibling deriver above, this binds its viz's own REQUIRED field and + * nothing else — `kanban → groupByField`, `calendar → startDateField`, + * `gallery → coverField`, `gantt → start/end`, `map → locationField`. A marker + * TITLE is not this seam's to bind. + * + * ## Why the derived title binding was removed (objectui#6343) * - * The sibling derivers each bind their viz's own REQUIRED field and no title — - * `kanban → groupByField`, `calendar → startDateField`, `gallery → coverField`, - * `gantt → start/end` — and by that measure this deriver was never the odd one - * out: it binds `locationField`, its own required field. The asymmetry is one - * layer down, at the RENDERERS: `ObjectKanban`, `ObjectCalendar` and - * `ObjectGantt` all resolve their item title through - * `@object-ui/core#getRecordDisplayName` (ADR-0079), so they need no derived - * title binding. `ObjectMap` does not — its `getMapConfig` fills an absent - * `titleField` with the LITERAL `'name'`, and the marker title is then a plain - * `record[titleField]` read. So for any object whose display field is not - * literally `name`, every marker popup titles itself `undefined`. + * It existed (objectui#5909) to route around a forge in `ObjectMap` that no + * longer exists: `getMapConfig` used to fill an absent `titleField` with the + * literal `'name'`, and the marker title was then a plain `record[titleField]` + * read — so an object whose display field was not literally `name` titled every + * marker popup `undefined`. objectui#5953 deleted that forge. `ObjectMap` now + * resolves marker titles through `@object-ui/core#getRecordDisplayName`, the + * same ADR-0079 resolver `ObjectKanban`, `ObjectCalendar` and `ObjectGantt` + * already used, which is exactly why none of them needs a derived binding + * either. * - * Deriving the title binding here is the fix available at this seam: the key is - * on `FLAT_MAP_CONFIG_KEYS`, so it survives `ListView`'s whitelisted flatten and - * reaches `getMapConfig` ahead of that `'name'` literal. It is NOT the general - * fix — an `ObjectMap` that resolved titles through `getRecordDisplayName` like - * its siblings would not need a derived binding at all, and would also cover the - * paths this seam never sees (a hand-declared block that omits `titleField`, and - * every non-interface-page map). Filed separately. + * What the binding left behind once the forge was gone was not redundancy but + * an INVERSION. It reaches the resolver as `options.titleField`, which is + * precedence **step 0** — ahead of `objectDef.titleField`, ahead of the + * declared `nameField` pointer (step 1/2), and ahead of the legacy + * `titleFormat` template (step 3). Measured against that ladder, a field name + * derived HERE can only ever change the answer by out-ranking something the + * object itself declared; in every other case it reproduces, at step 0, the + * string the resolver already computes further down. So the binding's entire + * live effect was to let a per-view guess outrank declared metadata — the + * governed-authority default says the declared side wins, and it now does. * - * `titleField` is omitted, not defaulted, when nothing resolves: an absent key - * lets whatever `ObjectMap` does today stand, whereas a fabricated one would be - * indistinguishable from a declared choice at the read site. + * The async window `ObjectMap` has before its `getObjectSchema` fetch lands is + * NOT an argument for keeping a static binding here: `ObjectKanban` fetches its + * object definition exactly the same way and carries no derived title. Closing + * that window is a renderer-side change (hand the definition down from + * `ListView`, which already holds it), and it would cover the paths this seam + * never sees — a hand-declared `map` block that omits `titleField`, and every + * non-interface-page map. + * + * An author's own `map.titleField` is unaffected: it is declared, it travels as + * the view-level `map` block that `ListView` merges per key over this bag, and + * `getRecordDisplayName` honours it at step 0 by design. */ -export function defaultMapFromObject( - objectDef: any, -): { locationField: string; titleField?: string } | undefined { +export function defaultMapFromObject(objectDef: any): { locationField: string } | undefined { const field = firstFieldMatching(objectDef, (_n, f) => LOCATION_TYPES.has(f.type)) ?? firstFieldMatching(objectDef, (n) => /location|address|geo|coords?|place|venue/i.test(n)); if (!field) return undefined; - const titleField = displayFieldOfObject(objectDef); - return { locationField: field, ...(titleField ? { titleField } : {}) }; + return { locationField: field }; } export function InterfaceListPage({ page, className, onConfigChange, reserveEditAffordance }: InterfaceListPageProps) {