-
Notifications
You must be signed in to change notification settings - Fork 0
Select: default creatable behavior trims input when onCreateOption is omitted #158
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
Select: default creatable behavior trims input when onCreateOption is omitted #158
Conversation
… omitted; update Storybook to document default
|
|
WalkthroughAdds a default creation handler for creatable Select options, enabling both sync/async onCreateOption via Promise.resolve, and updates docs to reflect default trimming/label-value behavior. Bumps components package version to 0.22.4. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Select as Select (creatable)
participant Handler as onCreateOption (optional)
rect rgba(230,245,255,0.5)
User->>Select: Type query q and choose "Create"
alt onCreateOption provided
Select->>Handler: (q)
Note right of Handler: May be sync or async
Handler-->>Select: Promise<SelectOption>
else no onCreateOption
Select->>Select: defaultCreate(q)\n(trim q, build {label,value})
end
end
Select->>Select: onValueChange(created.value)
Select-->>User: Close popover
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
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 (4)
apps/docs/src/remix-hook-form/select.stories.tsx (1)
533-533: Docs: clarify that the created value is a string.To avoid ambiguity for consumers using non-string values, explicitly note that the default creatable value is a string.
- description="Creatable option enabled (defaults to trimming the input and using it as label/value)" + description="Creatable option enabled (defaults to trimming the input and using it as a string label/value)"packages/components/src/ui/select.tsx (3)
254-259: Handle async errors from onCreateOption.If onCreateOption throws/rejects, the component stays in an odd state. Wrap in try/catch and only close on success.
- onSelect={async () => { - const defaultCreate = (input: string): SelectOption<T> => { - const normalized = input.trim(); - // Keep value as a string; schema can coerce on submit if needed - return { label: normalized, value: normalized as unknown as T }; - }; - const created = await Promise.resolve((onCreateOption ?? defaultCreate)(q)); - onValueChange?.(created.value); - popoverState.close(); - }} + onSelect={async () => { + const defaultCreate = (input: string): SelectOption<T> => { + const normalized = input.trim(); + // Keep value as a string; schema can coerce on submit if needed + return { label: normalized, value: normalized as unknown as T }; + }; + try { + const created = await Promise.resolve((onCreateOption ?? defaultCreate)(q)); + onValueChange?.(created.value); + popoverState.close(); + } catch (err) { + // Swallow to keep the popover open and allow retry/custom error handling upstream + console.error('Select creatable onCreateOption failed:', err); + } + }}
254-256: Minor: redundant trim.
qis alreadysearchQuery.trim(). Trimming again is harmless but unnecessary.- const defaultCreate = (input: string): SelectOption<T> => { - const normalized = input.trim(); + const defaultCreate = (_input: string): SelectOption<T> => { + const normalized = q; // already trimmed above
254-259: Type-safety: defaultcreatablereturns string; casting toTis unsound
Restrict the defaultcreatablepath toT = string(via a type overload) or require explicitonCreateOptionfor non-string types.
📜 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(1 hunks)packages/components/src/ui/select.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/select.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/select.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/select.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/select.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/select.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/select.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/select.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/select.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/select.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/select.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/select.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/select.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/select.tsx
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid cross-package relative imports in the monorepo
Files:
packages/components/src/ui/select.tsx
|
@codegen-sh let's use the cursor rule instruction for npm versioning to bump a patch version to our component library
|
|
Hey Jake! 🚀 View PR #159 on Github
This follows the documented steps in
If you want, I can also:
Would love your feedback! 👍 💻 View my work • 🛑 Stop • React 👍 or 👎 |
…trim behavior + docs
Requested by Jake Ruesink.
Summary
creatableis enabled and noonCreateOptionis provided, Select now defaults to trimming the input and creating{ label: normalized, value: normalized }wherenormalized = input.trim().Why
Details
packages/components/src/ui/select.tsx: provide a safe default creator when none is supplied; still supports asynconCreateOption.apps/docs/src/remix-hook-form/select.stories.tsx: "Creatable" story uses default behavior and description clarifies the default.Notes
@lambdacurry/formstype-check passes;apps/docstype-check has pre-existing issues unrelated to this change.💻 View my work • About Codegen
⛔ Remove Codegen from PR • 🚫 Ban action checks
Summary by CodeRabbit
New Features
Documentation
Chores