Fix phone input handling of 11-digit autofilled numbers with leading 1 #134
Conversation
|
|
WalkthroughAdds US phone input logic to accept 11-digit numbers starting with “1” by normalizing to 10 digits for value while allowing entry/display; updates keyboard gating accordingly. Introduces a new test for the autofill case. Also reformats root workspaces in package.json and bumps components package version. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant PI as PhoneInput (US)
participant F as Formatter
participant S as Internal State
participant H as onChange Handler
U->>PI: Type digits (e.g., 12025550123)
PI->>PI: Extract digits
alt 11 digits AND startsWith('1')
PI->>PI: normalized = digits[1..10]
note right of PI: New: allow entry of 11th digit<br/>but drop leading '1' for value
else
PI->>PI: normalized = first 10 digits
end
PI->>F: formatUS(digits)
F-->>PI: "(202) 555-0123"
PI->>S: set display value
PI->>H: onChange(normalized)
opt Further typing
PI->>PI: Keyboard gate checks:<br/>- allow 11th digit only if leading '1'<br/>- otherwise stop at 10
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
📝 Storybook Preview: View Storybook This preview will be updated automatically when you push new changes to this PR.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/components/src/ui/phone-input.tsx (2)
26-33: Generalize leading “1” handling and DRY the normalization.Current logic only drops the leading “1” for exactly 11 digits. For robustness, handle any length ≥ 11 that starts with “1” (e.g., pasted “+1…” or extra trailing digits) by taking the 10 digits after the leading 1. Also, centralize this in a helper to avoid divergence between format/display and onChange normalization.
Apply this diff in the shown ranges:
@@ -function formatUS(digits: string): string { - // Handle case where an 11-digit number with leading "1" is provided (common in autofill) - let d = digits; - if (digits.length === 11 && digits.startsWith('1')) { - d = digits.slice(1); // Remove the leading "1" country code - } else { - d = digits.slice(0, 10); // Otherwise just take first 10 digits as before - } +function formatUS(digits: string): string { + // Normalize: if leading +1 pattern, drop it and take the next 10; otherwise cap to 10 + const d = + digits.startsWith('1') && digits.length >= 11 + ? digits.slice(1, 11) + : digits.slice(0, 10); @@ - // Remove the slice(0, 10) to allow handling 11-digit numbers with leading 1 const digits = extractDigits(String(value)); setDisplay(formatUS(digits));@@ - // Remove the slice(0, 10) to allow handling 11-digit numbers with leading 1 - const digits = extractDigits(raw); - // Handle case where an 11-digit number with leading "1" is provided - let normalizedDigits = digits; - if (digits.length === 11 && digits.startsWith('1')) { - normalizedDigits = digits.slice(1); // Remove the leading "1" country code - } else { - normalizedDigits = digits.slice(0, 10); // Otherwise just take first 10 digits - } + const digits = extractDigits(raw); + // Normalize consistently with formatter: + const normalizedDigits = + digits.startsWith('1') && digits.length >= 11 + ? digits.slice(1, 11) + : digits.slice(0, 10); const formatted = formatUS(digits); setDisplay(formatted); onChange?.(normalizedDigits || undefined);Optionally, to fully remove duplication, add this helper outside the shown ranges and use it in both places:
function normalizeUSDigits(d: string): string { return d.startsWith('1') && d.length >= 11 ? d.slice(1, 11) : d.slice(0, 10); }Then replace the inlined normalization with normalizeUSDigits(digits), and use const d = normalizeUSDigits(digits) in formatUS().
Also applies to: 91-104
112-116: Keydown gating doesn’t actually detect the “11 with leading 1” case.You derive currentDigits from display, which never reaches length 11 in US mode, so the exception is unreachable. Consider basing the decision on the prospective next digits to permit the 11th character when it creates a valid “1 + 10” shape.
Apply this diff:
- - // 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')); - - if (!isModifier && isNumberKey && isComplete) { + // Determine prospective digits after this key press to gate input correctly + const nextDigits = isNumberKey ? extractDigits(display + e.key) : currentDigits; + const allowNext = + nextDigits.length <= 10 || (nextDigits.length === 11 && nextDigits.startsWith('1')); + + if (!isModifier && isNumberKey && !allowNext) { // Prevent adding more digits once 10-digit US number is complete e.preventDefault(); return; }Note: This still ignores selection/replace scenarios; if needed, we can incorporate selectionStart/selectionEnd to compute nextDigits precisely.
apps/docs/src/remix-hook-form/phone-input.test.tsx (1)
119-132: Consider asserting the normalized value submitted (10 digits without the leading 1).Right now we validate display only. A follow-up test can ensure form submission stores "2025550123" (no leading 1), guarding against regressions in onChange normalization.
Example test you can add:
it('submits normalized 10-digit value when 11-digit with leading 1 is entered', async () => { const user = userEvent.setup(); render(<TestPhoneInputForm />); const usaPhoneInput = screen.getByLabelText('USA Phone Number') as HTMLInputElement; await user.type(usaPhoneInput, '12025550123'); await user.click(screen.getByRole('button', { name: /submit/i })); await waitFor(() => { // mockFetcher.submit should have been called with FormData expect(useFetcher().submit).toHaveBeenCalled(); }); const submitCall = (useFetcher().submit as jest.Mock).mock.calls[0]; const submitted = submitCall[0] as FormData; // Depending on your useRemixForm integration, you may need to adapt this extraction expect(submitted.get('usaPhone')).toBe('2025550123'); });If the submit payload shape differs, I can adjust the assertion to match your form lib’s submission contract.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/docs/src/remix-hook-form/phone-input.test.tsx(1 hunks)package.json(1 hunks)packages/components/package.json(1 hunks)packages/components/src/ui/phone-input.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
{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
**/*.{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/phone-input.test.tsxpackages/components/src/ui/phone-input.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/phone-input.test.tsxpackages/components/src/ui/phone-input.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/phone-input.test.tsxpackages/components/src/ui/phone-input.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/phone-input.test.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/phone-input.test.tsxpackages/components/src/ui/phone-input.tsx
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo-organization.mdc)
Avoid circular dependencies
Files:
apps/docs/src/remix-hook-form/phone-input.test.tsxpackages/components/src/ui/phone-input.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.
Files:
apps/docs/src/remix-hook-form/phone-input.test.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/phone-input.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.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/phone-input.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/phone-input.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/phone-input.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (3)
package.json (1)
5-8: LGTM: Formatting-only workspace array change.No functional impact. Workspaces remain apps/* and packages/*.
packages/components/src/ui/phone-input.tsx (1)
72-75: LGTM: Sync path now delegates length handling to formatter.Removing the premature slice centralizes normalization in formatUS, reducing drift.
apps/docs/src/remix-hook-form/phone-input.test.tsx (1)
119-132: Great coverage for the autofill case.This verifies the UX outcome for '1' + 10 digits and matches the new normalization/formatting behavior.
| { | ||
| "name": "@lambdacurry/forms", | ||
| "version": "0.19.4", | ||
| "version": "0.19.5", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Version bump looks right; confirm presence of a changeset and cross-package alignment.
Patch bump to 0.19.5 matches the scope of the fix. Please verify a corresponding changeset exists and that no other packages pin a mismatched version to this package.
Run this script to check for a changeset mentioning @lambdacurry/forms and to list internal dependents that reference it:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check for Changesets mentioning @lambdacurry/forms =="
fd -H -a -t f .changeset | xargs -r rg -n "@lambdacurry/forms" -n -C1 || echo "No changeset found mentioning @lambdacurry/forms"
echo
echo "== List packages that depend on @lambdacurry/forms (deps/peerDeps/devDeps) =="
fd package.json packages apps | while read -r f; do
jq -r --arg name "@lambdacurry/forms" '
.name as $pkg
| {deps: .dependencies?[$name], peer: .peerDependencies?[$name], dev: .devDependencies?[$name]}
| select(.deps or .peer or .dev)
| [$pkg, (if .deps then "deps" else (if .peer then "peerDeps" else "devDeps" end) end), ( .deps // .peer // .dev )]
| @tsv
' "$f" 2>/dev/null | awk -v file="$f" -F'\t' '{ print file ": " $0 }'
done || trueLength of output: 1372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Package name and version from packages/components/package.json:"
jq -r '.name, .version' packages/components/package.jsonLength of output: 241
Add missing changeset and align internal dependency version
It looks like there’s no changeset recording the bump to 0.19.5 for @lambdacurry/forms, and one internal consumer is still using a loose “*” range.
Please address the following before merging:
- Add a new changeset under
.changeset/that bumps@lambdacurry/formsto0.19.5with a brief description of the fix. - In
apps/docs/package.json, update the dependency on@lambdacurry/formsfrom"*"to match the new version range (e.g."^0.19.5").
🤖 Prompt for AI Agents
In packages/components/package.json around line 3, the package was bumped to
0.19.5 but there is no changeset and an internal consumer uses a loose "*"
range; add a new changeset file under .changeset/ (e.g.,
yyyy-mm-dd-bump-forms.md) that lists @lambdacurry/forms bumped to 0.19.5 with a
one-line description of the fix, and then update apps/docs/package.json to
change the @lambdacurry/forms dependency from "*" to the matching
semver-compatible range (for example "^0.19.5") so internal consumers align with
the new version.
Description
This PR fixes an issue with the phone number input component where autofilled 11-digit US phone numbers (with a leading "1") were being incorrectly formatted. Previously, the component would keep the leading "1" and truncate the last digit of the actual phone number. Now, it correctly identifies and removes the leading "1" country code while preserving all 10 digits of the actual phone number.
Changes
formatUSfunction to detect 11-digit numbers with leading "1" and remove the country codeTesting
Requested by Jake Ruesink
Summary by CodeRabbit
Bug Fixes
Tests
Chores