Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: country prefill bug #299

Merged
merged 3 commits into from
Jan 31, 2024
Merged

fix: country prefill bug #299

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fe2d16a
fix: country prefill bug
madassdev Jan 31, 2024
beedcbe
Merge branch 'main' into 0e356-bug-phone-country-pre-fill
JhumanJ Jan 31, 2024
d10fcc9
Merge branch 'main' into 0e356-bug-phone-country-pre-fill
JhumanJ Jan 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 21 additions & 11 deletions client/components/forms/PhoneInput.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ export default {
}
}
},
compVal(newVal, oldVal){
this.initState()
},
Comment on lines +96 to +98
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

selectedCountryCode (newVal, oldVal) {
if (this.compVal && newVal && oldVal) {
this.compVal = this.compVal.replace(oldVal.code + oldVal.dial_code, newVal.code + newVal.dial_code)
Expand All @@ -102,17 +105,7 @@ export default {

mounted () {
if (this.compVal) {
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
}
this.initState()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
if (!this.selectedCountryCode) {
this.selectedCountryCode = this.getCountryBy()
Expand All @@ -131,6 +124,7 @@ export default {
},
onInput (event) {
this.inputVal = event?.target?.value.replace(/[^0-9]/g, '')

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

},
onChangeCountryCode () {
if (!this.selectedCountryCode && this.countries.length > 0) {
Expand All @@ -139,6 +133,22 @@ export default {
if (this.canOnlyCountry && (this.inputVal === null || this.inputVal === '' || !this.inputVal)) {
this.compVal = this.selectedCountryCode.code + this.selectedCountryCode.dial_code
}
},
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
}
Comment on lines +137 to +151
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 way substring parameters 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 parsePhoneNumber returns undefined or an invalid object, which could lead to runtime errors if phoneObj.nationalNumber or phoneObj.country are accessed without checking.

  • Correct the substring parameters to properly extract the country code.

  • Add error handling for cases where parsePhoneNumber does not return a valid phone object.

}
}
}
Expand Down
Loading