feat: implement scroll-to-error functionality for @lambdacurry/forms #140
Conversation
- Add core scrollToFirstError utility with configurable options - Create useScrollToErrorOnSubmit hook for Remix form integration - Add ScrollToErrorOnSubmit component for declarative usage - Leverage existing data-slot selectors for seamless integration - Support both client-side and server-side validation errors - Include retry logic for async rendering scenarios - Export new functionality from main package entry points
|
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds a scroll-to-error utility, a Remix hook and component that trigger it after form submission or on mount, updates barrel exports to expose them, re-exports a TextareaField alias, and introduces Storybook docs demonstrating usage. Bumps components package version to 0.20.0. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Form as Remix Form
participant Hook as useScrollToErrorOnSubmit
participant Util as scrollToFirstError
participant DOM
User->>Form: Submit
Form->>Hook: formState (errors, isSubmitting -> false)
activate Hook
Note over Hook: After delay, if enabled and errors present
Hook->>Util: scrollToFirstError(errors, options)
activate Util
Util->>DOM: query selectors, scroll, (optional) focus
deactivate Util
deactivate Hook
sequenceDiagram
autonumber
participant App as App Mount (SSR)
participant Hook as useScrollToErrorOnSubmit
participant Util as scrollToFirstError
participant DOM
App->>Hook: Initialize with options
Note over Hook: If scrollOnMount && scrollOnServerErrors && errors
Hook->>Util: scrollToFirstError(errors, options)
Util->>DOM: query, scroll, focus (retry if needed)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/components/package.json (1)
37-43: Deduplicate peer vs. runtime deps for router/remix-hook-form.
These must be peerDependencies (and devDependencies for build), not runtime dependencies, to avoid duplicate installs and version conflicts in host apps.Apply:
"peerDependencies": { "react": "^19.0.0", "react-router": "^7.0.0", "react-router-dom": "^7.0.0", "remix-hook-form": "7.1.0" }, "dependencies": { @@ - "react-router": "^7.6.3", - "react-router-dom": "^7.6.3", - "remix-hook-form": "7.1.0", + // (moved to devDependencies for type-check/build only) @@ }, "devDependencies": { @@ + "react-router": "^7.6.3", + "react-router-dom": "^7.6.3", + "remix-hook-form": "7.1.0", "react": "^19.0.0",Also applies to: 71-75
🧹 Nitpick comments (9)
packages/components/src/ui/phone-input.tsx (1)
106-121: Prevent >11 digits in US mode and align typing guard with normalization.
Current isComplete logic allows typing past 11 when leading '1'. Tighten the guard.Apply:
const handleKeyDown = (e: KeyboardEvent<HTMLInputElement>) => { if (!isInternational) { const currentDigits = extractDigits(display); const isNumberKey = NUMBER_KEY_REGEX.test(e.key); const isModifier = e.ctrlKey || e.metaKey || e.altKey; const allowed = ['Backspace', 'Delete', 'ArrowLeft', 'ArrowRight', 'Tab', 'Home', 'End', 'Enter']; - // Allow typing if we have fewer than 10 digits or if we have 11 digits but the first is '1' - const isComplete = currentDigits.length >= 10 && !(currentDigits.length === 11 && currentDigits.startsWith('1')); + // Max: 10 digits, or 11 when the first digit is the US country code '1' + const isAtMax = + currentDigits.startsWith('1') ? currentDigits.length >= 11 : currentDigits.length >= 10; - if (!isModifier && isNumberKey && isComplete) { + if (!isModifier && isNumberKey && isAtMax) { // Prevent adding more digits once 10-digit US number is complete e.preventDefault(); return; } if (allowed.includes(e.key)) return; // Allow other typical keys; restriction handled by formatting } };packages/components/src/utils/scrollToError.ts (1)
59-67: Clarify return semantics (retry path).
Function returns true even when only scheduling a retry. Consider documenting this or returning a Promise in a future minor to reflect eventual success/failure.packages/components/src/remix-hook-form/us-state-select.tsx (1)
7-7: Don't override consumer placeholder; reorder props.Placing
{...props}first causesplaceholder="Select a state"to always win. Mirror the UI variant so consumers can override it.- return <Select {...props} options={US_STATES} placeholder="Select a state" />; + return <Select options={US_STATES} placeholder="Select a state" {...props} />;packages/components/src/remix-hook-form/canada-province-select.tsx (1)
7-7: Ensure placeholder is overrideable; reorder props.Like the UI counterpart, default props should come before
{...props}.- return <Select {...props} options={CANADA_PROVINCES} placeholder="Select a province" />; + return <Select options={CANADA_PROVINCES} placeholder="Select a province" {...props} />;packages/components/src/remix-hook-form/components/ScrollToErrorOnSubmit.tsx (1)
1-14: Rename file to kebab‑case to match repo conventions.Guidelines require kebab‑case filenames. Recommend
scroll-to-error-on-submit.tsxand adjust barrel exports accordingly.packages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts (1)
1-10: Optional: simplify memo deps.Use
useMemo(() => ({ ...scrollOptions }), [behavior, block, inline, offset, shouldFocus, retryAttempts])to ensure a stable object identity and avoid accidental mutation.packages/components/src/utils/index.ts (3)
1-1: Use explicit named and type re‑exports per package index guidelines.Avoid wildcard re‑exports to control API surface and aid tree‑shaking.
-export * from './scrollToError'; +export { scrollToFirstError } from './scrollToError'; +export type { ScrollToErrorOptions } from './scrollToError';
1-1: Prefer funneling top‑level exports through the utils barrel.In
packages/components/src/index.ts, re‑export from./utilsrather than deep path.Example:
export { scrollToFirstError } from './utils'; export type { ScrollToErrorOptions } from './utils';
1-1: Rename scrollToError.ts → scroll-to-error.ts and update importsRename packages/components/src/utils/scrollToError.ts → packages/components/src/utils/scroll-to-error.ts and update these references:
packages/components/src/utils/index.ts
- change
export * from './scrollToError';→export * from './scroll-to-error';packages/components/src/index.ts
- change
export { scrollToFirstError } from './utils/scrollToError';export type { ScrollToErrorOptions } from './utils/scrollToError';- to
export { scrollToFirstError } from './utils/scroll-to-error';export type { ScrollToErrorOptions } from './utils/scroll-to-error';packages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts
- change
import { type ScrollToErrorOptions, scrollToFirstError } from '../../utils/scrollToError';→from '../../utils/scroll-to-error';-export { scrollToFirstError } from './scrollToError'; -export type { ScrollToErrorOptions } from './scrollToError'; +export { scrollToFirstError } from './scroll-to-error'; +export type { ScrollToErrorOptions } from './scroll-to-error';
📜 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 (22)
apps/docs/src/remix-hook-form/phone-input.stories.tsx(1 hunks)apps/docs/src/remix-hook-form/select.test.tsx(1 hunks)package.json(1 hunks)packages/components/package.json(1 hunks)packages/components/src/index.ts(1 hunks)packages/components/src/remix-hook-form/canada-province-select.tsx(1 hunks)packages/components/src/remix-hook-form/components/ScrollToErrorOnSubmit.tsx(1 hunks)packages/components/src/remix-hook-form/components/index.ts(1 hunks)packages/components/src/remix-hook-form/hooks/index.ts(1 hunks)packages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts(1 hunks)packages/components/src/remix-hook-form/index.ts(1 hunks)packages/components/src/remix-hook-form/phone-input.tsx(1 hunks)packages/components/src/remix-hook-form/select.tsx(3 hunks)packages/components/src/remix-hook-form/us-state-select.tsx(1 hunks)packages/components/src/ui/canada-province-select.tsx(1 hunks)packages/components/src/ui/data/canada-provinces.ts(1 hunks)packages/components/src/ui/data/us-states.ts(1 hunks)packages/components/src/ui/index.ts(0 hunks)packages/components/src/ui/phone-input.tsx(1 hunks)packages/components/src/ui/us-state-select.tsx(1 hunks)packages/components/src/utils/index.ts(1 hunks)packages/components/src/utils/scrollToError.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/components/src/ui/index.ts
🧰 Additional context used
📓 Path-based instructions (28)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
Handle server-side validation using getValidatedFormData with zodResolver and return errors as needed
Files:
packages/components/src/utils/index.tspackages/components/src/remix-hook-form/index.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.tspackages/components/src/remix-hook-form/components/index.tspackages/components/src/remix-hook-form/hooks/index.tspackages/components/src/ui/data/us-states.tspackages/components/src/utils/scrollToError.tspackages/components/src/index.tspackages/components/src/ui/data/canada-provinces.ts
**/*.{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/utils/index.tspackages/components/src/remix-hook-form/index.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.tsapps/docs/src/remix-hook-form/phone-input.stories.tsxpackages/components/src/remix-hook-form/components/index.tspackages/components/src/remix-hook-form/us-state-select.tsxapps/docs/src/remix-hook-form/select.test.tsxpackages/components/src/remix-hook-form/hooks/index.tspackages/components/src/ui/data/us-states.tspackages/components/src/utils/scrollToError.tspackages/components/src/remix-hook-form/phone-input.tsxpackages/components/src/ui/phone-input.tsxpackages/components/src/index.tspackages/components/src/ui/data/canada-provinces.tspackages/components/src/ui/us-state-select.tsxpackages/components/src/remix-hook-form/canada-province-select.tsxpackages/components/src/remix-hook-form/select.tsxpackages/components/src/remix-hook-form/components/ScrollToErrorOnSubmit.tsxpackages/components/src/ui/canada-province-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:
packages/components/src/utils/index.tspackages/components/src/remix-hook-form/index.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.tsapps/docs/src/remix-hook-form/phone-input.stories.tsxpackages/components/src/remix-hook-form/components/index.tspackages/components/src/remix-hook-form/us-state-select.tsxapps/docs/src/remix-hook-form/select.test.tsxpackages/components/src/remix-hook-form/hooks/index.tspackages/components/src/ui/data/us-states.tspackages/components/src/utils/scrollToError.tspackages/components/src/remix-hook-form/phone-input.tsxpackages/components/src/ui/phone-input.tsxpackages/components/src/index.tspackages/components/src/ui/data/canada-provinces.tspackages/components/src/ui/us-state-select.tsxpackages/components/src/remix-hook-form/canada-province-select.tsxpackages/components/src/remix-hook-form/select.tsxpackages/components/src/remix-hook-form/components/ScrollToErrorOnSubmit.tsxpackages/components/src/ui/canada-province-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:
packages/components/src/utils/index.tspackages/components/src/remix-hook-form/index.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.tsapps/docs/src/remix-hook-form/phone-input.stories.tsxpackages/components/src/remix-hook-form/components/index.tspackages/components/src/remix-hook-form/us-state-select.tsxapps/docs/src/remix-hook-form/select.test.tsxpackages/components/src/remix-hook-form/hooks/index.tspackages/components/src/ui/data/us-states.tspackages/components/src/utils/scrollToError.tspackages/components/src/remix-hook-form/phone-input.tsxpackages/components/src/ui/phone-input.tsxpackages/components/src/index.tspackages/components/src/ui/data/canada-provinces.tspackages/components/src/ui/us-state-select.tsxpackages/components/src/remix-hook-form/canada-province-select.tsxpackages/components/src/remix-hook-form/select.tsxpackages/components/src/remix-hook-form/components/ScrollToErrorOnSubmit.tsxpackages/components/src/ui/canada-province-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
packages/components/src/**/*.{ts,tsx}: Keep React components pure and fully typed (TypeScript + React, ES modules)
Export symbols as named exports only (avoid default exports)
Files:
packages/components/src/utils/index.tspackages/components/src/remix-hook-form/index.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.tspackages/components/src/remix-hook-form/components/index.tspackages/components/src/remix-hook-form/us-state-select.tsxpackages/components/src/remix-hook-form/hooks/index.tspackages/components/src/ui/data/us-states.tspackages/components/src/utils/scrollToError.tspackages/components/src/remix-hook-form/phone-input.tsxpackages/components/src/ui/phone-input.tsxpackages/components/src/index.tspackages/components/src/ui/data/canada-provinces.tspackages/components/src/ui/us-state-select.tsxpackages/components/src/remix-hook-form/canada-province-select.tsxpackages/components/src/remix-hook-form/select.tsxpackages/components/src/remix-hook-form/components/ScrollToErrorOnSubmit.tsxpackages/components/src/ui/canada-province-select.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/utils/index.tspackages/components/src/remix-hook-form/index.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.tsapps/docs/src/remix-hook-form/phone-input.stories.tsxpackages/components/src/remix-hook-form/components/index.tspackages/components/src/remix-hook-form/us-state-select.tsxapps/docs/src/remix-hook-form/select.test.tsxpackages/components/src/remix-hook-form/hooks/index.tspackages/components/src/ui/data/us-states.tspackages/components/src/utils/scrollToError.tspackages/components/src/remix-hook-form/phone-input.tsxpackages/components/src/ui/phone-input.tsxpackages/components/src/index.tspackages/components/src/ui/data/canada-provinces.tspackages/components/src/ui/us-state-select.tsxpackages/components/src/remix-hook-form/canada-province-select.tsxpackages/components/src/remix-hook-form/select.tsxpackages/components/src/remix-hook-form/components/ScrollToErrorOnSubmit.tsxpackages/components/src/ui/canada-province-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)
Filenames should be kebab-case (e.g., text-field.tsx, data-table-filter/**)
Use PascalCase for components and types; hooks use camelCase with a use* prefix
Files:
packages/components/src/utils/index.tspackages/components/src/remix-hook-form/index.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.tsapps/docs/src/remix-hook-form/phone-input.stories.tsxpackages/components/src/remix-hook-form/components/index.tspackages/components/src/remix-hook-form/us-state-select.tsxapps/docs/src/remix-hook-form/select.test.tsxpackages/components/src/remix-hook-form/hooks/index.tspackages/components/src/ui/data/us-states.tspackages/components/src/utils/scrollToError.tspackages/components/src/remix-hook-form/phone-input.tsxpackages/components/src/ui/phone-input.tsxpackages/components/src/index.tspackages/components/src/ui/data/canada-provinces.tspackages/components/src/ui/us-state-select.tsxpackages/components/src/remix-hook-form/canada-province-select.tsxpackages/components/src/remix-hook-form/select.tsxpackages/components/src/remix-hook-form/components/ScrollToErrorOnSubmit.tsxpackages/components/src/ui/canada-province-select.tsx
packages/components/src/**/*index.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Re-export with explicit names and types; avoid mixing default and named exports
Files:
packages/components/src/utils/index.tspackages/components/src/remix-hook-form/index.tspackages/components/src/remix-hook-form/components/index.tspackages/components/src/remix-hook-form/hooks/index.tspackages/components/src/index.ts
**/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Prefer local index.ts barrel files when useful for organized imports/exports
Files:
packages/components/src/utils/index.tspackages/components/src/remix-hook-form/index.tspackages/components/src/remix-hook-form/components/index.tspackages/components/src/remix-hook-form/hooks/index.tspackages/components/src/index.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use cross-package relative imports; respect package boundaries
Files:
packages/components/src/utils/index.tspackages/components/src/remix-hook-form/index.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.tspackages/components/src/remix-hook-form/components/index.tspackages/components/src/remix-hook-form/us-state-select.tsxpackages/components/src/remix-hook-form/hooks/index.tspackages/components/src/ui/data/us-states.tspackages/components/src/utils/scrollToError.tspackages/components/src/remix-hook-form/phone-input.tsxpackages/components/src/ui/phone-input.tsxpackages/components/src/index.tspackages/components/src/ui/data/canada-provinces.tspackages/components/src/ui/us-state-select.tsxpackages/components/src/remix-hook-form/canada-province-select.tsxpackages/components/src/remix-hook-form/select.tsxpackages/components/src/remix-hook-form/components/ScrollToErrorOnSubmit.tsxpackages/components/src/ui/canada-province-select.tsx
packages/components/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Define explicit props interfaces and follow React 19 ref patterns
Files:
packages/components/src/utils/index.tspackages/components/src/remix-hook-form/index.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.tspackages/components/src/remix-hook-form/components/index.tspackages/components/src/remix-hook-form/us-state-select.tsxpackages/components/src/remix-hook-form/hooks/index.tspackages/components/src/ui/data/us-states.tspackages/components/src/utils/scrollToError.tspackages/components/src/remix-hook-form/phone-input.tsxpackages/components/src/ui/phone-input.tsxpackages/components/src/index.tspackages/components/src/ui/data/canada-provinces.tspackages/components/src/ui/us-state-select.tsxpackages/components/src/remix-hook-form/canada-province-select.tsxpackages/components/src/remix-hook-form/select.tsxpackages/components/src/remix-hook-form/components/ScrollToErrorOnSubmit.tsxpackages/components/src/ui/canada-province-select.tsx
{packages/components,apps/docs}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
{packages/components,apps/docs}/**/*.{ts,tsx}: Organize imports automatically (Biome) and maintain consistent import order
For form-aware components, use Zod schemas with proper messages, Remix fetcher.Form, and show FormMessage errors
Files:
packages/components/src/utils/index.tspackages/components/src/remix-hook-form/index.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.tsapps/docs/src/remix-hook-form/phone-input.stories.tsxpackages/components/src/remix-hook-form/components/index.tspackages/components/src/remix-hook-form/us-state-select.tsxapps/docs/src/remix-hook-form/select.test.tsxpackages/components/src/remix-hook-form/hooks/index.tspackages/components/src/ui/data/us-states.tspackages/components/src/utils/scrollToError.tspackages/components/src/remix-hook-form/phone-input.tsxpackages/components/src/ui/phone-input.tsxpackages/components/src/index.tspackages/components/src/ui/data/canada-provinces.tspackages/components/src/ui/us-state-select.tsxpackages/components/src/remix-hook-form/canada-province-select.tsxpackages/components/src/remix-hook-form/select.tsxpackages/components/src/remix-hook-form/components/ScrollToErrorOnSubmit.tsxpackages/components/src/ui/canada-province-select.tsx
packages/components/src/remix-hook-form/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
Form-aware components should be named as ComponentName (e.g., TextField, Checkbox)
Files:
packages/components/src/remix-hook-form/index.tspackages/components/src/remix-hook-form/us-state-select.tsxpackages/components/src/remix-hook-form/phone-input.tsxpackages/components/src/remix-hook-form/canada-province-select.tsxpackages/components/src/remix-hook-form/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/remix-hook-form/index.tspackages/components/src/remix-hook-form/us-state-select.tsxpackages/components/src/remix-hook-form/phone-input.tsxpackages/components/src/ui/phone-input.tsxpackages/components/src/ui/us-state-select.tsxpackages/components/src/remix-hook-form/canada-province-select.tsxpackages/components/src/remix-hook-form/select.tsxpackages/components/src/ui/canada-province-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
Use Storybook Test Runner (Playwright) for tests in apps/docs
Files:
apps/docs/src/remix-hook-form/phone-input.stories.tsxapps/docs/src/remix-hook-form/select.test.tsx
apps/docs/src/remix-hook-form/**/*.stories.tsx
📄 CodeRabbit inference engine (.cursor/rules/storybook-testing.mdc)
apps/docs/src/remix-hook-form/**/*.stories.tsx: Always place decorators on individual stories, not in meta configuration.
Never place decorators in meta configuration.
Use kebab-case for story file names in 'apps/docs/src/remix-hook-form/'.
Always include all three test phases in stories: default state, invalid submission, and valid submission.
Use play functions for comprehensive interaction testing in Storybook stories.
Always use fetcher.Form instead of regular form elements in stories that require React Router integration.
Always click before clearing inputs with userEvent in interaction tests.
Use findBy* queries for async elements instead of getBy* in Storybook interaction tests.
Never use CSS selectors when semantic queries (role, label, etc.) are available in canvas queries.
Each story should test one primary workflow and not multiple unrelated scenarios.
Group related test functions together and export individual test functions for reusability.
Use semantic queries (getByRole, getByLabelText, etc.) as the preferred method for selecting elements in interaction tests.
Meta configuration in stories should be kept clean and minimal, avoiding unnecessary parameters and decorators.
Always import required testing utilities: Meta, StoryObj, expect, userEvent, and withReactRouterStubDecorator in Storybook test stories.
Test both client-side and server-side validation in form stories.
Stories should serve as both documentation and automated tests.
Tests should complete in under 10 seconds for fast feedback.
Use step grouping in play functions for better debugging and organization of complex interactions.
Never use getBy* queries for async elements in Storybook interaction tests.
Never clear inputs without clicking first in userEvent interactions.
Never use regular forms instead of fetcher.Form in stories that require React Router integration.
Never test multiple unrelated scenarios in one story.
Never use CSS selectors when semantic queries are available.
Group rel...
Files:
apps/docs/src/remix-hook-form/phone-input.stories.tsx
apps/docs/**
📄 CodeRabbit inference engine (.cursor/rules/storybook-testing.mdc)
apps/docs/**: Build Storybook static files before running Playwright tests locally.
Install Playwright browsers and system dependencies before running interaction tests.Serve built Storybook from apps/docs when running yarn serve
Files:
apps/docs/src/remix-hook-form/phone-input.stories.tsxapps/docs/src/remix-hook-form/select.test.tsx
apps/docs/**/*.stories.@(tsx|mdx)
📄 CodeRabbit inference engine (AGENTS.md)
apps/docs/**/*.stories.@(tsx|mdx): Add stories that exercise component states to support interaction tests
Use per-story decorators and router stub decorator for Storybook interaction tests
Files:
apps/docs/src/remix-hook-form/phone-input.stories.tsx
packages/components/src/remix-hook-form/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
packages/components/src/remix-hook-form/**/*.tsx: Use remix-hook-form for form state management and validation
Maintain FormField consistency for all form inputs
Ensure proper accessibility and validation integration in form components
Always define Zod schemas with proper error messages for form validation
Use the required imports for Remix Hook Form integration, Zod, and base UI components in form-aware components
Follow the wrapper component pattern for all form-aware components
Use the form setup pattern for form initialization in Remix
Use Zod schemas for all form validation
Provide clear, user-friendly error messages in validation
Validate on blur and submit, not on every keystroke
Always display field-level errors using FormMessage
Handle both client-side and server-side validation errors in form components
Provide loading states during form submission
Clear errors appropriately when fields are corrected
Use React.memo for expensive form components
Avoid unnecessary re-renders by properly structuring form state
Consider field-level subscriptions for large forms
All form fields must have proper labels
Use ARIA attributes for complex form interactions
Ensure keyboard navigation works correctly in form components
Provide clear error announcements for screen readers
Files:
packages/components/src/remix-hook-form/us-state-select.tsxpackages/components/src/remix-hook-form/phone-input.tsxpackages/components/src/remix-hook-form/canada-province-select.tsxpackages/components/src/remix-hook-form/select.tsxpackages/components/src/remix-hook-form/components/ScrollToErrorOnSubmit.tsx
**/*.test.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate interaction tests as *.test.tsx near stories/components
Files:
apps/docs/src/remix-hook-form/select.test.tsx
{package.json,packages/**/package.json,apps/**/package.json}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Use consistent versioning across packages
Files:
packages/components/package.jsonpackage.json
{packages/**/package.json,apps/**/package.json}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Define peerDependencies, dependencies, and devDependencies appropriately in package.json
Files:
packages/components/package.json
packages/components/package.json
📄 CodeRabbit inference engine (.cursor/rules/versioning-with-npm.mdc)
Version bump updates packages/components/package.json for @lambdacurry/forms
Files:
packages/components/package.json
packages/*/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Verify package.json exports fields correctly define public API
Files:
packages/components/package.json
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/data/us-states.tspackages/components/src/ui/phone-input.tsxpackages/components/src/ui/data/canada-provinces.tspackages/components/src/ui/us-state-select.tsxpackages/components/src/ui/canada-province-select.tsx
package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use Yarn 4 (packageManager field) at the repository root
Files:
package.json
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/phone-input.tsxpackages/components/src/ui/us-state-select.tsxpackages/components/src/ui/canada-province-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/phone-input.tsxpackages/components/src/ui/us-state-select.tsxpackages/components/src/ui/canada-province-select.tsx
🧬 Code graph analysis (9)
packages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts (1)
packages/components/src/utils/scrollToError.ts (2)
ScrollToErrorOptions(3-10)scrollToFirstError(38-70)
apps/docs/src/remix-hook-form/phone-input.stories.tsx (1)
packages/components/src/remix-hook-form/phone-input.tsx (1)
PhoneInput(12-29)
packages/components/src/remix-hook-form/us-state-select.tsx (3)
packages/components/src/ui/us-state-select.tsx (2)
USStateSelectProps(4-4)USStateSelect(6-8)packages/components/src/remix-hook-form/select.tsx (2)
SelectProps(7-20)Select(22-52)packages/components/src/ui/data/us-states.ts (1)
US_STATES(3-55)
packages/components/src/utils/scrollToError.ts (1)
packages/components/src/index.ts (2)
ScrollToErrorOptions(5-5)scrollToFirstError(4-4)
packages/components/src/ui/us-state-select.tsx (3)
packages/components/src/remix-hook-form/us-state-select.tsx (2)
USStateSelectProps(4-4)USStateSelect(6-8)packages/components/src/ui/select.tsx (2)
SelectProps(25-35)Select(37-214)packages/components/src/ui/data/us-states.ts (1)
US_STATES(3-55)
packages/components/src/remix-hook-form/canada-province-select.tsx (4)
packages/components/src/ui/canada-province-select.tsx (2)
CanadaProvinceSelectProps(4-4)CanadaProvinceSelect(6-8)packages/components/src/remix-hook-form/select.tsx (2)
SelectProps(7-20)Select(22-52)packages/components/src/ui/select.tsx (2)
SelectProps(25-35)Select(37-214)packages/components/src/ui/data/canada-provinces.ts (1)
CANADA_PROVINCES(3-17)
packages/components/src/remix-hook-form/select.tsx (1)
packages/components/src/ui/select.tsx (2)
Select(37-214)SelectProps(25-35)
packages/components/src/remix-hook-form/components/ScrollToErrorOnSubmit.tsx (1)
packages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts (2)
UseScrollToErrorOnSubmitOptions(5-10)useScrollToErrorOnSubmit(12-54)
packages/components/src/ui/canada-province-select.tsx (4)
packages/components/src/remix-hook-form/canada-province-select.tsx (2)
CanadaProvinceSelectProps(4-4)CanadaProvinceSelect(6-8)packages/components/src/remix-hook-form/select.tsx (2)
SelectProps(7-20)Select(22-52)packages/components/src/ui/select.tsx (2)
SelectProps(25-35)Select(37-214)packages/components/src/ui/data/canada-provinces.ts (1)
CANADA_PROVINCES(3-17)
🔇 Additional comments (15)
packages/components/package.json (1)
29-29: LGTM: files field reformat is harmless.
No behavior change; keep as is.package.json (1)
5-5: LGTM: workspaces formatting only.
No functional change.packages/components/src/remix-hook-form/phone-input.tsx (1)
1-5: Good: switch to type-only React import.
Reduces runtime bundle; matches React 19 automatic JSX.Confirm tsconfig has "jsx": "react-jsx" and no code relies on React default import at runtime.
apps/docs/src/remix-hook-form/phone-input.stories.tsx (1)
40-41: LGTM: JSX collapsed to single line.
Story still meets testing guidelines (findBy*, per‑story decorator, three phases).packages/components/src/ui/data/us-states.ts (1)
1-1: LGTM! Type-only import optimization.The change from runtime import to type-only import for
SelectOptionis correct since this file only uses the type for array typing, not at runtime. This reduces the bundle size by avoiding unnecessary runtime dependencies.packages/components/src/remix-hook-form/hooks/index.ts (1)
1-1: LGTM! Proper barrel export for new scroll-to-error hook.This barrel export correctly exposes the
useScrollToErrorOnSubmithook through the hooks index, maintaining consistency with the existing export pattern.packages/components/src/ui/us-state-select.tsx (2)
2-2: LGTM! Type-only import optimization aligned with JSX Transform.The change to
import { Select, type SelectProps }correctly separates runtime imports from type-only imports, reducing bundle size. This aligns with the project's adoption of React's automatic JSX transform where explicit React imports are no longer needed for JSX compilation.
7-7: LGTM! Cleaner component render with proper prop spreading.The single-line return is more concise while maintaining the same functionality. Props spreading with
{...props}is correctly placed after the specific props to allow overrides.packages/components/src/remix-hook-form/index.ts (1)
1-4: LGTM! Well-organized public API expansion for scroll-to-error functionality.The new exports are properly organized with clear commenting, making the scroll-to-error functionality available at the package's public API level. The structure maintains backward compatibility while extending functionality.
packages/components/src/ui/data/canada-provinces.ts (1)
1-1: LGTM! Type-only import optimization aligned with US states pattern.This change mirrors the same optimization made in
us-states.ts, consistently using type-only imports forSelectOptionacross data files. This reduces runtime dependencies while maintaining type safety.apps/docs/src/remix-hook-form/select.test.tsx (1)
1-1: LGTM! Type-only import optimization for Storybook context.The change to
import type { StoryContext }is correct sinceStoryContextis only used as a type annotation in function parameters, not at runtime. This optimization aligns with the broader pattern of minimizing runtime imports throughout the codebase.packages/components/src/remix-hook-form/components/index.ts (1)
1-1: LGTM! Proper barrel export for new scroll-to-error component.This barrel export correctly exposes the
ScrollToErrorOnSubmitcomponent and its props interface through the components index, maintaining consistency with the existing export pattern and completing the public API surface for the scroll-to-error functionality.packages/components/src/ui/canada-province-select.tsx (1)
7-7: LGTM — correct prop precedence.
options/defaultplaceholderprecede{...props}, allowing consumer overrides where intended.packages/components/src/index.ts (1)
4-5: LGTM — public exports match PR objectives.Adding
scrollToFirstErrorandScrollToErrorOptionsat the root is correct and tree‑shaking friendly.packages/components/src/remix-hook-form/select.tsx (1)
1-1: LGTM — type‑only React import and refactor are safe.Only type positions reference
React, JSX relies on the automatic runtime, and the API remains unchanged.Also applies to: 4-5, 22-22, 46-46
packages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts
Show resolved
Hide resolved
| // Handle form submission errors | ||
| useEffect(() => { | ||
| if (!enabled) return; | ||
| const hasErrors = Object.keys(formState.errors).length > 0; | ||
|
|
||
| // Scroll after submission attempt when errors exist | ||
| if (!formState.isSubmitting && hasErrors) { | ||
| const timeoutId = setTimeout(() => { | ||
| scrollToFirstError(formState.errors, memoizedScrollOptions); | ||
| }, delay); | ||
|
|
||
| return () => clearTimeout(timeoutId); | ||
| } | ||
| }, [formState.errors, formState.isSubmitting, enabled, delay, memoizedScrollOptions]); | ||
|
|
||
| // Handle server-side validation errors on mount (Remix SSR) | ||
| useEffect(() => { | ||
| if (!(enabled && scrollOnMount) || !scrollOnServerErrors) return; | ||
| const hasErrors = Object.keys(formState.errors).length > 0; | ||
|
|
||
| if (hasErrors && !formState.isSubmitting) { | ||
| const timeoutId = setTimeout(() => { | ||
| scrollToFirstError(formState.errors, memoizedScrollOptions); | ||
| }, delay); | ||
|
|
||
| return () => clearTimeout(timeoutId); | ||
| } | ||
| }, [enabled, scrollOnMount, scrollOnServerErrors, formState.errors, formState.isSubmitting, delay, memoizedScrollOptions]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Verification helper: check for other wrappers with prop order pitfalls.
Run to spot any <Select {...props} ... placeholder=...> in wrappers.
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts --type=tsx -C1 '\<Select\s*\{\.\.\.props\}[^>]*\bplaceholder=' packages/components/src | sed -n '1,200p'Length of output: 27
🏁 Script executed:
#!/bin/bash
rg -n '<Select[^>]*\{\.{3}props\}[^>]*placeholder' -g '*.tsx' -g '*.ts' packages/components/srcLength of output: 302
Don't override consumer-provided placeholder in Select wrappers
Two wrappers spread {...props} before an explicit placeholder, causing the wrapper's placeholder to always win; change to spread {...props} last or use placeholder={props.placeholder ?? '…'}.
- packages/components/src/remix-hook-form/canada-province-select.tsx:7
- packages/components/src/remix-hook-form/us-state-select.tsx:7
🤖 Prompt for AI Agents
In packages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts
around lines 26 to 53, review comment actually targets two Select wrapper files
where the wrapper placeholder is overriding consumer-provided values; update the
wrappers (packages/components/src/remix-hook-form/canada-province-select.tsx and
packages/components/src/remix-hook-form/us-state-select.tsx) so that either
{...props} is spread last (so consumer placeholder wins) or set
placeholder={props.placeholder ?? 'Your default placeholder'} (use the consumer
prop if present, otherwise fall back to the default); apply the change
consistently in both files and ensure TypeScript types still align.
packages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts
Outdated
Show resolved
Hide resolved
…onality - Created scroll-to-error.stories.tsx with hook and component examples - Demonstrates both useScrollToErrorOnSubmit hook and ScrollToErrorOnSubmit component - Includes comprehensive form with multiple sections for scroll testing - Added interactive test scenarios for validation and success cases - Fixed TypeScript issues and accessibility concerns - Optimized performance with regex constants
- Split RadioGroup and RadioGroupItem imports to separate lines - RadioGroupItem is exported from radio-group-item.tsx, not radio-group.tsx - This resolves the build error: 'RadioGroupItem is not exported by radio-group.tsx' Co-authored-by: Jake Ruesink <jake@lambdacurry.com>
- Added optional methods parameter to UseScrollToErrorOnSubmitOptions - Hook now accepts methods explicitly or falls back to context - Updated story to pass methods explicitly to fix TypeError - This resolves the 'Cannot read properties of null' error in Storybook
- Move useScrollToErrorOnSubmit hook inside RemixFormProvider context - Create separate ScrollToErrorHookForm component to properly use the hook - Fix TypeError: Cannot read properties of null (reading 'handleSubmit') - Ensure proper form context is available when hook is called
- Add null check in useScrollToErrorOnSubmit hook to prevent 'Cannot read properties of null (reading handleSubmit)' error - Remove duplicate ScrollToErrorComponentExample component declaration - Hook now gracefully handles cases where no form methods are available - Adds warning message when form context is missing
- Remove undefined 'methods' parameter from useScrollToErrorOnSubmit hook - Hook will use useRemixFormContext() automatically when inside RemixFormProvider - This fixes the ReferenceError that was preventing the story from rendering
- Get methods from useRemixFormContext() instead of undefined variable - This fixes the remaining ReferenceError: methods is not defined - Now the component properly accesses form methods for reset and submit state
- Simplified the ScrollToErrorHookForm story by removing redundant test scenarios for form submission and validation. - Enhanced readability by organizing the form structure and ensuring proper context usage. - Removed unnecessary regex constants and unused imports to optimize performance and maintainability.
…a-storybook-examples
jaruesink
temporarily deployed
to
pr-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts (2)
41-55: Gate submit effect by submitCount to prevent double scroll.- if (!formState.isSubmitting && hasErrors) { + if (!formState.isSubmitting && hasErrors && formState.submitCount > 0) { const timeoutId = setTimeout(() => { scrollToFirstError(formState.errors, memoizedScrollOptions); }, delay); return () => clearTimeout(timeoutId); } - }, [formState.errors, formState.isSubmitting, enabled, delay, memoizedScrollOptions]); + }, [formState.errors, formState.isSubmitting, formState.submitCount, enabled, delay, memoizedScrollOptions]);
56-76: Run SSR/server‑error path only before any client submit.- useEffect(() => { + useEffect(() => { if (!(enabled && scrollOnMount) || !scrollOnServerErrors) return; const hasErrors = Object.keys(formState.errors).length > 0; - if (hasErrors && !formState.isSubmitting) { + if (hasErrors && !formState.isSubmitting && formState.submitCount === 0) { const timeoutId = setTimeout(() => { scrollToFirstError(formState.errors, memoizedScrollOptions); }, delay); return () => clearTimeout(timeoutId); } }, [ enabled, scrollOnMount, scrollOnServerErrors, formState.errors, formState.isSubmitting, + formState.submitCount, delay, memoizedScrollOptions, ]);packages/components/src/utils/scrollToError.ts (1)
22-30: Use block/inline, add SSR guard, and focus a focusable target.Options
block/inlineare defined but unused; function can run in SSR; focus may land on non‑focusable nodes.Apply:
-const scrollToElement = (element: HTMLElement, offset: number, behavior: ScrollBehavior): void => { - const elementRect = element.getBoundingClientRect(); - const offsetTop = elementRect.top + window.pageYOffset - offset; - - window.scrollTo({ - top: Math.max(0, offsetTop), - behavior, - }); -}; +const scrollToElement = ( + element: HTMLElement, + offset: number, + behavior: ScrollBehavior, + block: ScrollLogicalPosition, + inline: ScrollLogicalPosition, +): void => { + element.scrollIntoView({ behavior, block, inline }); + if (offset) { + requestAnimationFrame(() => window.scrollBy({ top: -offset, behavior: 'auto' })); + } +};-const focusElement = (element: HTMLElement, shouldFocus: boolean, behavior: ScrollBehavior): void => { - if (shouldFocus && element.focus) { - setTimeout(() => element.focus(), behavior === 'smooth' ? 300 : 0); - } -}; +const focusElement = (element: HTMLElement, shouldFocus: boolean, behavior: ScrollBehavior): void => { + if (!shouldFocus) return; + const target = getFocusable(element) ?? element; + setTimeout(() => target.focus?.(), behavior === 'smooth' ? 300 : 0); +};-export const scrollToFirstError = (errors: FieldErrors, options: ScrollToErrorOptions = {}) => { - const { behavior = 'smooth', offset = 80, shouldFocus = true, retryAttempts = 3 } = options; +export const scrollToFirstError = (errors: FieldErrors, options: ScrollToErrorOptions = {}) => { + if (typeof window === 'undefined' || typeof document === 'undefined') return false; + const { + behavior = 'smooth', + block = 'center', + inline = 'nearest', + offset = 80, + shouldFocus = true, + retryAttempts = 3, + } = options;And add this helper near the other utils:
const getFocusable = (el: HTMLElement): HTMLElement | null => { if (el.matches('input,select,textarea,button,[tabindex]:not([tabindex="-1"])')) return el; return el.querySelector('input,select,textarea,button,[tabindex]:not([tabindex="-1"])') as HTMLElement | null; };Also applies to: 32-36, 38-40
🧹 Nitpick comments (7)
packages/components/src/utils/scrollToError.ts (2)
57-64: Fix return semantics: currently always “true” on retry.Spec says “warn/return false if none found.” Return false when scheduling a retry; true only on immediate success.
- if (attempt < retryAttempts) { - setTimeout(() => attemptScroll(attempt + 1), 100); - return true; - } + if (attempt < retryAttempts) { + setTimeout(() => attemptScroll(attempt + 1), 100); + return false; // not scrolled yet + }Optionally document that the boolean reflects immediate success, not eventual success.
Also applies to: 67-68
3-10: Unused options (block/inline) — wire through or drop.The interface exposes
blockandinlinebut implementation ignored them; resolve by applying the earlier diff or remove from the API.packages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts (1)
1-4: Optional: make the hook generically typed instead ofany.-import { useEffect, useMemo } from 'react'; +import { useEffect, useMemo } from 'react'; +import type { FieldValues } from 'react-hook-form'; import { useRemixFormContext } from 'remix-hook-form'; import type { UseRemixFormReturn } from 'remix-hook-form'; export interface UseScrollToErrorOnSubmitOptions extends ScrollToErrorOptions { delay?: number; enabled?: boolean; scrollOnServerErrors?: boolean; scrollOnMount?: boolean; - methods?: UseRemixFormReturn<any>; // Optional methods parameter + methods?: UseRemixFormReturn<FieldValues>; // Optional methods parameter } -export const useScrollToErrorOnSubmit = (options: UseScrollToErrorOnSubmitOptions = {}) => { +export const useScrollToErrorOnSubmit = (options: UseScrollToErrorOnSubmitOptions = {}) => {If you want full generics, we can add
<TFieldValues extends FieldValues = FieldValues>to both the interface and hook.Also applies to: 6-12, 14-19
apps/docs/src/remix-hook-form/scroll-to-error.stories.tsx (4)
11-16: Import Storybook test utilities (expect/userEvent) for interaction tests.-import type { Meta, StoryObj } from '@storybook/react-vite'; +import type { Meta, StoryObj } from '@storybook/react-vite'; +import { expect, userEvent, within } from '@storybook/test';
599-612: Add play() to exercise invalid submit (client‑side) for the hook flow.export const HookApproach: Story = { name: 'Hook Approach (useScrollToErrorOnSubmit)', decorators: [ @@ ], + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const submit = await canvas.findByRole('button', { name: /submit form/i }); + await userEvent.click(submit); + const firstErr = await canvas.findByText(/first name must be at least 2 characters/i); + expect(firstErr).toBeInTheDocument(); + expect((document.activeElement as HTMLElement | null)?.getAttribute('name')).toBe('firstName'); + }, };
614-627: Add play() to exercise invalid submit (client‑side) for the component flow.export const ComponentApproach: Story = { name: 'Component Approach (ScrollToErrorOnSubmit)', decorators: [ @@ ], + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const submit = await canvas.findByRole('button', { name: /submit form/i }); + await userEvent.click(submit); + const firstErr = await canvas.findByText(/first name must be at least 2 characters/i); + expect(firstErr).toBeInTheDocument(); + expect((document.activeElement as HTMLElement | null)?.getAttribute('name')).toBe('firstName'); + }, };
629-642: Missing server‑error and valid‑submission interaction tests.Per docs guidelines, add play() to:
- simulate server error (fill fields, use email taken@example.com, assert focus on email message),
- simulate valid submission (fill all fields, assert success message).
I can generate the two play() implementations tuned to your Select/Radio components if you want.
📜 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 ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
apps/docs/src/remix-hook-form/scroll-to-error.stories.tsx(1 hunks)packages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts(1 hunks)packages/components/src/utils/scrollToError.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
Handle server-side validation using getValidatedFormData with zodResolver and return errors as needed
Files:
packages/components/src/utils/scrollToError.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts
**/*.{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/utils/scrollToError.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.tsapps/docs/src/remix-hook-form/scroll-to-error.stories.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/utils/scrollToError.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.tsapps/docs/src/remix-hook-form/scroll-to-error.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/utils/scrollToError.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.tsapps/docs/src/remix-hook-form/scroll-to-error.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
packages/components/src/**/*.{ts,tsx}: Keep React components pure and fully typed (TypeScript + React, ES modules)
Export symbols as named exports only (avoid default exports)
Files:
packages/components/src/utils/scrollToError.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts
{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/utils/scrollToError.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.tsapps/docs/src/remix-hook-form/scroll-to-error.stories.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)
Filenames should be kebab-case (e.g., text-field.tsx, data-table-filter/**)
Use PascalCase for components and types; hooks use camelCase with a use* prefix
Files:
packages/components/src/utils/scrollToError.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.tsapps/docs/src/remix-hook-form/scroll-to-error.stories.tsx
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use cross-package relative imports; respect package boundaries
Files:
packages/components/src/utils/scrollToError.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts
packages/components/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Define explicit props interfaces and follow React 19 ref patterns
Files:
packages/components/src/utils/scrollToError.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts
{packages/components,apps/docs}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
{packages/components,apps/docs}/**/*.{ts,tsx}: Organize imports automatically (Biome) and maintain consistent import order
For form-aware components, use Zod schemas with proper messages, Remix fetcher.Form, and show FormMessage errors
Files:
packages/components/src/utils/scrollToError.tspackages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.tsapps/docs/src/remix-hook-form/scroll-to-error.stories.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
Use Storybook Test Runner (Playwright) for tests in apps/docs
Files:
apps/docs/src/remix-hook-form/scroll-to-error.stories.tsx
apps/docs/src/remix-hook-form/**/*.stories.tsx
📄 CodeRabbit inference engine (.cursor/rules/storybook-testing.mdc)
apps/docs/src/remix-hook-form/**/*.stories.tsx: Always place decorators on individual stories, not in meta configuration.
Never place decorators in meta configuration.
Use kebab-case for story file names in 'apps/docs/src/remix-hook-form/'.
Always include all three test phases in stories: default state, invalid submission, and valid submission.
Use play functions for comprehensive interaction testing in Storybook stories.
Always use fetcher.Form instead of regular form elements in stories that require React Router integration.
Always click before clearing inputs with userEvent in interaction tests.
Use findBy* queries for async elements instead of getBy* in Storybook interaction tests.
Never use CSS selectors when semantic queries (role, label, etc.) are available in canvas queries.
Each story should test one primary workflow and not multiple unrelated scenarios.
Group related test functions together and export individual test functions for reusability.
Use semantic queries (getByRole, getByLabelText, etc.) as the preferred method for selecting elements in interaction tests.
Meta configuration in stories should be kept clean and minimal, avoiding unnecessary parameters and decorators.
Always import required testing utilities: Meta, StoryObj, expect, userEvent, and withReactRouterStubDecorator in Storybook test stories.
Test both client-side and server-side validation in form stories.
Stories should serve as both documentation and automated tests.
Tests should complete in under 10 seconds for fast feedback.
Use step grouping in play functions for better debugging and organization of complex interactions.
Never use getBy* queries for async elements in Storybook interaction tests.
Never clear inputs without clicking first in userEvent interactions.
Never use regular forms instead of fetcher.Form in stories that require React Router integration.
Never test multiple unrelated scenarios in one story.
Never use CSS selectors when semantic queries are available.
Group rel...
Files:
apps/docs/src/remix-hook-form/scroll-to-error.stories.tsx
apps/docs/**
📄 CodeRabbit inference engine (.cursor/rules/storybook-testing.mdc)
apps/docs/**: Build Storybook static files before running Playwright tests locally.
Install Playwright browsers and system dependencies before running interaction tests.Serve built Storybook from apps/docs when running yarn serve
Files:
apps/docs/src/remix-hook-form/scroll-to-error.stories.tsx
apps/docs/**/*.stories.@(tsx|mdx)
📄 CodeRabbit inference engine (AGENTS.md)
apps/docs/**/*.stories.@(tsx|mdx): Add stories that exercise component states to support interaction tests
Use per-story decorators and router stub decorator for Storybook interaction tests
Files:
apps/docs/src/remix-hook-form/scroll-to-error.stories.tsx
🧬 Code graph analysis (3)
packages/components/src/utils/scrollToError.ts (1)
packages/components/src/index.ts (2)
ScrollToErrorOptions(5-5)scrollToFirstError(4-4)
packages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts (1)
packages/components/src/utils/scrollToError.ts (2)
ScrollToErrorOptions(3-10)scrollToFirstError(38-68)
apps/docs/src/remix-hook-form/scroll-to-error.stories.tsx (3)
packages/components/src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts (1)
useScrollToErrorOnSubmit(14-77)packages/components/src/ui/button.tsx (1)
Button(40-43)packages/components/src/remix-hook-form/components/ScrollToErrorOnSubmit.tsx (1)
ScrollToErrorOnSubmit(7-12)
⏰ 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
| const selectors = [ | ||
| '[data-slot="form-message"]', // Target error message first (best UX) | ||
| '[data-slot="form-control"][aria-invalid="true"]', // Input with error state | ||
| ]; |
There was a problem hiding this comment.
Complete selector priority per LC‑321.
Add fallbacks: [data-slot="form-control"] and [data-slot="form-item"].
- const selectors = [
- '[data-slot="form-message"]', // Target error message first (best UX)
- '[data-slot="form-control"][aria-invalid="true"]', // Input with error state
- ];
+ const selectors = [
+ '[data-slot="form-message"]', // error message
+ '[data-slot="form-control"][aria-invalid="true"]', // invalid input
+ '[data-slot="form-control"]', // any control (fallback)
+ '[data-slot="form-item"]', // item wrapper (last resort)
+ ];Also applies to: 50-55
🤖 Prompt for AI Agents
In packages/components/src/utils/scrollToError.ts around lines 45–48 (and also
apply same change to lines 50–55), the selector priority is incomplete; update
the selectors array to include fallbacks so it tries
'[data-slot="form-message"]', then
'[data-slot="form-control"][aria-invalid="true"]', then
'[data-slot="form-control"]', and finally '[data-slot="form-item"]'; similarly
update the other selectors block at 50–55 to include those same fallback entries
in the same priority order so the function will find error nodes reliably per
LC‑321.
|
@codegen-sh pull the latest changes from main, and then bump our components package to the next minor version with a summary message of the changes, don't publish just bump the version and commit and push up
|
- Fix: adjust spacing in RadioGroupItemField label for better a11y - Patch version bump for accessibility improvements Co-authored-by: lcmohsen <mohsen@lambdacurry.dev>
- Replace single useEffect with ResizeObserver for dynamic width tracking - Automatically handles responsive changes, content updates, and parent resizing - More reliable than previous approach that only measured width once on mount - Uses borderBoxSize for more accurate measurements with fallback to offsetWidth - Maintains portal benefits while ensuring dropdown width always matches trigger
- Create FixedWidth500px story to test dropdown width matching - Container is fixed at 500px width to verify ResizeObserver functionality - Includes automated tests to verify trigger and dropdown widths match - Logs actual widths for debugging width matching issues - Tests both width verification and functional selection
- Add check for ResizeObserver availability before using it - Fallback to simple width measurement when ResizeObserver is not available - Improve fallback chain for borderBoxSize/contentBoxSize measurements - Ensures tests pass in environments without ResizeObserver support
- Cast entry.target to HTMLElement to access offsetWidth property - Resolves TypeScript compilation error while maintaining ResizeObserver functionality - Ensures compatibility with test environments that may not support borderBoxSize
- Set initial width immediately before setting up ResizeObserver - Ensure original behavior is preserved for compatibility - Prevent timing conflicts that were causing popover rendering issues
- Replace PopoverContent with direct PopoverPrimitive.Content usage - Remove hardcoded w-72 (288px) width that was overriding inline styles - Maintain all popover animations and positioning behavior - Now the inline width style can properly control dropdown width - This should fix the width matching issue where dropdown was stuck at 288px
- Add console logging to track menuWidth changes and ResizeObserver updates - Add w-full class to inner div and ul to ensure content takes full width - This should help debug why dropdown isn't matching full container width - Inner content should now properly expand to match the applied width style
- Remove complex JavaScript width measurement logic - Use --radix-popover-trigger-width CSS variable instead - Much cleaner approach that leverages Radix UI's built-in functionality - Automatically handles width matching without JavaScript overhead - This should provide perfect width matching with the trigger
- Remove FixedWidth500px test story that was failing - Remove FixedWidthSelectExample component and fixedWidthRouterDecorator - Clean up test code that's no longer needed - Prepare for merge by removing test-only code
- Add activeIndex state tracking with default value 0 - Implement ArrowUp/ArrowDown key handlers with bounds checking - Reset activeIndex when filtered items change - Add aria-activedescendant on search input for accessibility - Generate unique IDs for each option element - Add visual highlighting for active items (bg-gray-50) - Implement scroll-into-view for active items - Update Enter key to select active item instead of enterCandidate - Add comprehensive keyboard navigation test story - Create changeset for patch version bump - Maintain all existing visuals, API, and functionality Co-authored-by: Jake Ruesink <jaruesink@gmail.com>
- Consolidated useEffect hooks to ensure activeIndex is properly set - Added delay in test to account for component initialization timing - The test was failing because activeIndex wasn't being set to 0 consistently when dropdown opens The issue appears to be a race condition between component rendering and state updates.
- Split useEffect for filtered items and dropdown open state - Ensure activeIndex is always reset to 0 when dropdown opens - Improve test stability with longer wait time and additional checks
- Add ArrowUp/ArrowDown navigation with activeIndex state management - Implement Enter key selection with dropdown closing - Add proper accessibility support with aria-activedescendant - Include visual feedback with data-active attribute - Add scroll behavior for active items - Reset active index when filtering changes - Fix DOM element caching issues in tests - Add timing delays for React re-renders All keyboard navigation functionality is working correctly: - Arrow keys navigate through options - Enter selects active item and closes dropdown - Dropdown reopens correctly with reset active index - Filtering integration works properly - Accessibility standards maintained
…ests - Remove .changeset/keyboard-navigation-select.md file - Update @lambdacurry/forms package to version 0.19.6 (patch) - Remove KeyboardNavigation story with failing arrow key tests - Keep keyboard navigation functionality for manual testing
- Add scroll-to-error functionality with scrollToFirstError utility - Add useScrollToErrorOnSubmit hook for Remix form integration - Add ScrollToErrorOnSubmit component for declarative usage - Leverage existing data-slot selectors for seamless integration - Support both client-side and server-side validation errors - Include retry logic for async rendering scenarios - Maintain backward compatibility with existing exports
|
✅ Version bump complete! I've successfully: 🔄 Pulled latest changes from main branch Summary of changes in v0.20.0:
The version bump is now ready in PR #140! 🎯 💻 View my work • 🛑 Stop • React 👍 or 👎 |
… and memoization - Updated methods parameter in UseScrollToErrorOnSubmitOptions to use FieldValues for better type safety. - Improved memoization of scroll options to prevent unnecessary re-renders. - Adjusted condition for handling server-side validation errors on mount to ensure proper functionality.
…oll-to-error-to' of github.com:lambda-curry/forms into codegen/lc-321-complete-implementation-guide-adding-scroll-to-error-to
…-adding-scroll-to-error-to
jaruesink
temporarily deployed
to
pr-preview
GitHub Actions
Inactive
🎯 Overview
This PR implements comprehensive scroll-to-error functionality for the @lambdacurry/forms library as specified in LC-321. The implementation provides both hook-based and component-based APIs for automatically scrolling to form validation errors.
✨ Key Features
Core Utilities
scrollToFirstErrorutility function with configurable optionsdata-slotselectors (no new DOM attributes needed)React Integration
useScrollToErrorOnSubmithook for programmatic usageScrollToErrorOnSubmitcomponent for declarative usageremix-hook-formcontextSmart Error Detection
🔧 Implementation Details
Phase 1: Core Utilities
src/utils/scrollToError.ts- Core scroll functionalitydata-slot="form-message",data-slot="form-control", etc.Phase 2: React Hooks & Components
src/remix-hook-form/hooks/useScrollToErrorOnSubmit.ts- React hooksrc/remix-hook-form/components/ScrollToErrorOnSubmit.tsx- React componentuseMemofor performancePhase 3: Export Configuration
src/index.tsto export scroll utilitiessrc/remix-hook-form/index.tsto export new functionality📚 Usage Examples
Hook Usage (Recommended)
Component Usage
Advanced Configuration
🎨 Design Decisions
Selector Strategy
Uses existing component selectors in priority order:
[data-slot="form-message"]- Targets error messages (best UX)[data-slot="form-control"][aria-invalid="true"]- Targets inputs with errors[data-slot="form-control"]- Any form control as fallback[data-slot="form-item"]- Form container as final fallbackPerformance Optimizations
Accessibility
🧪 Quality Assurance
🚀 Benefits
data-slotselectorsThis implementation integrates seamlessly with the existing @lambdacurry/forms architecture while providing robust scroll-to-error functionality for both client-side and server-side validation scenarios.
Requested by: Jake Ruesink
Issue: LC-321
💻 View my work • 👤 Initiated by
Jake Ruesink• About Codegen⛔ Remove Codegen from PR • 🚫 Ban action checks
Summary by CodeRabbit