Skip to content

feat(tokens): badge and banner token updates#341

Open
najlaskr wants to merge 10 commits intocloudflare:mainfrom
najlaskr:badge-banner-updates
Open

feat(tokens): badge and banner token updates#341
najlaskr wants to merge 10 commits intocloudflare:mainfrom
najlaskr:badge-banner-updates

Conversation

@najlaskr
Copy link
Copy Markdown
Contributor

Summary

Reworks the Badge component colour variants to be semantic-first and updates the Banner backgrounds to align with new colour tokens.

Changes

  • Replaces subtle variants with new semantic tokens
  • Retains raw colour variants as secondary options for product-specific use cases
  • Updates background and border values to compensate for the semantic token colour changes while preserving the existing visual appearance of all three variants
  • Updates Badges docs with markdown styling to reflect the new semantic-first structure

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Mar 31, 2026

npm i https://pkg.pr.new/@cloudflare/kumo@341

commit: 379b3e8

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 31, 2026

Docs Preview

View docs preview

Commit: 379b3e8

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 31, 2026

Visual Regression Report

23 screenshot(s) with visual changes:

Button / Basic

90 px (0.09%) changed

Before After Diff
Before After Diff

Button / Variant: Primary

1,130 px (1.11%) changed

Before After Diff
Before After Diff

Button / Variant: Secondary

161 px (0.16%) changed

Before After Diff
Before After Diff

Button / Variant: Destructive

3,170 px (3.12%) changed

Before After Diff
Before After Diff

Button / Variant: Outline

254 px (0.25%) changed

Before After Diff
Before After Diff

Button / Variant: Secondary Destructive

0 px (0%) changed

Before After Diff
Before After Diff

Button / Sizes

477 px (0.47%) changed

Before After Diff
Before After Diff

Button / Loading State

198 px (0.2%) changed

Before After Diff
Before After Diff

Button / Disabled State

171 px (0.17%) changed

Before After Diff
Before After Diff

Button / Title

542 px (0.53%) changed

Before After Diff
Before After Diff

Dialog / Dialog Basic

192 px (0.19%) changed

Before After Diff
Before After Diff

Dialog / Dialog Alert

3,858 px (3.8%) changed

Before After Diff
Before After Diff

Dialog / Dialog Confirmation

3,296 px (3.25%) changed

Before After Diff
Before After Diff

Dialog / Dialog With Select

244 px (0.24%) changed

Before After Diff
Before After Diff

Dialog / Dialog With Combobox

130 px (0.13%) changed

Before After Diff
Before After Diff

Dialog / Dialog With Dropdown

130 px (0.13%) changed

Before After Diff
Before After Diff

Dialog (Open)

10,138 px (0.05%) changed

Before After Diff
Before After Diff

Select / Select Basic

496 px (0.49%) changed

Before After Diff
Before After Diff

Select / Select Without Label

170 px (0.17%) changed

Before After Diff
Before After Diff

Select / Select With Field

0 px (0%) changed

Before After Diff
Before After Diff

Select / Select Loading

0 px (0%) changed

Before After Diff
Before After Diff

Select / Select Multiple

931 px (0.92%) changed

Before After Diff
Before After Diff

Select (Open)

237 px (0%) changed

Before After Diff
Before After Diff
7 screenshot(s) unchanged
  • Button / Variant: Ghost
  • Button / With Icon
  • Button / Icon Only
  • Dialog / Dialog With Actions
  • Select / Select With Tooltip
  • Select / Select Custom Rendering
  • Select / Select Complex

Generated by Kumo Visual Regression

@geoquant
Copy link
Copy Markdown
Collaborator

/bonk review pls

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Mar 31, 2026

Now let me check the visual regression report more closely and understand what's happening with the unrelated Button and Dialog changes:

github run

@stritt
Copy link
Copy Markdown
Collaborator

stritt commented Apr 1, 2026

/bonk review ignore visual regressions and just look at the code

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Apr 1, 2026

UnknownError: ProviderInitError

github run

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Apr 1, 2026

@stritt Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

@stritt
Copy link
Copy Markdown
Collaborator

stritt commented Apr 2, 2026

/bonk review this code please

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Apr 2, 2026

Code Review: Badge and Banner Token Updates

This is a well-structured PR that successfully reworks the Badge component to use semantic-first tokens while preserving color variants for product-specific use cases. The Banner background/border adjustments are a thoughtful compensation for the token color changes.

