-
Notifications
You must be signed in to change notification settings - Fork 18
fix: address migration feedback from shadcn/Radix to Kumo #42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
geoquant
wants to merge
15
commits into
cloudflare:main
Choose a base branch
from
geoquant:geoquant/kumo-fixes-for-matt
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+641
−297
Open
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
a81225b
fix(docs): add required Tailwind 4 @source directive to all setup exa…
jonnie-clouds ec91373
fix(cli): resolve broken doc/docs/ls commands by fixing registry path
jonnie-clouds e08d405
fix(dialog): wrap sub-components to isolate @base-ui/react type refer…
jonnie-clouds 11ccec6
fix(docs): add confirmation dialog recipe showing disablePointerDismi…
jonnie-clouds 0db514c
fix(label): render as <label> element with htmlFor support
jonnie-clouds 92be40d
feat(input): add Textarea alias for InputArea
jonnie-clouds 2871202
feat(toast): add ToastProvider alias for Toasty
jonnie-clouds 53b6436
fix(docs): update Select basic example to use items prop
jonnie-clouds 06f8689
fix(docs): document icon-only button pattern with required aria-label
jonnie-clouds 0999d85
feat(button): require aria-label on icon-only buttons via discriminat…
jonnie-clouds ab5e089
chore: add changeset for migration feedback fixes
jonnie-clouds e765467
feat(docs): add dev-only HMR plugin for instant kumo component reload
jonnie-clouds 36314ce
docs: add @source path disclaimer and Tailwind v4 callout
jonnie-clouds ee1ece4
fix(registry): handle union types (anyOf/oneOf) in prop extraction
jonnie-clouds 93cc97a
fix(docs): correct CSS import order — kumo styles before tailwindcss
jonnie-clouds File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| --- | ||
| "@cloudflare/kumo": minor | ||
| "@cloudflare/kumo-docs-astro": minor | ||
| --- | ||
|
|
||
| fix(cli): resolve broken doc/docs/ls commands by fixing registry path from catalog/ to ai/ | ||
| fix(dialog): wrap sub-components to isolate @base-ui/react type references from downstream consumers | ||
| fix(label): render as `<label>` element with htmlFor support instead of `<span>` | ||
| feat(input): add Textarea alias for InputArea | ||
| feat(toast): add ToastProvider alias for Toasty | ||
| feat(button): require aria-label on icon-only buttons (shape="square" | "circle") via discriminated union | ||
| fix(docs): add Tailwind 4 @source directive to usage example, add confirmation dialog recipe, update Select basic example, document icon-only button aria-label pattern |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
118 changes: 118 additions & 0 deletions
118
packages/kumo-docs-astro/src/lib/vite-plugin-kumo-hmr.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| import { resolve, dirname } from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
|
|
||
| const __dirname = dirname(fileURLToPath(import.meta.url)); | ||
|
|
||
| // Resolve once — points at the sibling kumo package root | ||
| const kumoRoot = resolve(__dirname, "../../../kumo"); | ||
| const kumoSrc = resolve(kumoRoot, "src"); | ||
|
|
||
| /** | ||
| * Map every `@cloudflare/kumo` sub-path export to its source equivalent. | ||
| * | ||
| * In dev mode Vite will resolve these to the raw .ts/.tsx source files, | ||
| * which means file-watcher-based HMR works instantly — no rebuild of | ||
| * the kumo package required. | ||
| * | ||
| * In production builds (astro build) this plugin is NOT loaded, so the | ||
| * normal package.json `exports` field is used (dist/), which validates | ||
| * the real consumer experience. | ||
| */ | ||
| const aliases: Record<string, string> = { | ||
| // Main barrel — resolves to source index.ts | ||
| "@cloudflare/kumo": resolve(kumoSrc, "index.ts"), | ||
|
|
||
| // CSS styles — resolve to source CSS | ||
| "@cloudflare/kumo/styles/tailwind": resolve(kumoSrc, "styles/kumo.css"), | ||
| "@cloudflare/kumo/styles/standalone": resolve( | ||
| kumoSrc, | ||
| "styles/kumo-standalone.css", | ||
| ), | ||
| "@cloudflare/kumo/styles": resolve(kumoSrc, "styles/kumo.css"), | ||
|
|
||
| // JSON registry — these live outside src/ and are NOT built, so same | ||
| // path works in dev and prod. We alias anyway so Vite can resolve | ||
| // the workspace:* link correctly and watch the file. | ||
| "@cloudflare/kumo/ai/component-registry.json": resolve( | ||
| kumoRoot, | ||
| "ai/component-registry.json", | ||
| ), | ||
| }; | ||
|
|
||
| /** | ||
| * Vite plugin that rewires `@cloudflare/kumo` imports to the raw source | ||
| * files of the sibling package during `astro dev`. | ||
| * | ||
| * **Why not just use `resolve.alias`?** | ||
| * `resolve.alias` is a simple prefix match — it can't distinguish | ||
| * `@cloudflare/kumo` from `@cloudflare/kumo-figma` without a trailing | ||
| * slash, and it can't handle the overlapping sub-path exports cleanly. | ||
| * A plugin gives us exact-match control. | ||
| */ | ||
| export function kumoHmrPlugin() { | ||
| return { | ||
| name: "vite-plugin-kumo-hmr", | ||
| enforce: "pre" as const, | ||
|
|
||
| resolveId(source: string) { | ||
| // Exact match first (most imports) | ||
| if (aliases[source]) { | ||
| return aliases[source]; | ||
| } | ||
|
|
||
| // Sub-path component imports: @cloudflare/kumo/components/button | ||
| // → packages/kumo/src/components/button/index.ts | ||
| if (source.startsWith("@cloudflare/kumo/components/")) { | ||
| const componentName = source.replace( | ||
| "@cloudflare/kumo/components/", | ||
| "", | ||
| ); | ||
| return resolve(kumoSrc, `components/${componentName}/index.ts`); | ||
| } | ||
|
|
||
| // Primitives: @cloudflare/kumo/primitives/dialog | ||
| // → packages/kumo/src/primitives/dialog.ts | ||
| if (source.startsWith("@cloudflare/kumo/primitives/")) { | ||
| const primitiveName = source.replace( | ||
| "@cloudflare/kumo/primitives/", | ||
| "", | ||
| ); | ||
| return resolve(kumoSrc, `primitives/${primitiveName}.ts`); | ||
| } | ||
| if (source === "@cloudflare/kumo/primitives") { | ||
| return resolve(kumoSrc, "primitives/index.ts"); | ||
| } | ||
|
|
||
| // Utils barrel | ||
| if (source === "@cloudflare/kumo/utils") { | ||
| return resolve(kumoSrc, "utils/index.ts"); | ||
| } | ||
|
|
||
| // Catalog barrel | ||
| if (source === "@cloudflare/kumo/catalog") { | ||
| return resolve(kumoSrc, "catalog/index.ts"); | ||
| } | ||
|
|
||
| // Registry barrel | ||
| if (source === "@cloudflare/kumo/registry") { | ||
| return resolve(kumoSrc, "registry/index.ts"); | ||
| } | ||
|
|
||
| // Catch-all for any other @cloudflare/kumo/styles/* CSS imports | ||
| if (source.startsWith("@cloudflare/kumo/styles/")) { | ||
| const styleName = source.replace("@cloudflare/kumo/styles/", ""); | ||
| return resolve(kumoSrc, `styles/${styleName}.css`); | ||
| } | ||
|
|
||
| return undefined; | ||
| }, | ||
|
|
||
| configResolved(config: { server: { fs: { allow: string[] } } }) { | ||
| // Append kumo source to the existing allow list rather than replacing it. | ||
| // Using config() would shallow-merge and override Astro/Vite defaults. | ||
| if (config.server?.fs?.allow) { | ||
| config.server.fs.allow.push(kumoRoot); | ||
| } | ||
| }, | ||
| }; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does
disablePointerDismissalalso disable escape key dismissal?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ascorbic From my testing... no, disablePointerDismissal does not disable Escape key dismissal.
That actually seems like the correct behavior if we want to align with alertdialog patterns. Pressing Esc should be equivalent to activating the Cancel / Discard action and should return focus to the trigger, per the ARIA Authoring Practices:
https://www.w3.org/WAI/ARIA/apg/patterns/alertdialog/examples/alertdialog/
Also, the prop name explicitly refers to pointer dismissal, so it makes sense that it only blocks outside pointer interactions, not keyboard dismissal via Esc.