Add type prop to Select searchInputProps #162
Conversation
|
|
WalkthroughAdds a docs story demonstrating a numeric search input in a select and updates CommandInput to accept an optional input Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Story as Docs Story
participant UISelect as UISelect (Select w/ search)
participant Command as Command
participant CmdInput as CommandInput
participant Native as native <input>
Story->>UISelect: render NumberSearchTypeExample
UISelect->>Command: include command/search UI
Command->>CmdInput: render CommandInput (type="number")
CmdInput->>Native: asChild -> native <input type="number">
Note right of Story: Play test opens combobox\nand asserts input[type=number]
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🧰 Additional context used📓 Path-based instructions (18)**/*.{tsx,ts}📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
Files:
{apps,packages}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Files:
{apps,packages}/**/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Files:
apps/docs/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Files:
{apps,packages}/**/src/**/*.{tsx,ts}📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Files:
apps/docs/src/remix-hook-form/**/*.stories.tsx📄 CodeRabbit inference engine (.cursor/rules/storybook-testing.mdc)
Files:
**/*.tsx📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,mdx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/docs/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.stories.@(tsx|mdx)📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/components/src/ui/**/*.tsx📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
Files:
packages/components/src/ui/*.{tsx,ts}📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
Files:
packages/components/src/{remix-hook-form,ui}/*.{tsx,ts}📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
Files:
packages/components/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Files:
packages/components/src/ui/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Files:
packages/components/src/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/**/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧬 Code graph analysis (1)packages/components/src/ui/command.tsx (1)
🔇 Additional comments (3)
Comment |
|
📝 Storybook Preview: View Storybook This preview will be updated automatically when you push new changes to this PR.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/docs/src/remix-hook-form/select.stories.tsx (2)
637-673: Consider extracting the options array to improve maintainability.The component correctly demonstrates a number-typed search input, and the logic is sound. However, the 20 inline options could be extracted to a constant for better reusability and readability.
Apply this diff to extract the options:
+const NUMBER_OPTIONS = Array.from({ length: 20 }, (_, i) => ({ + label: String(i + 1), + value: i + 1, +})); + const NumberSearchTypeExample = () => { - // Use UI Select directly with number-valued options - const numberOptions = [ - { label: '1', value: 1 }, - { label: '2', value: 2 }, - { label: '3', value: 3 }, - { label: '4', value: 4 }, - { label: '5', value: 5 }, - { label: '6', value: 6 }, - { label: '7', value: 7 }, - { label: '8', value: 8 }, - { label: '9', value: 9 }, - { label: '10', value: 10 }, - { label: '11', value: 11 }, - { label: '12', value: 12 }, - { label: '13', value: 13 }, - { label: '14', value: 14 }, - { label: '15', value: 15 }, - { label: '16', value: 16 }, - { label: '17', value: 17 }, - { label: '18', value: 18 }, - { label: '19', value: 19 }, - { label: '20', value: 20 }, - ]; const [value, setValue] = React.useState<number | undefined>(10); return ( <div style={{ width: 320 }}> <UISelect<number> - options={numberOptions} + options={NUMBER_OPTIONS} placeholder="Pick a number" searchInputProps={{ type: 'number' }} value={value} onValueChange={setValue} /> </div> ); };
675-693: Remove the redundant decorator.The story correctly tests that the search input has
type="number". The play function is well-structured and focused. However, the decorator(StoryFn) => <StoryFn />is a no-op and can be removed.Apply this diff to simplify the story:
export const NumberSearchInputType: Story = { - decorators: [ - // No router needed for this simple UI-only story - (StoryFn) => <StoryFn />, - ], render: () => <NumberSearchTypeExample />, play: async ({ canvasElement, step }) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/docs/src/remix-hook-form/select.stories.tsx(2 hunks)packages/components/src/ui/command.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (18)
packages/components/src/ui/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
packages/components/src/ui/**/*.tsx: Build on @radix-ui components as the foundation for base UI components
Follow the component composition pattern for UI components that accept form integration
Files:
packages/components/src/ui/command.tsx
packages/components/src/ui/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
Base UI components should be named as ComponentName in ui/ directory
Files:
packages/components/src/ui/command.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
**/*.{tsx,ts}: Props interfaces should be named as ComponentNameProps
Form schemas should be named formSchema or componentNameSchema
Files:
packages/components/src/ui/command.tsxapps/docs/src/remix-hook-form/select.stories.tsx
packages/components/src/{remix-hook-form,ui}/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
Always export both the component and its props type
Files:
packages/components/src/ui/command.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
{apps,packages}/**/*.{ts,tsx}: Use package name imports for published packages (e.g., import { TextField } from '@lambdacurry/forms/remix-hook-form')
Import from specific entry points (e.g., import { TextField } from '@lambdacurry/forms/remix-hook-form/text-field')
Do not use relative imports across packages (e.g., avoid import { TextField } from '../../packages/components/src/remix-hook-form/text-field')
Order imports: 1) external libraries, 2) internal package imports, 3) cross-package imports, 4) type-only imports (grouped separately)
Files:
packages/components/src/ui/command.tsxapps/docs/src/remix-hook-form/select.stories.tsx
{apps,packages}/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
{apps,packages}/**/src/**/*.{ts,tsx}: Use relative imports within the same package (e.g., import { FormControl } from './form')
Use relative imports for sibling directories (e.g., import { Button } from '../ui/button')
Files:
packages/components/src/ui/command.tsxapps/docs/src/remix-hook-form/select.stories.tsx
packages/components/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
packages/components/src/**/*.{ts,tsx}: Always export both component and props type (e.g., export { ComponentName }; export type { ComponentNameProps };)
Use named exports for components for better tree-shaking (e.g., export const ComponentName = ...; avoid default exports)
Avoid default exports for components
Use tree-shaking friendly exports
Files:
packages/components/src/ui/command.tsx
{apps,packages}/**/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
{apps,packages}/**/src/**/*.{tsx,ts}: Use kebab-case for component file names (e.g., text-field.tsx, data-table.tsx)
Match component name in PascalCase to the exported component from the file (e.g., text-field.tsx exports TextField)
Files:
packages/components/src/ui/command.tsxapps/docs/src/remix-hook-form/select.stories.tsx
packages/components/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Don't import from remix-hook-form package in ui components
Files:
packages/components/src/ui/command.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Avoid circular dependencies
**/*.{ts,tsx}: Indent with 2 spaces, max line width 120, and use single quotes (Biome-enforced)
Use PascalCase for component and type names
Name hooks in camelCase with a use* prefix
Organize imports automatically with Biome
Use named exports only
Files:
packages/components/src/ui/command.tsxapps/docs/src/remix-hook-form/select.stories.tsx
packages/components/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all @lambdacurry/forms source under packages/components/src and treat it as the only editable source for the package
Files:
packages/components/src/ui/command.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: Keep React components pure and fully typed
Define explicit props interfaces for React components
Use React 19 ref patterns (forwardRef, useImperativeHandle as appropriate)
Files:
packages/components/src/ui/command.tsxapps/docs/src/remix-hook-form/select.stories.tsx
**/*.{ts,tsx,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use kebab-case for filenames (e.g., text-field.tsx, data-table-filter/**)
Files:
packages/components/src/ui/command.tsxapps/docs/src/remix-hook-form/select.stories.tsx
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid cross-package relative imports in the monorepo
Files:
packages/components/src/ui/command.tsx
apps/docs/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
In apps/docs, import from package name instead of relative paths for cross-package dependencies
Files:
apps/docs/src/remix-hook-form/select.stories.tsx
apps/docs/src/remix-hook-form/**/*.stories.tsx
📄 CodeRabbit inference engine (.cursor/rules/storybook-testing.mdc)
; always use fetcher.Form with methods.handleSubmit for router integration
apps/docs/src/remix-hook-form/**/*.stories.tsx: Follow the three-phase test pattern in play: default state, invalid submission, then valid submission
Use play functions with @storybook/test utilities (userEvent, expect, canvas/within) for interaction tests
Place decorators on individual stories, not on the meta object
Keep meta configuration minimal (title, component, parameters, tags) and avoid global decorators
Use withReactRouterStubDecorator per story when routing/fetcher behavior is needed; configure routes, loaders, and actions in the decorator
For async elements, use findBy* queries; use queryBy* to assert non-existence; avoid getBy* for async content
Prefer semantic queries (ByRole, ByLabelText, etc.) over CSS selectors or data-testid; avoid CSS selectors when accessible queries exist
Always click an input before clearing; then clear and type
Each story should test one primary workflow; do not bundle multiple unrelated scenarios in a single story
Use step grouping in play for complex workflows
Include required imports for interaction tests: Meta/StoryObj types and { expect, userEvent, within } from @storybook/test; import withReactRouterStubDecorator
Use a controlled form wrapper pattern: useFetcher, useRemixForm with zodResolver(formSchema), RemixFormProvider, and fetcher.Form
Never use a regular
Define Zod schemas and infer types for form data; wire zodResolver into useRemixForm
Test both client-side and server-side validation (loader/action paths) when applicable
Use router decorator configurations suitable to the scenario: loader for server fetch, action for form submissions, or no loader for client-only
Organize and export reusable helper test functions; group related test helpers together
Use role-based queries for portaled UI (listbox/option), open trigger first, query portal via within(document.body), and assert teardown with waitFor
Give triggers of portaled widgets a...
Files:
apps/docs/src/remix-hook-form/select.stories.tsx
apps/docs/**
📄 CodeRabbit inference engine (AGENTS.md)
Place Storybook docs, examples, and UI tests in apps/docs
Files:
apps/docs/src/remix-hook-form/select.stories.tsx
**/*.stories.@(tsx|mdx)
📄 CodeRabbit inference engine (AGENTS.md)
**/*.stories.@(tsx|mdx): Add stories to exercise states for critical interactions (forms, validation, a11y, filters)
Use per-story decorators as needed for setup
Structure Storybook play tests in three phases (setup, act, assert)
Files:
apps/docs/src/remix-hook-form/select.stories.tsx
🧬 Code graph analysis (1)
packages/components/src/ui/command.tsx (1)
packages/components/src/ui/utils/index.ts (1)
cn(4-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (2)
packages/components/src/ui/command.tsx (1)
37-49: LGTM! Clean implementation of the type prop.The asChild pattern is correctly used to pass through the
typeprop while maintaining Radix UI's composition model. The default value of'text'is sensible, and all props are properly forwarded.apps/docs/src/remix-hook-form/select.stories.tsx (1)
2-2: LGTM! Required imports for the new story.The React namespace import is needed for
React.useStatein the new component, and the UISelect import enables direct usage of the UI component without form integration.Also applies to: 4-4
| type CommandInputProps = React.ComponentPropsWithoutRef<typeof CommandPrimitive.Input> & { | ||
| type?: React.InputHTMLAttributes<HTMLInputElement>['type']; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Export the CommandInputProps type.
The type definition follows the correct naming convention, but it should be exported for external use and better type safety when composing components.
As per coding guidelines: "Always export both the component and its props type"
Apply this diff to export the type:
-type CommandInputProps = React.ComponentPropsWithoutRef<typeof CommandPrimitive.Input> & {
+export type CommandInputProps = React.ComponentPropsWithoutRef<typeof CommandPrimitive.Input> & {
type?: React.InputHTMLAttributes<HTMLInputElement>['type'];
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type CommandInputProps = React.ComponentPropsWithoutRef<typeof CommandPrimitive.Input> & { | |
| type?: React.InputHTMLAttributes<HTMLInputElement>['type']; | |
| }; | |
| export type CommandInputProps = React.ComponentPropsWithoutRef<typeof CommandPrimitive.Input> & { | |
| type?: React.InputHTMLAttributes<HTMLInputElement>['type']; | |
| }; |
🤖 Prompt for AI Agents
In packages/components/src/ui/command.tsx around lines 33 to 35, the
CommandInputProps type is currently declared but not exported; update the
declaration to export the type so consumers can import it (e.g., change the
declaration to export type CommandInputProps = ...), and ensure any existing
imports/exports in the file remain consistent with the new exported type.
| {...props} | ||
| /> | ||
| <CommandPrimitive.Input asChild {...props}> | ||
| <input |
There was a problem hiding this comment.
@lcmohsen why do we need asChild and input here? Is CommandPrimitive.Input not already an input?
There was a problem hiding this comment.
The input type is hardcoded in cmdk
https://github.com/pacocoursey/cmdk/blob/main/cmdk/src/index.tsx
lcmohsen
force-pushed
the
fix/add-type-prop-to-select
branch
from
1f126f9 to
212dd20
Compare
…tory to demonstrate number input type in UI Select
lcmohsen
force-pushed
the
fix/add-type-prop-to-select
branch
from
212dd20 to
a913877
Compare
Add a
typeprop to the searchInputProps of the component. Summary by CodeRabbit New Features Command input now supports setting the input type (e.g., number) for more precise data entry. Select component’s search field can be configured as a numeric input, improving number-only workflows. Documentation Added a story demonstrating a numeric search input for the Select component with an interactive check that the field is type="number". Chores Components package bumped to v0.22.5.