Skip to content

Commit e9b377e

Browse files
claude[bot]claude
andauthored
feat(spec): warn when a grid view's rowColor declares no colors map (#15101)
`RowColorConfigSchema` requires `field` and leaves `colors` optional, so `rowColor: { field: 'status' }` parses, publishes and colours nothing: the only renderer that reads the block bails unless both are present. Every key involved is declared and live, so neither unknown-key rejection nor the liveness ledger can see it — the ADR-0078 silent half. `checkViewCompleteness` now emits `view/row-color-without-colors` (warning, path `rowColor.colors`) when a grid list view binds a non-empty `rowColor.field` and declares no usable `colors` map. Both spellings of "no map" are flagged: `colors` absent (the resolver's own guard returns early) and `colors: {}` (truthy, so it passes that guard, then matches no value). The message names the view, the bound field and the runtime line; the fix prescribes the map. Scope is measured from the renderer, not inferred from the schema: the rule fires only on the view type whose adapter forwards `rowColor` at all. On the other list view types the block is inert for a different reason, and prescribing a colours map would not fix it — recorded in the module as a non-rule rather than enforced. `RowColorConfigSchema.field`'s describe is rewritten so it no longer reads as if a colour is derived without a map. The accept set does not move: `colors` stays optional and no member is added, removed or narrowed. Claude-Session: https://claude.ai/code/session_0174WZTU6XcFcS7g2kykC53i Co-authored-by: Claude <noreply@anthropic.com>
1 parent b812a54 commit e9b377e

7 files changed

Lines changed: 202 additions & 5 deletions

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
`view/row-color-without-colors` — a new author-time completeness warning for a `rowColor` block that colours nothing
6+
7+
`RowColorConfigSchema` requires `field` and leaves `colors` optional, so a list view
8+
authored as `rowColor: { field: 'status' }` parses, publishes and colours no row: the
9+
only renderer that reads the block needs BOTH keys, and its row-className resolver
10+
returns before it reads a record when the map is missing. Every key involved is a
11+
declared, live one, so neither unknown-key rejection nor the liveness ledger could see
12+
it — the ADR-0078 silent half, in the family `validateFunctionalCompleteness` already
13+
gates (a `summary` with no `summaryOperations`, a `select` with no `options`).
14+
15+
`checkViewCompleteness` now emits a `warning` finding at `rowColor.colors` when a grid
16+
list view binds a non-empty `rowColor.field` and declares no usable `colors` map. Both
17+
spellings of "no map" are flagged — `colors` absent, and `colors: {}`, which passes the
18+
renderer's own guard and then matches no value, so it is the same dead shape spelled
19+
out. The finding names the view, the bound field and the runtime line that makes it
20+
true, and prescribes the map.
21+
22+
The accept set does not move: `colors` stays optional and nothing is added, removed or
23+
narrowed. Only the diagnostic is new, plus a rewritten `.describe()` on
24+
`RowColorConfigSchema.field` that no longer reads as if a colour is derived without a
25+
map.
26+
27+
Scope is measured, not assumed: the rule fires only on the view type whose renderer
28+
actually reads `rowColor` (the grid branch of the list-view adapter's per-type props
29+
switch). On a kanban or gallery view the block is inert too, but for a different
30+
reason — the key is never forwarded there at all — so prescribing a `colors` map would
31+
be a false prescription. That is recorded in the module, not enforced.

content/docs/references/ui/view.mdx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,7 +1009,7 @@ View filter rule
10091009

10101010
| Property | Type | Required | Description |
10111011
| :--- | :--- | :--- | :--- |
1012-
| **field** | `string` || Field to derive color from (typically a select/status field) |
1012+
| **field** | `string` || Field whose value is looked up in the `colors` map below to pick a row colour (typically a select/status field). The map is what does the colouring — with no `colors`, no row is ever coloured, whatever this field holds. Author-time diagnostic `view/row-color-without-colors` reports that combination. |
10131013
| **colors** | `Record<string, string>` | optional | Map of field value to color (hex/token) |
10141014

10151015
### Nested Shape: `ListView.bulkActionDefs[number]`
@@ -1395,7 +1395,7 @@ View filter rule
13951395

13961396
| Property | Type | Required | Description |
13971397
| :--- | :--- | :--- | :--- |
1398-
| **field** | `string` || Field to derive color from (typically a select/status field) |
1398+
| **field** | `string` || Field whose value is looked up in the `colors` map below to pick a row colour (typically a select/status field). The map is what does the colouring — with no `colors`, no row is ever coloured, whatever this field holds. Author-time diagnostic `view/row-color-without-colors` reports that combination. |
13991399
| **colors** | `Record<string, string>` | optional | Map of field value to color (hex/token) |
14001400

14011401
### Nested Shape: `ObjectListView.bulkActionDefs[number]`
@@ -1553,7 +1553,7 @@ Row color configuration based on field values
15531553

15541554
| Property | Type | Required | Description |
15551555
| :--- | :--- | :--- | :--- |
1556-
| **field** | `string` || Field to derive color from (typically a select/status field) |
1556+
| **field** | `string` || Field whose value is looked up in the `colors` map below to pick a row colour (typically a select/status field). The map is what does the colouring — with no `colors`, no row is ever coloured, whatever this field holds. Author-time diagnostic `view/row-color-without-colors` reports that combination. |
15571557
| **colors** | `Record<string, string>` | optional | Map of field value to color (hex/token) |
15581558

15591559

packages/spec/api-surface/kernel.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,7 @@
442442
"UpgradeSnapshotParsed (type)",
443443
"UpgradeSnapshotSchema (const)",
444444
"VIEW_LAYOUT_WITHOUT_BINDING (const)",
445+
"VIEW_ROW_COLOR_WITHOUT_COLORS (const)",
445446
"VIEW_TREE_WITHOUT_PARENT_FIELD (const)",
446447
"ValidationError (type)",
447448
"ValidationErrorSchema (const)",

packages/spec/export-origins/kernel.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,7 @@
442442
"UpgradeSnapshotParsed": "src/kernel/package-upgrade.zod.ts#UpgradeSnapshotParsed (type)",
443443
"UpgradeSnapshotSchema": "src/kernel/package-upgrade.zod.ts#UpgradeSnapshotSchema (const)",
444444
"VIEW_LAYOUT_WITHOUT_BINDING": "src/kernel/functional-completeness.ts#VIEW_LAYOUT_WITHOUT_BINDING (const)",
445+
"VIEW_ROW_COLOR_WITHOUT_COLORS": "src/kernel/functional-completeness.ts#VIEW_ROW_COLOR_WITHOUT_COLORS (const)",
445446
"VIEW_TREE_WITHOUT_PARENT_FIELD": "src/kernel/functional-completeness.ts#VIEW_TREE_WITHOUT_PARENT_FIELD (const)",
446447
"ValidationError": "src/kernel/plugin-validator.zod.ts#ValidationError (type)",
447448
"ValidationErrorSchema": "src/kernel/plugin-validator.zod.ts#ValidationErrorSchema (const)",

packages/spec/src/kernel/functional-completeness.test.ts

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
FIELD_CHOICE_WITHOUT_OPTIONS,
2929
VIEW_LAYOUT_WITHOUT_BINDING,
3030
VIEW_TREE_WITHOUT_PARENT_FIELD,
31+
VIEW_ROW_COLOR_WITHOUT_COLORS,
3132
WEBHOOK_WITHOUT_TRIGGERS,
3233
} from './functional-completeness';
3334

@@ -277,6 +278,78 @@ describe('checkViewCompleteness — the tree parent pointer (the silent-flat hal
277278
});
278279
});
279280

281+
describe('checkViewCompleteness — rowColor without a colour map (the parse-clean no-op)', () => {
282+
// `RowColorConfigSchema` requires `field` and leaves `colors` optional, so
283+
// `rowColor: { field }` parses, publishes, and colours nothing: objectui's
284+
// `useRowColor.ts` returns `undefined` unless BOTH are present. Every key is
285+
// one we know, which is why nothing else in the stack can see it.
286+
287+
it('flags `rowColor: { field }` with no `colors` as a WARNING', () => {
288+
const f = only(checkViewCompleteness({ type: 'grid', rowColor: { field: 'status' } }) as never);
289+
expect(f.rule).toBe(VIEW_ROW_COLOR_WITHOUT_COLORS);
290+
expect(f.severity).toBe('warning');
291+
expect(f.path).toBe('rowColor.colors');
292+
// The first sentence, verbatim: it names the view, the bound field and the
293+
// consequence — a prescription with no diagnosis is the shape ADR-0078 §6
294+
// rejects.
295+
expect(f.message).toContain(
296+
'A `grid` view whose `rowColor` binds `status` and declares no `colors` map never colours a row',
297+
);
298+
// …and the runtime line that makes it true.
299+
expect(f.message).toContain('if (!config?.field || !config.colors) return undefined');
300+
// The prescription is machine-pastable and carries the authored field back.
301+
expect(f.fix).toContain("field: 'status'");
302+
expect(f.fix).toContain('colors');
303+
});
304+
305+
it('is silent once a `colors` map is declared — the negative fixture', () => {
306+
expect(checkViewCompleteness({
307+
type: 'grid',
308+
rowColor: { field: 'status', colors: { open: '#0f0' } },
309+
})).toEqual([]);
310+
});
311+
312+
it('flags `colors: {}` the same — an empty map passes the guard and then matches nothing', () => {
313+
// The renderer's own test is `!config.colors`, which an empty object
314+
// PASSES; the lookup one line down matches no value and the row keeps its
315+
// default background anyway. Mirroring the guard expression alone would
316+
// have blessed this shape; the rule mirrors the outcome.
317+
const f = only(checkViewCompleteness({ type: 'grid', rowColor: { field: 'status', colors: {} } }) as never);
318+
expect(f.rule).toBe(VIEW_ROW_COLOR_WITHOUT_COLORS);
319+
expect(f.path).toBe('rowColor.colors');
320+
});
321+
322+
it('does NOT flag the view types whose renderer never reads `rowColor` — the pinned NON-rule', () => {
323+
// objectui's ListView adapter forwards `rowColor` in its `grid` branch
324+
// only. On a kanban board the block is inert too, but declaring a `colors`
325+
// map would not fix it — so warning here would be a false prescription,
326+
// which is what this module refuses to ship. Recorded, not enforced.
327+
for (const type of ['kanban', 'gallery', 'chart', 'timeline']) {
328+
const findings = checkViewCompleteness({ type, rowColor: { field: 'status' } });
329+
expect(findings.map((f) => f.rule)).not.toContain(VIEW_ROW_COLOR_WITHOUT_COLORS);
330+
}
331+
});
332+
333+
it('stays silent without a `field` to bind — that half is the schema\'s to refuse', () => {
334+
// `field` is REQUIRED by `RowColorConfigSchema`, so a block without one is
335+
// a parse error, not a completeness finding. Asserting it here would
336+
// double-report the same defect in two different vocabularies.
337+
expect(checkViewCompleteness({ type: 'grid', rowColor: {} })).toEqual([]);
338+
expect(checkViewCompleteness({ type: 'grid', rowColor: { field: '' } })).toEqual([]);
339+
});
340+
341+
it('leaves a non-record `colors` alone — only what was verified is asserted', () => {
342+
// The schema refuses these at parse; this module is not a second parser.
343+
expect(checkViewCompleteness({ type: 'grid', rowColor: { field: 'status', colors: 'red' } })).toEqual([]);
344+
expect(checkViewCompleteness({ type: 'grid', rowColor: { field: 'status', colors: [] } })).toEqual([]);
345+
});
346+
347+
it('never throws on junk', () => {
348+
expect(checkViewCompleteness({ type: 'grid', rowColor: 'red' })).toEqual([]);
349+
expect(checkViewCompleteness({ type: 'grid', rowColor: null })).toEqual([]);
350+
});
351+
});
352+
280353
describe('checkWebhookCompleteness — the rule the runtime comment argued against', () => {
281354
it('flags a webhook with no `triggers` as an ERROR', () => {
282355
const f = only(checkWebhookCompleteness({ name: 'notify_slack', url: 'https://x' }) as never);
@@ -329,6 +402,7 @@ describe('registry hygiene', () => {
329402
'field/relationship-without-reference',
330403
'field/summary-without-operations',
331404
'view/layout-without-binding',
405+
'view/row-color-without-colors',
332406
'view/tree-without-parent-field',
333407
'webhook/without-triggers',
334408
]);
@@ -343,9 +417,10 @@ describe('registry hygiene', () => {
343417
...checkFieldCompleteness({ type: 'checkboxes' }),
344418
...checkViewCompleteness({ type: 'kanban' }),
345419
...checkViewCompleteness({ type: 'tree', tree: {} }, { name: 'unit', fields: {} }),
420+
...checkViewCompleteness({ type: 'grid', rowColor: { field: 'status' } }),
346421
...checkWebhookCompleteness({ url: 'https://x' }),
347422
];
348-
expect(all).toHaveLength(8);
423+
expect(all).toHaveLength(9);
349424
for (const f of all) {
350425
expect(f.fix.length).toBeGreaterThan(8);
351426
expect(f.message.length).toBeGreaterThan(60);

packages/spec/src/kernel/functional-completeness.ts

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@
4646
* - `checkboxes` w/o `options` sits between the two: it shares the multi
4747
* branch's free-form validator behaviour, but a checkbox group with zero
4848
* boxes is almost certainly an omission — so it is a WARNING, not an error.
49+
* - a grid view's `rowColor` w/o `colors` → objectui `plugin-grid`'s
50+
* `useRowColor.ts` `if (!config?.field || !config.colors) return undefined;`
51+
* — the row-className resolver bails before it reads a single row, so every
52+
* row keeps the default colour. See {@link VIEW_ROW_COLOR_WITHOUT_COLORS}.
4953
* - `webhook` w/o `triggers` → `auto-enqueuer.ts` `if (triggers.size === 0) …
5054
* return null`. Note this one needed a SECOND source: that skip site's own
5155
* comment blesses the empty case as "a manual-only webhook", which reads
@@ -78,6 +82,7 @@ export const FIELD_RELATIONSHIP_WITHOUT_REFERENCE = 'field/relationship-without-
7882
export const FIELD_CHOICE_WITHOUT_OPTIONS = 'field/choice-without-options';
7983
export const VIEW_LAYOUT_WITHOUT_BINDING = 'view/layout-without-binding';
8084
export const VIEW_TREE_WITHOUT_PARENT_FIELD = 'view/tree-without-parent-field';
85+
export const VIEW_ROW_COLOR_WITHOUT_COLORS = 'view/row-color-without-colors';
8186
export const WEBHOOK_WITHOUT_TRIGGERS = 'webhook/without-triggers';
8287

8388
/** Every rule id this module can emit — pinned by tests so ids cannot drift. */
@@ -88,6 +93,7 @@ export const FUNCTIONAL_COMPLETENESS_RULES = [
8893
FIELD_CHOICE_WITHOUT_OPTIONS,
8994
VIEW_LAYOUT_WITHOUT_BINDING,
9095
VIEW_TREE_WITHOUT_PARENT_FIELD,
96+
VIEW_ROW_COLOR_WITHOUT_COLORS,
9197
WEBHOOK_WITHOUT_TRIGGERS,
9298
] as const;
9399

@@ -315,6 +321,59 @@ function hasDetectableParentField(object: AnyRec): boolean {
315321
|| ((def.type === 'lookup' || def.type === 'master_detail') && own !== undefined && def.reference === own));
316322
}
317323

324+
/**
325+
* The view types whose renderer actually READS `rowColor`, measured rather
326+
* than inferred from the schema: `rowColor` is declared on every list view,
327+
* but objectui's ListView adapter forwards it in exactly one branch of its
328+
* per-type props switch — `case 'grid'` (`packages/plugin-list/src/
329+
* ListView.tsx`, `...(rowColorConfig ? { rowColor: rowColorConfig } : {})`).
330+
* `kanban` / `gallery` / `calendar` / `timeline` / `gantt` / `map` / `tree` /
331+
* `chart` each build their own props and never carry the key; `page` mounts a
332+
* published page through a different renderer entirely.
333+
*
334+
* ⛔ So this rule is NOT widened to every list view type, however tempting the
335+
* schema's shape makes it. On a `kanban` view a `rowColor` block is inert too
336+
* — but it is inert for a DIFFERENT reason, and this rule's prescription
337+
* ("declare a `colors` map") would not fix it: the key is never read there
338+
* with or without a map. Warning would be a false prescription, the failure
339+
* mode the module doc and ADR-0078 §6 exist to prevent. That non-grid
340+
* inertness is recorded here, not enforced — the `gallery` disposition in
341+
* {@link VIEW_BINDING_BLOCKS} for the same reason.
342+
*
343+
* The `default:` arm of that switch shares the grid branch, so an unrecognised
344+
* view type would carry `rowColor` too — unreachable from this predicate,
345+
* which returns early on a view with no string `type` and never sees a
346+
* defaulted one (it runs on the NORMALIZED, pre-parse stack).
347+
*/
348+
const ROW_COLOR_VIEW_TYPES: ReadonlySet<string> = new Set(['grid']);
349+
350+
/**
351+
* Whether a `rowColor` block declares no usable colour map.
352+
*
353+
* Both spellings of "no map" are flagged, and they reach the same dead end by
354+
* two different routes in objectui's `useRowColor.ts` (`plugin-grid`):
355+
*
356+
* - `colors` ABSENT — the resolver's own guard,
357+
* `if (!config?.field || !config.colors) return undefined;`, returns before
358+
* it reads a row. Nothing is ever coloured.
359+
* - `colors: {}` — an empty object is truthy, so it PASSES that guard; the
360+
* lookup one line down (`hasOwnProperty.call(config.colors, value)`) then
361+
* matches no value, `if (!color) return undefined`, and nothing is coloured
362+
* either. Mirroring the guard expression alone would have blessed this one;
363+
* the rule mirrors the OUTCOME the guard produces, which is the same.
364+
*
365+
* An empty map is not an "I meant it" marker the way an action's
366+
* `locations: []` is — turning row colouring off has its own spellings (omit
367+
* the `rowColor` block, or leave the toolbar toggle `userActions.rowColor`
368+
* off), so `{}` is the same dead shape spelled out. Same reasoning as
369+
* `triggers: []` in {@link checkWebhookCompleteness}.
370+
*
371+
* Any OTHER shape (a string, an array) is left alone: the schema refuses it at
372+
* parse, and this module only asserts what it verified.
373+
*/
374+
const hasNoColorMap = (colors: unknown): boolean =>
375+
colors === undefined || colors === null || (isRec(colors) && Object.keys(colors).length === 0);
376+
318377
/**
319378
* Completeness of a single list-view definition (a container's `list` /
320379
* `listViews.*` entry).
@@ -343,6 +402,17 @@ function hasDetectableParentField(object: AnyRec): boolean {
343402
* rule fires only when BOTH halves fail — `parentField` undeclared AND nothing
344403
* on the bound object the renderer would detect — so a view that renders
345404
* correctly by auto-detection is never warned about.
405+
*
406+
* ## Why `rowColor` gets a rule of its own
407+
*
408+
* `RowColorConfigSchema` requires `field` and leaves `colors` optional, so
409+
* `rowColor: { field: 'status' }` parses, publishes and colours nothing — the
410+
* only renderer needs BOTH ({@link ROW_COLOR_VIEW_TYPES},
411+
* {@link hasNoColorMap}). It is the same ADR-0078 silent half as a `summary`
412+
* with no `summaryOperations`: every key is one we know, so the unknown-key
413+
* rejection cannot see it and the liveness ledger cannot either — `rowColor`
414+
* IS live, it is this instance that is dead. WARNING rather than error: only
415+
* the colouring is lost, the rows still render (ADR-0078 §1).
346416
*/
347417
export function checkViewCompleteness(view: unknown, boundObject?: unknown): CompletenessFinding[] {
348418
if (!isRec(view)) return [];
@@ -396,6 +466,25 @@ export function checkViewCompleteness(view: unknown, boundObject?: unknown): Com
396466
}
397467
}
398468

469+
if (ROW_COLOR_VIEW_TYPES.has(type) && isRec(view.rowColor)) {
470+
const field = isNonEmptyString(view.rowColor.field) ? view.rowColor.field : undefined;
471+
if (field !== undefined && hasNoColorMap(view.rowColor.colors)) {
472+
out.push({
473+
rule: VIEW_ROW_COLOR_WITHOUT_COLORS,
474+
severity: 'warning',
475+
path: 'rowColor.colors',
476+
message:
477+
`A \`${type}\` view whose \`rowColor\` binds \`${field}\` and declares no \`colors\` map never `
478+
+ 'colours a row: the grid\'s row-className resolver returns before it reads a record (objectui '
479+
+ '`useRowColor.ts` — `if (!config?.field || !config.colors) return undefined`), so every row keeps '
480+
+ 'the default background while parsing and publishing report success. An empty `colors: {}` is the '
481+
+ 'same dead shape spelled out — it passes that guard and then matches no value. The map is what '
482+
+ 'does the colouring; the field only says which value to look up.',
483+
fix: `rowColor: { field: '${field}', colors: { '<field_value>': '<hex_or_token>' } }`,
484+
});
485+
}
486+
}
487+
399488
return out;
400489
}
401490

packages/spec/src/ui/view.zod.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -874,7 +874,7 @@ export const RowColorConfigSchema = lazySchema(() => strictObject({
874874
surface: 'this row color configuration',
875875
history: VIEW_HISTORY,
876876
}, {
877-
field: z.string().describe('Field to derive color from (typically a select/status field)'),
877+
field: z.string().describe('Field whose value is looked up in the `colors` map below to pick a row colour (typically a select/status field). The map is what does the colouring — with no `colors`, no row is ever coloured, whatever this field holds. Author-time diagnostic `view/row-color-without-colors` reports that combination.'),
878878
colors: z.record(z.string(), z.string()).optional().describe('Map of field value to color (hex/token)'),
879879
}).describe('Row color configuration based on field values'));
880880

0 commit comments

Comments
 (0)