Skip to content

feat(badge): unify badge interface to support array values in DescriptionCard and DataTable to render multiple badges#278

Merged
IzumiSy merged 8 commits into
mainfrom
feat/badge-array-support
May 26, 2026
Merged

feat(badge): unify badge interface to support array values in DescriptionCard and DataTable to render multiple badges#278
IzumiSy merged 8 commits into
mainfrom
feat/badge-array-support

Conversation

@IzumiSy

@IzumiSy IzumiSy commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

In typical AppShell applications, DataTable is used on list pages and DescriptionCard on detail pages — users navigate from a table row to its detail view. To provide a consistent badge experience across this common pattern, this PR unifies badge rendering into a shared BadgeList component that both DataTable and DescriptionCard use, with support for array values and overflow handling (maxVisible + hover popover).

Usage

// DescriptionCard — array badges with maxVisible
<DescriptionCard
  fields={[{
    key: "tags",
    type: "badge",
    meta: {
      badgeVariantMap: { urgent: "error", fragile: "warning" },
      maxVisible: 3,
    },
  }]}
  data={{ tags: ["urgent", "fragile", "heavy", "oversized"] }}
/>

// DataTable — badge column with array accessor
column({
  ...infer("tags"),
  type: "badge",
  typeOptions: {
    badgeVariantMap: { Premium: "warning", Office: "outline-info" },
    maxVisible: 2,
  },
})

Screenshots

DescriptionCard with multiple badges:

スクリーンショット 2026-05-22 16 37 53

DataTable with multiple badges:

スクリーンショット 2026-05-22 18 13 26

Changes

BadgeList component (badge-list.tsx)

  • Consolidate badge-utils.ts into badge-list.tsx as single source of truth
  • Shared BadgeList component renders one or more badges with overflow popover
  • Overflow popover opens on hover (openOnHover)
  • Exports: BadgeVariant, BadgeOptions, BadgeListProps, BadgeList, resolveBadgeVariant, resolveBadgeLabel

Array badge support

  • DescriptionCard badge field now accepts array values (string[])
  • DataTable badge column accepts string[] from accessor
  • Single values continue to work as before (fully backward-compatible)

Unified badge interface

  • Shared BadgeOptions interface used by both DataTable and DescriptionCard
  • resolveBadgeVariant() / resolveBadgeLabel() shared utilities
  • Default badge variant unified to "outline-neutral" across both DataTable and DescriptionCard (previously DataTable used "neutral" and DescriptionCard used "outline-neutral")
  • DataTable's BadgeCellOptions extends BadgeOptions with maxVisible
  • Keep BadgeVariantType as a deprecated alias for backward compatibility
  • DataTable renderBadge now correctly shows placeholder for empty arrays (consistent with DescriptionCard)

inferColumns() fix

  • Remove default render from infer() return value so type renderers take priority
  • Previously, column({ ...infer("field"), type: "badge" }) would use the generic formatValue render instead of the badge renderer
  • Add renderText fallback for boolean (✓/✗), Date (toLocaleDateString), and object (JSON.stringify) to avoid regressions
  • Columns without explicit type or render now display for null/empty values (aligns with typed-column behavior)

DataTable demo

  • Add tags field with multiple badges and maxVisible: 2 to product table
  • Convert status column from custom render to declarative type: "badge" + typeOptions

Tests

  • badge-list.test.tsx: 18 tests covering utilities and component behavior
  • data-table.test.tsx: Added tests for renderText fallback (boolean, Date, object), array badge, and empty array placeholder
  • field-helpers.test.ts: Updated expectations for removed render property

@pkg-pr-new

pkg-pr-new Bot commented May 22, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@tailor-platform/app-shell@278
npm i https://pkg.pr.new/@tailor-platform/app-shell-sdk-plugin@278
npm i https://pkg.pr.new/@tailor-platform/app-shell-vite-plugin@278

commit: 92e2900

@IzumiSy IzumiSy changed the title feat(badge): support array values in DescriptionCard and unify badge interface feat(badge): unify badge interface to support array values in DescriptionCard and DataTable to render multiple badges May 22, 2026
@IzumiSy IzumiSy self-assigned this May 22, 2026
@IzumiSy

IzumiSy commented May 22, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generated by API Design Review for issue #278

Comment thread packages/core/src/components/data-table/cell-renderers.tsx Outdated
Comment thread packages/core/src/components/description-card/field-renderers.test.tsx Outdated
IzumiSy added 6 commits May 25, 2026 11:02
…interface

- DescriptionCard badge field now accepts array values, rendering multiple badges
- Extract shared BadgeVariant type and BadgeOptions interface into badge-utils.ts
- DataTable's BadgeCellOptions now extends shared BadgeOptions
- Add badgeLabelMap and defaultBadgeVariant to DescriptionCard FieldMeta
- Deprecate BadgeVariantType in favor of shared BadgeVariant
- Add resolveBadgeVariant/resolveBadgeLabel shared utilities
- Add example of array badge usage in vite-app
- Add maxVisible option to FieldMeta for badge fields
- Overflow badges shown in a popover on click with full Badge UI
- +N trigger is a simple text element with hover highlight (no dot)
- Add popover interaction test
- Extend badge accessor type to accept string[] in addition to primitives
- renderBadge handles arrays with multiple Badge rendering
- Add maxVisible + Popover overflow (same pattern as DescriptionCard)
- Update type test: badge accessor now allows arrays
- Add rendering tests for multi-badge and maxVisible in DataTable
- Merge badge-utils.ts into badge-list.tsx as single source of truth
- Remove default render from inferColumns() to fix type renderer priority
- Add renderText fallback for boolean/Date/object values
- Add openOnHover to BadgeList overflow popover
- Add tags badge column to DataTable demo (multiple badges + maxVisible)
- Convert status column from custom render to type: badge
- Add comprehensive tests for BadgeList, renderText fallback, and field-helpers
- renderBadge now returns placeholder for empty arrays (consistent with DescriptionCard)
- Align default badge variant to 'outline-neutral' across DataTable and DescriptionCard
- Remove redundant BadgeVariant override in DescriptionCard field-renderers
- Add changeset
@IzumiSy
IzumiSy force-pushed the feat/badge-array-support branch from c967c63 to 412c160 Compare May 25, 2026 02:02
… test