Overall Assessment

Approachable with minor fixes

Issues Found

1. Inconsistent token reference in theme-kumo.css

In packages/kumo/src/styles/theme-kumo.css around line 146, there is an inconsistency:

  • Light mode uses color-kumo-neutral-125
  • Dark mode uses color-neutral-800 (missing kumo- prefix)

This should be color-kumo-neutral-800 for consistency with other tokens.

2. Outdated documentation example

In packages/kumo-docs-astro/src/pages/components/badge.mdx line 41, the Usage example shows:

return <Badge variant="neutral">New</Badge>;

Since neutral is now in the "other color token variants" section and secondary is the new default, consider updating this to:

return <Badge variant="secondary">New</Badge>;

3. Unnecessary type workaround in BadgeDemo.tsx

In packages/kumo-docs-astro/src/components/demos/BadgeDemo.tsx:

  • Line 4: The BadgeDemoVariant type extends with | "info", but info is already a proper variant in KUMO_BADGE_VARIANTS
  • Lines 4-9: The type casting workaround appears unnecessary now that info is a standard variant

Consider removing this workaround and using the Badge component directly.

4. JSDoc mentions non-existent variants

In packages/kumo/src/components/badge/badge.tsx around lines 132-134, the JSDoc mentions "neutral-subtle" and "inverted" as additional token variants, but:

  • neutral-subtle is not defined in KUMO_BADGE_VARIANTS (the secondary variant uses bg-kumo-fill instead)
  • inverted was removed from exported variants (now only used internally for primary)

Please update the documentation comment to match the actual available variants.

Summary

The architectural changes are solid. These are minor cleanup items that should be addressed before merging.

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Apr 2, 2026

I've completed my review of PR #341 and posted my findings as a comment.

Summary

Overall Assessment: ✅ Approvable with minor fixes

The PR successfully reworks the Badge component to use semantic-first tokens (primary, secondary, error, warning, success, info) while preserving color token variants for product-specific use cases. The Banner background/border adjustments from /10 to /50 opacity are a thoughtful compensation for the semantic token color changes.

Issues Identified:

  1. Inconsistent token reference (theme-kumo.css:146): The dark mode value for --color-kumo-fill-hover uses color-neutral-800 instead of color-kumo-neutral-800, inconsistent with the light mode value.

  2. Outdated documentation example (badge.mdx:41): The Usage example shows variant="neutral" but should use variant="secondary" to match the new semantic-first recommendation.

  3. Unnecessary type workaround (BadgeDemo.tsx:4-9): The type cast extending with "info" is no longer needed since info is now a proper variant in the component.

  4. JSDoc mentions non-existent variants (badge.tsx:132-134): The docs mention "neutral-subtle" and "inverted" variants that don't exist in KUMO_BADGE_VARIANTS.

These are all minor cleanup items—the architectural changes are solid and the implementation follows the project's conventions well.

github run

@najlaskr
Copy link
Copy Markdown
Contributor Author

najlaskr commented Apr 2, 2026

I've completed my review of PR #341 and posted my findings as a comment.

Summary

Overall Assessment: ✅ Approvable with minor fixes

The PR successfully reworks the Badge component to use semantic-first tokens (primary, secondary, error, warning, success, info) while preserving color token variants for product-specific use cases. The Banner background/border adjustments from /10 to /50 opacity are a thoughtful compensation for the semantic token color changes.

Issues Identified:

  1. Inconsistent token reference (theme-kumo.css:146): The dark mode value for --color-kumo-fill-hover uses color-neutral-800 instead of color-kumo-neutral-800, inconsistent with the light mode value.
  2. Outdated documentation example (badge.mdx:41): The Usage example shows variant="neutral" but should use variant="secondary" to match the new semantic-first recommendation.
  3. Unnecessary type workaround (BadgeDemo.tsx:4-9): The type cast extending with "info" is no longer needed since info is now a proper variant in the component.
  4. JSDoc mentions non-existent variants (badge.tsx:132-134): The docs mention "neutral-subtle" and "inverted" variants that don't exist in KUMO_BADGE_VARIANTS.

These are all minor cleanup items—the architectural changes are solid and the implementation follows the project's conventions well.

github run

Issue #1: this is intentional since we don't need to create a new token that has the exact same values as the tailwind token so keeping it as is.

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.

3 participants