refactor(core): reorganize UI into component library#121
refactor(core): reorganize UI into component library#121zrosenbauer wants to merge 14 commits intomainfrom
Conversation
…ayout modules Move UI components from flat structure into organized subdirectories: - prompts/ for user input components (Confirm, Select, TextInput, etc.) - display/ for presentational components - layout/ for structural components (FullScreen, ScrollArea, Tabs) - Consolidate output module into single file - Add screen module with provider and context exports Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
🦋 Changeset detectedLatest commit: 57f1abb The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The reduce with a void accumulator was semantically incorrect for side-effect iteration. Revert to map which is the idiomatic choice in this codebase. Co-Authored-By: Claude <noreply@anthropic.com>
Merge origin/main into feat/component-lib, resolving conflicts in: - packages/core/src/index.ts: keep expanded type exports from main + ScreenContext from branch - packages/core/src/ui/output.tsx: adopt cancelled/error spinner states from main using theme abstractions Co-Authored-By: Claude <noreply@anthropic.com>
Merging this PR will not alter performance
Comparing Footnotes
|
Remove all backwards compatibility for the deprecated `spinner` option on `cli()`, `createContext()`, and `createTestContext()`. Consumers must migrate to `status` — no deprecation period, hard break. - Remove `spinner` from CliOptions, RuntimeOptions, CreateContextOptions, TestContextOptions and all pass-through call sites - Remove compat `resolveStatus` logic that wrapped spinner in Status - Remove "backward compatibility" comment on screen re-exports - Delete deprecated-spinner test case - Clean up unused Spinner type imports Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR reorganizes the public UI export surface by introducing new barrel modules ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/context/create-context.ts (1)
25-33:⚠️ Potential issue | 🟡 MinorRemove stale
Spinneroverride claim fromCreateContextOptionsdocs.Line 30-Line 31 still document direct
Spinnerinjection, but this option was removed fromCreateContextOptions. The API docs are now inaccurate.🛠️ Suggested doc fix
- * custom {`@link` Log}, {`@link` Prompts}, {`@link` Status}, and {`@link` Spinner} - * implementations; when omitted, default `@clack/prompts`-backed instances + * custom {`@link` Log}, {`@link` Prompts}, and {`@link` Status} + * implementations; when omitted, default `@clack/prompts`-backed instances🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/context/create-context.ts` around lines 25 - 33, The JSDoc for CreateContextOptions incorrectly claims a direct Spinner override; update the documentation in create-context.ts (the CreateContextOptions docs used by createContext / CommandContext) to remove the stale "Spinner" injection mention and only list the actual supported overrides (e.g., Log, Prompts, Status), ensuring the doc text matches the current CreateContextOptions shape and available overrides.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/designs/component-library.md`:
- Line 77: Add language identifiers to all fenced code blocks in the component
library doc (the directory structure and visual example blocks) so linting
recognizes them; specifically update each triple-backtick block (the directory
structure example and other plaintext examples around the visual examples) to
start with ```text instead of ```; ensure every fenced block between the noted
examples includes the `text` language tag.
In `@packages/core/src/ui/display/alert.tsx`:
- Line 79: The component coerces ReactNode to a string via String(children)
which yields "[object Object]" for JSX; change the prop type so
AlertProps.children is string (not ReactNode), update any usages/prop types that
reference AlertProps and the component signature, and then use children directly
when building contentStr (referencing contentStr and the component that reads
AlertProps.children) to ensure only text is accepted and rendered correctly.
In `@packages/core/src/ui/display/progress-bar.tsx`:
- Line 67: The calculation of ratio in the ProgressBar component can divide by
zero; update the ratio computation (the const ratio in progress-bar.tsx /
ProgressBar) to guard against max === 0 (or non-positive max) before
dividing—e.g., if max is zero or <= 0 treat ratio as 0 (or clamp appropriately),
otherwise compute Math.min(1, Math.max(0, value / max)); apply the same guard
anywhere else that uses value/max.
In `@packages/core/src/ui/display/status-message.tsx`:
- Line 59: The Text element in StatusMessage is coercing children with
String(children) which breaks ReactNode rendering; update the Text rendering in
the StatusMessage component to output children as a ReactNode instead of
string-coercing it (preserve the intended leading space by rendering an explicit
space node or spacing prop and then render children directly). Locate the Text
line that currently reads <Text>{` ${String(children)}`}</Text> and replace it
so it renders a literal space (if needed) and the children ReactNode without
calling String(), ensuring complex children (JSX, elements) render correctly.
In `@packages/core/src/ui/prompts/autocomplete.tsx`:
- Around line 131-139: The Delete key is currently treated the same as
Backspace; update the key handling so key.backspace removes the character before
the cursor (keep using removeCharAt(search, cursorOffset - 1), decrement
cursorOffset and setFocusIndex(0)), while key.delete removes the character at
the cursor (call removeCharAt(search, cursorOffset) only when cursorOffset <
search.length, do not decrement cursorOffset, and still call setSearch and
setFocusIndex(0)); ensure both branches return after handling. Reference the
current handlers around key.backspace / key.delete, removeCharAt, setSearch,
setCursorOffset, and setFocusIndex.
In `@packages/core/src/ui/prompts/group-multi-select.tsx`:
- Around line 245-255: The moveFocus function can land on a disabled option at
the list boundary; update moveFocus(current: number, direction: number, items:
readonly FlatItem[]) to scan in the given direction until it finds an enabled
option and return that index, but if the scan reaches out-of-bounds without
finding any enabled option, return the original current index instead of the
last in-bounds index. Use the existing symbols (current, direction, items,
FlatItem, item.kind === 'option' and item.option.disabled) to implement a loop
that advances next while skipping disabled options and stops with current if no
enabled candidate is found.
In `@packages/core/src/ui/prompts/multi-select.tsx`:
- Around line 141-151: The OptionRow elements use option.label as the React key
which can collide; update the key in the map that renders OptionRow (the block
passing option, indicator, isFocused, isSelected, isDisabled) to use a stable
unique identifier such as option.value (or if value may not be unique, a
combined key like `${option.value}-${index}` or an explicit id field) instead of
option.label so React reconciliation is deterministic; locate the map that
renders OptionRow (references: OptionRow, option.label, focusedIndex) and
replace the key accordingly.
In `@packages/core/src/ui/prompts/navigation.ts`:
- Around line 98-110: The exported function resolveInitialIndex currently
accepts two positional parameters; change its signature to use a single object
parameter with destructured properties (e.g., { options, defaultValue }: {
options: readonly PromptOption<TValue>[], defaultValue?: TValue }) and update
all callers (such as select.tsx where it's invoked) to pass an object instead of
two args; preserve the generic TValue and return type and ensure type
annotations for PromptOption and optional defaultValue remain correct.
---
Outside diff comments:
In `@packages/core/src/context/create-context.ts`:
- Around line 25-33: The JSDoc for CreateContextOptions incorrectly claims a
direct Spinner override; update the documentation in create-context.ts (the
CreateContextOptions docs used by createContext / CommandContext) to remove the
stale "Spinner" injection mention and only list the actual supported overrides
(e.g., Log, Prompts, Status), ensuring the doc text matches the current
CreateContextOptions shape and available overrides.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6ee727fa-2e1a-4614-ac26-6d045471dfe4
⛔ Files ignored due to path filters (1)
.changeset/reorganize-ui-components.mdis excluded by!.changeset/**
📒 Files selected for processing (81)
docs/designs/component-library.mdexamples/diagnostic-output/package.jsonpackages/core/src/cli.tspackages/core/src/context/create-context.tspackages/core/src/index.tspackages/core/src/runtime/runtime.tspackages/core/src/runtime/types.tspackages/core/src/screen/index.tspackages/core/src/screen/output/index.tspackages/core/src/screen/output/screen-log.tspackages/core/src/screen/output/screen-report.tspackages/core/src/screen/output/screen-spinner.tspackages/core/src/screen/output/store-key.tspackages/core/src/screen/output/store.tspackages/core/src/screen/output/types.tspackages/core/src/screen/output/use-output-store.tspackages/core/src/screen/provider.tsxpackages/core/src/screen/screen.test.tspackages/core/src/screen/screen.tsxpackages/core/src/stories/decorators.tsxpackages/core/src/stories/viewer/components/field-control.tsxpackages/core/src/stories/viewer/components/preview.tsxpackages/core/src/stories/viewer/components/props-editor.tsxpackages/core/src/stories/viewer/components/sidebar.tsxpackages/core/src/stories/viewer/stories-app.tsxpackages/core/src/stories/viewer/stories-check.tsxpackages/core/src/test/context.test.tspackages/core/src/test/context.tspackages/core/src/test/types.tspackages/core/src/types/cli.tspackages/core/src/ui/confirm.tsxpackages/core/src/ui/display/alert.stories.tsxpackages/core/src/ui/display/alert.tsxpackages/core/src/ui/display/error-message.tsxpackages/core/src/ui/display/index.tspackages/core/src/ui/display/progress-bar.stories.tsxpackages/core/src/ui/display/progress-bar.tsxpackages/core/src/ui/display/spinner.stories.tsxpackages/core/src/ui/display/spinner.tsxpackages/core/src/ui/display/status-message.stories.tsxpackages/core/src/ui/display/status-message.tsxpackages/core/src/ui/index.tspackages/core/src/ui/layout/fullscreen.test.tspackages/core/src/ui/layout/fullscreen.tsxpackages/core/src/ui/layout/index.tspackages/core/src/ui/layout/scroll-area.tsxpackages/core/src/ui/layout/tabs.tsxpackages/core/src/ui/layout/use-size.test.tspackages/core/src/ui/layout/use-size.tsxpackages/core/src/ui/multi-select.tsxpackages/core/src/ui/output.tsxpackages/core/src/ui/password-input.tsxpackages/core/src/ui/prompts/autocomplete.stories.tsxpackages/core/src/ui/prompts/autocomplete.tsxpackages/core/src/ui/prompts/confirm.stories.tsxpackages/core/src/ui/prompts/confirm.tsxpackages/core/src/ui/prompts/cursor-value.tsxpackages/core/src/ui/prompts/group-multi-select.stories.tsxpackages/core/src/ui/prompts/group-multi-select.tsxpackages/core/src/ui/prompts/index.tspackages/core/src/ui/prompts/input-state.tspackages/core/src/ui/prompts/multi-select.stories.tsxpackages/core/src/ui/prompts/multi-select.tsxpackages/core/src/ui/prompts/navigation.tspackages/core/src/ui/prompts/option-row.tsxpackages/core/src/ui/prompts/password-input.stories.tsxpackages/core/src/ui/prompts/password-input.tsxpackages/core/src/ui/prompts/path-input.stories.tsxpackages/core/src/ui/prompts/path-input.tsxpackages/core/src/ui/prompts/select-key.stories.tsxpackages/core/src/ui/prompts/select-key.tsxpackages/core/src/ui/prompts/select.stories.tsxpackages/core/src/ui/prompts/select.tsxpackages/core/src/ui/prompts/string-utils.tspackages/core/src/ui/prompts/text-input.stories.tsxpackages/core/src/ui/prompts/text-input.tsxpackages/core/src/ui/prompts/types.tspackages/core/src/ui/select.tsxpackages/core/src/ui/spinner.tsxpackages/core/src/ui/text-input.tsxpackages/core/src/ui/theme.ts
💤 Files with no reviewable changes (9)
- packages/core/src/runtime/runtime.ts
- packages/core/src/ui/multi-select.tsx
- packages/core/src/ui/spinner.tsx
- packages/core/src/ui/select.tsx
- packages/core/src/cli.ts
- packages/core/src/ui/text-input.tsx
- packages/core/src/ui/confirm.tsx
- packages/core/src/screen/output/index.ts
- packages/core/src/ui/password-input.tsx
- Add language identifiers to fenced code blocks in component-library.md - Restrict Alert and StatusMessage children to string (prevents [object Object]) - Guard ProgressBar against division by zero when max is 0 - Fix Delete key in Autocomplete to remove char at cursor (not before) - Prevent focus landing on disabled option at boundary in GroupMultiSelect - Use option.value as React key in MultiSelect to avoid collisions - Refactor resolveInitialIndex to use object destructuring per conventions Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
- Add @kidd-cli/cli as root devDependency for direct `kidd stories` access - Patch Module._resolveFilename in story importer to handle TypeScript ESM-style .js -> .ts/.tsx extension mapping (same strategy as tsx/ts-node) - Replace strikethrough on disabled options with gray color (matches clack) - Add (disabled) text fallback when colors are not supported - Use gray + dim for disabled option hints Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/core/src/ui/prompts/select.tsx (1)
115-125:⚠️ Potential issue | 🟠 MajorUse a stable key to avoid reconciliation bugs.
At Line 124,
key={option.label}can collide when labels repeat. That can misrender focus/selection state between rows.Proposed fix
<OptionRow - key={option.label} + key={String(option.value)} option={option} indicator={indicator} isFocused={index === focusedIndex}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/ui/prompts/select.tsx` around lines 115 - 125, The list uses option.label as the React key in the options.map rendering (OptionRow) which can collide for duplicate labels; change the key to a stable unique identifier such as option.id or, if no id exists, use the map index only as a fallback (e.g., key={option.id ?? `option-${index}`}) so each OptionRow has a stable, unique key and avoids reconciliation/focus/selection bugs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/designs/component-library.md`:
- Around line 645-650: The fenced code block containing the dependency graph
(the block with "foundation", "├── prompts-core", "├── prompts-extended", "└──
display") is missing a language identifier; update that fenced block to use the
plaintext language identifier by changing the opening fence from ``` to ```text
so it matches other plaintext examples in the doc.
In `@packages/core/src/ui/prompts/autocomplete.tsx`:
- Around line 103-106: When handling the down-arrow case in the autocomplete key
handler, guard against an empty filtered list so focusIndex isn't set to -1:
check filtered.length > 0 before computing next and calling setFocusIndex; if
filtered is empty, do nothing (or keep focusIndex at -1/0 per desired behavior).
Update the block that uses key.downArrow, Math.min(filtered.length - 1,
focusIndex + 1), setFocusIndex and the focused = filtered[next] access to only
run when filtered.length > 0 to avoid negative indices and out-of-bounds access.
In `@packages/core/src/ui/prompts/group-multi-select.tsx`:
- Around line 332-337: The key generator itemKey for FlatItem can produce
collisions when two options in the same group have identical labels; update
itemKey to use a stable, unique identifier (e.g., option.value) for option keys
instead of option.label, ensuring option.value is stringified if necessary;
change the option branch in itemKey to generate
`option-${groupName}-${String(option.value)}` and verify the FlatItem/option
types (and any generic TValue) support or coerce to a string to avoid runtime
key collisions.
---
Duplicate comments:
In `@packages/core/src/ui/prompts/select.tsx`:
- Around line 115-125: The list uses option.label as the React key in the
options.map rendering (OptionRow) which can collide for duplicate labels; change
the key to a stable unique identifier such as option.id or, if no id exists, use
the map index only as a fallback (e.g., key={option.id ?? `option-${index}`}) so
each OptionRow has a stable, unique key and avoids
reconciliation/focus/selection bugs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f0b29016-2307-4cc0-868c-7d15296e2688
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (17)
codspeed.ymldocs/designs/component-library.mdexamples/tui/package.jsonpackage.jsonpackages/core/src/stories/importer.tspackages/core/src/stories/viewer/components/preview.tsxpackages/core/src/stories/viewer/stories-app.tsxpackages/core/src/ui/display/alert.tsxpackages/core/src/ui/display/progress-bar.tsxpackages/core/src/ui/display/status-message.tsxpackages/core/src/ui/index.tspackages/core/src/ui/prompts/autocomplete.tsxpackages/core/src/ui/prompts/group-multi-select.tsxpackages/core/src/ui/prompts/multi-select.tsxpackages/core/src/ui/prompts/navigation.tspackages/core/src/ui/prompts/option-row.tsxpackages/core/src/ui/prompts/select.tsx
Static test/story data doesn't need to be frozen — it's already a const binding that nothing mutates. Co-Authored-By: Claude <noreply@anthropic.com>
- Remove docs/designs/component-library.md design doc - Guard focus movement in autocomplete when filtered list is empty - Fix key collision in group-multi-select by using option.value instead of label Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/ui/prompts/autocomplete.tsx`:
- Around line 93-104: The up-arrow branch for the autocomplete currently only
clamps the lower bound, so if focusIndex is greater than filtered.length - 1 the
new index stays out of range; update the calculation of next in that branch (the
code that sets focusIndex via setFocusIndex) to clamp both ends using
filtered.length - 1 (e.g., compute next = Math.max(0, Math.min(filtered.length -
1, focusIndex - 1))) and then use that next when calling setFocusIndex and
onChange (referencing focusIndex, filtered, setFocusIndex, and onChange).
In `@packages/core/src/ui/prompts/group-multi-select.tsx`:
- Around line 75-80: The focusIndex state is initialized once and can become
out-of-bounds when flatItems (from buildFlatItems using options and
selectableGroups) changes; add a reconciliation effect that runs when flatItems
changes to clamp or reset focusIndex via setFocusIndex to a valid entry (e.g.,
min(current, flatItems.length - 1)) and prefer the first enabled/selectable item
(skip disabled rows) so keyboard navigation and initial focus never point at a
removed or disabled item; reference the flatItems variable,
focusIndex/setFocusIndex state, and the buildFlatItems/selectableGroups logic
when implementing this fix.
- Around line 84-88: The early return in the useInput handler prevents Enter
handling when flatItems is empty; update the handler in useInput (the callback
referencing flatItems) so it does not unconditionally return on flatItems.length
=== 0 — instead only skip navigation/selection logic for empty lists but still
allow the Enter/Return branch to run so non-required prompts can submit [] and
required prompts can run validation; adjust the conditional to check key.name
(e.g., 'return'/'enter') and only bypass other key handling when flatItems is
empty while preserving Enter processing.
- Around line 90-122: Replace the sequential if-returns in the input handler
with a ts-pattern match() dispatch so control flow is explicit and conforms to
the style guide: use match({ key, input }) (or match(key) and match on input
where appropriate) to branch on upArrow, downArrow, input === ' ' (space) and
return; in each arm call the same side-effecting helpers
(setFocusIndex(moveFocus(...)), toggleItem -> setSelected -> setError(undefined)
-> onChange, and onSubmit) and preserve the required validation (required &&
selected.length === 0) and early returns inside the matching arm; update imports
to include match from 'ts-pattern' and keep references to setFocusIndex,
moveFocus, toggleItem, setSelected, onChange, onSubmit unchanged so the behavior
remains identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7de35f6a-2277-4ead-adf9-0f09866ebbf6
📒 Files selected for processing (2)
packages/core/src/ui/prompts/autocomplete.tsxpackages/core/src/ui/prompts/group-multi-select.tsx
Co-Authored-By: Claude <noreply@anthropic.com>
- Clamp focusIndex on upArrow in autocomplete when filtered list shrinks - Sync focusIndex with flatItems length in group-multi-select via useEffect - Move Enter handling before empty list guard so non-required prompts can submit [] Co-Authored-By: Claude <noreply@anthropic.com>
|
Re: using |
Summary
prompts/,display/, andlayout/subdirectoriesAutocomplete,GroupMultiSelect,PathInput,SelectKeyAlert,ProgressBar,Spinner,StatusMessagescreen/module fromui/with provider, context, and output storeoutput.tsxfileTest plan
pnpm checkpasses (typecheck + lint + format)pnpm testpasses