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
20 changes: 20 additions & 0 deletions .changeset/map-marker-title-binding.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
---
'@object-ui/app-shell': patch
---

Interface-page maps: derive a marker-title binding from the object's display field

An ADR-0047 interface page that whitelists `map` derives its map binding with
`defaultMapFromObject`, which bound only `locationField`. With no `titleField`
reaching `ObjectMap`, `getMapConfig` filled the gap with the literal `'name'`
and the marker title is a plain `record[titleField]` read — so on any object
whose display field is not `name` (for example one keyed by `title`), every
marker popup titled itself `undefined`.

The derivation now also binds the object's display field, resolved with the
field-name half of ADR-0079's precedence: the declared `nameField` (and its
`displayNameField` / `NAME_FIELD_KEY` aliases), otherwise the shared
`deriveTitleField` scan from `@object-ui/core` — the same ranking the kanban,
calendar and gantt renderers resolve titles through, so a map and a board over
one object agree on what a record is called. When nothing resolves the key is
omitted rather than defaulted. A hand-declared `map` block still wins per key.
120 changes: 116 additions & 4 deletions packages/app-shell/src/views/InterfaceListPage.mapConfig.test.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -67,7 +67,8 @@ vi.mock('@object-ui/react', async (importOriginal) => {
};
});

import { InterfaceListPage } from './InterfaceListPage';
import { getRecordDisplayName } from '@object-ui/core';
import { InterfaceListPage, defaultMapFromObject } from './InterfaceListPage';

const OBJECT_NAME = 'showcase_task';
const VIEW_ID = `${OBJECT_NAME}.work_map`;
Expand DownExpand Up@@ -126,8 +127,11 @@ 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, unchanged.
expect(optionsMap).toEqual({ locationField: 'location' });
// …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' });
});

it('keeps the auto-derived binding ALONGSIDE a partial authored block', async () => {
Expand All@@ -138,7 +142,7 @@ describe('InterfaceListPage forwards the view-level `map` block (objectui#5042)'
const { map, optionsMap } = await renderWith({ map: { titleField: 'title' } });

expect(map).toEqual({ titleField: 'title' });
expect(optionsMap).toEqual({ locationField: 'location' });
expect(optionsMap).toEqual({ locationField: 'location', titleField: 'title' });
});

it('CONTROL: the legacy `options.map` bag is still forwarded on its own path', async () => {
Expand All@@ -150,3 +154,111 @@ describe('InterfaceListPage forwards the view-level `map` block (objectui#5042)'
expect(optionsMap).toEqual({ locationField: 'location', titleField: 'legacy_title' });
});
});

/**
* objectui#5909 — the derived marker-title binding.
*
* ## What the siblings actually do (measured, because the card asserted it)
*
* 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.
*
* 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`.
*
* So these arms pin the binding against the SAME ADR-0079 field ranking the
* sibling renderers use, rather than against a rule invented here.
*/
describe('defaultMapFromObject derives the marker title (objectui#5909)', () => {
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' });
});

it('accepts the deprecated `displayNameField` / `NAME_FIELD_KEY` aliases', () => {
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({
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({
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);
});

it('CONTROL: no location field still derives nothing at all', () => {
expect(defaultMapFromObject({ fields: { title: { type: 'text' } } })).toBeUndefined();
});

// 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));
});
});
73 changes: 69 additions & 4 deletions packages/app-shell/src/views/InterfaceListPage.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,6 +23,7 @@ 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';
Expand DownExpand Up@@ -181,13 +182,77 @@ export function defaultGanttFromObject(objectDef: any): { startDateField: string
return { startDateField: start, endDateField: end, ...(progress ? { progressField: progress } : {}) };
}

// Map needs a location/geo field (or address). Auto-derive from a location-typed
// field, else a field whose name looks geographic.
export function defaultMapFromObject(objectDef: any): { locationField: string } | undefined {
/**
* 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)
*
* 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`.
*
* 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.
*
* `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.
*/
export function defaultMapFromObject(
objectDef: any,
): { locationField: string; titleField?: 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));
return field ? { locationField: field } : undefined;
if (!field) return undefined;
const titleField = displayFieldOfObject(objectDef);
return { locationField: field, ...(titleField ? { titleField } : {}) };
}

export function InterfaceListPage({ page, className, onConfigChange, reserveEditAffordance }: InterfaceListPageProps) {
Expand Down
Loading