Skip to content

Commit c6ae7b2

Browse files
os-justinclaude
andauthored
fix(plugin-kanban): decline to draw a card badge with no label (#8733)
* fix(plugin-kanban): decline to draw a card badge with no label `ObjectKanban`'s explicit-card-field loop opens with `if (raw == null || raw === '') continue;` — which an empty array passes — and then forks on `isPicklist`, true for any field carrying declared `options`. That fork never calls `getCellRenderer`, so the shared `@object-ui/fields` renderers' rule that an empty array is not a cell value cannot reach it: it resolves a label and a colour itself and pushes a plain object onto `cardBadges`. With nothing to resolve the label came out as the empty string, and the card drew a fully styled, fully coloured pill with no children in it — on the most commonly authored shape of the three, because a picklist that declares its options is the normal case. Both `cardBadges.push` sites now decline a badge whose resolved label is empty. That is narrower than teaching the kanban what an "empty value" is: at the push site the only question left is whether there is a label to draw, and asking exactly that also closes the half that is independent of arrays — any value resolving to an empty label drew the same pill, the empty array being merely the shape that is easy to hit. The comparison is against the empty string rather than falsiness, so a legitimately authored '0' label keeps rendering. The legacy semantic-field heuristic that runs when a board authors no `cardFields` resolves its label the same way and had the same defect; it is repaired in the same shape, skipping the push rather than breaking so an unlabelled field cannot consume one of the two badge slots. Pinned at the DOM, not at `cardBadges`: counts taken from rendered nodes scoped per card, with the counting selector proved to match exactly one node per badge. Every populated badge is pinned byte-for-byte against markup captured before the repair. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S * chore(changeset): declare the kanban empty-badge repair Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent fd9c50a commit c6ae7b2

3 files changed

Lines changed: 346 additions & 0 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
'@object-ui/plugin-kanban': patch
3+
---
4+
5+
Kanban cards no longer draw an empty coloured badge for a card field that resolves to no label.
6+
7+
The explicit-card-field loop's guard (`raw == null || raw === ''`) let an empty array
8+
through, and the branch it then took for any field carrying declared `options` never
9+
reached the shared `@object-ui/fields` cell renderers — it resolved a label and colour
10+
itself. With nothing to resolve, a fully styled, fully coloured pill was drawn with
11+
nothing inside it, on the most commonly authored shape of all: a picklist that declares
12+
its options.
13+
14+
Both badge-push sites now decline a badge whose resolved label is empty. This also closes
15+
the half that has nothing to do with arrays — any value resolving to an empty label drew
16+
the same pill. A legitimately authored `'0'` label still renders: the test is against the
17+
empty string, not falsiness.
Lines changed: 305 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,305 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* objectui#8489 — a kanban card must not draw a badge it has no label for.
11+
*
12+
* ── The defect ────────────────────────────────────────────────────────────
13+
* `ObjectKanban`'s explicit-card-field loop opens with
14+
* `if (raw == null || raw === '') continue;` — which an empty array passes —
15+
* and then FORKS on `isPicklist`, true for any field carrying declared
16+
* `options`. That fork never calls `getCellRenderer`, so objectui#8481's
17+
* renderer fix (an empty array is not a cell value) cannot reach it: the
18+
* branch resolves a label and a colour itself and pushes a plain object onto
19+
* `cardBadges`. With nothing to resolve, the label came out as the empty
20+
* string and the card drew a fully styled, fully coloured pill with no
21+
* children in it — the most commonly authored shape of the three, because a
22+
* picklist that declares its options is the normal case.
23+
*
24+
* ── What the repair is, and what it deliberately is NOT ───────────────────
25+
* The kanban does not learn what an "empty value" is. At the push site the
26+
* only question left is *is there a label to draw?*, so the branch declines
27+
* to push a badge whose RESOLVED label is empty. That is strictly narrower
28+
* than an emptiness judgement, and it closes both halves at once.
29+
*
30+
* ── Why the empty array is the easy case and not the subject ──────────────
31+
* ANY value that resolves to an empty label draws the same empty pill. A fix
32+
* that special-cases `[]` leaves the defect standing while reading as
33+
* complete, so the DISCRIMINATING pin below is `stringifiesToNothing` — a
34+
* NON-array stored value whose `String(...)` is empty. The empty array comes
35+
* along for free; built the other way round, it would not.
36+
*
37+
* Measured while writing these: the third shape the card imagined — an
38+
* OPTION whose declared `label` is the empty string — cannot reach the push
39+
* site as an empty label on its own. `opt?.label || String(raw)` falls back
40+
* to the stringified value, and the i18n hop cannot introduce one either:
41+
* `useObjectLabel`'s `resolve()` rejects a translation that is `''` and
42+
* returns its fallback (`packages/i18n/src/useObjectLabel.ts`). So the empty
43+
* label always arrives through `String(raw)`, by either of the two shapes
44+
* pinned here. Recorded because it is the reason there is no third case.
45+
*
46+
* ── Non-regression, derived from the two plausible WRONG fixes ────────────
47+
* "drop any badge whose label is falsy" would also drop a legitimately
48+
* authored `'0'` label, so `'0'` is pinned rendering normally, byte for
49+
* byte. "drop empty arrays" leaves the second half standing, so the
50+
* non-array case is pinned separately from the array one.
51+
*
52+
* ── Both push sites ───────────────────────────────────────────────────────
53+
* `cardBadges.push` happens twice in `ObjectKanban.tsx`: the explicit
54+
* `cardFields` loop, and the legacy semantic-field heuristic that runs when a
55+
* board authors no `cardFields` at all. Both resolve a label exactly the same
56+
* way (the file's own comment says so) and both had the defect, so both are
57+
* pinned; a repair on one alone leaves the pill drawable from the other.
58+
*
59+
* ── Why these assertions are at the DOM ───────────────────────────────────
60+
* `cardBadges` is an implementation array one refactor away from meaning
61+
* nothing; the defect is what the card DRAWS. Every count below is taken
62+
* from rendered nodes, scoped to one card through the `role="listitem"` /
63+
* `aria-label` handle `SortableCard` gives every card. `queryByText('')`
64+
* cannot express "no pill" (and `queryByText` throws on multiple matches as
65+
* well as none), so the pins COUNT nodes at a selector — and the first case
66+
* below verifies that selector matches exactly ONE node per badge, because
67+
* `.rounded-full` alone matches two nodes per avatar elsewhere in this repo.
68+
*/
69+
70+
import React from 'react';
71+
import { describe, it, expect, afterEach, vi } from 'vitest';
72+
import { render, waitFor, cleanup, screen } from '@testing-library/react';
73+
import '@testing-library/jest-dom';
74+
import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react';
75+
// Registers `object-kanban`.
76+
import './index';
77+
// The board renders inside `KanbanRenderer`'s `React.lazy` boundary. Importing
78+
// the chunk at module scope bills the cold transform to the import phase
79+
// instead of racing a `waitFor` budget under full parallelism (objectui#3010);
80+
// same specifier as `index.tsx`'s factory, so ESM's module cache makes that
81+
// factory resolve immediately.
82+
import './KanbanImpl';
83+
84+
/**
85+
* A stored value that stringifies to nothing while being neither `null`,
86+
* nor the empty string, nor an array — the discriminating shape. It clears
87+
* the loop's `raw == null || raw === ''` guard, misses the option lookup
88+
* (nothing declares the empty value), and leaves `String(raw)` as the only
89+
* label the branch can resolve.
90+
*/
91+
const stringifiesToNothing = { toString: () => '' };
92+
93+
const TAGS_FIELD = {
94+
name: 'tags',
95+
type: 'multiselect',
96+
label: 'Tags',
97+
options: [{ value: 'alpha', label: 'Alpha', color: 'indigo' }],
98+
};
99+
100+
/** `'0'` is a legitimately authored label, and it is falsy. */
101+
const TIER_FIELD = {
102+
name: 'tier',
103+
type: 'picklist',
104+
label: 'Tier',
105+
options: [
106+
{ value: 'zero', label: '0', color: 'blue' },
107+
{ value: 'one', label: 'One', color: 'green' },
108+
],
109+
};
110+
111+
const CHANNEL_FIELD = {
112+
name: 'channel',
113+
type: 'picklist',
114+
label: 'Channel',
115+
options: [{ value: 'email', label: 'Email', color: 'amber' }],
116+
};
117+
118+
/** The legacy heuristic's own badge field — reached with no `cardFields`. */
119+
const PRIORITY_FIELD = {
120+
name: 'priority',
121+
type: 'picklist',
122+
label: 'Priority',
123+
options: [{ value: 'high', label: 'High', color: 'red' }],
124+
};
125+
126+
const OBJECT_SCHEMA = {
127+
name: 'test_object',
128+
fields: {
129+
id: { type: 'text' },
130+
name: { type: 'text', label: 'Name' },
131+
status: { type: 'text' },
132+
tags: TAGS_FIELD,
133+
tier: TIER_FIELD,
134+
channel: CHANNEL_FIELD,
135+
priority: PRIORITY_FIELD,
136+
},
137+
};
138+
139+
const ROWS: any[] = [
140+
// Every badge this board can draw correctly, on one card.
141+
{ id: 'c1', name: 'Populated card', status: 'open', tags: ['alpha'], tier: 'zero', channel: 'email' },
142+
// The easy half: an empty array clears the loop's guard.
143+
{ id: 'c2', name: 'Empty array card', status: 'open', tags: [] },
144+
// The discriminating half: NOT an array, still resolves to no label.
145+
{ id: 'c3', name: 'Stringifies to nothing card', status: 'open', channel: stringifiesToNothing },
146+
];
147+
148+
/** The legacy heuristic reads `priority` off the record, with no field list. */
149+
const LEGACY_ROWS: any[] = [
150+
{ id: 'l1', name: 'Legacy populated card', status: 'open', priority: 'high' },
151+
{ id: 'l2', name: 'Legacy empty array card', status: 'open', priority: [] },
152+
{ id: 'l3', name: 'Legacy stringifies to nothing card', status: 'open', priority: stringifiesToNothing },
153+
];
154+
155+
const LANES = [{ id: 'open', title: 'Open' }];
156+
157+
function makeDataSource(rows: any[]) {
158+
return {
159+
find: vi.fn().mockResolvedValue({ data: rows, total: rows.length }),
160+
findOne: vi.fn(),
161+
create: vi.fn(),
162+
update: vi.fn(),
163+
delete: vi.fn(),
164+
getObjectSchema: vi.fn().mockResolvedValue(OBJECT_SCHEMA),
165+
} as any;
166+
}
167+
168+
/**
169+
* The three populated pills, captured from a run BEFORE the repair. The
170+
* repair may not move one byte of them: it only declines a push, so the
171+
* markup of every badge that still has a label has to be what it was.
172+
*/
173+
const POPULATED_BADGE_HTML: string[] = [
174+
'<div class="inline-flex items-center whitespace-nowrap rounded-full border px-2.5 py-0.5 transition-colors focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 text-xs font-normal bg-indigo-50 text-indigo-700 border-indigo-200 dark:bg-indigo-950/40 dark:text-indigo-300 dark:border-indigo-900/60">Alpha</div>',
175+
'<div class="inline-flex items-center whitespace-nowrap rounded-full border px-2.5 py-0.5 transition-colors focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 text-xs font-normal bg-blue-50 text-blue-700 border-blue-200 dark:bg-blue-950/40 dark:text-blue-300 dark:border-blue-900/60">0</div>',
176+
'<div class="inline-flex items-center whitespace-nowrap rounded-full border px-2.5 py-0.5 transition-colors focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 text-xs font-normal bg-green-50 text-green-700 border-green-200 dark:bg-green-950/40 dark:text-green-300 dark:border-green-900/60">Email</div>',
177+
];
178+
179+
/** The same capture for the legacy heuristic's own pill. */
180+
const LEGACY_POPULATED_BADGE_HTML: string[] = [
181+
'<div class="inline-flex items-center whitespace-nowrap rounded-full border px-2.5 py-0.5 transition-colors focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 text-xs font-normal bg-red-50 text-red-700 border-red-200 dark:bg-red-950/40 dark:text-red-300 dark:border-red-900/60">High</div>',
182+
];
183+
184+
afterEach(cleanup);
185+
186+
/**
187+
* Every badge pill drawn inside one card, in document order.
188+
*
189+
* The selector is the `Badge` primitive's own cva base — `inline-flex` and
190+
* `whitespace-nowrap` together with `rounded-full`. Radix's `Avatar.Root` and
191+
* `AvatarFallback`, the two nodes that make a bare `.rounded-full` count
192+
* double elsewhere in this repo, carry neither, so this matches exactly one
193+
* node per badge. The `renders exactly one node per badge` case below proves
194+
* that rather than asserting it.
195+
*/
196+
function badgesIn(card: HTMLElement): HTMLElement[] {
197+
return Array.from(
198+
card.querySelectorAll<HTMLElement>('div.inline-flex.whitespace-nowrap.rounded-full'),
199+
);
200+
}
201+
202+
/** The card whose accessible name is `title` — `SortableCard`'s own handle. */
203+
function cardNamed(title: string): HTMLElement {
204+
return screen.getByRole('listitem', { name: title });
205+
}
206+
207+
async function renderBoard(rows: any[], cardFields?: string[]) {
208+
const adapter = makeDataSource(rows);
209+
const result = render(
210+
<SchemaRendererProvider dataSource={adapter as any}>
211+
<SchemaRenderer
212+
schema={
213+
{
214+
type: 'object-kanban',
215+
objectName: 'test_object',
216+
groupBy: 'status',
217+
columns: LANES,
218+
...(cardFields ? { cardFields } : {}),
219+
} as any
220+
}
221+
/>
222+
</SchemaRendererProvider>,
223+
);
224+
await waitFor(() => expect(result.container.textContent).toContain(rows[0].name));
225+
return result;
226+
}
227+
228+
describe('objectui#8489 — the card declines to draw a badge with no label', () => {
229+
describe('the explicit cardFields picklist branch', () => {
230+
it('renders exactly one node per badge at the counting selector', async () => {
231+
await renderBoard(ROWS, ['tags', 'tier', 'channel']);
232+
const populated = cardNamed('Populated card');
233+
234+
// Three declared, populated card fields -> three pills, and the
235+
// selector resolves them one-for-one. Without this the counts below
236+
// could not distinguish "no badge" from "a selector that never matched".
237+
expect(badgesIn(populated).map((b) => b.textContent)).toEqual(['Alpha', '0', 'Email']);
238+
});
239+
240+
it('draws no badge for an empty array', async () => {
241+
await renderBoard(ROWS, ['tags', 'tier', 'channel']);
242+
243+
// Mapped to outerHTML so a failure prints the pill that should not exist.
244+
expect(badgesIn(cardNamed('Empty array card')).map((b) => b.outerHTML)).toEqual([]);
245+
});
246+
247+
it('draws no badge for a non-array value that stringifies to nothing', async () => {
248+
await renderBoard(ROWS, ['tags', 'tier', 'channel']);
249+
250+
expect(
251+
badgesIn(cardNamed('Stringifies to nothing card')).map((b) => b.outerHTML),
252+
).toEqual([]);
253+
});
254+
255+
it('leaves every populated badge byte-identical to before the repair', async () => {
256+
await renderBoard(ROWS, ['tags', 'tier', 'channel']);
257+
258+
expect(badgesIn(cardNamed('Populated card')).map((b) => b.outerHTML)).toEqual(
259+
POPULATED_BADGE_HTML,
260+
);
261+
});
262+
263+
it("keeps a legitimately authored '0' label, which is falsy", async () => {
264+
await renderBoard(ROWS, ['tags', 'tier', 'channel']);
265+
const populated = cardNamed('Populated card');
266+
267+
const zero = badgesIn(populated).filter((b) => b.textContent === '0');
268+
expect(zero).toHaveLength(1);
269+
});
270+
});
271+
272+
describe('the legacy semantic-field heuristic (no cardFields authored)', () => {
273+
it('renders exactly one node per badge at the counting selector', async () => {
274+
await renderBoard(LEGACY_ROWS);
275+
276+
expect(badgesIn(cardNamed('Legacy populated card')).map((b) => b.textContent)).toEqual([
277+
'High',
278+
]);
279+
});
280+
281+
it('leaves every populated badge byte-identical to before the repair', async () => {
282+
await renderBoard(LEGACY_ROWS);
283+
284+
expect(badgesIn(cardNamed('Legacy populated card')).map((b) => b.outerHTML)).toEqual(
285+
LEGACY_POPULATED_BADGE_HTML,
286+
);
287+
});
288+
289+
it('draws no badge for an empty array', async () => {
290+
await renderBoard(LEGACY_ROWS);
291+
292+
expect(
293+
badgesIn(cardNamed('Legacy empty array card')).map((b) => b.outerHTML),
294+
).toEqual([]);
295+
});
296+
297+
it('draws no badge for a non-array value that stringifies to nothing', async () => {
298+
await renderBoard(LEGACY_ROWS);
299+
300+
expect(
301+
badgesIn(cardNamed('Legacy stringifies to nothing card')).map((b) => b.outerHTML),
302+
).toEqual([]);
303+
});
304+
});
305+
});

‎packages/plugin-kanban/src/ObjectKanban.tsx‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,23 @@ export const ObjectKanban: React.FC<ObjectKanbanComponentProps> = ({
626626
const colorClass = hexBadge
627627
? hexBadge.className
628628
: getBadgeColorClasses(opt?.color, raw);
629+
// objectui#8489 — decline to draw a badge with no label in it.
630+
// The loop's own `raw == null || raw === ''` guard lets an empty
631+
// array through, and this branch never reaches `getCellRenderer`,
632+
// so objectui#8481's rule for the shared renderers ("an empty
633+
// array is not a cell value") cannot help it: with nothing to
634+
// resolve, the label came out as the empty string and the card
635+
// drew a fully styled, fully coloured pill with no children in it.
636+
//
637+
// Deliberately NOT an emptiness judgement about the VALUE — the
638+
// kanban needs none. By this point the only question left is
639+
// whether there is a label to draw, and asking exactly that also
640+
// covers every non-array value resolving to nothing; the empty
641+
// array is simply the shape that is easy to hit.
642+
//
643+
// Compared against the empty string, NOT for falsiness: `'0'` is a
644+
// legitimately authored option label and has to keep rendering.
645+
if (translatedLabel === '') continue;
629646
cardBadges.push({ label: translatedLabel, colorClass, colorStyle: hexBadge?.style });
630647
} else {
631648
// Route through the same registry that Grid/Gallery use so
@@ -684,6 +701,13 @@ export const ObjectKanban: React.FC<ObjectKanbanComponentProps> = ({
684701
const colorClass = hexBadge
685702
? hexBadge.className
686703
: getBadgeColorClasses(option?.color, v);
704+
// objectui#8489, on the reasoning spelled out at the explicit
705+
// branch above: this heuristic resolves its label exactly the same
706+
// way, so `[]` — or any value stringifying to nothing — reached the
707+
// push with an empty label and drew the same empty pill. Skipping
708+
// the push rather than breaking keeps the remaining badge fields
709+
// eligible: an unlabelled one must not consume a badge slot.
710+
if (label === '') continue;
687711
cardBadges.push({ label, colorClass, colorStyle: hexBadge?.style });
688712
if (cardBadges.length >= 2) break;
689713
}

0 commit comments

Comments
 (0)