From 6231c6131c2ab3f8b357b24651b702de3330a191 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 19:05:35 +0000 Subject: [PATCH 1/2] feat(plugin-list): read the spec's view-level map block on list views MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ListMapConfigSchema` (objectstack#9340) has been authorable and validated since the @objectstack/spec 17.1.0 pin — it flows into this repo's own `ListViewSchema` by reference — but nothing read it. `ListView`'s `case 'map'` forwarded only the legacy `schema.options.map` bag, so declaring the typed block changed nothing at runtime and marker titles fell back to the renderer's placeholder. The block now merges over that bag, per key, which is the precedence the sibling visualization blocks in the same file already use. Both sources go through the existing objectui#5177 whitelist and the branch still emits the flat form, keeping `getMapConfig`'s objectui#5018 rule true. The switcher capability gate had the same hole with a sharper consequence: a view binding coordinates in the spec block was filtered out of its own `appearance.allowedVisualizations` and fell back to `['grid']`. It now asks the same merged config the render seam forwards. `InterfaceListPage` forwards the referenced view's block alongside the auto-derived binding rather than replacing it, so a partial authored block keeps the derived coordinates. No defaults are added for zoom/center — objectui#5000's fit-to-records behaviour is unchanged. Fixes #5042 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EuPCi56cnGyykygi3z9w4m --- .../listview-map-view-level-config-5042.md | 36 +++ .../InterfaceListPage.mapConfig.test.tsx | 152 ++++++++++ .../app-shell/src/views/InterfaceListPage.tsx | 10 + packages/plugin-list/src/ListView.tsx | 76 ++++- .../ListView.mapViewLevelConfig.test.tsx | 271 ++++++++++++++++++ packages/plugin-map/package.json | 1 + .../ObjectMap.listViewMapConfigReach.test.tsx | 211 ++++++++++++++ pnpm-lock.yaml | 3 + 8 files changed, 754 insertions(+), 6 deletions(-) create mode 100644 .changeset/listview-map-view-level-config-5042.md create mode 100644 packages/app-shell/src/views/InterfaceListPage.mapConfig.test.tsx create mode 100644 packages/plugin-list/src/__tests__/ListView.mapViewLevelConfig.test.tsx create mode 100644 packages/plugin-map/src/ObjectMap.listViewMapConfigReach.test.tsx diff --git a/.changeset/listview-map-view-level-config-5042.md b/.changeset/listview-map-view-level-config-5042.md new file mode 100644 index 0000000000..44ca3ccd04 --- /dev/null +++ b/.changeset/listview-map-view-level-config-5042.md @@ -0,0 +1,36 @@ +--- +'@object-ui/plugin-list': minor +'@object-ui/app-shell': minor +--- + +**Behaviour change:** the spec's view-level `map` block on a list view is now read at +runtime. `ListMapConfigSchema` (objectstack#9340) has been authorable and validated since +the `@objectstack/spec` 17.1.0 pin — it flows into this repo's own `ListViewSchema` by +reference — but nothing consumed it: `ListView`'s `case 'map'` forwarded only the legacy +`schema.options.map` bag, so declaring `map: { titleField: 'title', locationField: +'location' }` on a view changed nothing and marker titles fell back to the renderer's +placeholder. + +The block now reaches `plugin-map` and drives every one of its seven reads — coordinate +extraction, marker title and description, and the initial camera. Precedence follows the +convention the sibling visualization blocks in the same file already set: the view-level +block wins over `options.map`, per key, exactly as `kanban` / `calendar` / `gallery` / +`timeline` / `gantt` each merge their spec config over the legacy bag. Both sources go +through the existing objectui#5177 key whitelist, and the branch still emits the flat +form, so `getMapConfig`'s objectui#5018 precedence rule ("neither flattener emits a `map` +key at all") stays true. + +The visualization switcher had the same gap with a sharper consequence: the capability +gate that decides which visualizations are offered also read `options.map` alone, so a +view binding its coordinates in the spec block was filtered out of its own +`appearance.allowedVisualizations` and fell back to `['grid']`. The gate now asks the same +merged config the render seam forwards, so the two cannot disagree — including for a +binding split across the two sources. + +`InterfaceListPage` (ADR-0047 interface pages) forwards the referenced view's `map` block +for the same reason. It is passed alongside the auto-derived `options.map` rather than +replacing it, so a partial authored block — `map: { titleField: 'title' }` — keeps the +derived coordinate binding instead of dropping it. + +No defaults are introduced for `zoom` / `center`: an undeclared camera stays undeclared, +so the fit-to-queried-records behaviour ruled in objectui#5000 is unchanged. diff --git a/packages/app-shell/src/views/InterfaceListPage.mapConfig.test.tsx b/packages/app-shell/src/views/InterfaceListPage.mapConfig.test.tsx new file mode 100644 index 0000000000..9098d1374c --- /dev/null +++ b/packages/app-shell/src/views/InterfaceListPage.mapConfig.test.tsx @@ -0,0 +1,152 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#5042, the interface-page seam. + * + * `ListView` learned to read the spec's view-level `map` block + * (`ListMapConfigSchema`), but an ADR-0047 interface page builds the list-view + * schema itself, and its map binding was assembled from `options.map` alone: + * + * const mapCfg = (view.options as any)?.map ?? (allowedSet.has('map') ? … ) + * + * So a referenced view declaring the typed block had it dropped one seam ABOVE + * `ListView`, and the auto-derivation silently stood in for it — the same + * declared-but-inert shape the card is about, on the path the showcase map page + * actually renders through. + * + * The block is forwarded as `map` rather than folded into `mapCfg` with `??` + * like the sibling bindings, and the third case here is why: `??` would let a + * PARTIAL authored block replace the derivation wholesale, so + * `map: { titleField: 'title' }` alone would drop the auto-derived + * `locationField` and the page would render no markers at all. Forwarding both + * lets `ListView` merge them per key, which is the precedence the card settled. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import React from 'react'; + +vi.mock('react-router-dom', () => ({ + useSearchParams: () => [new URLSearchParams(), vi.fn()], + useNavigate: () => vi.fn(), +})); + +vi.mock('@object-ui/i18n', async (importOriginal) => { + const actual = await (importOriginal as any)(); + return { + ...actual, + useObjectTranslation: () => ({ t: (_k: string, o?: any) => o?.defaultValue ?? _k }), + }; +}); + +vi.mock('@object-ui/auth', async (importOriginal) => { + const actual = await (importOriginal as any)(); + return { ...actual, useAuth: () => ({}) }; +}); + +// Only the map-config half of the schema this page builds is under test. +vi.mock('@object-ui/plugin-list', () => ({ + ListView: (props: any) => ( +
+ ), +})); + +let testDataSource: any; +let testObjects: any[]; + +vi.mock('@object-ui/react', async (importOriginal) => { + const actual = await (importOriginal as any)(); + return { + ...actual, + useAdapter: () => testDataSource, + useMetadata: () => ({ objects: testObjects }), + }; +}); + +import { InterfaceListPage } from './InterfaceListPage'; + +const OBJECT_NAME = 'showcase_task'; +const VIEW_ID = `${OBJECT_NAME}.work_map`; + +/** An object with a location-typed field, so `defaultMapFromObject` can derive. */ +const makeObjectDef = (view: Record) => ({ + name: OBJECT_NAME, + fields: { + title: { type: 'text' }, + location: { type: 'location' }, + }, + listViews: { + [VIEW_ID]: { name: VIEW_ID, type: 'map', columns: ['title', 'location'], ...view }, + }, +}); + +const page = { + name: 'showcase_task_map', + label: 'Work Map', + interfaceConfig: { + source: OBJECT_NAME, + sourceView: 'work_map', + recordAction: 'none', + appearance: { allowedVisualizations: ['map'] }, + }, +}; + +async function renderWith(view: Record) { + testDataSource = {}; + testObjects = [makeObjectDef(view)]; + render(); + await waitFor(() => expect(screen.queryByTestId('list-view-map-config')).not.toBeNull()); + const el = screen.getByTestId('list-view-map-config'); + return { + map: JSON.parse(el.getAttribute('data-map') || 'null'), + optionsMap: JSON.parse(el.getAttribute('data-options-map') || 'null'), + }; +} + +describe('InterfaceListPage forwards the view-level `map` block (objectui#5042)', () => { + beforeEach(() => { + testDataSource = undefined; + testObjects = []; + }); + + // THE DISCRIMINATING ARM — before the fix `map` was `null` here. + it('forwards the referenced view’s spec `map` block verbatim', async () => { + const { map } = await renderWith({ + map: { locationField: 'location', titleField: 'title', zoom: 9 }, + }); + + expect(map).toEqual({ locationField: 'location', titleField: 'title', zoom: 9 }); + }); + + it('CONTROL: with no view-level block, no `map` key is emitted at all', async () => { + const { map, optionsMap } = await renderWith({}); + + expect(map).toBeNull(); + // …and the ADR-0047 auto-derivation still fires, unchanged. + expect(optionsMap).toEqual({ locationField: 'location' }); + }); + + it('keeps the auto-derived binding ALONGSIDE a partial authored block', async () => { + // The case that rules out folding the block into `mapCfg` with `??`: the + // 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. + const { map, optionsMap } = await renderWith({ map: { titleField: 'title' } }); + + expect(map).toEqual({ titleField: 'title' }); + expect(optionsMap).toEqual({ locationField: 'location' }); + }); + + it('CONTROL: the legacy `options.map` bag is still forwarded on its own path', async () => { + const { map, optionsMap } = await renderWith({ + options: { map: { locationField: 'location', titleField: 'legacy_title' } }, + }); + + expect(map).toBeNull(); + expect(optionsMap).toEqual({ locationField: 'location', titleField: 'legacy_title' }); + }); +}); diff --git a/packages/app-shell/src/views/InterfaceListPage.tsx b/packages/app-shell/src/views/InterfaceListPage.tsx index 6eaa56076a..5394421083 100644 --- a/packages/app-shell/src/views/InterfaceListPage.tsx +++ b/packages/app-shell/src/views/InterfaceListPage.tsx @@ -347,6 +347,13 @@ export function InterfaceListPage({ page, className, onConfigChange, reserveEdit view.gantt ?? (allowedSet.has('gantt') ? defaultGanttFromObject(objectDef) : undefined); // Map binding lives under options.map (locationField); auto-derive when // whitelisted so a map interface page renders without hand-wiring. + // + // The referenced view's own spec-level `map` block is NOT folded in here — + // it is forwarded as `map` on the schema below, so `ListView` merges it + // per key over this bag (objectui#5042). Collapsing the two with `??`, the + // way the sibling bindings above do, would make a partial authored block + // REPLACE the derivation: `map: { titleField: 'title' }` alone would drop + // the auto-derived `locationField` and the page would render no markers. const mapCfg = (view.options as any)?.map ?? (allowedSet.has('map') ? defaultMapFromObject(objectDef) : undefined); @@ -393,6 +400,9 @@ export function InterfaceListPage({ page, className, onConfigChange, reserveEdit gallery, timeline, gantt, + // The spec's view-level `map` block (`ListMapConfigSchema`), forwarded + // verbatim so `ListView` can merge it over the `options.map` bag below. + ...((view as any).map ? { map: (view as any).map } : {}), ...((mapCfg || (view.options as any)) ? { options: { ...((view.options as any) ?? {}), ...(mapCfg ? { map: mapCfg } : {}) } } : {}), // Presentation policy — the page layer (ADR-0047). diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index a179a4e792..43eb960471 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -81,6 +81,42 @@ function pickFlatMapConfig(mapConfig: unknown): Record { return Object.fromEntries(FLAT_MAP_CONFIG_KEYS.filter((key) => key in source).map((key) => [key, source[key]])); } +/** + * The effective map configuration for a list view: the spec's VIEW-LEVEL `map` + * block merged OVER the legacy `options.map` bag, per key. + * + * `map` is `ListMapConfigSchema` (objectstack#9340, consumable here since the + * `@objectstack/spec` 17.1.0 pin) — a strict, seven-key block that flows into + * this package's own `ListViewSchema` by reference (it is not in + * `LIST_VIEW_LOCAL_OVERRIDES`, so `specFieldsExcept` imports it). It was + * authorable and validated but never read: `case 'map'` forwarded only + * `schema.options?.map`, so declaring it changed nothing at runtime + * (objectui#5042). + * + * PRECEDENCE — the view-level block wins, per key. Both halves of that are the + * convention already set by every sibling visualization in this file, not a new + * rule: `kanban`, `calendar`, `gallery`, `timeline` and `gantt` each spread + * `schema.options?.` FIRST and `schema.` LAST, which is a per-key + * override in the view-level block's favour. (`tree` and `chart` also put the + * view-level block first, but with `||` — whole-block replacement rather than a + * merge. The direction is unanimous across all seven; only the granularity + * differs, and this follows the five that merge, which are also the five that + * flatten config into props the way the map branch does.) + * + * Both sides go through the same whitelist, so the typed block cannot + * reintroduce the `style` namespace collision that objectui#5177 closed. + * + * NOT a second validation of the seven keys — that reading belongs to + * `getMapConfig` in `ObjectMap.tsx` and stays there (objectui#5018). This is + * the whitelist-flatten that already existed, applied to one more source. + */ +function resolveListMapConfig(schema: { map?: unknown; options?: { map?: unknown } }): Record { + return { + ...pickFlatMapConfig(schema.options?.map), + ...pickFlatMapConfig(schema.map), + }; +} + /** * The list view's props. * @@ -1807,8 +1843,19 @@ export const ListView = React.forwardRef(({ resolvable.push('gantt'); } - // Check for Map capabilities - if (schema.options?.map?.locationField || (schema.options?.map?.latitudeField && schema.options?.map?.longitudeField)) { + // Check for Map capabilities (spec config takes precedence) + // + // Asked of the SAME merged config the render branch forwards + // (`resolveListMapConfig`), not of `options.map` alone: the gate and the + // seam must answer one question, or a view that binds its coordinates in + // the view-level `map` block renders fine but is filtered out of + // `allowedVisualizations` below — whitelist ∩ resolvable — and falls back + // to `['grid']`. That is what made the spec block inert for the SWITCHER + // even where the forward alone would have been enough (objectui#5042). + // Sharing the resolver also means a split binding (`latitudeField` on the + // block, `longitudeField` in the bag) is judged the way it will render. + const mapConfig = resolveListMapConfig(schema); + if (mapConfig.locationField || (mapConfig.latitudeField && mapConfig.longitudeField)) { resolvable.push('map'); } @@ -1834,7 +1881,7 @@ export const ListView = React.forwardRef(({ } return resolvable; - }, [schema.options, schema.viewType, schema.kanban, schema.calendar, schema.gantt, schema.gallery, schema.timeline, (schema as any).tree, schema.appearance?.allowedVisualizations]); + }, [schema.options, schema.viewType, schema.kanban, schema.calendar, schema.gantt, schema.gallery, schema.timeline, schema.map, (schema as any).tree, schema.appearance?.allowedVisualizations]); // Sync view from props React.useEffect(() => { @@ -2131,17 +2178,34 @@ export const ListView = React.forwardRef(({ ...(schema.options?.gantt || {}), ...(schema.gantt || {}), }; - case 'map': + case 'map': { // Whitelisted flatten (objectui#5177) — see `FLAT_MAP_CONFIG_KEYS`. // `schema.options.map` is an untyped bag; a raw spread here forwarded // every key the author wrote, including `style`, which `ObjectMap`'s // `FlatMapConfigKeys` declares OUT of this flat form. + // + // The spec's view-level `map` block merges over that bag — see + // `resolveListMapConfig` for the precedence and its sibling evidence. + // + // Emitted in the FLAT form, deliberately, exactly as before: a nested + // `map` key would win OUTRIGHT at `getMapConfig` (objectui#5018), which + // would turn this per-key merge into whole-block replacement of the bag + // and would trip `warnOnShadowedFlatMapKeys`. That precedence rule is + // written around the flatten product — "neither flattener emits a `map` + // key at all" — and this branch keeps that true. + // + // No camera is synthesized here: `pickFlatMapConfig` copies only keys + // the author actually wrote, so an undeclared `zoom`/`center` stays + // absent and `ObjectMap` still fits the camera to the queried records + // (objectui#5000, objectui#4941). + const mapConfig = resolveListMapConfig(schema); return { type: 'object-map', ...baseProps, - locationField: schema.options?.map?.locationField || 'location', - ...pickFlatMapConfig(schema.options?.map), + locationField: mapConfig.locationField || 'location', + ...mapConfig, }; + } case 'tree': { // Self-referencing tree-grid. Config lives under view.tree.* (direct) // or options.tree.* (app-shell object pages). parentField auto-detects diff --git a/packages/plugin-list/src/__tests__/ListView.mapViewLevelConfig.test.tsx b/packages/plugin-list/src/__tests__/ListView.mapViewLevelConfig.test.tsx new file mode 100644 index 0000000000..0ca5307bd1 --- /dev/null +++ b/packages/plugin-list/src/__tests__/ListView.mapViewLevelConfig.test.tsx @@ -0,0 +1,271 @@ +/** + * 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. + */ + +/** + * objectui#5042 — the spec's VIEW-LEVEL `map` block was authorable and + * validated, and inert. + * + * `ListMapConfigSchema` (objectstack#9340) flows into this repo's own + * `ListViewSchema` by reference — it is absent from `LIST_VIEW_LOCAL_OVERRIDES`, + * so `specFieldsExcept` imports it — which means `map: { … }` on a list view + * PARSED, type-checked and travelled all the way to `ListView`. It just was not + * read: `case 'map'` forwarded `schema.options?.map` and nothing else, so + * authoring the key changed nothing at runtime. The switcher had the same hole + * one level up, and a worse consequence: the capability gate that decides which + * visualizations are offered also looked only at `options.map`, so a view whose + * coordinates were bound in the spec block was filtered OUT of its own + * `appearance.allowedVisualizations` and fell back to `['grid']`. + * + * These pin the forward, the precedence, and the switcher gate. The end-to-end + * consequence — the block reaching `getMapConfig` and driving the seven reads — + * is pinned in `plugin-map`'s `ObjectMap.listViewMapConfigReach.test.tsx`, + * where the real reader and the maplibre mock live. + * + * PRECEDENCE, and why it is not a new rule: every sibling visualization in + * `ListView.tsx` already puts the view-level block ahead of the legacy + * `options.` bag. Five of them (`kanban`, `calendar`, `gallery`, + * `timeline`, `gantt`) spread `options` first and the view-level block last — + * a per-key override. Two (`tree`, `chart`) use `||`, replacing the bag + * wholesale. The DIRECTION is unanimous across all seven; only the granularity + * differs, and `map` follows the five that merge — which are also the five that + * flatten config into props the way the map branch does. + */ + +import React from 'react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { ComponentRegistry } from '@object-ui/core'; +import { render, waitFor, screen, fireEvent } from '@testing-library/react'; +import { ListView } from '../ListView'; +import { SchemaRendererProvider } from '@object-ui/react'; + +let captured: Array> = []; + +ComponentRegistry.register( + 'object-map', + (props: Record) => { + captured.push(props); + return
; + }, + { namespace: 'test', label: 'Map spy', category: 'view' }, +); + +const makeDataSource = () => ({ + find: vi.fn().mockResolvedValue([]), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ name: 'store', fields: {} }), +}); + +const BASE = { + type: 'list-view', + objectName: 'store', + viewType: 'map', + columns: ['name'], +} as const; + +/** Mount `ListView` on the given view and return the schema handed to `object-map`. */ +async function mapSchemaFor(view: Record) { + captured = []; + const dataSource = makeDataSource() as any; + render( + + + , + ); + await waitFor(() => expect(captured.length).toBeGreaterThan(0)); + return captured[captured.length - 1].schema as Record; +} + +describe('ListView forwards the view-level `map` block to plugin-map (objectui#5042)', () => { + beforeEach(() => { + captured = []; + }); + + // THE DISCRIMINATING ARM. Before the fix every one of these assertions read + // `undefined`: the block was parsed and then dropped on the floor. + it('forwards every one of the seven declared keys from the view-level block alone', async () => { + const schema = await mapSchemaFor({ + map: { + latitudeField: 'lat', + longitudeField: 'lng', + locationField: 'geo', + titleField: 'title', + descriptionField: 'blurb', + zoom: 12, + center: [40.7128, -74.006], + }, + }); + + expect(schema.type).toBe('object-map'); + expect(schema.latitudeField).toBe('lat'); + expect(schema.longitudeField).toBe('lng'); + expect(schema.locationField).toBe('geo'); + expect(schema.titleField).toBe('title'); + expect(schema.descriptionField).toBe('blurb'); + expect(schema.zoom).toBe(12); + expect(schema.center).toEqual([40.7128, -74.006]); + }); + + it('emits the FLAT form, never a nested `map` key', async () => { + // Not cosmetic. `getMapConfig` treats a nested `map` block as winning + // OUTRIGHT over the flat spelling (objectui#5018) — emitting one here would + // turn this per-key merge into whole-block replacement of the legacy bag + // and would trip `warnOnShadowedFlatMapKeys`. That precedence rule is + // written around the flattener: "neither flattener emits a `map` key at + // all". This keeps that sentence true. + const schema = await mapSchemaFor({ map: { locationField: 'geo' } }); + + expect(schema.locationField).toBe('geo'); + expect(Object.prototype.hasOwnProperty.call(schema, 'map')).toBe(false); + }); + + describe('precedence — view-level block over `options.map`, per key', () => { + it('lets the view-level block win on a key both declare', async () => { + const schema = await mapSchemaFor({ + options: { map: { locationField: 'geo', titleField: 'bag_title' } }, + map: { titleField: 'spec_title' }, + }); + + expect(schema.titleField).toBe('spec_title'); + }); + + it('keeps a key only the legacy bag declares — it is a merge, not a replacement', async () => { + // The arm that separates the five merging siblings from the two that use + // `||`. Under whole-block replacement `locationField` would be gone here + // and the map would render no markers at all. + const schema = await mapSchemaFor({ + options: { map: { locationField: 'geo', titleField: 'bag_title' } }, + map: { titleField: 'spec_title' }, + }); + + expect(schema.locationField).toBe('geo'); + }); + }); + + describe('controls', () => { + it('CONTROL: `options.map` alone still works, unchanged', async () => { + const schema = await mapSchemaFor({ + options: { map: { latitudeField: 'lat', longitudeField: 'lng', titleField: 'bag_title' } }, + }); + + expect(schema.latitudeField).toBe('lat'); + expect(schema.longitudeField).toBe('lng'); + expect(schema.titleField).toBe('bag_title'); + }); + + it('CONTROL: with no map config at all, only the `locationField` default is emitted', async () => { + const schema = await mapSchemaFor({}); + + expect(schema.locationField).toBe('location'); + // objectui#5000 / objectui#4941: no camera is synthesized anywhere on + // this path. `zoom`/`center` carry no spec default precisely so that + // "no declaration" stays distinguishable from a declared camera at the + // read site — declaring one here would silently overrule that and + // suppress the fit-to-records the absence is supposed to trigger. + expect(Object.prototype.hasOwnProperty.call(schema, 'zoom')).toBe(false); + expect(Object.prototype.hasOwnProperty.call(schema, 'center')).toBe(false); + }); + + it('CONTROL: a declared camera is forwarded only because the author wrote it', async () => { + const schema = await mapSchemaFor({ map: { locationField: 'geo', zoom: 3 } }); + + expect(schema.zoom).toBe(3); + expect(Object.prototype.hasOwnProperty.call(schema, 'center')).toBe(false); + }); + + it('applies the objectui#5177 whitelist to the view-level block too', async () => { + // `style` is also `BaseSchema.style` (inline CSS, legal on every node). + // The spec block is strict and could not carry it from a validated + // author, but `ListView` never parses — it renders what it is handed — + // so the new source goes through the same whitelist as the old one. + const schema = await mapSchemaFor({ + map: { locationField: 'geo', style: 'https://tiles.example.com/style.json', bogusKey: 'nope' }, + }); + + expect(schema.locationField).toBe('geo'); + expect(Object.prototype.hasOwnProperty.call(schema, 'style')).toBe(false); + expect(Object.prototype.hasOwnProperty.call(schema, 'bogusKey')).toBe(false); + }); + }); +}); + +/** + * The switcher half. `appearance.allowedVisualizations` is the author + * whitelist (ADR-0047) and the offered set is `whitelist ∩ resolvable` — so a + * capability gate that cannot see the spec block does not merely fail to offer + * `map`, it filters the author's own whitelist down to nothing and falls back + * to `['grid']`. That is the switcher consequence of the same missing read. + */ +describe('ListView offers `map` in the switcher from the view-level block (objectui#5042)', () => { + beforeEach(() => { + captured = []; + }); + + /** Mount a grid-typed view with the switcher shown, then open it. */ + const renderSwitcher = (view: Record) => { + const dataSource = makeDataSource() as any; + render( + + + , + ); + const trigger = screen.queryByTestId('view-switcher-dropdown'); + if (trigger) fireEvent.click(trigger); + }; + + /** + * Find a visualization option by accessible name, in either switcher form — + * the inline segmented control exposes `role="tab"`, the collapsed dropdown + * plain buttons. Mirrors `ListView.test.tsx`'s helper. + */ + const queryViewOption = (name: string) => + screen.queryByRole('tab', { name }) ?? screen.queryByRole('button', { name }); + + it('offers `map` when the binding is in the view-level block', () => { + renderSwitcher({ + appearance: { allowedVisualizations: ['grid', 'map'] }, + map: { locationField: 'geo' }, + }); + + expect(queryViewOption('Map')).toBeTruthy(); + }); + + it('CONTROL: `options.map` alone still resolves the capability', () => { + renderSwitcher({ + appearance: { allowedVisualizations: ['grid', 'map'] }, + options: { map: { locationField: 'geo' } }, + }); + + expect(queryViewOption('Map')).toBeTruthy(); + }); + + it('CONTROL: no binding anywhere leaves `map` unresolvable', () => { + renderSwitcher({ appearance: { allowedVisualizations: ['grid', 'map'] } }); + + expect(queryViewOption('Map')).toBeNull(); + }); + + it('resolves a binding SPLIT across the two sources, the way it will render', () => { + // The gate asks the merged config, so a split lat/lng pair is judged the + // same way the render seam resolves it. Asking `options.map` alone said + // "not resolvable" while the seam would have rendered it fine. + renderSwitcher({ + appearance: { allowedVisualizations: ['grid', 'map'] }, + options: { map: { latitudeField: 'lat' } }, + map: { longitudeField: 'lng' }, + }); + + expect(queryViewOption('Map')).toBeTruthy(); + }); +}); diff --git a/packages/plugin-map/package.json b/packages/plugin-map/package.json index c4d6e79a56..8411170892 100644 --- a/packages/plugin-map/package.json +++ b/packages/plugin-map/package.json @@ -46,6 +46,7 @@ "react-dom": "^18.0.0 || ^19.0.0" }, "devDependencies": { + "@object-ui/plugin-list": "workspace:*", "@types/react": "19.2.18", "@types/react-dom": "19.2.4", "@vitejs/plugin-react": "^6.0.5", diff --git a/packages/plugin-map/src/ObjectMap.listViewMapConfigReach.test.tsx b/packages/plugin-map/src/ObjectMap.listViewMapConfigReach.test.tsx new file mode 100644 index 0000000000..2ef89a5879 --- /dev/null +++ b/packages/plugin-map/src/ObjectMap.listViewMapConfigReach.test.tsx @@ -0,0 +1,211 @@ +/** + * 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. + */ + +/** + * objectui#5042 — the END of the chain: a view-level `map` block authored on a + * list view reaches `getMapConfig` intact and drives its reads. + * + * `ListView.mapViewLevelConfig.test.tsx` (plugin-list) pins the forward itself, + * against a spy. A spy can only ever show that a prop was PASSED, and this card + * is not about a prop — the reported symptom was `undefined` marker titles, one + * read site downstream of a config that never arrived. So this file drives the + * real `ListView` into the real `ObjectMap` and asserts what an author would + * see: the markers, their titles, and the camera. + * + * Two things make that seam worth its own test rather than an assumed join: + * + * - `getMapConfig` reaches the flat form ONLY through + * `if (schema.locationField || schema.latitudeField)`. `ListView` satisfies + * that gate today via its `locationField: … || 'location'` default; drop the + * default and every other flat key — `titleField` included — is silently + * ignored, with no type error and no failing unit test on either side. + * - The two packages agree on the FLAT spelling, and nothing but a test + * holds them to it: a nested `map` key would win outright here + * (objectui#5018) and quietly change the precedence the forward implements. + * + * `@object-ui/plugin-list` is a devDependency of this package for exactly this + * file — dev-only (no runtime source here imports it) and acyclic + * (`plugin-list` does not depend on `plugin-map`). The alternative homes cannot + * host it: `plugin-list` cannot resolve `react-map-gl` to mock it, and + * `apps/console`, which declares both plugins, cannot either. + */ + +import React from 'react'; +import { render, screen, waitFor, fireEvent } from '@testing-library/react'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { ComponentRegistry } from '@object-ui/core'; +import { SchemaRendererProvider } from '@object-ui/react'; +import { ListView } from '@object-ui/plugin-list'; +import { ObjectMap } from './ObjectMap'; +import { FIT_MAX_ZOOM, FIT_PADDING_PX } from './camera'; + +let capturedProps: any = null; + +vi.mock('react-map-gl/maplibre', () => ({ + default: (props: any) => { + capturedProps = props; + return
{props.children}
; + }, + Map: ({ children }: any) =>
{children}
, + NavigationControl: () =>
, + Marker: ({ children, longitude, latitude, onClick }: any) => ( +
onClick?.({ originalEvent: { stopPropagation() {} } })} + > + {children} +
+ ), + Popup: ({ children }: any) =>
{children}
, +})); + +// The real renderer, under the tag `ListView`'s `case 'map'` emits. +ComponentRegistry.register('object-map', ObjectMap as any, { + namespace: 'test', + label: 'Object Map', + category: 'view', +}); + +/** + * Showcase-shaped records: the platform's `location` value is `{ lat, lng }`, + * and the marker text field is `title` — the pair the card names. + */ +const records = [ + { id: '1', title: 'Install rooftop unit', blurb: 'Bldg A', location: { lat: 47.6062, lng: -122.3321 } }, + { id: '2', title: 'Replace filters', blurb: 'Bldg B', location: { lat: 37.7749, lng: -122.4194 } }, +]; + +const makeDataSource = () => ({ + find: vi.fn().mockResolvedValue(records), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ + name: 'showcase_task', + fields: { + title: { type: 'text' }, + blurb: { type: 'text' }, + location: { type: 'location' }, + }, + }), +}); + +/** Mount a `map` list view and settle both the ListView fetch and the map. */ +async function renderListViewMap(view: Record) { + capturedProps = null; + const dataSource = makeDataSource() as any; + render( + + + , + ); + await waitFor(() => expect(screen.queryByLabelText('Map')).not.toBeNull()); + await waitFor(() => expect(capturedProps).not.toBeNull()); +} + +describe('a view-level `map` block reaches getMapConfig through ListView (objectui#5042)', () => { + beforeEach(() => { + capturedProps = null; + }); + + // THE DISCRIMINATING ARM — `titleField`, the card's own symptom. + it('renders marker titles from the declared `titleField`', async () => { + await renderListViewMap({ map: { locationField: 'location', titleField: 'title' } }); + + await waitFor(() => expect(screen.getAllByTestId('map-marker').length).toBe(2)); + + // The title is a real read, not a forwarded prop: `getMapConfig` resolves + // `titleField`, the marker transform reads `record[titleField]`, and the + // popup renders it. Before the forward landed this read `'Marker'` — the + // literal `getMapConfig` falls back to — which is the `undefined`/placeholder + // marker-title symptom the card was filed for. + fireEvent.click(screen.getAllByTestId('map-marker')[0]); + await waitFor(() => expect(screen.queryByTestId('map-popup')).not.toBeNull()); + expect(screen.getByText('Install rooftop unit')).toBeTruthy(); + }); + + it('renders the marker description from the declared `descriptionField`', async () => { + await renderListViewMap({ + map: { locationField: 'location', titleField: 'title', descriptionField: 'blurb' }, + }); + + await waitFor(() => expect(screen.getAllByTestId('map-marker').length).toBe(2)); + fireEvent.click(screen.getAllByTestId('map-marker')[0]); + await waitFor(() => expect(screen.queryByTestId('map-popup')).not.toBeNull()); + expect(screen.getByText('Bldg A')).toBeTruthy(); + }); + + it('honours a declared camera — the declaration suppresses the fit', async () => { + await renderListViewMap({ + map: { locationField: 'location', titleField: 'title', zoom: 12, center: [40.7128, -74.006] }, + }); + + const { bounds, zoom, longitude, latitude } = capturedProps.initialViewState; + + // `center` is `[lat, lng]` (ObjectMapConfigSchema's own description). + expect(latitude).toBe(40.7128); + expect(longitude).toBe(-74.006); + expect(zoom).toBe(12); + // The whole point of carrying no spec default for `zoom`/`center` + // (objectui#5000): a DECLARED camera opts out of the fit. If the block had + // not reached here, `bounds` would be set and these three would be the + // fitted values instead. + expect(bounds).toBeUndefined(); + }); + + it('CONTROL: with no camera declared, the map still fits the queried records', async () => { + await renderListViewMap({ map: { locationField: 'location', titleField: 'title' } }); + + const { bounds, fitBoundsOptions } = capturedProps.initialViewState; + + expect(bounds).toBeDefined(); + expect(fitBoundsOptions).toEqual({ padding: FIT_PADDING_PX, maxZoom: FIT_MAX_ZOOM }); + }); + + it('CONTROL: the legacy `options.map` bag still reaches the same reads', async () => { + await renderListViewMap({ options: { map: { locationField: 'location', titleField: 'title' } } }); + + await waitFor(() => expect(screen.getAllByTestId('map-marker').length).toBe(2)); + fireEvent.click(screen.getAllByTestId('map-marker')[0]); + await waitFor(() => expect(screen.queryByTestId('map-popup')).not.toBeNull()); + expect(screen.getByText('Install rooftop unit')).toBeTruthy(); + }); + + it('precedence: the view-level block wins over the bag at the READ site', async () => { + // Both name a marker-title field; the records carry distinct values, so the + // rendered popup says which config actually drove the read. + await renderListViewMap({ + options: { map: { locationField: 'location', titleField: 'blurb' } }, + map: { titleField: 'title' }, + }); + + await waitFor(() => expect(screen.getAllByTestId('map-marker').length).toBe(2)); + fireEvent.click(screen.getAllByTestId('map-marker')[0]); + await waitFor(() => expect(screen.queryByTestId('map-popup')).not.toBeNull()); + + expect(screen.getByText('Install rooftop unit')).toBeTruthy(); + // …and the coordinate binding the bag alone supplied still worked, which is + // what makes this a per-key merge rather than a replacement: markers exist + // at all only because `locationField` survived from the bag. + expect(screen.getAllByTestId('map-marker').length).toBe(2); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b7437cb1ce..dc8e5f92a4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2324,6 +2324,9 @@ importers: specifier: ^4.4.3 version: 4.4.3 devDependencies: + '@object-ui/plugin-list': + specifier: workspace:* + version: link:../plugin-list '@types/react': specifier: 19.2.18 version: 19.2.18 From 304ad757998e33a3fef567d8ebc114ccee0bf1c7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 19:36:44 +0000 Subject: [PATCH 2/2] test(plugin-list): label the declared-camera arm as discriminating, not a control Ablation A measured it RED: it authors a view-level `map` block, so removing the forward breaks it. Calling it a CONTROL claimed the opposite and would have misread the next ablation's output. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EuPCi56cnGyykygi3z9w4m --- .../src/__tests__/ListView.mapViewLevelConfig.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/plugin-list/src/__tests__/ListView.mapViewLevelConfig.test.tsx b/packages/plugin-list/src/__tests__/ListView.mapViewLevelConfig.test.tsx index 0ca5307bd1..aa4986e73a 100644 --- a/packages/plugin-list/src/__tests__/ListView.mapViewLevelConfig.test.tsx +++ b/packages/plugin-list/src/__tests__/ListView.mapViewLevelConfig.test.tsx @@ -173,7 +173,7 @@ describe('ListView forwards the view-level `map` block to plugin-map (objectui#5 expect(Object.prototype.hasOwnProperty.call(schema, 'center')).toBe(false); }); - it('CONTROL: a declared camera is forwarded only because the author wrote it', async () => { + it('forwards a declared camera, and only the half the author wrote', async () => { const schema = await mapSchemaFor({ map: { locationField: 'geo', zoom: 3 } }); expect(schema.zoom).toBe(3);