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..aa4986e73a --- /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('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); + 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