fix: country prefill bug #299
Conversation
WalkthroughThe update involves enhancing the Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| compVal(newVal, oldVal){ | ||
| this.initState() | ||
| }, |
There was a problem hiding this comment.
The compVal method is designed to handle changes in the component's value but currently only calls this.initState() without actually comparing newVal and oldVal or handling them in any specific way. This might not align with the PR objectives that suggest compVal should effectively deal with new and old values to ensure the country prefill functionality works as intended.
Consider implementing logic within compVal that utilizes newVal and oldVal to adjust the component's state or value accordingly.
| } | ||
| this.inputVal = phoneObj.nationalNumber | ||
| } | ||
| this.initState() |
There was a problem hiding this comment.
The mounted hook calls this.initState() conditionally based on the presence of this.compVal. This approach might not be reliable for initializing the component's state, especially if compVal is not set at the time of mounting. It's crucial to ensure that the component is correctly initialized regardless of the compVal state.
Ensure that the component's initialization logic in mounted does not solely depend on compVal. Consider always calling initState and handling any necessary conditions within initState itself.
| @@ -131,6 +124,7 @@ export default { | |||
| }, | |||
| onInput (event) { | |||
| this.inputVal = event?.target?.value.replace(/[^0-9]/g, '') | |||
|
|
|||
There was a problem hiding this comment.
The onInput method strips all non-numeric characters from the input value. While this helps in removing whitespace and ensuring data quality, it also removes any plus signs (+) that might be part of an international phone number prefix. This could potentially alter the intended input value.
Consider allowing the plus sign (+) in the input value to support international phone numbers fully. Adjust the regex to exclude the plus sign from being removed.
| initState() { | ||
| if(this.compVal === null){ | ||
| return; | ||
| } | ||
| if (!this.compVal.startsWith('+')) { | ||
| this.selectedCountryCode = this.getCountryBy(this.compVal.substring(2, 0)) | ||
| } | ||
|
|
||
| const phoneObj = parsePhoneNumber(this.compVal) | ||
| if (phoneObj !== undefined && phoneObj) { | ||
| if (!this.selectedCountryCode && phoneObj.country !== undefined && phoneObj.country) { | ||
| this.selectedCountryCode = this.getCountryBy(phoneObj.country) | ||
| } | ||
| this.inputVal = phoneObj.nationalNumber | ||
| } |
There was a problem hiding this comment.
The initState method attempts to parse and set the phone number and country code from compVal. However, there are a few concerns:
-
Line 142 uses
this.compVal.substring(2, 0), which will always return an empty string due to the waysubstringparameters are ordered. It seems like the intention was to extract the country code, but this will not work as expected. -
The method does not handle cases where
parsePhoneNumberreturnsundefinedor an invalid object, which could lead to runtime errors ifphoneObj.nationalNumberorphoneObj.countryare accessed without checking. -
Correct the
substringparameters to properly extract the country code. -
Add error handling for cases where
parsePhoneNumberdoes not return a valid phone object.
Summary by CodeRabbit