feat(ui): Add PhoneInput component with international formatting support #408
Conversation
feat(ui): Add PhoneInput component with international formatting support This commit adds a new PhoneInput component that provides: - International phone number formatting using libphonenumber-js - Real-time validation for phone numbers - Support for EN (GB), DE, and PL country codes - Auto-formatting as users type - Proper handling of pasted phone numbers - Default country code support (PL as default) - Three variants: PhoneInput, PhoneInputWithLabel, and PhoneInputWithDetails - Consistent API with existing Input component Addresses o2sdev#306
|
@mithilP007 is attempting to deploy a commit to the Hycom Team Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughA new phone input feature is introduced with three tightly integrated components (PhoneInput, PhoneInputWithLabel, PhoneInputWithDetails) built on libphonenumber-js. All components handle real-time formatting, validation, input sanitization, and paste handling for international phone numbers with supported countries (GB, DE, PL). Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PhoneInput
participant Formatter as AsYouType
participant Validator as getNumber()
User->>PhoneInput: Type/Paste Characters
PhoneInput->>PhoneInput: Sanitize Input<br/>(allow +, digits, spaces, etc.)
PhoneInput->>Formatter: Format via AsYouType<br/>(with defaultCountry)
Formatter-->>PhoneInput: Formatted String
PhoneInput->>PhoneInput: Update displayValue
PhoneInput->>Validator: Validate Formatted Number
Validator-->>PhoneInput: isValid Boolean
PhoneInput->>PhoneInput: Invoke onChange<br/>(formatted value, isValid)
PhoneInput->>PhoneInput: Invoke onValidationChange<br/>(isValid)
PhoneInput-->>User: Display Formatted Number<br/>+ Validation State
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
packages/ui/src/elements/phone-input.tsx (2)
81-81: Consider dynamic placeholder based on defaultCountry.The placeholder is hardcoded to
"+48 500 500 500"(Polish format) but will display even whendefaultCountryis set to'GB'or'DE'. This could mislead users.You can make the placeholder dynamic:
const placeholders: Record<CountryCode, string> = { PL: '+48 500 500 500', DE: '+49 30 12345678', GB: '+44 20 7123 4567', }; // Then in the component: placeholder={placeholders[defaultCountry] || placeholders.PL}Apply the same change to lines 153 and 225.
44-48: Consider logging errors in development.The error handling silently returns the unformatted input without logging what went wrong. While this is acceptable for production, adding a development-time log could aid debugging.
} catch (error) { + if (process.env.NODE_ENV === 'development') { + console.warn('Phone number formatting error:', error); + } setIsValid(false); onValidationChange?.(false); return input; }Apply the same change to lines 119-123 and 191-195.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/ui/src/elements/phone-input.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/ui/src/elements/phone-input.tsx (1)
packages/ui/src/elements/input.tsx (4)
InputProps(11-22)InputWithDetailsProps(144-149)InputWithLabel(162-162)InputWithDetails(162-162)
🔇 Additional comments (2)
packages/ui/src/elements/phone-input.tsx (2)
9-14: LGTM!The type definition correctly extends
InputPropsand provides custom signatures for phone-specific behavior.
235-235: LGTM!The exports are clean and provide the three component variants as intended.
| @@ -0,0 +1,235 @@ | |||
| import * as React from 'react'; | |||
| import { parsePhoneNumber, AsYouType, CountryCode } from 'libphonenumber-js'; | |||
There was a problem hiding this comment.
Remove unused import.
The parsePhoneNumber import is never used in this file.
Apply this diff:
-import { parsePhoneNumber, AsYouType, CountryCode } from 'libphonenumber-js';
+import { AsYouType, CountryCode } from 'libphonenumber-js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { parsePhoneNumber, AsYouType, CountryCode } from 'libphonenumber-js'; | |
| import { AsYouType, CountryCode } from 'libphonenumber-js'; |
🤖 Prompt for AI Agents
In packages/ui/src/elements/phone-input.tsx around line 2, the import list
includes parsePhoneNumber which is unused; remove parsePhoneNumber from the
import statement so it only imports AsYouType and CountryCode from
'libphonenumber-js' to eliminate the dead import and any linter warnings.
🧩 Analysis chain
Verify bundle size impact and metadata approach.
The PR objectives and issue #306 specifically requested minimizing bundle size by using MetadataJson with metadata for only the supported countries (GB, DE, PL) instead of full metadata. However, the current implementation imports from libphonenumber-js, which includes metadata for all 200+ countries.
Please verify whether:
- Tree-shaking effectively removes unused country metadata with your bundler configuration, or
- You should use
libphonenumber-js/minorlibphonenumber-js/corewith custom metadata as described in the libphonenumber-js documentation.
Run the following script to check the current bundle size impact:
🏁 Script executed:
#!/bin/bash
# Check if custom metadata is configured and measure bundle impact
echo "Checking for libphonenumber-js usage in packages/ui..."
rg -n "libphonenumber-js" packages/ui/
echo -e "\nChecking package.json for libphonenumber-js..."
cat packages/ui/package.json | jq '.dependencies."libphonenumber-js"'
echo -e "\nSearching for any metadata configuration..."
fd -e ts -e tsx -e js -e jsx . packages/ui/ --exec rg -l "MetadataJson|metadata\.min\.json|libphonenumber-js/(min|core)"Length of output: 273
🏁 Script executed:
#!/bin/bash
# Check root package.json and monorepo structure
echo "Checking root package.json for libphonenumber-js..."
cat package.json | jq '.dependencies."libphonenumber-js" // .devDependencies."libphonenumber-js" // empty'
echo -e "\nChecking for bundler configuration..."
ls -la | grep -E "vite|webpack|esbuild|tsconfig"
echo -e "\nSearching entire repo for custom metadata approach..."
rg -n "metadata" --type=ts --type=tsx --type=json -C2 | head -50
echo -e "\nListing all references to phone-input..."
fd -n "phone-input" --type=fLength of output: 513
🏁 Script executed:
#!/bin/bash
# Search for the specific issue mentioned in the review comment (#306)
# to understand the actual requirements
echo "Searching commit messages or PR context for issue #306..."
git log --all --oneline --grep="306\|MetadataJson\|supported countries" 2>/dev/null | head -20
echo -e "\nSearching for any custom metadata JSON files..."
fd -e json . | grep -i metadataLength of output: 180
🏁 Script executed:
#!/bin/bash
# Check for monorepo configuration and dependency resolution
echo "Checking root package.json for workspace configuration..."
cat package.json | jq '.workspaces // empty'
echo -e "\nChecking packages/ui/package.json completely..."
cat packages/ui/package.json
echo -e "\nSearching for any libphonenumber-js installation or reference..."
rg "libphonenumber" -i
echo -e "\nChecking if there's a lock file that might have the dependency..."
ls -la | grep -E "package-lock|yarn.lock|pnpm-lock"Length of output: 3567
🏁 Script executed:
#!/bin/bash
# Check root package.json for libphonenumber-js and verify workspace dependency resolution
echo "Checking root package.json dependencies..."
cat package.json | jq '.dependencies, .devDependencies' | grep -A 50 "libphonenumber"
echo -e "\nChecking complete root package.json for libphonenumber-js..."
rg "libphonenumber" package.json
echo -e "\nVerifying if libphonenumber-js is in node_modules..."
ls -la node_modules/ 2>/dev/null | grep -i libphone || echo "node_modules check skipped (not in sandbox)"
echo -e "\nChecking phone-input.tsx file contents..."
cat packages/ui/src/elements/phone-input.tsxLength of output: 7794
Add libphonenumber-js dependency to packages/ui/package.json and implement custom metadata to minimize bundle size.
Two critical issues were found:
-
Missing dependency declaration:
libphonenumber-jsis imported but not declared inpackages/ui/package.json. In this monorepo workspace structure, the package must explicitly declare its dependencies. -
Bundle size not optimized: The code includes a comment about "Minimal metadata for supported countries" and defines
SUPPORTED_COUNTRIESfor only 3 countries (GB, DE, PL), but the implementation uses the fulllibphonenumber-jslibrary which bundles metadata for 200+ countries. To minimize bundle size as intended, implement custom metadata by usinglibphonenumber-js/minwith a custom metadata file containing only the required countries' data, as documented in the libphonenumber-js documentation.
Required changes:
- Add
"libphonenumber-js": "<version>"topackages/ui/package.jsondependencies - Replace the current import with
libphonenumber-js/minand provide custom metadata
There was a problem hiding this comment.
@mithilP007 yes, please optimize loaded metadata so that not all countries are imported
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| import * as React from 'react'; | ||
| import { parsePhoneNumber, AsYouType, CountryCode } from 'libphonenumber-js'; | ||
| import { Input, InputWithLabel, InputWithDetails, InputProps, InputWithDetailsProps } from './input'; | ||
| import { cn } from '@o2s/ui/lib/utils'; |
There was a problem hiding this comment.
Remove unused import.
The cn utility import is never used in this file.
Apply this diff:
-import { cn } from '@o2s/ui/lib/utils';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { cn } from '@o2s/ui/lib/utils'; |
🤖 Prompt for AI Agents
In packages/ui/src/elements/phone-input.tsx around line 4, the import "cn" from
'@o2s/ui/lib/utils' is unused; remove that import statement to clean up the file
and avoid linting errors, ensuring no other parts of the file reference "cn"
before committing.
| // Minimal metadata for supported countries (EN, DE, PL) | ||
| const SUPPORTED_COUNTRIES: CountryCode[] = ['GB', 'DE', 'PL']; |
There was a problem hiding this comment.
Remove or utilize the unused constant.
The SUPPORTED_COUNTRIES constant is defined but never referenced. Either remove it or use it to validate the defaultCountry prop.
🤖 Prompt for AI Agents
In packages/ui/src/elements/phone-input.tsx around lines 6 to 7, the
SUPPORTED_COUNTRIES constant is declared but unused; either remove it or apply
it to validate the defaultCountry prop. Fix by using SUPPORTED_COUNTRIES to
check incoming defaultCountry (e.g., if
(!SUPPORTED_COUNTRIES.includes(defaultCountry)) log/warn or throw and fall back
to a safe default such as 'GB' or the first entry), update the component prop
handling to perform this validation before rendering, and remove the constant
entirely if you prefer not to enforce validation. Ensure the validation
preserves types (CountryCode) and add a unit or runtime test to cover the
fallback behavior.
| const PhoneInput = React.forwardRef<HTMLInputElement, PhoneInputOwnProps>( | ||
| ({ defaultCountry = 'PL', value = '', onChange, onValidationChange, ...props }, ref) => { | ||
| const [displayValue, setDisplayValue] = React.useState(value); | ||
| const [isValid, setIsValid] = React.useState(false); | ||
|
|
||
| React.useEffect(() => { | ||
| setDisplayValue(value); | ||
| }, [value]); | ||
|
|
||
| const formatPhoneNumber = React.useCallback( | ||
| (input: string) => { | ||
| if (!input) return ''; | ||
|
|
||
| try { | ||
| // Try to parse the phone number | ||
| const asYouType = new AsYouType(defaultCountry); | ||
| const formatted = asYouType.input(input); | ||
|
|
||
| // Check if it's valid | ||
| const phoneNumber = asYouType.getNumber(); | ||
| const valid = phoneNumber ? phoneNumber.isValid() : false; | ||
|
|
||
| setIsValid(valid); | ||
| onValidationChange?.(valid); | ||
|
|
||
| return formatted; | ||
| } catch (error) { | ||
| setIsValid(false); | ||
| onValidationChange?.(false); | ||
| return input; | ||
| } | ||
| }, | ||
| [defaultCountry, onValidationChange] | ||
| ); | ||
|
|
||
| const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const inputValue = e.target.value; | ||
|
|
||
| // Only allow valid characters (numbers, +, spaces, parentheses, hyphens) | ||
| const sanitized = inputValue.replace(/[^0-9+\s()-]/g, ''); | ||
|
|
||
| const formatted = formatPhoneNumber(sanitized); | ||
| setDisplayValue(formatted); | ||
| onChange?.(formatted, isValid); | ||
| }; | ||
|
|
||
| const handlePaste = (e: React.ClipboardEvent<HTMLInputElement>) => { | ||
| e.preventDefault(); | ||
| const pastedText = e.clipboardData.getData('text'); | ||
| const sanitized = pastedText.replace(/[^0-9+\s()-]/g, ''); | ||
| const formatted = formatPhoneNumber(sanitized); | ||
| setDisplayValue(formatted); | ||
| onChange?.(formatted, isValid); | ||
| }; | ||
|
|
||
| return ( | ||
| <Input | ||
| {...props} | ||
| ref={ref} | ||
| type="tel" | ||
| value={displayValue} | ||
| onChange={handleChange} | ||
| onPaste={handlePaste} | ||
| placeholder="+48 500 500 500" | ||
| /> | ||
| ); | ||
| } | ||
| ); | ||
|
|
||
| PhoneInput.displayName = 'PhoneInput'; | ||
|
|
||
| export type PhoneInputWithLabelProps = Omit<InputWithDetailsProps, 'onChange' | 'value'> & { | ||
| defaultCountry?: CountryCode; | ||
| value?: string; | ||
| onChange?: (value: string, isValid: boolean) => void; | ||
| onValidationChange?: (isValid: boolean) => void; | ||
| }; | ||
|
|
||
| const PhoneInputWithLabel = React.forwardRef<HTMLInputElement, PhoneInputWithLabelProps>( | ||
| ({ defaultCountry = 'PL', value = '', onChange, onValidationChange, ...props }, ref) => { | ||
| const [displayValue, setDisplayValue] = React.useState(value); | ||
| const [isValid, setIsValid] = React.useState(false); | ||
|
|
||
| React.useEffect(() => { | ||
| setDisplayValue(value); | ||
| }, [value]); | ||
|
|
||
| const formatPhoneNumber = React.useCallback( | ||
| (input: string) => { | ||
| if (!input) return ''; | ||
|
|
||
| try { | ||
| const asYouType = new AsYouType(defaultCountry); | ||
| const formatted = asYouType.input(input); | ||
| const phoneNumber = asYouType.getNumber(); | ||
| const valid = phoneNumber ? phoneNumber.isValid() : false; | ||
|
|
||
| setIsValid(valid); | ||
| onValidationChange?.(valid); | ||
|
|
||
| return formatted; | ||
| } catch (error) { | ||
| setIsValid(false); | ||
| onValidationChange?.(false); | ||
| return input; | ||
| } | ||
| }, | ||
| [defaultCountry, onValidationChange] | ||
| ); | ||
|
|
||
| const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const inputValue = e.target.value; | ||
| const sanitized = inputValue.replace(/[^0-9+\s()-]/g, ''); | ||
| const formatted = formatPhoneNumber(sanitized); | ||
| setDisplayValue(formatted); | ||
| onChange?.(formatted, isValid); | ||
| }; | ||
|
|
||
| const handlePaste = (e: React.ClipboardEvent<HTMLInputElement>) => { | ||
| e.preventDefault(); | ||
| const pastedText = e.clipboardData.getData('text'); | ||
| const sanitized = pastedText.replace(/[^0-9+\s()-]/g, ''); | ||
| const formatted = formatPhoneNumber(sanitized); | ||
| setDisplayValue(formatted); | ||
| onChange?.(formatted, isValid); | ||
| }; | ||
|
|
||
| return ( | ||
| <InputWithLabel | ||
| {...props} | ||
| ref={ref} | ||
| type="tel" | ||
| value={displayValue} | ||
| onChange={handleChange} | ||
| onPaste={handlePaste} | ||
| placeholder="+48 500 500 500" | ||
| /> | ||
| ); | ||
| } | ||
| ); | ||
|
|
||
| PhoneInputWithLabel.displayName = 'PhoneInputWithLabel'; | ||
|
|
||
| export type PhoneInputWithDetailsProps = Omit<InputWithDetailsProps, 'onChange' | 'value'> & { | ||
| defaultCountry?: CountryCode; | ||
| value?: string; | ||
| onChange?: (value: string, isValid: boolean) => void; | ||
| onValidationChange?: (isValid: boolean) => void; | ||
| }; | ||
|
|
||
| const PhoneInputWithDetails = React.forwardRef<HTMLInputElement, PhoneInputWithDetailsProps>( | ||
| ({ defaultCountry = 'PL', value = '', onChange, onValidationChange, caption, errorMessage, ...props }, ref) => { | ||
| const [displayValue, setDisplayValue] = React.useState(value); | ||
| const [isValid, setIsValid] = React.useState(false); | ||
|
|
||
| React.useEffect(() => { | ||
| setDisplayValue(value); | ||
| }, [value]); | ||
|
|
||
| const formatPhoneNumber = React.useCallback( | ||
| (input: string) => { | ||
| if (!input) return ''; | ||
|
|
||
| try { | ||
| const asYouType = new AsYouType(defaultCountry); | ||
| const formatted = asYouType.input(input); | ||
| const phoneNumber = asYouType.getNumber(); | ||
| const valid = phoneNumber ? phoneNumber.isValid() : false; | ||
|
|
||
| setIsValid(valid); | ||
| onValidationChange?.(valid); | ||
|
|
||
| return formatted; | ||
| } catch (error) { | ||
| setIsValid(false); | ||
| onValidationChange?.(false); | ||
| return input; | ||
| } | ||
| }, | ||
| [defaultCountry, onValidationChange] | ||
| ); | ||
|
|
||
| const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const inputValue = e.target.value; | ||
| const sanitized = inputValue.replace(/[^0-9+\s()-]/g, ''); | ||
| const formatted = formatPhoneNumber(sanitized); | ||
| setDisplayValue(formatted); | ||
| onChange?.(formatted, isValid); | ||
| }; | ||
|
|
||
| const handlePaste = (e: React.ClipboardEvent<HTMLInputElement>) => { | ||
| e.preventDefault(); | ||
| const pastedText = e.clipboardData.getData('text'); | ||
| const sanitized = pastedText.replace(/[^0-9+\s()-]/g, ''); | ||
| const formatted = formatPhoneNumber(sanitized); | ||
| setDisplayValue(formatted); | ||
| onChange?.(formatted, isValid); | ||
| }; | ||
|
|
||
| return ( | ||
| <InputWithDetails | ||
| {...props} | ||
| ref={ref} | ||
| type="tel" | ||
| value={displayValue} | ||
| onChange={handleChange} | ||
| onPaste={handlePaste} | ||
| placeholder="+48 500 500 500" | ||
| caption={caption} | ||
| errorMessage={errorMessage} | ||
| /> | ||
| ); | ||
| } | ||
| ); | ||
|
|
||
| PhoneInputWithDetails.displayName = 'PhoneInputWithDetails'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract shared logic to eliminate code duplication.
All three components (PhoneInput, PhoneInputWithLabel, PhoneInputWithDetails) contain nearly identical logic (~150 lines duplicated):
- State management (
displayValue,isValid) useEffectfor prop synchronizationformatPhoneNumbercallbackhandleChangehandlerhandlePastehandler
This violates DRY principles and triples the maintenance burden.
Recommended approach: Extract the shared logic into a custom hook:
function usePhoneInput(
value: string,
defaultCountry: CountryCode,
onChange?: (value: string, isValid: boolean) => void,
onValidationChange?: (isValid: boolean) => void
) {
const [displayValue, setDisplayValue] = React.useState(value);
const [isValid, setIsValid] = React.useState(false);
React.useEffect(() => {
setDisplayValue(value);
}, [value]);
const formatPhoneNumber = React.useCallback(
(input: string) => {
if (!input) return { formatted: '', valid: false };
try {
const asYouType = new AsYouType(defaultCountry);
const formatted = asYouType.input(input);
const phoneNumber = asYouType.getNumber();
const valid = phoneNumber ? phoneNumber.isValid() : false;
setIsValid(valid);
onValidationChange?.(valid);
return { formatted, valid };
} catch (error) {
setIsValid(false);
onValidationChange?.(false);
return { formatted: input, valid: false };
}
},
[defaultCountry, onValidationChange]
);
const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
const inputValue = e.target.value;
const sanitized = inputValue.replace(/[^0-9+\s()-]/g, '');
const { formatted, valid } = formatPhoneNumber(sanitized);
setDisplayValue(formatted);
onChange?.(formatted, valid);
};
const handlePaste = (e: React.ClipboardEvent<HTMLInputElement>) => {
e.preventDefault();
const pastedText = e.clipboardData.getData('text');
const sanitized = pastedText.replace(/[^0-9+\s()-]/g, '');
const { formatted, valid } = formatPhoneNumber(sanitized);
setDisplayValue(formatted);
onChange?.(formatted, valid);
};
return {
displayValue,
isValid,
handleChange,
handlePaste,
};
}Then simplify each component:
const PhoneInput = React.forwardRef<HTMLInputElement, PhoneInputOwnProps>(
({ defaultCountry = 'PL', value = '', onChange, onValidationChange, ...props }, ref) => {
const { displayValue, handleChange, handlePaste } = usePhoneInput(
value,
defaultCountry,
onChange,
onValidationChange
);
return (
<Input
{...props}
ref={ref}
type="tel"
value={displayValue}
onChange={handleChange}
onPaste={handlePaste}
placeholder="+48 500 500 500"
/>
);
}
);🤖 Prompt for AI Agents
In packages/ui/src/elements/phone-input.tsx around lines 18 to 233, the three
components duplicate the same state, effect, formatting, change and paste
handlers; extract that shared logic into a custom hook (e.g. usePhoneInput) that
accepts (value, defaultCountry, onChange, onValidationChange), manages
displayValue and isValid, synchronizes value via useEffect, exposes a
formatPhoneNumber that returns {formatted, valid}, and provides handleChange and
handlePaste which sanitize input, call formatPhoneNumber, update displayValue
and call onChange with the current valid flag; then refactor PhoneInput,
PhoneInputWithLabel and PhoneInputWithDetails to call the hook and use the
returned displayValue, handleChange and handlePaste (and use the returned valid
when invoking onChange) to remove the duplicated code.
| const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const inputValue = e.target.value; | ||
|
|
||
| // Only allow valid characters (numbers, +, spaces, parentheses, hyphens) | ||
| const sanitized = inputValue.replace(/[^0-9+\s()-]/g, ''); | ||
|
|
||
| const formatted = formatPhoneNumber(sanitized); | ||
| setDisplayValue(formatted); | ||
| onChange?.(formatted, isValid); | ||
| }; |
There was a problem hiding this comment.
Critical: Race condition in validation status.
The isValid variable passed to onChange at line 61 is stale—it holds the value from the previous render, not the newly computed validation status from formatPhoneNumber. Because setIsValid is asynchronous, the updated value won't be available until the next render, causing onChange to receive outdated validation information.
Example:
- User types "5" →
isValidisfalse,onChange("5", false)✓ - User types "500500500" →
formatPhoneNumbersetsisValidtotrue, butonChange("500500500", false)is called with the oldfalse✗
This affects all three components in this file.
Solution: Return the validation status from formatPhoneNumber instead of relying on state:
const formatPhoneNumber = React.useCallback(
(input: string) => {
if (!input) return '';
try {
const asYouType = new AsYouType(defaultCountry);
const formatted = asYouType.input(input);
const phoneNumber = asYouType.getNumber();
const valid = phoneNumber ? phoneNumber.isValid() : false;
setIsValid(valid);
onValidationChange?.(valid);
- return formatted;
+ return { formatted, valid };
} catch (error) {
setIsValid(false);
onValidationChange?.(false);
- return input;
+ return { formatted: input, valid: false };
}
},
[defaultCountry, onValidationChange]
);
const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
const inputValue = e.target.value;
const sanitized = inputValue.replace(/[^0-9+\s()-]/g, '');
- const formatted = formatPhoneNumber(sanitized);
+ const { formatted, valid } = formatPhoneNumber(sanitized);
setDisplayValue(formatted);
- onChange?.(formatted, isValid);
+ onChange?.(formatted, valid);
};Apply the same fix to handlePaste and to the other two components (PhoneInputWithLabel, PhoneInputWithDetails).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/ui/src/elements/phone-input.tsx around lines 53 to 62, the handler
passes the stale isValid state to onChange; change formatPhoneNumber to return
both the formatted value and its validation boolean, then in handleChange: call
formatPhoneNumber to get (formatted, valid), setDisplayValue(formatted),
setIsValid(valid), and call onChange?.(formatted, valid); apply the same pattern
to handlePaste and to the other two components (PhoneInputWithLabel,
PhoneInputWithDetails) so they all use the returned validation result instead of
the old state.
| export type PhoneInputWithLabelProps = Omit<InputWithDetailsProps, 'onChange' | 'value'> & { | ||
| defaultCountry?: CountryCode; | ||
| value?: string; | ||
| onChange?: (value: string, isValid: boolean) => void; | ||
| onValidationChange?: (isValid: boolean) => void; | ||
| }; |
There was a problem hiding this comment.
Incorrect base type for PhoneInputWithLabel.
PhoneInputWithLabelProps extends InputWithDetailsProps, which includes caption and errorMessage props. However, the PhoneInputWithLabel component renders InputWithLabel (line 146) and never uses these props. This type mismatch allows consumers to pass props that will be silently ignored.
According to the relevant code snippets, InputWithDetailsProps = InputWithLabelProps & { caption, errorMessage }. Use the appropriate base type:
-export type PhoneInputWithLabelProps = Omit<InputWithDetailsProps, 'onChange' | 'value'> & {
+export type PhoneInputWithLabelProps = Omit<InputWithLabelProps, 'onChange' | 'value'> & {
defaultCountry?: CountryCode;
value?: string;
onChange?: (value: string, isValid: boolean) => void;
onValidationChange?: (isValid: boolean) => void;
};You'll also need to import InputWithLabelProps from ./input on line 3.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/ui/src/elements/phone-input.tsx around lines 89 to 94, the
PhoneInputWithLabelProps incorrectly extends InputWithDetailsProps (which
includes caption and errorMessage that PhoneInputWithLabel never uses); change
the base type to Omit<InputWithLabelProps, 'onChange' | 'value'> so the props
reflect InputWithLabel usage, update the generic additions (defaultCountry?,
value?, onChange?, onValidationChange?) accordingly, and add an import for
InputWithLabelProps from './input' at the top (line 3).
marcinkrasowski
left a comment
marcinkrasowski
left a comment
There was a problem hiding this comment.
@mithilP007 please take a look at the comments, both mine and those from @CodeRabbit
|
|
||
| export type PhoneInputOwnProps = PhoneInputProps & { ref?: React.Ref<HTMLInputElement> }; | ||
|
|
||
| const PhoneInput = React.forwardRef<HTMLInputElement, PhoneInputOwnProps>( |
There was a problem hiding this comment.
please add a Storybook file for this component so it can actually be tested, once it's done I will check how it works
| value={displayValue} | ||
| onChange={handleChange} | ||
| onPaste={handlePaste} | ||
| placeholder="+48 500 500 500" |
There was a problem hiding this comment.
placeholder CAN NOT be hardcoded
| value={displayValue} | ||
| onChange={handleChange} | ||
| onPaste={handlePaste} | ||
| placeholder="+48 500 500 500" |
There was a problem hiding this comment.
again, please remove hardcoded placeholder
2 participants
Description
This PR adds a new PhoneInput component that provides international phone number formatting and validation support as requested in #306.
What does this PR do?
PhoneInputcomponent built on top of the existingInputcomponentlibphonenumber-jsPhoneInput,PhoneInputWithLabel, andPhoneInputWithDetailsImplementation Details
packages/ui/src/elements/phone-input.tsxlibphonenumber-jsfor formatting and validationTODO (to be completed)
libphonenumber-jsdependency topackages/ui/package.jsonphone-input.stories.tsx)Related Issue
Closes #306
Framework Experience Feedback
The framework was well-documented and easy to navigate. The existing Input component provided a solid foundation to build upon. The monorepo structure is clear and the component organization makes it easy to find related files. The issue description was very detailed with good references to existing code patterns.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.