- renderBadge now filters array items and returns placeholder when all
  values are null or empty strings
- Add tests for [null, null] and ["", ""] badge column cases
- Change field-renderers test to use user.hover instead of user.click
  to match the openOnHover popover behavior
*/
function BadgeFieldRenderer({ field }: { field: ResolvedField }) {
if (isEmpty(field.value)) {
const values = Array.isArray(field.value)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this into an if/else function instead? Nested ternaries are a bit hard to read/understand

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I guess nested ternaries should be prohibied by linting.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to update oxlint rules to prevent nested ternaries, so let me work on it in the next PR.

resolveLabel: resolveLabelProp,
badgeClassName,
}: BadgeListProps): React.ReactNode {
const values = Array.isArray(value) ? value : [value];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From an API design perspective, wouldn't it make more sense to force value to always be an array even if there's only one? That way we don't have to check the variable type and consistently enforce it as an array through TS

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point to raise.

The reason I went with T | T[] is to optimize for caller ergonomics — most usages will pass a single value, and forcing callers to wrap it as an array adds repeated boilerplate at every call site. Internal type consistency is preserved by normalizing on the first line, so the rest of the component still operates on T[] with full type safety.

This also matches how React's own APIs behave — children, style, and similar props all accept T | T[] and normalize internally, which I think sets a reasonable precedent for forgiving input shapes in shared components.

Given AppShell is consumed across many surfaces — including by AI coding agents that often forget the [x] wrap and incur an extra type-error-then-fix round trip — I'd lean on this pattern to minimize friction at call sites.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah fair enough, as long as coding agents consistently follow this pattern I don't see an issue :)

if (isEmpty(value)) return PLACEHOLDER;
if (typeof value === "boolean") return value ? "✓" : "✗";
if (value instanceof Date) return value.toLocaleDateString();
if (typeof value === "object") return JSON.stringify(value);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intentional? The value can look pretty strange with this

I also think it might more sense to convert this to a Switch case command for readability

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the mixture of multiple types of comparison using typeof and instanceof, so can we really make everything in one switch?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that'll be fine, because switch evaluates on a boolean, this can be like:

function renderText(value: unknown): ReactNode {
  switch (true) {
    case isEmpty(value):
      return PLACEHOLDER;
    case typeof value === "boolean":
      return value ? "✓" : "✗";
    case value instanceof Date:
      return value.toLocaleDateString();
    case typeof value === "object":
      return JSON.stringify(value);
    default:
      return String(value);
  }
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not super fussed though, it's only 4 conditions so it's not too bad right now, but if we want to add more conditions it might be useful.

More concerned about stringifying objects, maybe it should throw an error instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

siwtch (true) looks stylish, but I remember it disables type narrowing, doesn't it? I don't like that if so.

More concerned about stringifying objects, maybe it should throw an error instead?

I cannot ensure if throwing errors there does not break any of the current projects that use AppShell, so best not touch that for backward behaviour compatibility. Showing data to users is not harmful if that's just raw JSON.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

siwtch (true) looks stylish, but I remember it disables type narrowing, doesn't it? I don't like that if so.

Ah, I probably didn't know we can do it now? Then, let's go with switch one!

microsoft/TypeScript#53681

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I probably didn't know we can do it now? Then, let's go with switch one!

Yeah since TS version 5.4 it's possible, but like I said, there's only 4 conditions so I'm okay if we don't do it haha.

I cannot ensure if throwing errors there does not break any of the current projects that use AppShell, so best not touch that for backward behaviour compatibility. Showing data to users is not harmful if that's just raw JSON.

Ah right, yeah I guess in that case it's fine, we should probably start enforcing it in the future though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored renderText to use switch(true) in 92e2900.

options?.badgeVariantMap?.[key] ?? options?.defaultBadgeVariant ?? "neutral";
const label = options?.badgeLabelMap?.[key] ?? key;
return <Badge variant={variant}>{label}</Badge>;
const items = Array.isArray(value) ? value : value != null ? [value] : [];

@erickteowarang erickteowarang May 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as below, I think we should enforce this to remain an array so we don't constantly transform/re-transform it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe #278 (comment) will answer this too?

@IzumiSy
IzumiSy requested a review from erickteowarang May 25, 2026 08:43
case isEmpty(value):
return PLACEHOLDER;
case typeof value === "boolean":
return value ? "✓" : "✗";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ふと思ったんですが、ここは✓✕より○✕の方が日本っぽくないですか? 😄

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

たしかにそうですね。しかし、このbooleanのfallback rendererって使ってる人いない可能性もあるので、撤廃してもいい可能性あります。

@IzumiSy
IzumiSy merged commit 2197c3f into main May 26, 2026
5 checks passed
@IzumiSy
IzumiSy deleted the feat/badge-array-support branch May 26, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants