chore(application-system): Translate country list in phone prefix dropdown #16708
Conversation
WalkthroughThe changes in this pull request introduce localization support for phone input components. The Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
norda-gunni
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16708 +/- ##
==========================================
- Coverage 36.53% 36.51% -0.02%
==========================================
Files 6888 6890 +2
Lines 143659 143717 +58
Branches 40931 41029 +98
==========================================
- Hits 52490 52484 -6
- Misses 91169 91233 +64 Flags with carried forward coverage won't be shown. Click here to find out more.
... and 24 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 34 Total Test Services: 0 Failed, 32 Passed Test ServicesThis report shows up to 10 services
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
norda-gunni
force-pushed
the
as-translate-countries
branch
from
35afcfa to
3125225
Compare
norda-gunni
force-pushed
the
as-translate-countries
branch
from
3125225 to
4621033
Compare
norda-gunni
requested review from
RunarVestmann and
jonnigs
and removed request for
RunarVestmann
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (6)
libs/shared/form-fields/src/lib/PhoneInputController/PhoneInputController.tsx (1)
16-16: Add JSDoc documentation for the locale propFor better maintainability and to help library consumers, consider adding JSDoc documentation explaining the purpose and expected values of the
localeprop.+ /** The locale used for phone input localization (e.g., 'is' or 'en') */ locale?: Localelibs/island-ui/core/src/lib/PhoneInput/countryCodes.ts (1)
Data inconsistencies found between English and Icelandic country code arrays
The verification revealed several inconsistencies:
Country codes are not consistently matched between arrays:
- Some codes appear only once instead of twice (e.g., AG, AN, AS, AX)
- This indicates missing translations or mismatched country codes
Dial codes show inconsistent counts:
- Multiple entries for some dial codes (+1, +500, +672)
- This suggests potential duplicate or mismatched entries
Format properties are preserved correctly between translations for:
- Canada: (###) ###-####
- Denmark: ## ## ## ##
- Iceland: ###-####
- Poland: ## ### ## ##
- Ukraine: ## ###-##-##
- United Kingdom: #### ######
- United States: (###) ###-####
Please review and align the country codes and dial codes between the English and Icelandic arrays to ensure one-to-one mapping.
🔗 Analysis chain
Line range hint
1-941: Verify data consistency between English and Icelandic arrays.Let's ensure both arrays maintain consistent data across translations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that both arrays have matching entries for codes and dial codes # Extract and compare the number of entries echo "Comparing array lengths..." EN_COUNT=$(grep -c "dial_code:" libs/island-ui/core/src/lib/PhoneInput/countryCodes.ts) IS_COUNT=$(grep -c "dial_code:" libs/island-ui/core/src/lib/PhoneInput/countryCodes.ts) if [ "$EN_COUNT" -ne "$IS_COUNT" ]; then echo "Error: Arrays have different lengths" exit 1 fi # Compare country codes echo "Checking country codes consistency..." grep -A 1 "code:" libs/island-ui/core/src/lib/PhoneInput/countryCodes.ts | grep "code:" | sort | uniq -c | grep -v " 2" && echo "Error: Inconsistent country codes found" # Compare dial codes echo "Checking dial codes consistency..." grep -A 1 "dial_code:" libs/island-ui/core/src/lib/PhoneInput/countryCodes.ts | grep "dial_code:" | sort | uniq -c | grep -v " 2" && echo "Error: Inconsistent dial codes found" # Check format properties echo "Checking format properties consistency..." grep -A 3 "format:" libs/island-ui/core/src/lib/PhoneInput/countryCodes.tsLength of output: 63768
libs/island-ui/core/src/lib/PhoneInput/PhoneInput.tsx (4)
26-30: Consider defaulting thelangparameter ingetCountryCodesto simplify function callsCurrently,
getCountryCodesrequires alangparameter, and you're defaulting to'is'at each call site usinglocale || 'is'. By setting a default value forlangin the function signature, you can reduce code redundancy and prevent potential errors.Apply this diff to set a default value for
lang:const getCountryCodes = ( - lang: Locale, + lang: Locale = 'is', allowedCountryCodes?: string[], ): StringOption[] => {
68-71: Set a default value forlangingetDefaultCountryCodeto prevent repeated defaultingSimilarly to
getCountryCodes, setting a default value forlangingetDefaultCountryCodecan simplify calls and eliminate the need to defaultlocaleat each invocation.Apply this diff:
const getDefaultCountryCode = ( - lang: Locale, + lang: Locale = 'is', phoneNumber?: string, ) => {
149-153: Simplify function calls by removing redundant default locale logicWith default values set for
langin bothgetDefaultCountryCodeandgetCountryCodes, you can remove thelocale || 'is'logic from these function calls.Apply these diffs:
const defaultCountryCode = getDefaultCountryCode( - locale || 'is', + locale, value || defaultValue, )const countryCodes = getCountryCodes( - locale || 'is', + locale, allowedCountryCodes, )
186-189: Utilize default parameter forlanginhandleInputChangeSince
getDefaultCountryCodenow defaultslangto'is', you can simplify the function call by passinglocaledirectly.Apply this diff:
const updatedCC = getDefaultCountryCode( - locale || 'is', + locale, e.currentTarget.value, )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
libs/application/ui-fields/src/lib/PhoneFormField/PhoneFormField.tsx(3 hunks)libs/island-ui/core/src/lib/PhoneInput/PhoneInput.tsx(6 hunks)libs/island-ui/core/src/lib/PhoneInput/countryCodes.ts(4 hunks)libs/shared/form-fields/src/lib/PhoneInputController/PhoneInputController.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
libs/application/ui-fields/src/lib/PhoneFormField/PhoneFormField.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/island-ui/core/src/lib/PhoneInput/PhoneInput.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/island-ui/core/src/lib/PhoneInput/countryCodes.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/shared/form-fields/src/lib/PhoneInputController/PhoneInputController.tsx (2)
Pattern libs/shared/**/*: "Confirm that the code adheres to the following:
- Cross-application utility functions, types, and constants.
- TypeScript best practices to ensure type safety and reusability.
- Documentation and examples for library consumers."
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (7)
libs/application/ui-fields/src/lib/PhoneFormField/PhoneFormField.tsx (2)
12-12: LGTM! Clean TypeScript implementation.
The explicit import of Locale type and the destructuring of useLocale hook follow TypeScript best practices and maintain good tree-shaking capabilities.
Also applies to: 40-40
67-67: Consider avoiding type assertion for locale prop.
While the type assertion works, it would be better to ensure type safety at compile time. Consider defining the type for lang from useLocale directly or using type guards if necessary.
Let's verify the typing of useLocale:
libs/shared/form-fields/src/lib/PhoneInputController/PhoneInputController.tsx (3)
5-5: LGTM: Clean import of shared type
The import of Locale from shared types follows best practices for shared libraries.
52-52: LGTM: Clean prop destructuring
The locale prop is correctly destructured and maintains alphabetical ordering.
85-85: Verify PhoneInput component's locale prop implementation
The locale prop is correctly passed down to PhoneInput component.
Let's verify the PhoneInput component's implementation of the locale prop:
✅ Verification successful
PhoneInput component correctly implements the locale prop
The locale prop is properly implemented in the PhoneInput component:
- It's defined in the PhoneInputProps type
- It's used to determine the country codes list (via
getCountryCodesfunction) - It affects the display of country names in the dropdown (switches between English and Icelandic)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify PhoneInput component's locale prop implementation
# Expected: Find the locale prop definition in PhoneInput component
# Search for PhoneInput component's props interface
ast-grep --pattern 'interface $PROPS_NAME {
$$$
locale?: $TYPE
$$$
}'
# Search for locale usage in PhoneInput component
rg -A 5 'locale.*=.*props' libs/island-ui/core/src/lib/PhoneInput/
Length of output: 13095
Script:
#!/bin/bash
# Let's search for PhoneInput component's implementation and props
fd PhoneInput.tsx -x cat {}
# Also search for any PhoneInput interface or type definitions
ast-grep --pattern 'type PhoneInputProps = {
$$$
}'
ast-grep --pattern 'interface PhoneInputProps {
$$$
}'
Length of output: 14224
libs/island-ui/core/src/lib/PhoneInput/countryCodes.ts (2)
1-1: LGTM! Clean export structure.
The file properly exports the country code arrays following TypeScript conventions.
Also applies to: 638-639
639-941: LGTM! Well-structured Icelandic translations.
The Icelandic translations:
- Maintain consistent data structure with the English version
- Preserve special format properties for relevant countries
- Include all countries from the English list
norda-gunni
force-pushed
the
as-translate-countries
branch
from
4621033 to
6cf3a36
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
libs/island-ui/core/src/lib/PhoneInput/countryCodes.ts (1)
638-940: Consider refactoring to reduce duplicationThe EN and IS arrays share the same structure with only names being different. Consider using a base array with translations:
interface CountryCodeBase { flag: string; code: string; dial_code: string; format?: string; } interface CountryCodeTranslations { en: string; is: string; } const baseCountryCodes: (CountryCodeBase & { id: string })[] = [/*...*/] const translations: Record<string, CountryCodeTranslations> = {/*...*/} export const countryCodesEN = baseCountryCodes.map(country => ({ ...country, name: translations[country.id].en })) export const countryCodesIS = baseCountryCodes.map(country => ({ ...country, name: translations[country.id].is }))libs/island-ui/core/src/lib/PhoneInput/PhoneInput.tsx (2)
19-19: Optimize import of country code data to reduce bundle sizeImporting both
countryCodesISandcountryCodesENmay unnecessarily increase the bundle size since only one is needed at runtime based on the locale. Consider dynamically importing the country code data or restructuring it to leverage tree-shaking, thereby improving bundling efficiency.
113-113: Set a default value forlocaleto simplify codeBy setting a default value for
localewhen destructuring props, you can avoid repeatedly usinglocale || 'is'throughout the component, enhancing code readability.Apply this diff to set a default value:
const { name, label, - locale, + locale = 'is', errorMessage = '', hasError = Boolean(errorMessage), // ...other props } = propsThen, you can simplify the following lines by removing the defaulting:
- const defaultCountryCode = getDefaultCountryCode( - locale || 'is', - value || defaultValue, - ) + const defaultCountryCode = getDefaultCountryCode( + locale, + value || defaultValue, + )- const countryCodes = getCountryCodes(locale || 'is', allowedCountryCodes) + const countryCodes = getCountryCodes(locale, allowedCountryCodes)- const countryCodeList = locale === 'is' ? countryCodesIS : countryCodesEN + const countryCodeList = locale === 'is' ? countryCodesIS : countryCodesEN- const updatedCC = getDefaultCountryCode( - locale || 'is', - e.currentTarget.value, - ) + const updatedCC = getDefaultCountryCode( + locale, + e.currentTarget.value, + )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
libs/application/ui-fields/src/lib/PhoneFormField/PhoneFormField.tsx(3 hunks)libs/island-ui/core/src/lib/PhoneInput/PhoneInput.tsx(6 hunks)libs/island-ui/core/src/lib/PhoneInput/countryCodes.ts(4 hunks)libs/shared/form-fields/src/lib/PhoneInputController/PhoneInputController.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/application/ui-fields/src/lib/PhoneFormField/PhoneFormField.tsx
- libs/shared/form-fields/src/lib/PhoneInputController/PhoneInputController.tsx
🧰 Additional context used
📓 Path-based instructions (2)
libs/island-ui/core/src/lib/PhoneInput/PhoneInput.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/island-ui/core/src/lib/PhoneInput/countryCodes.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (1)
libs/island-ui/core/src/lib/PhoneInput/countryCodes.ts (1)
1-1:
Breaking change: Renamed export from countryCodes to countryCodesEN
This rename improves code clarity but requires updates in all importing files.
✅ Verification successful
No breaking changes detected - rename is safe
The search results show that the old countryCodes export was not directly imported anywhere in the codebase. The PhoneInput component already uses the new countryCodesEN and countryCodesIS imports internally, and other files only import the PhoneInput component itself. The only other usage of countryCodes is in countries.ts, but this is a different constant with a different type.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all files importing the old constant name
rg "import.*\{.*countryCodes.*\}.*from.*PhoneInput"
Length of output: 53
Script:
#!/bin/bash
# Try a broader search for any imports from PhoneInput
rg "from.*PhoneInput"
# Also search for direct usage of countryCodes
rg "countryCodes[^A-Za-z]"
# Search for any file that imports from the PhoneInput directory
rg "import.*from.*PhoneInput"
Length of output: 2356
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
libs/island-ui/core/src/lib/PhoneInput/countryCodes.ts (1)
646-948: LGTM with suggestions for improvement.The Icelandic translations are well-implemented and maintain consistency with the English version. Consider these improvements:
- Extract phone number formats into a separate constant to avoid duplication between language versions
- Consider sorting countries alphabetically within each language array for easier maintenance
Example of format extraction:
const PHONE_FORMATS = { CA: '(###) ###-####', DK: '## ## ## ##', GB: '#### ######', IS: '###-####', PL: '## ### ## ##', UA: '## ###-##-##', US: '(###) ###-####', } as const;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
libs/island-ui/core/src/lib/PhoneInput/countryCodes.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/island-ui/core/src/lib/PhoneInput/countryCodes.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (2)
libs/island-ui/core/src/lib/PhoneInput/countryCodes.ts (2)
1-7: LGTM! Well-defined TypeScript interface.
The CountryCode interface properly defines all required fields with appropriate types.
365-365: LGTM! Country entries updated correctly.
The changes properly update the Netherlands Antilles flag and add the North Macedonia entry.
Also applies to: 386-386
norda-gunni
added
the
deprecated:automerge
norda-gunni
added
deprecated:automerge
…pdown (#16708) * chore(application-system): Translate country list in phone prefix dropdown * PR comments --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
…pdown (#16708) * chore(application-system): Translate country list in phone prefix dropdown * PR comments --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
...
Attach a link to issue if relevant
What
Specify what you're trying to achieve
Why
Specify why you need to achieve this
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation