-
Notifications
You must be signed in to change notification settings - Fork 0
feat(Select): add type prop to Select and add NumberSearchInputType s… #160
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
Conversation
…tory to demonstrate number input type in UI Select
|
|
WalkthroughAdds a new NumberSearchInputType story in Remix Hook Form select stories. Updates CommandInput to accept an explicit input type, render a native input via asChild, and set displayName. The story validates numeric input type behavior. No other files are modified. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Story as Story: NumberSearchInputType
participant UISelect
participant CommandInput
participant CmdPrimitive as CommandPrimitive.Input
participant Native as <input>
User->>Story: Render story
Story->>UISelect: Render with search input type="number"
UISelect->>CommandInput: props { type: "number" }
CommandInput->>CmdPrimitive: asChild
CmdPrimitive->>Native: Render <input type="number">
Note right of Native: Type attribute propagated
User->>Native: Enter numeric value
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
📝 Storybook Preview: View Storybook This preview will be updated automatically when you push new changes to this PR.
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/docs/src/remix-hook-form/select.stories.tsx (2)
1-13: Reorganize imports per coding guidelines.External library imports should precede internal package imports. Move lines 8-12 (external libraries:
@storybook/*,react-router,remix-hook-form,zod) before lines 3-7 (internal package imports:@lambdacurry/forms/*).Apply this diff to reorder imports:
import { zodResolver } from '@hookform/resolvers/zod'; import * as React from 'react'; +import type { Meta, StoryObj } from '@storybook/react-vite'; +import { expect, userEvent, within } from '@storybook/test'; +import { type ActionFunctionArgs, useFetcher } from 'react-router'; +import { getValidatedFormData, RemixFormProvider, useRemixForm } from 'remix-hook-form'; +import { z } from 'zod'; import { CanadaProvinceSelect, Select, USStateSelect } from '@lambdacurry/forms/remix-hook-form'; import { Select as UISelect } from '@lambdacurry/forms/ui/select'; import { Button } from '@lambdacurry/forms/ui/button'; import { CANADA_PROVINCES } from '@lambdacurry/forms/ui/data/canada-provinces'; import { US_STATES } from '@lambdacurry/forms/ui/data/us-states'; -import type { Meta, StoryObj } from '@storybook/react-vite'; -import { expect, userEvent, within } from '@storybook/test'; -import { type ActionFunctionArgs, useFetcher } from 'react-router'; -import { getValidatedFormData, RemixFormProvider, useRemixForm } from 'remix-hook-form'; -import { z } from 'zod'; import { withReactRouterStubDecorator } from '../lib/storybook/react-router-stub';As per coding guidelines.
639-660: Consider generating number options programmatically.The manually listed number options (1-20) could be simplified for better maintainability.
Apply this diff to generate options programmatically:
- 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 numberOptions = Array.from({ length: 20 }, (_, i) => ({ + label: String(i + 1), + value: i + 1, + }));
📜 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)
**/*.{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:
apps/docs/src/remix-hook-form/select.stories.tsxpackages/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:
apps/docs/src/remix-hook-form/select.stories.tsxpackages/components/src/ui/command.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:
apps/docs/src/remix-hook-form/select.stories.tsxpackages/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,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:
apps/docs/src/remix-hook-form/select.stories.tsxpackages/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:
apps/docs/src/remix-hook-form/select.stories.tsxpackages/components/src/ui/command.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
**/*.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:
apps/docs/src/remix-hook-form/select.stories.tsxpackages/components/src/ui/command.tsx
**/*.{ts,tsx,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use kebab-case for filenames (e.g., text-field.tsx, data-table-filter/**)
Files:
apps/docs/src/remix-hook-form/select.stories.tsxpackages/components/src/ui/command.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
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
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
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
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
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
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid cross-package relative imports in the monorepo
Files:
packages/components/src/ui/command.tsx
🧬 Code graph analysis (1)
packages/components/src/ui/command.tsx (1)
packages/components/src/ui/utils/index.ts (1)
cn(4-6)
🔇 Additional comments (2)
apps/docs/src/remix-hook-form/select.stories.tsx (1)
675-693: LGTM! Story correctly validates numeric input type.The story properly tests that
searchInputProps={{ type: 'number' }}is applied to the search input element. The test structure follows Storybook best practices with async queries and role-based selectors.packages/components/src/ui/command.tsx (1)
33-51: LGTM! Type prop correctly implemented with asChild pattern.The implementation correctly:
- Extends
CommandPrimitive.Inputprops with an optionaltypefield- Uses the
asChildpattern to delegate behavior to a native<input>element- Applies the
typeattribute to the rendered input- Maintains proper prop spreading and className handling
- Sets
displayNameappropriately
Add
typeprop to the 's input component. Summary by CodeRabbit New Features CommandInput now supports an input type prop (e.g., number), enabling numeric search/select inputs in UI components. Documentation Added a Storybook example showcasing a number-based search/select input (NumberSearchInputType), including an interactive check to verify the input type.