From eb813566e1e7bc3d6ec95d2d7a4b8296ff03a116 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 14:21:11 +0000 Subject: [PATCH] =?UTF-8?q?feat(core):=20isEmptyValue=20=E2=80=94=20the=20?= =?UTF-8?q?shared=20emptiness=20floor,=20and=20five=20surfaces=20that=20st?= =?UTF-8?q?ate=20their=20answer=20against=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five surfaces each held their own answer to "is this value empty", and objectui#8481 was the third rediscovery of the same hole. The weakest common claim — null, undefined, the empty string, the empty array — now lives in `@object-ui/core` as `isEmptyValue`, below every consumer. The floor was not invented: `evaluator/optionRules.ts` had spelled exactly those four members privately, and this promotes that copy. Every surface that answers differently keeps its own answer, rewritten as an explicit call on the floor with the justification at the site: - `hasCellValue` and `RelatedList.isValueEmpty` extend it with a trim; - `BooleanCellRenderer` extends it with every non-boolean (false stays a value); the date cells with every falsy scalar (the epoch stays empty); - `JsonCellRenderer` DECLINES its `[]` member — the array literal is drawn on purpose — and `LocationCellRenderer` / `AddressCellRenderer` inherit that through the JSON fallback; `FileCellRenderer` states "0 files". Two visible fixes: a gallery card and a kanban card holding `[]` in a card field now omit that field, as they already did for `null`, instead of drawing a labelled "No value" em-dash beside fields that were omitted. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB --- .changeset/8496-emptiness-floor.md | 23 +++ packages/core/src/evaluator/optionRules.ts | 11 +- packages/core/src/index.ts | 7 + .../__tests__/emptiness-floor-8496.test.ts | 128 ++++++++++++ packages/core/src/utils/emptiness.ts | 104 ++++++++++ .../emptinessFloorExtensions-8496.test.tsx | 182 ++++++++++++++++++ packages/fields/src/index.tsx | 178 +++++++++++------ packages/plugin-detail/src/RelatedList.tsx | 20 +- .../emptinessFloorExtensions-8496.test.tsx | 146 ++++++++++++++ packages/plugin-detail/src/emptiness.ts | 28 ++- .../ObjectKanban.emptinessFloor-8496.test.tsx | 141 ++++++++++++++ packages/plugin-kanban/src/ObjectKanban.tsx | 24 ++- packages/plugin-list/src/ObjectGallery.tsx | 17 +- ...ObjectGallery.emptinessFloor-8496.test.tsx | 113 +++++++++++ 14 files changed, 1036 insertions(+), 86 deletions(-) create mode 100644 .changeset/8496-emptiness-floor.md create mode 100644 packages/core/src/utils/__tests__/emptiness-floor-8496.test.ts create mode 100644 packages/core/src/utils/emptiness.ts create mode 100644 packages/fields/src/__tests__/emptinessFloorExtensions-8496.test.tsx create mode 100644 packages/plugin-detail/src/__tests__/emptinessFloorExtensions-8496.test.tsx create mode 100644 packages/plugin-kanban/src/ObjectKanban.emptinessFloor-8496.test.tsx create mode 100644 packages/plugin-list/src/__tests__/ObjectGallery.emptinessFloor-8496.test.tsx diff --git a/.changeset/8496-emptiness-floor.md b/.changeset/8496-emptiness-floor.md new file mode 100644 index 0000000000..85914b4c97 --- /dev/null +++ b/.changeset/8496-emptiness-floor.md @@ -0,0 +1,23 @@ +--- +'@object-ui/core': minor +'@object-ui/fields': patch +'@object-ui/plugin-detail': patch +'@object-ui/plugin-list': patch +'@object-ui/plugin-kanban': patch +--- + +Add `isEmptyValue` to `@object-ui/core` — the weakest common claim about "is +this value empty": `null`, `undefined`, the empty string, the empty array, and +never a fifth member (objectui#8496, director seat, decision batch #86). + +Five surfaces had each grown their own copy of those four members, and +objectui#8481 was the third rediscovery of the same hole. They now call the +shared floor and state their own answer against it: `record:details`' +`hasCellValue` and `RelatedList` extend it with a trim, `BooleanCellRenderer` +with every non-boolean, the date cells with every falsy scalar; `JsonCellRenderer` +declines its `[]` member out loud (the array literal is drawn on purpose) and +`FileCellRenderer` states "0 files" instead. + +Two visible fixes come with it: a gallery card and a kanban card holding an +empty array in a card field now OMIT that field, as they already did for `null`, +instead of drawing a labelled "No value" em-dash for it. diff --git a/packages/core/src/evaluator/optionRules.ts b/packages/core/src/evaluator/optionRules.ts index d5c36a9806..fbda36cd3d 100644 --- a/packages/core/src/evaluator/optionRules.ts +++ b/packages/core/src/evaluator/optionRules.ts @@ -31,6 +31,7 @@ */ import type { DependsOnInput } from '@object-ui/types'; import { evalFieldPredicate, type FieldRulePredicate } from './fieldRules.js'; +import { isEmptyValue } from '../utils/emptiness.js'; /** * Minimal shape of a select/radio option this module reads. Deliberately has no @@ -68,10 +69,12 @@ export function resolveDependsOnFields(dependsOn: DependsOnInput): string[] { .filter((f): f is string => typeof f === 'string' && f.length > 0); } -/** A value counts as "empty" (dependency unmet) when nullish, blank, or an empty array. */ -function isEmptyValue(v: unknown): boolean { - return v === undefined || v === null || v === '' || (Array.isArray(v) && v.length === 0); -} +// A dependency counts as UNMET on exactly the shared floor — `null`, +// `undefined`, the empty string, the empty array — and this module is where +// those four members were first written down. objectui#8496 promoted them out +// of here into `utils/emptiness.ts` (byte-for-byte the same four) so the four +// other surfaces that had each re-spelled them could stop. No extension and no +// declension: a gated option list asks the floor and nothing more. /** * True when at least one `dependsOn` field is empty in the record — the option diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 68c48e5abe..72b347dd97 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -23,6 +23,13 @@ export * from './utils/dom-props.js'; export * from './utils/filter-converter.js'; export * from './utils/managedBy.js'; export * from './utils/extract-records.js'; +// The emptiness FLOOR (objectui#8496, director seat, decision batch #86): the +// weakest common claim about "is this value empty" — `null`, `undefined`, the +// empty string, the empty array — below `plugin-detail`, `plugin-list`, +// `plugin-kanban` and `@object-ui/fields`, each of which used to spell those +// four members privately. Surfaces EXTEND it or DECLINE a member out loud; ⛔ +// the floor itself never grows past the four. +export * from './utils/emptiness.js'; export * from './utils/expand-fields.js'; // The RETIREMENT gate (objectui#4914, maintainer ruling B). Homed here rather // than in `@object-ui/fields` because `@object-ui/components` is one of its six diff --git a/packages/core/src/utils/__tests__/emptiness-floor-8496.test.ts b/packages/core/src/utils/__tests__/emptiness-floor-8496.test.ts new file mode 100644 index 0000000000..967d77e415 --- /dev/null +++ b/packages/core/src/utils/__tests__/emptiness-floor-8496.test.ts @@ -0,0 +1,128 @@ +/** + * 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. + */ + +/** + * The FLOOR itself (objectui#8496 — director seat, decision batch #86). + * + * This file pins the cheap half: the four members, and the ⛔ that keeps them + * four. The EXPENSIVE half — that no surface's deliberate disagreement was + * flattened into the floor — is pinned next to each surface: + * `emptinessFloorExtensions-8496.test.tsx` in `@object-ui/fields` and in + * `@object-ui/plugin-detail`, `galleryEmptinessFloor-8496.test.tsx` in + * `@object-ui/plugin-list`, `kanbanEmptinessFloor-8496.test.tsx` in + * `@object-ui/plugin-kanban`. + * + * ⚠️ A suite that only proves the floor works proves the half that was never + * in doubt. Read the four files above as one pin. + */ + +import { describe, it, expect } from 'vitest'; +import { isEmptyValue } from '../emptiness.js'; +import { isOptionGroupGated, isValueStillOffered } from '../../evaluator/optionRules.js'; + +/** The four members, and nothing else is one. */ +const MEMBERS: Array<[string, unknown]> = [ + ['null', null], + ['undefined', undefined], + ['the empty string', ''], + ['the empty array', []], +]; + +/** + * Every candidate FIFTH member, each with the measurement that refused it. + * These are values, and the floor calling any of them empty is the failure + * this table exists to catch. + */ +const REFUSED_FIFTH_MEMBERS: Array<[string, unknown, string]> = [ + ['a whitespace-only string', ' ', + 'EMPTY only on record:details and RelatedList (objectui#8350) — an extension, not a member'], + ['an empty object literal', {}, + 'measured a VALUE and pinned (objectui#8474): a type-aware renderer draws it'], + ['a populated object', { a: 1 }, 'a populated object is drawn by a type-aware renderer'], + ['a one-entry array', [1], 'one entry is one thing to draw'], + ['an array of one undefined', [undefined], 'length 1: the container has an entry'], + ['zero', 0, 'a stored zero is a value on every surface'], + ['false', false, 'BooleanCellRenderer keeps false a value (objectui#8582)'], + ['the numeric epoch', 0, "DateCellRenderer's `!value` calls it empty — that is its extension"], + ['the Date epoch', new Date(0), + 'Object.keys(new Date(0)).length === 0, which is why that shape is not the test'], + ['a populated Map', new Map([['a', 1]]), + 'Object.keys() is empty on it — a false-empty the floor must not have'], + ['a populated Set', new Set([1]), 'same false-empty shape as Map'], + ['a class instance behind getters', new (class { get a() { return 1; } })(), + 'same false-empty shape: state that Object.keys() cannot see'], + ['the string "0"', '0', 'a non-empty string is a value however falsy it coerces'], + ['NaN', NaN, 'falsy, but not one of the four members'], +]; + +describe('objectui#8496 — the emptiness floor in @object-ui/core', () => { + describe('THE FLOOR — exactly four members', () => { + for (const [label, value] of MEMBERS) { + it(`${label} is EMPTY`, () => { + expect(isEmptyValue(value), `${label} must be a floor member`).toBe(true); + }); + } + }); + + describe('⛔ THE FLOOR NEVER GROWS — every candidate fifth member is a VALUE', () => { + for (const [label, value, why] of REFUSED_FIFTH_MEMBERS) { + it(`${label} is a VALUE — ${why}`, () => { + expect( + isEmptyValue(value), + `${label}: the floor grew a fifth member. ${why}`, + ).toBe(false); + }); + } + }); + + describe('THE MEMBER COUNT — stated as a number, so a widening cannot pass unnoticed', () => { + it('exactly 4 of the probed shapes are empty', () => { + const probes: unknown[] = [ + ...MEMBERS.map(([, v]) => v), + ...REFUSED_FIFTH_MEMBERS.map(([, v]) => v), + ]; + expect( + probes.filter((v) => isEmptyValue(v)).length, + 'the floor answered EMPTY for something outside its four members', + ).toBe(MEMBERS.length); + }); + }); + + /** + * The floor was not invented: it was PROMOTED out of this package's own + * private copy in `evaluator/optionRules.ts`, which had spelled the same four + * members since before the card. These two exports are that copy's only + * readers, so their answers are the promotion's non-regression evidence. + */ + describe('THE PROMOTION — core’s own former private copy still answers the same', () => { + for (const [label, value] of MEMBERS) { + it(`a dependency holding ${label} gates the option list`, () => { + expect( + isOptionGroupGated('parent', { parent: value }), + `${label}: an unmet dependency must still gate`, + ).toBe(true); + }); + } + + it('a dependency holding a value does NOT gate', () => { + expect(isOptionGroupGated('parent', { parent: 'cn' })).toBe(false); + expect(isOptionGroupGated('parent', { parent: 0 })).toBe(false); + expect(isOptionGroupGated('parent', { parent: false })).toBe(false); + }); + + it('an empty value is always still offered (nothing to clear)', () => { + for (const [label, value] of MEMBERS) { + expect( + isValueStillOffered(value, [{ label: 'A', value: 'a' }]), + `${label}: an empty value has no stale choice to clear`, + ).toBe(true); + } + expect(isValueStillOffered('gone', [{ label: 'A', value: 'a' }])).toBe(false); + }); + }); +}); diff --git a/packages/core/src/utils/emptiness.ts b/packages/core/src/utils/emptiness.ts new file mode 100644 index 0000000000..9ec86ba9f2 --- /dev/null +++ b/packages/core/src/utils/emptiness.ts @@ -0,0 +1,104 @@ +/** + * ObjectUI — the shared emptiness floor + * 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. + */ + +/** + * THE floor under "is this value empty" (objectui#8496 — director seat, + * decision batch #86, 2026-09-08, option B). + * + * Exactly four members, and it never grows past them: + * + * `null` · `undefined` · the empty string · the empty array + * + * ## What a floor IS, and what it is not + * + * It is the WEAKEST claim the surfaces below it can all make — not the answer + * any one of them gives. A caller does one of two things with it, and both are + * legitimate: + * + * - **extends** it — `isEmptyValue(v) || ` — when its surface + * calls MORE things empty (a grid trims whitespace; a boolean column calls + * every non-boolean empty); + * - **declines a member** in the open — `isEmptyValue(v) && !Array.isArray(v)` + * — when its surface has MEASURED that member to be a value there (a `json` + * cell draws the two-character literal `[]` on purpose, objectui#8474). + * + * What is NOT legitimate is a sixth private re-spelling of these four members. + * That is the defect this function exists to close: `plugin-detail`'s + * `hasCellValue`, `RelatedList`'s `isValueEmpty`, `ObjectGallery`'s and + * `ObjectKanban`'s inline guards and the guard idioms across + * `@object-ui/fields`' cell renderers each grew their own copy, and + * objectui#8481 was the THIRD rediscovery of the same hole — objectui#8474 and + * objectui#8459 had each closed it at their own door first. A copy that agrees + * today stops agreeing; one definition cannot. + * + * ## ⛔ The floor never grows past those four members + * + * The ruling fixed the member list, and every candidate fifth member is a + * measured disagreement rather than an oversight: + * + * - **whitespace-only strings.** `' '` is EMPTY on `record:details` and in + * `RelatedList` (objectui#8350 measured the damage a blank cell does there) + * and a VALUE on the gallery, the kanban and the shared renderers. Both are + * right for their surface, so the trim is an EXTENSION, not a member. + * - **`{}`.** Measured as a VALUE and pinned (objectui#8474): a populated or + * empty object literal is handed to a type-aware renderer that draws it, and + * the shape that would sweep it in — `Object.keys(v).length === 0` — is also + * true of `new Date(0)`, of a populated `Map`, of a populated `Set` and of + * any class instance whose state sits behind getters. + * - **`0` / `false`.** Values everywhere. `BooleanCellRenderer` keeping + * `false` a value is the pinned case (objectui#8582). + * + * ## Why `@object-ui/core` and not `@object-ui/types` + * + * It is a runtime predicate, not a protocol type, so it belongs in the engine. + * The ruling made that conditional on a measurement — `@object-ui/fields` is + * the lowest consumer, and if it did not already depend on `core` the floor + * would have had to fall back to `types`. Measured on the implementing branch: + * `@object-ui/fields`' `package.json` lists `@object-ui/core` in + * `dependencies`, and its barrel already imports from it. No new dependency + * edge is created by this file, in either direction — `core` reaches no + * consumer, which is why exporting the helper from `@object-ui/fields` instead + * (option C) was refused: the gallery and the kanban would then import a + * `fields` helper to decide whether to call a `fields` renderer. + * + * ## "Empty" is two questions; this floor answers the half both share + * + * objectui#8496's later evidence (comment 5603203484) measured that the word + * has split in two on this codebase: SCALAR-MISSING (`EmptyValue`, the em-dash + * affordance whose accessible name is fixed) and COLLECTION-EMPTY + * (`EmptyDescription`, an author's own sentence). The floor serves both and + * does not have to choose: its four members ARE two scalar-missing members, + * one blank scalar and one empty collection, and no call site asks a boolean to + * tell those apart — each one knows statically which affordance it is drawing. + * ⛔ So this function is deliberately NOT the place to grow a second axis. Which + * COMPONENT states the emptiness is a different question, carried by + * objectui#8570 / objectui#8526 / objectui#8507. + * + * ## Readers + * + * `isOptionGroupGated` / `isValueStillOffered` here in `core` (this function's + * origin: it was written privately in `evaluator/optionRules.ts`, byte-for-byte + * these four members, before the ruling promoted it); `hasCellValue` and + * `RelatedList.isValueEmpty` in `@object-ui/plugin-detail`; `ObjectGallery`'s + * card-field row filter; `ObjectKanban`'s card-field loop; and the cell-renderer + * guards in `@object-ui/fields`. + * + * The extensions and the declensions are pinned — the assertion that each + * surface still answers DIFFERENTLY from the floor, not merely that the floor + * works — in `__tests__/emptiness-floor-8496.test.ts` here and in + * `emptinessFloorExtensions-8496.test.tsx` in `@object-ui/fields` and + * `@object-ui/plugin-detail`. + */ +export function isEmptyValue(value: unknown): boolean { + return ( + value === undefined || + value === null || + value === '' || + (Array.isArray(value) && value.length === 0) + ); +} diff --git a/packages/fields/src/__tests__/emptinessFloorExtensions-8496.test.tsx b/packages/fields/src/__tests__/emptinessFloorExtensions-8496.test.tsx new file mode 100644 index 0000000000..132b26e8a9 --- /dev/null +++ b/packages/fields/src/__tests__/emptinessFloorExtensions-8496.test.tsx @@ -0,0 +1,182 @@ +/** + * 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. + */ + +/** + * The EXPENSIVE half of objectui#8496: proof that putting the floor in + * `@object-ui/core` FLATTENED NOTHING. + * + * The ruling (director seat, decision batch #86, option B) is two clauses, and + * only the second one is hard: the floor goes below every consumer, AND every + * surface that answers differently keeps its own answer. A suite that only + * proves `isEmptyValue` works proves the clause nobody doubted. So every case + * below asserts a DISAGREEMENT with the floor — either a member this renderer + * DECLINES, or an extension it makes past the four. + * + * ⛔ If one of these goes red because a renderer now "just calls the floor", + * that is not a test to update. It is the flattening the ruling forbids. + */ + +import React from 'react'; +import { describe, it, expect } from 'vitest'; +import { render } from '@testing-library/react'; +import { getCellRenderer, resolveCellRendererType } from '../index'; +import { isEmptyValue } from '@object-ui/core'; + +/** Resolve + render exactly the way a consumer builds a read-mode cell. */ +function renderCell(type: string, value: unknown, field: Record = {}) { + const Renderer = getCellRenderer(resolveCellRendererType({ type }) || type); + return render( + , + ); +} + +/** The shared "No value" affordance — a muted glyph carrying an aria-label. */ +const affordance = (root: HTMLElement) => + root.querySelector('[data-slot="empty-value"]'); + +/** The four floor members, by name. */ +const FLOOR: Array<[string, unknown]> = [ + ['null', null], + ['undefined', undefined], + ["''", ''], + ['[]', []], +]; + +describe('objectui#8496 — @object-ui/fields renderers against the floor', () => { + /** + * The renderers whose guard IS the floor, with no clause of their own. These + * are the cases the promotion had to keep byte-identical. + */ + describe('THE FLOOR REACHED — every member answered by the shared affordance', () => { + for (const type of ['select', 'status', 'multiselect', 'tags', 'lookup', 'master_detail', 'text', 'formula', 'color']) { + for (const [label, value] of FLOOR) { + it(`\`${type}\` holding ${label} draws the affordance`, () => { + const { container } = renderCell(type, value, { options: [{ value: 'a', label: 'A' }] }); + expect( + affordance(container), + `${type} holding ${label}: the floor member lost its affordance`, + ).not.toBeNull(); + }); + } + } + }); + + /** + * ⛔ THE DECLENSIONS. Each of these renderers calls `[]` a VALUE, and each + * one was measured before it was allowed to. The floor says `[]` is empty; + * these four say otherwise, out loud, in code. + */ + describe('⛔ NOT FLATTENED — the renderers that DECLINE the floor’s `[]` member', () => { + it('the floor itself calls [] empty — the premise these cases disagree with', () => { + expect(isEmptyValue([])).toBe(true); + }); + + for (const type of ['json', 'object', 'composite', 'record']) { + it(`\`${type}\` holding [] still prints the array literal (objectui#8474, pinned)`, () => { + const { container } = renderCell(type, []); + expect( + container.textContent, + `${type}: the two-character literal is the measured answer here`, + ).toContain('[]'); + expect( + affordance(container), + `${type} holding []: drawing the affordance flattens objectui#8474`, + ).toBeNull(); + }); + } + + for (const type of ['location', 'geolocation', 'address']) { + it(`\`${type}\` holding [] keeps the unknown shape visible through its JSON fallback`, () => { + const { container } = renderCell(type, []); + expect(container.textContent, `${type}: an unknown shape must stay visible`).toContain('[]'); + expect( + affordance(container), + `${type} holding []: swallowing the shape is what the JSON fallback exists to prevent`, + ).toBeNull(); + }); + } + + for (const type of ['file', 'video', 'audio']) { + it(`\`${type}\` holding [] states its COUNT rather than "No value"`, () => { + const { container } = renderCell(type, []); + expect( + container.textContent, + `${type}: "0 files" is an answer the em-dash cannot give`, + ).toContain('0'); + expect( + affordance(container), + `${type} holding []: the count is the measured answer here`, + ).toBeNull(); + }); + } + }); + + /** + * ⛔ THE EXTENSIONS. Each renderer below calls something EMPTY that the floor + * calls a value. Deleting the extension would leave a green floor and a + * broken cell. + */ + describe('⛔ NOT FLATTENED — the renderers that answer MORE than the floor', () => { + it('`boolean` calls every NON-BOOLEAN empty — the floor says nothing about them (objectui#8582)', () => { + for (const [label, value] of [["the string 'false'", 'false'], ['0', 0], ['{}', {}], ["'x'", 'x']] as Array<[string, unknown]>) { + expect(isEmptyValue(value), `${label} is not a floor member`).toBe(false); + const { container } = renderCell('boolean', value); + expect( + affordance(container), + `boolean holding ${label}: only a real boolean is a value of a boolean column`, + ).not.toBeNull(); + } + }); + + it('⛔ but `false` STAYS A VALUE — the member the floor must never grow', () => { + const { container } = renderCell('boolean', false, { name: 'completed' }); + expect( + affordance(container), + 'boolean holding false: a stored false is a value, not a blank', + ).toBeNull(); + }); + + for (const type of ['date', 'datetime']) { + it(`\`${type}\` deliberately calls the numeric EPOCH empty — its \`!value\` extension`, () => { + expect(isEmptyValue(0), '0 is not a floor member').toBe(false); + const { container } = renderCell(type, 0); + expect( + affordance(container), + `${type} holding 0: the epoch is empty here on purpose (the ruling names this one)`, + ).not.toBeNull(); + }); + } + + it('`user` extends the floor with every falsy scalar', () => { + const { container } = renderCell('user', 0); + expect( + affordance(container), + 'user holding 0: an unresolved reference of zero is not a user', + ).not.toBeNull(); + }); + + for (const type of ['number', 'currency', 'percent', 'email', 'url', 'phone']) { + it(`\`${type}\` extends the floor with WHITESPACE, on the coerced text`, () => { + expect(isEmptyValue(' '), "' ' is not a floor member").toBe(false); + const { container } = renderCell(type, ' '); + expect( + affordance(container), + `${type} holding ' ': Number(' ') is 0, a digit the record never held`, + ).not.toBeNull(); + }); + } + + it('⛔ but `text` does NOT trim — the floor exactly, and the disagreement is the point', () => { + const { container } = renderCell('text', ' '); + expect( + affordance(container), + "text holding ' ': a stored string keeps its spaces; the trim belongs to the coercing renderers", + ).toBeNull(); + }); + }); +}); diff --git a/packages/fields/src/index.tsx b/packages/fields/src/index.tsx index 9e593ba1ef..41327228e8 100644 --- a/packages/fields/src/index.tsx +++ b/packages/fields/src/index.tsx @@ -8,7 +8,7 @@ import React from 'react'; import type { DateTimeFieldMetadata, FieldMetadata, SelectOptionMetadata } from '@object-ui/types'; -import { ComponentRegistry, percentDisplayValue, getRecordDisplayName, humanizeLabel, isMissingForRequired, formatDate, formatDateTime, formatDateTimeCompactParts, formatRelativeDate, extractRecords, type ComponentMeta, type DateDisplayOptions } from '@object-ui/core'; +import { ComponentRegistry, percentDisplayValue, getRecordDisplayName, humanizeLabel, isEmptyValue, isMissingForRequired, formatDate, formatDateTime, formatDateTimeCompactParts, formatRelativeDate, extractRecords, type ComponentMeta, type DateDisplayOptions } from '@object-ui/core'; // The platform's own value-shape contract, asked rather than restated // (objectui#6744). See `locationStoredValueSchemaFor` below for why this is a // runtime import in the barrel and not a hand-written coordinate range. @@ -333,6 +333,31 @@ import { coerceToSafeValue } from './coerceToSafeValue.js'; export { coerceToSafeValue }; /** + * ## This package and the emptiness FLOOR (objectui#8496) + * + * `@object-ui/core`'s `isEmptyValue` is the weakest common claim — `null`, + * `undefined`, `''`, `[]` — and every guard in this file now stands in a STATED + * relation to it instead of re-spelling its members. There are three relations, + * and all three are legitimate: + * + * - **the floor exactly** — `SelectCellRenderer`, `LookupCellRenderer`, + * `TextCellRenderer`, `FormulaCellRenderer`, `ColorSwatchCellRenderer`; + * - **the floor EXTENDED** — this helper (+ whitespace, on the coerced text); + * `UserCellRenderer` (+ every falsy scalar); `BooleanCellRenderer` + * (+ every non-boolean, objectui#8582); `DateCellRenderer` / + * `DateTimeCellRenderer` (+ every falsy scalar, so the numeric epoch is + * empty, and + every unparsable one, objectui#8581); + * - **the floor with a member DECLINED, out loud** — `JsonCellRenderer` draws + * the two-character literal for `[]` on purpose (objectui#8474 measured and + * kept it), `LocationCellRenderer` and `AddressCellRenderer` inherit that + * through their JSON fallback, and `FileCellRenderer` states "0 files". + * + * ⛔ Those disagreements are MEASURED, not drift: do not "finish the job" by + * making every renderer answer the floor. The pins that go red if one is + * flattened are `__tests__/emptinessFloorExtensions-8496.test.tsx`. + * + * --- + * * A coerced cell text with nothing in it is NOT a cell value (objectui#8490). * * `coerceToSafeValue([])` joins zero entries into `''`, and every renderer @@ -347,13 +372,23 @@ export { coerceToSafeValue }; * blank string, whatever produced it. Whitespace counts as blank for the same * reason — `Number(' ')` is `0` too. * - * ⛔ Not the package's general emptiness predicate — see `isEmptyMultiValue` - * below for why there is none. This answers ONE question for the renderers - * that coerce to text before they draw: "did the coercion leave anything to - * draw?". `BooleanCellRenderer` does not coerce to text and does not ask it. + * ⛔ Not the package's general emptiness predicate — the renderers do not agree + * on what "empty" means, and the roster above says where each one stands. This + * answers ONE question for the renderers that coerce to text before they draw: + * "did the coercion leave anything to draw?". `BooleanCellRenderer` does not + * coerce to text and does not ask it. */ function isBlankCellText(safe: ReturnType): boolean { - return safe == null || (typeof safe === 'string' && safe.trim() === ''); + // THE FLOOR by name, taken on the COERCED text rather than on the raw value + // (objectui#8496). `[]` never reaches it as an array — `coerceToSafeValue` + // joins zero entries into `''`, which is the floor's string member. + return ( + isEmptyValue(safe) || + // THE EXTENSION: whitespace counts as blank, because `Number(' ')` is `0` + // too. It is not a floor member — `' '` is a value on the gallery, the + // kanban and `TextCellRenderer`. + (typeof safe === 'string' && safe.trim() === '') + ); } /** @@ -648,7 +683,11 @@ function TruncatedText({ */ export function TextCellRenderer({ value }: CellRendererProps): React.ReactElement { const safe = coerceToSafeValue(value); - if (safe == null || safe === '') return ; + // THE FLOOR by name and nothing more (objectui#8496), on the coerced text. + // ⛔ Deliberately NOT `isBlankCellText`: a stored `' '` is a value of a text + // cell and keeps its spaces — the trim belongs to the renderers that go on to + // coerce the text into a number, a date or an `href`. + if (isEmptyValue(safe)) return ; return ; } @@ -829,6 +868,12 @@ export function BooleanCellRenderer({ value, field }: CellRendererProps): React. // no boolean here. `null` / `undefined` and `[]` (objectui#8490: an empty // array holds no boolean) are the same answer for the same reason. A real // `false` is a value and stays an unchecked box. + // + // THE FLOOR, STRICTLY EXTENDED (objectui#8496): every one of its four members + // is a non-boolean, so this one test already answers all of them and adding + // `isEmptyValue(value) ||` in front of it would be a dead disjunct. What the + // floor must NOT do here is grow a `false` member — that is the pinned + // disagreement on this renderer. if (typeof value !== 'boolean') { return ; } @@ -882,6 +927,11 @@ export function DateCellRenderer({ value, field }: CellRendererProps): React.Rea // disagrees with itself. const locale = useDisplayLocale(); const t = useFieldTranslate(); + // THE FLOOR, EXTENDED with every falsy scalar (objectui#8496). The extension + // is deliberate and is the pinned disagreement on this renderer: `0` — the + // numeric epoch — is EMPTY on a date cell, where the floor says nothing about + // it. The floor's own `[]` member is answered one line down, on the coerced + // text, because `[]` is truthy. ⛔ Do not "fix" this to spare the epoch. if (!value) return ; const safe = coerceToSafeValue(value); // `[]` is truthy, so it passed the guard above and reached `formatDate` as @@ -952,6 +1002,10 @@ export function DateTimeCellRenderer({ value, field }: CellRendererProps): React // `undefined`, i.e. the machine's locale, on every session. const locale = useDisplayLocale(); const t = useFieldTranslate(); + // THE FLOOR, EXTENDED with every falsy scalar — the numeric epoch included, + // spelled EXACTLY as `DateCellRenderer` one function up (objectui#8496). The + // floor's `[]` member is answered by the unparsable-date test below, which + // `coerceToSafeValue([])` reaches as `''`. if (!value) return ; const safe = coerceToSafeValue(value); const date = safe != null ? new Date(safe as string | number) : null; @@ -1568,46 +1622,6 @@ export function getSemanticHex(name?: string, fallback: string = '#3b82f6'): str return COLOR_NAME_HEX[name] ?? fallback; } -/** - * An array with zero entries is not a cell value (objectui#8481). - * - * Three renderers below open a MULTI-VALUE container and map their entries - * into it — `SelectCellRenderer` (a flex-wrap row of badges/dots), - * `LookupCellRenderer` (a flex-wrap row of record chips) and - * `UserCellRenderer` (an overlapping avatar stack). Each one's opening guard - * tested only `null`/`undefined`/`''`, so `[]` reached the array branch and - * mapped over zero entries: the renderer's whole output was a CHILDLESS - * container — no glyph, no `aria-label`, a visually blank cell. - * - * That blindness lived in the SHARED renderer, so it was the same blank cell - * on every surface. `@object-ui/plugin-detail` had already grown two private - * upstream pre-checks against it (objectui#8474's `hasCellValue`, and - * `RelatedList`'s `isValueEmpty` from objectui#8459); every consumer that does - * NOT pre-check — `ObjectGrid`, `ObjectGallery`, `ObjectKanban`, - * `ObjectDataTable` — reached the renderer directly and painted the blank. - * A renderer with nothing to draw says so itself rather than depending on - * every caller remembering to ask first. - * - * ⛔ Deliberately NOT the package's general emptiness predicate, and - * deliberately not exported. The renderers in this file do NOT agree on what - * "empty" means, and that disagreement is measured and in several places - * intentional: `JsonCellRenderer` draws the two-character literal for `[]` - * (objectui#8474 measured and kept that), `FileCellRenderer` states "0 files", - * `BooleanCellRenderer` treats `false` as a value while `DateCellRenderer`'s - * `!value` treats the epoch as empty. This helper answers ONE question — "is - * this a multi-value container with no entries to draw?" — for the renderers - * that ask it: the three below. `BooleanCellRenderer` asked it too between - * objectui#8490 and objectui#8582; its guard is now `typeof value !== 'boolean'`, - * which answers the same question for `[]` (an array is not a boolean) and for - * every non-boolean scalar besides. The renderers that coerce - * to text before they draw ask `isBlankCellText` instead — the same ruling, - * taken on the coerced string. Unifying the rest is a separate, contested - * change. - */ -function isEmptyMultiValue(value: unknown): boolean { - return Array.isArray(value) && value.length === 0; -} - /** * Select field cell renderer. * @@ -1626,10 +1640,18 @@ export function SelectCellRenderer({ value, field }: CellRendererProps): React.R const options: SelectOptionMetadata[] = selectField.options || []; const appearance: 'badge' | 'dot' = selectField.appearance === 'dot' ? 'dot' : 'badge'; - // `[]` is handled HERE rather than in the array branch below, because this - // is the statement the renderer makes about having nothing to draw - // (objectui#8481). - if (value == null || value === '' || isEmptyMultiValue(value)) return ; + // THE FLOOR by name and nothing more (objectui#8496). It used to be spelled + // out here as `value == null || value === '' || isEmptyMultiValue(value)` — + // the same four members, in the fourth of five private copies. + // + // `[]` is a floor MEMBER, and it is answered HERE rather than in the array + // branch below because this is the statement the renderer makes about having + // nothing to draw (objectui#8481): the branch opens a flex-wrap row of badges + // and maps zero entries into it, so its whole output was a CHILDLESS + // container — no glyph, no accessible name, a visually blank cell. The same + // shape is why `LookupCellRenderer` (a row of record chips) and + // `UserCellRenderer` (an overlapping avatar stack) ask the floor too. + if (isEmptyValue(value)) return ; // Match a stored value to a configured option, falling back to a // case-insensitive comparison so seed data with mixed case @@ -1870,6 +1892,10 @@ export function FileCellRenderer({ value, field }: CellRendererProps): React.Rea // conditional return violates the rules of hooks — the call would be skipped // for an empty value and hook order would desync between renders. const t = useFieldTranslate(); + // THE FLOOR WITH `[]` DECLINED, and extended with every other falsy scalar + // (objectui#8496). A file cell STATES ITS COUNT, so an empty array is a + // value here — it renders "0 files", which is an answer the em-dash cannot + // give. ⛔ Do not replace this with `isEmptyValue(value)`. if (!value) return ; const fileField = field as any; @@ -1938,6 +1964,10 @@ export function ImageCellRenderer({ value }: CellRendererProps): React.ReactElem [value], ); + // THE FLOOR, EXTENDED twice (objectui#8496): every falsy scalar, and every + // value that resolves to no displayable image. `[]` is covered by the second + // extension rather than by a floor call — unlike `FileCellRenderer` next + // door, an image cell has no count to state. if (!value || imgs.length === 0) return ; const imageAlt = (idx: number, name?: string) => @@ -2151,10 +2181,10 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R // Always call the hook (rules of hooks). It safely no-ops when inputs are missing. const resolvedName = useLookupName(referenceTo, primaryPrimitiveId, displayField); - // Same childless-container defect as `SelectCellRenderer` above: the array - // branch further down opens a flex-wrap row of chips and maps zero entries - // into it (objectui#8481). - if (value == null || value === '' || isEmptyMultiValue(value)) return ; + // THE FLOOR by name and nothing more (objectui#8496). Same childless-container + // defect as `SelectCellRenderer` above: the array branch further down opens a + // flex-wrap row of chips and maps zero entries into it (objectui#8481). + if (isEmptyValue(value)) return ; // A reference can arrive as a JSON-encoded object string — e.g. an // unresolved external-id reference '{"externalId":"Website Relaunch"}'. @@ -2310,7 +2340,9 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R */ export function FormulaCellRenderer({ value }: CellRendererProps): React.ReactElement { const safe = coerceToSafeValue(value); - if (safe == null || safe === '') return ; + // THE FLOOR by name and nothing more (objectui#8496), on the coerced text — + // same relation as `TextCellRenderer`, which this renderer's output mirrors. + if (isEmptyValue(safe)) return ; return ( {String(safe)} @@ -2407,9 +2439,12 @@ function UnresolvedUserReference({ * User/Owner field cell renderer (with avatars) */ export function UserCellRenderer({ value }: CellRendererProps): React.ReactElement { - // `!value` never saw `[]` — a truthy empty array reached the avatar-stack - // branch below and rendered an empty stack (objectui#8481). - if (!value || isEmptyMultiValue(value)) return ; + // THE FLOOR by name (objectui#8496) plus ONE extension: every falsy scalar. + // `!value` alone never saw `[]` — a truthy empty array reached the + // avatar-stack branch below and rendered an empty stack (objectui#8481) — + // and the floor alone would let `0` through to `UnresolvedUserReference`, + // which is not what a user reference of zero is. + if (isEmptyValue(value) || !value) return ; // A primitive is an UNRESOLVED reference, not "the ID/username" (objectui#8434). // The comment that stood here stated the branch's premise, and the premise was @@ -2563,7 +2598,13 @@ export function resolveCellRendererType(fieldOrType: string | { type?: string; f * stringified; primitives fall through to their string form. */ export function JsonCellRenderer({ value }: CellRendererProps): React.ReactElement { - if (value == null || value === '') return ; + // THE FLOOR WITH ONE MEMBER DECLINED, and the declension is the point + // (objectui#8496). `[]` is a floor member everywhere else in this file; here + // it is a VALUE and draws the two-character literal, because a `json` cell + // states the structure the record holds and "an empty array" is a structure. + // objectui#8474 measured that and pinned it. ⛔ Do not simplify this to + // `isEmptyValue(value)`: that flattens a decision already on the record. + if (isEmptyValue(value) && !Array.isArray(value)) return ; let text: string; if (typeof value === 'object') { try { @@ -2584,7 +2625,10 @@ export function JsonCellRenderer({ value }: CellRendererProps): React.ReactEleme * Renders a `color` value as a swatch alongside its hex/string value. */ export function ColorSwatchCellRenderer({ value }: CellRendererProps): React.ReactElement { - if (value == null) return ; + // THE FLOOR by name and nothing more (objectui#8496). `''` and `[]` reached + // the same affordance one branch down (`String([])` is `''`, which the blank + // test below caught); asking the floor here says so once, at the door. + if (isEmptyValue(value)) return ; // An object is not a colour (objectui#8596). `String({})` is // `'[object Object]'`, which this renderer handed to `background-color` — // an invalid declaration the browser drops, so the swatch drew a bordered @@ -2635,7 +2679,11 @@ import { RICH_TEXT_CELL_RENDERERS } from './widgets/richTextDisplay.js'; * or a `[lat, lng]` array. Falls back to compact JSON for anything else. */ export function LocationCellRenderer({ value }: CellRendererProps): React.ReactElement { - if (value == null || value === '') return ; + // THE FLOOR WITH `[]` DECLINED (objectui#8496), inherited rather than chosen: + // an unrecognized shape falls through to `JsonCellRenderer` below, whose + // pinned answer for `[]` is the array literal (objectui#8474). Declining the + // member here keeps the two ends of that fallback saying one thing. + if (isEmptyValue(value) && !Array.isArray(value)) return ; let lat: number | undefined; let lng: number | undefined; if (typeof value === 'object' && !Array.isArray(value)) { @@ -2694,7 +2742,11 @@ export function AddressCellRenderer({ value }: CellRendererProps): React.ReactEl // renderer in this file already uses, and it is provider-safe (it resolves // to `'en'` — the unchanged small-to-large order — with nothing mounted). const locale = useDisplayLocale(); - if (value == null || value === '') return ; + // THE FLOOR WITH `[]` DECLINED (objectui#8496), for the same inherited reason + // as `LocationCellRenderer`: this renderer's own docblock promises that an + // unknown shape stays visible through the JSON fallback rather than being + // swallowed, and `[]` is an unknown shape here. + if (isEmptyValue(value) && !Array.isArray(value)) return ; // A plain string address (some apps store one) is already display-ready. if (typeof value === 'string') return ; if (typeof value === 'object' && !Array.isArray(value)) { diff --git a/packages/plugin-detail/src/RelatedList.tsx b/packages/plugin-detail/src/RelatedList.tsx index aba06333a3..42d6b30664 100644 --- a/packages/plugin-detail/src/RelatedList.tsx +++ b/packages/plugin-detail/src/RelatedList.tsx @@ -49,6 +49,7 @@ import { compareSortValues, getRecordDisplayName, getSortValue, + isEmptyValue, isExpandableFieldType, isPlatformSortableField, isUnmaterializedFieldType, @@ -1050,6 +1051,13 @@ export const RelatedList: React.FC = ({ * * ## Why this does NOT delegate to `DetailSection`'s `hasCellValue` * + * ⚠️ objectui#8496 put the four members BOTH functions share into + * `@object-ui/core`'s `isEmptyValue` and had each call it. That is a shared + * FLOOR, not a merge: this predicate and `hasCellValue` stay two functions + * on purpose, because a grid COLUMN and a record ROW ask the question at two + * granularities, and objectui#8459 measured this one as the better-shaped + * answer here. ⛔ Do not "finish the job" by deleting one of them. + * * Measured, not assumed. `hasCellValue` answers `true` for every non-null * `object`, and `typeof [] === 'object'` — so it calls an EMPTY ARRAY a * VALUE. This surface calls it empty, and that is the answer a grid needs: @@ -1067,10 +1075,14 @@ export const RelatedList: React.FC = ({ * `__tests__/RelatedList.emptinessAgreement-8459.test.tsx`. */ const isValueEmpty = (v: any) => - v === null || - v === undefined || - (typeof v === 'string' && v.trim() === '') || - (Array.isArray(v) && v.length === 0); + // THE FLOOR, asked by name (objectui#8496): `null`, `undefined`, `''`, + // `[]`. Those four are no longer spelled here. + isEmptyValue(v) || + // THE EXTENSION, and the only one: a WHITESPACE-ONLY string is empty in a + // grid cell. It is not a floor member because the gallery, the kanban and + // the shared cell renderers all keep `' '` a value; only this surface + // and `record:details` trim, each for the reason objectui#8350 measured. + (typeof v === 'string' && v.trim() === ''); const pruneEmpty = (cols: any[]): any[] => { if (!relatedData.length) return cols; diff --git a/packages/plugin-detail/src/__tests__/emptinessFloorExtensions-8496.test.tsx b/packages/plugin-detail/src/__tests__/emptinessFloorExtensions-8496.test.tsx new file mode 100644 index 0000000000..8dcf2c39f8 --- /dev/null +++ b/packages/plugin-detail/src/__tests__/emptinessFloorExtensions-8496.test.tsx @@ -0,0 +1,146 @@ +/** + * 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. + */ + +/** + * `@object-ui/plugin-detail`'s two emptiness answers, stated as a DISAGREEMENT + * with the shared floor (objectui#8496, option B). + * + * Both `hasCellValue` and `RelatedList.isValueEmpty` now call + * `@object-ui/core`'s `isEmptyValue` for the four members they used to spell + * privately. What this file pins is the part the floor does NOT carry: + * + * - the TRIM. `' '` is a value to the floor, and empty on both of these + * surfaces — objectui#8350 measured what a visually blank cell costs on + * `record:details`, and objectui#8459 measured the same for a grid cell; + * - the REFUSALS. `{}`, a `Date`, `0` and `false` are values here, so a floor + * that grew any of them would be red below. + * + * ⛔ And the two predicates stay TWO. objectui#8459 measured `RelatedList`'s as + * the better-shaped answer for a grid and pinned it as deliberately separate; a + * shared floor is not permission to merge them. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import * as React from 'react'; +import { isEmptyValue } from '@object-ui/core'; + +// The real data-table and its cell renderers must be registered — the DOM case +// below reads what they DRAW. +import '@object-ui/components'; +import { hasCellValue } from '../emptiness'; +import { RelatedList } from '../RelatedList'; + +const EM_DASH = '—'; + +/** Desktop: `RelatedList` renders a gallery, not a table, on mobile. */ +beforeEach(() => { + Object.defineProperty(window, 'innerWidth', { configurable: true, value: 1280 }); +}); + +describe('objectui#8496 — plugin-detail extends the floor, and is not flattened into it', () => { + describe('THE FLOOR REACHED — hasCellValue answers all four members', () => { + for (const [label, value] of [ + ['null', null], + ['undefined', undefined], + ["''", ''], + ['[]', []], + ] as Array<[string, unknown]>) { + it(`${label} has NO cell value`, () => { + expect(isEmptyValue(value), `CONTROL: ${label} is a floor member`).toBe(true); + expect(hasCellValue(value), `${label}: the floor member lost its answer here`).toBe(false); + }); + } + }); + + describe('⛔ NOT FLATTENED — the TRIM, which the floor does not have', () => { + for (const blank of [' ', '\t', '\n ']) { + it(`a whitespace-only string (${JSON.stringify(blank)}) is EMPTY here and a VALUE to the floor`, () => { + expect( + isEmptyValue(blank), + 'CONTROL: the floor deliberately keeps whitespace a value — the gallery and the kanban rely on that', + ).toBe(false); + expect( + hasCellValue(blank), + 'the trim is this surface’s extension (objectui#8350); losing it repaints the blank cell', + ).toBe(false); + }); + } + + it('the extension is visible one layer up: a whitespace-only grid cell draws the em-dash', async () => { + expect(isEmptyValue(' '), 'CONTROL: the floor says this is a value').toBe(false); + + const dataSource = { + getObjectSchema: vi.fn(async () => ({ + name: 'line', + fields: { + product: { type: 'text', label: 'Product' }, + note: { type: 'text', label: 'Note' }, + }, + })), + find: vi.fn(async () => ({ + data: [ + { id: '1', product: 'Widget', note: ' ' }, + { id: '2', product: 'Gadget', note: 'real note' }, + ], + total: 2, + })), + }; + + const { container } = render( + , + ); + await waitFor(() => expect(container.querySelector('table')).not.toBeNull()); + await waitFor(() => expect(container.textContent).toContain('Widget')); + + const headers = Array.from(container.querySelectorAll('th')).map((th) => + (th.textContent ?? '').trim(), + ); + const idx = headers.indexOf('Note'); + expect(idx, 'the Note column survives — one row has a value').toBeGreaterThanOrEqual(0); + + const rows = container.querySelectorAll('tbody tr'); + const blankCell = rows[0]?.querySelectorAll('td')[idx]; + expect( + (blankCell?.textContent ?? '').trim(), + 'RelatedList still trims: the whitespace-only cell draws the placeholder, not a blank', + ).toBe(EM_DASH); + // CONTROL: the same column rendered BY VALUE for the sibling row. + const realCell = rows[1]?.querySelectorAll('td')[idx]; + expect((realCell?.textContent ?? '').trim(), 'CONTROL: the populated cell renders').toBe( + 'real note', + ); + }); + }); + + describe('⛔ NOT FLATTENED — the members hasCellValue refuses to let the floor grow', () => { + for (const [label, value, why] of [ + ['{}', {}, 'objectui#8474 measured it a VALUE: a type-aware renderer draws the literal'], + ['{ a: 1 }', { a: 1 }, 'a populated object is drawn by its type-aware renderer'], + ['[1]', [1], 'one entry is one thing to draw'], + ['0', 0, 'a stored zero is a value'], + ['false', false, 'a stored false is a value'], + ['new Date(0)', new Date(0), 'Object.keys() is empty on it — the false-empty shape the docblock refuses'], + ['a populated Map', new Map([['a', 1]]), 'same false-empty shape'], + ] as Array<[string, unknown, string]>) { + it(`${label} is a VALUE — ${why}`, () => { + expect(isEmptyValue(value), `CONTROL: ${label} is not a floor member`).toBe(false); + expect(hasCellValue(value), `${label}: ${why}`).toBe(true); + }); + } + }); +}); diff --git a/packages/plugin-detail/src/emptiness.ts b/packages/plugin-detail/src/emptiness.ts index 91e9fdde8a..9a8142fdbe 100644 --- a/packages/plugin-detail/src/emptiness.ts +++ b/packages/plugin-detail/src/emptiness.ts @@ -6,7 +6,7 @@ * LICENSE file in the root directory of this source tree. */ -import { recordDisplayValueAt } from '@object-ui/core'; +import { isEmptyValue, recordDisplayValueAt } from '@object-ui/core'; /** * Does this cell have anything to render? **THE** definition of emptiness on @@ -72,6 +72,12 @@ import { recordDisplayValueAt } from '@object-ui/core'; * * ## The one object shape that is NOT a value: `[]` (objectui#8474) * + * That member now arrives from the SHARED FLOOR rather than from a clause + * written here — `isEmptyValue` in `@object-ui/core` (objectui#8496). The + * reasoning below is what MEASURED it, and it is kept because it is the reason + * the floor may be asked here at all; the four members themselves are no longer + * this file's to spell. + * * Every example above is a POPULATED object. `typeof [] === 'object'`, so until * objectui#8474 an EMPTY array took the same branch and was a value — and there * the reasoning stops holding, because the type-aware renderer has nothing to @@ -136,18 +142,24 @@ import { recordDisplayValueAt } from '@object-ui/core'; * answers EMPTY for everything. */ export function hasCellValue(value: unknown): boolean { + // THE FLOOR, asked first and by name (objectui#8496): `null`, `undefined`, + // `''` and `[]`. `[]` is the member that has to be answered HERE rather than + // by the display-name authority below — that function answers "does this + // resolve to a NAME", and it calls `{}` and a `Date` empty too, correct for a + // title and a false-empty for a cell (objectui#8474). + if (isEmptyValue(value)) return false; // Object/array values belong to the cell renderers, not to the display-name // chain — see the docblock above. `typeof null === 'object'`, so null is - // excluded here and answered by the authority below. + // already gone at the floor; a POPULATED object reaching here is a value. if (value !== null && typeof value === 'object') { - // …with exactly one exception: an EMPTY array, which no cell renderer has - // anything to draw for (objectui#8474). Answered HERE rather than by - // falling through to the authority below: that function answers "does this - // resolve to a NAME", and it calls `{}` and a `Date` empty too — correct - // for a title, a false-empty for a cell. - if (Array.isArray(value) && value.length === 0) return false; return true; } + // THE EXTENSION, and the only one: a WHITESPACE-ONLY string is empty here + // and is not a floor member (objectui#8350 measured what a visually blank + // cell costs on this page; the gallery, the kanban and the shared renderers + // all keep `' '` a value, which is why the trim is an extension and not a + // fifth member of the floor). + // // A one-key synthetic record is how a VALUE asks the authority its question: // `recordDisplayValueAt` is keyed `(record, field)` because its callers read // a field off a record, while a call site's value has several sources (the diff --git a/packages/plugin-kanban/src/ObjectKanban.emptinessFloor-8496.test.tsx b/packages/plugin-kanban/src/ObjectKanban.emptinessFloor-8496.test.tsx new file mode 100644 index 0000000000..632acc824b --- /dev/null +++ b/packages/plugin-kanban/src/ObjectKanban.emptinessFloor-8496.test.tsx @@ -0,0 +1,141 @@ +/** + * 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. + */ + +/** + * `ObjectKanban`'s card-field loop asks the shared FLOOR (objectui#8496). + * + * ## What changed + * + * The loop opened with `raw == null || raw === ''` — three of the floor's four + * members, spelled privately, and the fourth (`[]`) fell through. objectui#8489 + * caught one consequence a step later (a fully coloured pill with no children, + * on the picklist fork) and repaired it AT THE LABEL, deliberately declining to + * make the kanban learn what "empty" means. The OTHER fork was still open: a + * non-picklist card field holding `[]` reached the shared cell renderer, which + * since objectui#8481 answers it with the "No value" em-dash — so the card drew + * a labelled placeholder for a field it omits outright when the value is `null`. + * + * Now the loop asks the floor by name, and objectui#8489's guard STAYS: it + * answers every non-array value that resolves to no label, which the floor says + * nothing about. Both pins have to be green at once. + * + * ## ⛔ What must not change + * + * The kanban does NOT trim. `' '` is a value here — that is why the floor's + * string member is `''` and not "blank". + */ + +import React from 'react'; +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, waitFor, cleanup, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; +import { isEmptyValue } from '@object-ui/core'; +// Registers `object-kanban`. +import './index'; +// The board renders inside `KanbanRenderer`'s `React.lazy` boundary; importing +// the chunk at module scope bills the cold transform to the import phase +// instead of racing a `waitFor` budget (objectui#3010). +import './KanbanImpl'; + +const OBJECT_SCHEMA = { + name: 'test_object', + fields: { + id: { type: 'text' }, + name: { type: 'text', label: 'Name' }, + status: { type: 'text' }, + // Deliberately NOT a picklist and carrying no `options`, so the loop takes + // the `getCellRenderer` fork rather than objectui#8489's badge fork. + notes: { type: 'text', label: 'Notes' }, + }, +}; + +const ROWS: any[] = [ + { id: 'c1', name: 'Populated card', status: 'open', notes: 'real note' }, + { id: 'c2', name: 'Empty array card', status: 'open', notes: [] }, + { id: 'c3', name: 'Whitespace card', status: 'open', notes: ' ' }, +]; + +const LANES = [{ id: 'open', title: 'Open' }]; + +function makeDataSource(rows: any[]) { + return { + find: vi.fn().mockResolvedValue({ data: rows, total: rows.length }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue(OBJECT_SCHEMA), + } as any; +} + +/** The card whose accessible name is `title` — `SortableCard`'s own handle. */ +const cardNamed = (title: string): HTMLElement => + screen.getByRole('listitem', { name: title }); + +async function renderBoard() { + const result = render( + + + , + ); + await waitFor(() => expect(result.container.textContent).toContain('Populated card')); + return result; +} + +afterEach(cleanup); + +describe('objectui#8496 — ObjectKanban asks the floor', () => { + it('CONTROL — a populated card field still renders its value under its label', async () => { + await renderBoard(); + const card = cardNamed('Populated card'); + expect(card.textContent, 'CONTROL: the card-field list rendered').toContain('real note'); + expect( + card.querySelectorAll('dt').length, + 'CONTROL: the field label is present as the list term', + ).toBe(1); + }); + + it('a card field holding [] is OMITTED, not drawn as a labelled em-dash', async () => { + expect(isEmptyValue([]), 'CONTROL: [] is a floor member').toBe(true); + + await renderBoard(); + const card = cardNamed('Empty array card'); + + expect( + card.querySelectorAll('dt').length, + 'a kanban card omits valueless fields; [] must not keep a label alive', + ).toBe(0); + expect( + card.querySelectorAll('[data-slot="empty-value"]').length, + 'the shared No-value affordance is what [] used to reach through getCellRenderer', + ).toBe(0); + }); + + it('⛔ NOT FLATTENED — the kanban does NOT trim: a whitespace-only value keeps its row', async () => { + expect(isEmptyValue(' '), 'CONTROL: the floor keeps whitespace a value').toBe(false); + + await renderBoard(); + const card = cardNamed('Whitespace card'); + + expect( + card.querySelectorAll('dt').length, + "a stored ' ' is a value on this surface — only record:details and RelatedList trim", + ).toBe(1); + }); +}); diff --git a/packages/plugin-kanban/src/ObjectKanban.tsx b/packages/plugin-kanban/src/ObjectKanban.tsx index 2a6d42aac9..6cd4f65a5f 100644 --- a/packages/plugin-kanban/src/ObjectKanban.tsx +++ b/packages/plugin-kanban/src/ObjectKanban.tsx @@ -25,6 +25,7 @@ import { extractRecords, buildExpandFields, getRecordDisplayName, + isEmptyValue, resolveNameField, } from '@object-ui/core'; import { getBadgeColorClasses, getBadgeHexAppearance, getCellRenderer, resolveCellRendererType } from '@object-ui/fields'; @@ -721,7 +722,11 @@ export const ObjectKanban: React.FC = ({ // value didn't get expanded (so we never show "8UY9zHWBfjYjYor4"). const resolveDisplay = (key: string): string | undefined => { const raw = (item as any)[key]; - if (raw == null || raw === '') return undefined; + // THE FLOOR by name (objectui#8496). `[]` is a member and used to reach + // the object branch below, which walked six name-ish keys over zero + // entries and returned `undefined` anyway — the same answer, spelled + // twice. + if (isEmptyValue(raw)) return undefined; if (typeof raw === 'object') { const obj = raw as Record; const candidates = ['name', 'full_name', 'display_name', 'label', 'title', 'username']; @@ -797,7 +802,15 @@ export const ObjectKanban: React.FC = ({ if (titleFieldsToSkip.has(f)) continue; const def = objectDef?.fields?.[f]; const raw = (item as any)[f]; - if (raw == null || raw === '') continue; + // THE FLOOR by name (objectui#8496), no extension: a card field with + // nothing in it is OMITTED, so this asks the floor and nothing more. + // ⚠️ `[]` is a MEMBER and used to fall through here — into the + // picklist branch, where it resolved to no label and drew a fully + // coloured pill with no children in it until objectui#8489 caught it + // one step later. That guard STAYS: it also answers every non-array + // value that resolves to nothing, which the floor says nothing about. + // ⛔ Do NOT trim — `' '` is deliberately a value on this surface. + if (isEmptyValue(raw)) continue; const isPicklist = def?.type === 'picklist' || def?.type === 'multipicklist' || @@ -914,8 +927,11 @@ export const ObjectKanban: React.FC = ({ // from semantic fields below (avoids "8UY9zHWBfjYjYor4" appearing as subtitle). const incomingDesc = (item as any).description; const descMissing = - incomingDesc == null || - incomingDesc === '' || + // THE FLOOR by name (objectui#8496) — `[]` is a member, and a card + // subtitle has no more to draw for it than for `null`. + isEmptyValue(incomingDesc) || + // THE EXTENSION: an id-shaped string is gibberish as a subtitle, the + // same rule about the VALUE that `resolveDisplay` applies above. (typeof incomingDesc === 'string' && isOpaqueId(incomingDesc)); // P2-4: keep the original record's `description` field intact so the diff --git a/packages/plugin-list/src/ObjectGallery.tsx b/packages/plugin-list/src/ObjectGallery.tsx index 16db5f8f69..c4ccd6a96c 100644 --- a/packages/plugin-list/src/ObjectGallery.tsx +++ b/packages/plugin-list/src/ObjectGallery.tsx @@ -8,7 +8,7 @@ import React, { useState, useEffect, useCallback, useMemo, useContext } from 'react'; import { useDataScope, SchemaRendererContext, useNavigationOverlay, useSafeFieldLabel, useSettledSchema } from '@object-ui/react'; -import { ComponentRegistry, buildExpandFields, getRecordDisplayName } from '@object-ui/core'; +import { ComponentRegistry, buildExpandFields, getRecordDisplayName, isEmptyValue } from '@object-ui/core'; import { cn, Card, CardContent, NavigationOverlay } from '@object-ui/components'; import { usePermissions } from '@object-ui/permissions'; import type { GalleryConfig, ObjectGallerySchema } from '@object-ui/types'; @@ -208,7 +208,9 @@ const resolveCoverUrl = ( coverField: string, ): string | undefined => { const raw = item?.[coverField]; - if (raw == null || raw === '') return undefined; + // THE FLOOR by name (objectui#8496), no extension: a cover field holding + // `[]` has no first entry either, so the four members are one answer here. + if (isEmptyValue(raw)) return undefined; return readFileValues(raw)[0]?.url; }; @@ -568,7 +570,16 @@ export const ObjectGallery: React.FC = (props) => {
{visibleFields.map((field) => { const value = (item as any)[field]; - if (value == null || value === '') return null; + // THE FLOOR by name (objectui#8496), no extension: a card + // row is OMITTED for a valueless field rather than drawn + // with a placeholder, so this asks the floor and nothing + // more. ⚠️ `[]` is a MEMBER, and it used to fall through + // here: the row survived and the shared renderer painted + // the em-dash affordance (objectui#8481) under a label, on + // a card that omits every other valueless field. ⛔ Do NOT + // trim — `' '` is deliberately a value on this surface; + // only `record:details` and `RelatedList` extend that far. + if (isEmptyValue(value)) return null; const enriched = buildEnrichedField(field); const rendererType = resolveCellRendererType(enriched as any) || enriched.type || 'text'; const CellRenderer = getCellRenderer(rendererType); diff --git a/packages/plugin-list/src/__tests__/ObjectGallery.emptinessFloor-8496.test.tsx b/packages/plugin-list/src/__tests__/ObjectGallery.emptinessFloor-8496.test.tsx new file mode 100644 index 0000000000..1435dabe98 --- /dev/null +++ b/packages/plugin-list/src/__tests__/ObjectGallery.emptinessFloor-8496.test.tsx @@ -0,0 +1,113 @@ +/** + * 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. + */ + +/** + * The gallery's card-field filter asks the shared FLOOR (objectui#8496). + * + * ## What changed, and why it is the ruling and not a drive-by + * + * The filter used to be `value == null || value === ''` — three of the floor's + * four members, spelled privately, which is the shape the card counted five + * times. The missing member is `[]`, and the gap was VISIBLE: a card omits + * every valueless field outright, yet an empty array fell through to the shared + * cell renderer, which since objectui#8481 answers it with the "No value" + * em-dash. So one card could show a labelled em-dash for `tags: []` while + * silently omitting the `null` field right beside it — two answers to one + * question on one card. + * + * ## ⛔ What did NOT change, and must not + * + * The gallery does NOT trim. `' '` is a value here, and that is the reason + * the floor's string member is `''` and not "blank": `record:details` and + * `RelatedList` trim, this surface and the kanban do not, so a floor that + * trimmed could not be the weakest common claim. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import { ObjectGallery } from '../ObjectGallery'; +import { SchemaRendererProvider } from '@object-ui/react'; +import { isEmptyValue } from '@object-ui/core'; + +const objectSchema = { + fields: { + name: { type: 'text', label: 'Name' }, + tags: { + type: 'multiselect', + label: 'Tags', + options: [{ value: 'alpha', label: 'Alpha', color: 'indigo' }], + }, + note: { type: 'text', label: 'Note' }, + }, +}; + +const data = [ + { id: 'a1', name: 'Populated card', tags: ['alpha'], note: 'real note' }, + { id: 'a2', name: 'Empty array card', tags: [], note: null }, + { id: 'a3', name: 'Whitespace card', tags: ['alpha'], note: ' ' }, +]; + +const mockDataSource = { + find: vi.fn().mockResolvedValue(data), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue(objectSchema), +}; + +const renderGallery = () => + render( + + + , + ); + +describe('objectui#8496 — ObjectGallery asks the floor', () => { + it('a card field holding [] is OMITTED, not drawn as a labelled em-dash', async () => { + expect(isEmptyValue([]), 'CONTROL: [] is a floor member').toBe(true); + + const { container } = renderGallery(); + await waitFor(() => expect(screen.getByText('Populated card')).toBeInTheDocument()); + // CONTROL: the gallery really did render card fields. + expect( + screen.getAllByText('Alpha').length, + 'CONTROL: the populated card fields rendered', + ).toBe(2); + expect(screen.getByText('real note'), 'CONTROL: a populated text field renders').toBeInTheDocument(); + + expect( + container.querySelectorAll('[data-slot="empty-value"]').length, + 'a gallery card omits valueless fields; it must not draw the No-value affordance for []', + ).toBe(0); + }); + + it('⛔ NOT FLATTENED — the gallery does NOT trim: a whitespace-only value is still drawn', async () => { + expect(isEmptyValue(' '), 'CONTROL: the floor keeps whitespace a value').toBe(false); + + const { container } = renderGallery(); + await waitFor(() => expect(screen.getByText('Whitespace card')).toBeInTheDocument()); + + // The whitespace value reaches TextCellRenderer and is drawn as stored. + // If the gallery ever grew the trim, this card would lose the row and the + // affordance count above would stop being a statement about `[]` alone. + const truncated = Array.from(container.querySelectorAll('div,span')).filter( + (el) => el.children.length === 0 && el.textContent === ' ', + ); + expect( + truncated.length, + "a stored ' ' is a value on this surface — only record:details and RelatedList trim", + ).toBeGreaterThan(0); + }); +});