[Update] - WEB: International phone number package #3753
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes replace the phone input library from Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant I as InternationalPhoneInput
participant C as CustomCountrySelect
participant O as External onChange Handler
U->>I: Enter phone number
I->>I: handlePhoneChange(newValue)
I-->>O: Propagate phone number change
U->>C: Open & type in country search
C->>C: Filter and display country options
C-->>I: Return selected country
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR updates the international phone number input component from 'react-phone-input-2' to 'react-phone-number-input' v3.4.12, with a complete implementation overhaul.
- Implemented a custom country selector with enhanced search functionality in
/apps/web/lib/components/inputs/international-phone-Input.tsx - Added comprehensive CSS styling in
globals.csswith proper dark mode support and accessibility features - Improved form integration with react-hook-form including proper validation using PHONE_REGEX
- Added proper disabled state handling for both the input field and country selector
- Updated the personal settings form to use the new component with direct user phone number binding
4 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/web/styles/globals.css (1)
391-1008:⚠️ Potential issueResolve pipeline failure by addressing cspell warnings on "phoneinput".
Your pipeline fails due to unknown-word errors for “phoneinput.” Either add “phoneinput” to your cspell dictionary or rename these classes to something cspell recognizes. Below is an example patch for cspell configuration:
{ "cspell": { "version": "0.2", + "words": ["phoneinput"] } }
🧹 Nitpick comments (2)
apps/web/lib/components/inputs/international-phone-Input.tsx (2)
1-1: Consider avoiding the broad ESLint disable.Disabling
react/display-nameentirely may mask potential issues. If feasible, re-enable the rule and refactor naming instead, to keep linting thorough.
8-8: Consider centralizing translations.Importing
react-phone-number-input/locale/en.jsonworks but might fragment translation handling. If you maintain all translations vianext-intl, consider mapping these country labels into your unified i18n approach.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
apps/web/lib/components/inputs/international-phone-Input.tsx(4 hunks)apps/web/lib/settings/personal-setting-form.tsx(2 hunks)apps/web/styles/globals.css(1 hunks)package.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/web/lib/components/inputs/international-phone-Input.tsx (1)
apps/web/lib/i18n/en.ts (1)
en(1-386)
🪛 GitHub Check: Cspell
apps/web/lib/components/inputs/international-phone-Input.tsx
[warning] 80-80:
Unknown word (phoneinput)
[warning] 80-80:
Unknown word (phoneinput)
[warning] 83-83:
Unknown word (phoneinput)
[warning] 91-91:
Unknown word (phoneinput)
[warning] 94-94:
Unknown word (phoneinput)
🪛 GitHub Actions: Check Spelling and Typos with cspell
apps/web/lib/components/inputs/international-phone-Input.tsx
[warning] 80-94: Unknown word (phoneinput)
[error] 80-94: 5 spelling issues found in 1 of the 3 files checked.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: deploy
🔇 Additional comments (10)
package.json (1)
182-182:Details
✅ Verification successful
Confirm removal of the old phone input dependency.
It looks like
react-phone-number-inputhas been introduced, replacingreact-phone-input-2. Please verify that all references to the old library are removed from the codebase and no residual imports remain.
🏁 Script executed:
#!/bin/bash # Description: Ensure no leftover references to `react-phone-input-2` remain. rg 'react-phone-input-2' -A 5Length of output: 29
I'll generate an additional shell script to search both the package files (i.e.
package.jsonandpackage-lock.json) and the entire repo for any occurrences of "react-phone-input-2" to ensure nothing was left behind.Please run this script and share the results so we can confirm that all references to the old dependency have indeed been removed.
🏁 Script executed:
#!/bin/bash echo "Checking for 'react-phone-input-2' in package.json:" rg -i "react-phone-input-2" package.json || echo "Not found in package.json." echo "Checking for 'react-phone-input-2' in package-lock.json (if present):" rg -i "react-phone-input-2" package-lock.json || echo "Not found in package-lock.json." echo "Searching the entire repository for 'react-phone-input-2':" rg -i "react-phone-input-2" || echo "No occurrences in the repository."Length of output: 914
Confirmed Removal of Legacy Dependency
All checks indicate that all references to
react-phone-input-2have been removed from the codebase. The search acrosspackage.json,package-lock.json(if present), and the entire repository produced no occurrences, and only the updated dependency remains inpackage.json:"react-phone-number-input": "^3.4.12",apps/web/lib/settings/personal-setting-form.tsx (2)
121-121: No concerns on the fallback assignment ofpreferredLanguage.When
userorpreferredLanguageis undefined, defaulting to an empty string is acceptable. The logic appears sound.
267-271: Verify consistent form behavior on phone number updates.Switching from
getValues('phoneNumber')touser?.phoneNumberis generally fine, but please confirm that the new approach reflects unsaved local changes in real-time. If the user modifies the phone number before saving, ensure that the displayed value remains consistent.apps/web/lib/components/inputs/international-phone-Input.tsx (7)
10-16: Generic type definitions look good.These type aliases for country options and callback semantics are clear and straightforward.
18-151: Custom country selector implementation is clear.The enhanced search and external click handling logic are well-written. No issues found.
🧰 Tools
🪛 GitHub Check: Cspell
[warning] 80-80:
Unknown word (phoneinput)
[warning] 80-80:
Unknown word (phoneinput)
[warning] 83-83:
Unknown word (phoneinput)
[warning] 91-91:
Unknown word (phoneinput)
[warning] 94-94:
Unknown word (phoneinput)🪛 GitHub Actions: Check Spelling and Typos with cspell
[warning] 80-94: Unknown word (phoneinput)
[error] 80-94: 5 spelling issues found in 1 of the 3 files checked.
168-181: Switch to uppercase default country code matches library expectations.Ensuring the default is
'US'is typically recommended by the library. This is a suitable update.
183-184: Appropriate local state initialization for phoneValue.Defaulting to the
valueprop or an empty string prevents undefined references. This looks fine.
187-204: Check phone validation for non-US patterns.Enforcing a minimum of 10 digits may not be valid universally. Confirm whether your target audience includes countries with shorter phone numbers and consider the library’s built-in validation if needed.
207-226: Ref-to-react-hook-form wiring is effectively implemented.The approach to sync the phone input value with form state and external handlers is nicely handled.
230-290: Layout and label structure are well integrated.The custom
countrySelectComponentusage is nicely factored. Overall, the final JSX structure is clean and consistent.
AnicetFantomas
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.cspell.json(1 hunks)apps/web/lib/components/inputs/international-phone-Input.tsx(1 hunks)apps/web/styles/globals.css(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .cspell.json
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/web/lib/components/inputs/international-phone-Input.tsx (1)
apps/web/lib/i18n/en.ts (1)
en(1-386)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (11)
apps/web/lib/components/inputs/international-phone-Input.tsx (6)
3-8: Good job upgrading to the react-phone-number-input library!The transition from
react-phone-input-2toreact-phone-number-inputis a nice improvement as it provides better internationalization support and accessibility features.
10-16: Well-defined TypeScript types.Good practice using explicit TypeScript types for the country selector components.
19-168: Excellent implementation of the CustomCountrySelect component.The custom country selector with search functionality is well-implemented with:
- Proper state management for dropdown visibility and search filtering
- Event handling for outside clicks to close the dropdown
- Accessibility attributes (aria-* attributes)
- Clean separation of concerns in the component structure
230-243: Good implementation of the handlePhoneChange function.This function correctly:
- Updates the internal state
- Notifies react-hook-form of the change
- Calls the external onChange handler if provided
This ensures proper state management and interoperability with the form library.
262-279: Well-configured PhoneInput component with accessibility features.The PhoneInput is properly configured with:
- International mode enabled
- Country calling code not editable
- Appropriate ARIA attributes
- Custom country selector
This provides a good user experience and meets accessibility standards.
197-197:Details
❓ Verification inconclusive
Check if your default country code still works as expected.
You've changed the default country code from lowercase 'us' to uppercase 'US'. Verify this works correctly with the new library.
🏁 Script executed:
#!/bin/bash # Check if other files in the codebase use a different case for country codes # Check for any other instances of defaultCountry being set rg -A 3 -B 3 'defaultCountry' --type ts --type tsxLength of output: 78
Default Country Code Usage – Please Verify Functionality
The change from lowercase
'us'to uppercase'US'appears isolated in the code (seeapps/web/lib/components/inputs/international-phone-Input.tsxat line 197). To ensure there are no other conflicting instances—and that the new uppercase code works with the updated library—please run the following updated search command (which adds TSX file support):#!/bin/bash rg --type-add 'tsx:*.tsx' -A 3 -B 3 'defaultCountry' .If no unexpected instances are found and manual tests confirm the library accepts
'US', then the change should be fine. Otherwise, please review any discrepancies.apps/web/styles/globals.css (5)
390-434: Comprehensive styling for the phone input component.The styling provides a complete foundation for the component with proper:
- Container sizing
- Label styling with dark mode variants
- Required field indicator
- Wrapper styling with appropriate border-radius
- Dark mode compatibility
- Error state handling
436-480: Good library override styles for customization.The styles effectively override the default PhoneInput library styles to match your application's design system while maintaining functionality.
483-543: Comprehensive dark mode support.Excellent implementation of dark mode variants for all dropdown components and selectors, ensuring a consistent user experience across themes.
559-760: Well-structured country selector styling.The CSS for the custom country selector is comprehensive, covering:
- Base layout
- Interactive states (hover, focus)
- Selected state
- Dropdown functionality
- Search input styling
- Disabled states
- Dark mode variants
611-620: Fixed padding inconsistency issue.The previous review noted an inconsistency with padding in
.phoneinput-search-container. This has been correctly fixed by using consistent 8px padding.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/web/lib/components/inputs/international-phone-Input.tsx (4)
271-271: Consider removing the type assertion for better type safety.Using
as anybypasses TypeScript's type checking. ThedefaultCountryprop should be properly typed according to the library's requirements.- defaultCountry={defaultCountry as any} + defaultCountry={defaultCountry as Country}
60-67: Consider memoizing filtered options for better performance.The filtering operation runs on every render when the search value or options change. While this is likely not a performance bottleneck with a limited number of countries, memoizing the filtered options could be a small optimization.
- // Filter options when search value changes - useEffect(() => { - if (!searchValue.trim()) { - setFilteredOptions(options); - return; - } - - const filtered = options.filter((option: Option) => { - const countryName = en[option.value as keyof typeof en]; - const searchableName = countryName || option.value; - return String(searchableName).toLowerCase().includes(searchValue.toLowerCase()); - }); - - setFilteredOptions(filtered); - }, [options, searchValue]); + // Filter options when search value changes + // useMemo to avoid recomputing on every render + const currentFilteredOptions = React.useMemo(() => { + if (!searchValue.trim()) { + return options; + } + + return options.filter((option: Option) => { + const countryName = en[option.value as keyof typeof en]; + const searchableName = countryName || option.value; + return String(searchableName).toLowerCase().includes(searchValue.toLowerCase()); + }); + }, [options, searchValue]); + + // Update state when filtered options change + useEffect(() => { + setFilteredOptions(currentFilteredOptions); + }, [currentFilteredOptions]);
156-158: Ensure consistent text directionality for country names.When displaying country names in different languages, it's important to ensure proper text directionality for RTL languages. Consider adding appropriate text direction attributes for internationalization support.
- <span className="phoneinput-country-label"> - {en[option.value as keyof typeof en] || option.value} - </span> + <span className="phoneinput-country-label" dir="auto"> + {en[option.value as keyof typeof en] || option.value} + </span>
112-112: Add i18n support for search placeholder text.The "Search countries..." text is hardcoded in English. For proper internationalization, this should use the translation system like other text in the application.
- placeholder="Search countries..." + placeholder={t('common.searchCountries')}Remember to add the corresponding translation key to your i18n files.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/lib/components/inputs/international-phone-Input.tsx(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/web/lib/components/inputs/international-phone-Input.tsx (1)
apps/web/lib/i18n/en.ts (1)
en(1-386)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (10)
apps/web/lib/components/inputs/international-phone-Input.tsx (10)
3-8: Good update of imports for the new phone input library.The switch from
react-phone-input-2toreact-phone-number-inputis appropriately reflected in the imports, including the necessary CSS styles and locale data. This aligns with the PR objective to update the international phone number package.
10-16: Well-defined TypeScript types for the custom country selector.These types provide good type safety for the country selector component, making the code more maintainable and self-documenting.
19-170: Excellent implementation of custom country selector with search functionality.The
CustomCountrySelectcomponent is a significant enhancement that provides:
- Improved UX with search functionality for countries
- Proper accessibility attributes and keyboard navigation
- Clean state management with filtered options
- Outside click detection to close the dropdown
This is a valuable addition that will make country selection more user-friendly, especially for mobile users or when dealing with many countries.
199-199: Correctly updated default country format to uppercase.The change from lowercase 'us' to uppercase 'US' aligns with the format expected by the new
react-phone-number-inputlibrary.
202-208: Good implementation of state management with value synchronization.The component now properly manages its internal state and stays in sync with external value changes through the useEffect hook, addressing a previous review comment.
230-234: Well-handled ref forwarding for react-hook-form integration.The component correctly forwards the ref to react-hook-form, ensuring proper form integration and validation.
237-249: Well-implemented phone change handler with proper events.The
handlePhoneChangefunction properly:
- Updates the internal state
- Triggers react-hook-form's onChange handler
- Calls the external onChange callback if provided
This ensures all parts of the application stay in sync with the phone input changes.
254-258: Clear and consistent CSS class naming convention.The CSS classes follow a consistent 'phoneinput-' prefix pattern, making the styling more maintainable and less likely to conflict with other components.
267-286: Good implementation of the new PhoneInput component with proper configuration.The component is properly configured with:
- International mode enabled
- Country calling code editing disabled (for consistency)
- Custom country selector integration
- Appropriate ARIA attributes for accessibility
This ensures a good user experience while maintaining accessibility.
288-296: Good error message handling with proper ARIA attributes.The error messages are properly displayed with ARIA attributes for accessibility, helping users understand validation issues.
|
|
||
| const [phoneValue, setPhoneValue] = useState(value || ''); | ||
| const inputRef = React.useRef<HTMLInputElement>(null); | ||
| useEffect(() => { |
There was a problem hiding this comment.
Problematic lines,The useEffect was executed each time the input was typed, because phoneValue changed ⇒ it rewrote the value from props.value, erasing what the user typed.
There was a problem hiding this comment.
useEffect(() => { if (externalValue !== undefined && externalValue !== phoneValue) { setPhoneValue(externalValue); } }, [externalValue]);
… handling and validation
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.cspell.json (1)
8-8: Fix duplicate entry "zipkin".The word "zipkin" appears twice in the word list (lines 8 and 15), which is redundant.
"words": [ "Złoty", "zipkin", " X", " X ", "hookform", "HCAPTCHA", "hcaptcha", - "zipkin", "CUBEJS",Also applies to: 15-15
apps/web/lib/components/inputs/international-phone-Input.tsx (2)
10-170: Well-implemented custom country selector with search functionality.The new
CustomCountrySelectcomponent enhances the user experience by adding search capabilities and improves accessibility with proper ARIA attributes.However, there's a potential issue with event listener cleanup:
useEffect(() => { const handleClickOutside = (event: MouseEvent) => { if (selectRef.current && !selectRef.current.contains(event.target as Node)) { setIsOpen(false); } }; document.addEventListener('mousedown', handleClickOutside); return () => { document.removeEventListener('mousedown', handleClickOutside); }; - }, []); + }, []); // This is fine as it should only run once, but adding a comment would clarify the intentionConsider adding keyboard navigation (arrow keys, Enter, Escape) for better accessibility:
useEffect(() => { + // Handle keyboard navigation + const handleKeyDown = (event: KeyboardEvent) => { + if (!isOpen) return; + if (event.key === 'Escape') { + setIsOpen(false); + } + }; + + document.addEventListener('keydown', handleKeyDown); + return () => { + document.removeEventListener('keydown', handleKeyDown); + }; + }, [isOpen]);
261-302: Improved component structure with proper accessibility attributes.The updated JSX structure includes proper accessibility attributes and error handling, which improves the overall user experience.
Consider adding phone number format validation feedback to help users enter correctly formatted numbers:
<PhoneInput international countryCallingCodeEditable={false} defaultCountry={defaultCountry as any} value={phoneValue} onChange={handlePhoneChange} disabled={disabled} inputRef={inputRef} + error={!!error || notValidBorder} countrySelectComponent={(props) => <CustomCountrySelect {...props} disabled={disabled} />} numberInputProps={{ id: inputId, className: inputClassName, 'aria-required': required, 'aria-invalid': notValidBorder || !!error, 'aria-describedby': error || notValidBorder ? `${name}-error` : undefined, ...registerProps, name }} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.cspell.json(2 hunks)apps/web/lib/components/inputs/international-phone-Input.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/web/lib/components/inputs/international-phone-Input.tsx (1)
apps/web/lib/i18n/en.ts (1)
en(1-386)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (8)
.cspell.json (1)
113-113: LGTM: Added "phoneinput" to the dictionary.This addition ensures the spell checker doesn't flag the new CSS class names used in the international phone input component.
apps/web/lib/components/inputs/international-phone-Input.tsx (7)
1-9: LGTM: Library replacement with proper imports.The code correctly imports the new
react-phone-number-inputpackage to replace the previousreact-phone-input-2library, including necessary CSS and locale data.
172-186: LGTM: Preserved original interface.The original
PhoneInputPropsinterface is maintained, ensuring backward compatibility with existing code that uses this component.
187-200: Good practice: Renamed value prop to avoid conflicts.Renaming the
valueprop toexternalValuehelps avoid semantic conflicts with internal state management, making the code more maintainable.
201-228: LGTM: Well-implemented form integration with proper validation.The component correctly integrates with react-hook-form and maintains the same validation logic as before.
229-234: Proper implementation of the effect to sync external and internal values.This useEffect correctly handles synchronization between external and internal state, addressing the issue mentioned in past review comments.
Note: This implementation properly avoids the previous issue where the effect would overwrite user input.
235-242: LGTM: Proper ref handling for form integration.The useEffect correctly connects the input ref to react-hook-form, ensuring proper form state management.
243-254: Well-structured centralized value management.The
handlePhoneChangefunction properly centralizes all value changes, updating both internal state and external handlers.
Description
Update the the international phone number input to a more recent package
Type of Change
Checklist
Previous screenshots
Please add here videos or images of the previous status
Current screenshots
Please add here videos or images of the current (new) status
Summary by CodeRabbit