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); + }); +});