feat(phone-number-input): show national (trunk) prefix indicator#2242
feat(phone-number-input): show national (trunk) prefix indicator#2242spike-rabbit wants to merge 1 commit into
Conversation
Display a non-editable `(0)` indicator between the country code and the phone number input for countries that use a droppable `0` trunk prefix (e.g. Germany), producing numbers like `+49 (0) 176 ...`. The prefix is detected via libphonenumber's national direct dialing prefix and is omitted for countries without a `0` trunk prefix (e.g. US, Italy).
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a national (trunk) prefix indicator to the phone number input component, refactoring selectedCountry to use Angular signals and adding a computed nationalPrefix property. The review feedback highlights critical runtime safety issues, specifically potential TypeError crashes in writeCountry() when handling undefined countries and in the validation logic due to unsafe non-null assertions on selectedCountry(). Additionally, the feedback suggests optimizing signal reads and eliminating non-null assertions in both the TypeScript logic and the HTML template by utilizing local variable bindings.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (!country) { | ||
| const countryCode = this.phoneUtil.getCountryCodeForRegion( | ||
| currentCountry ?? this.defaultCountry() ?? 'XX' | ||
| ); | ||
| if (countryCode) { | ||
| this.selectedCountry = { | ||
| country = { | ||
| isoCode: currentCountry, | ||
| countryCode, | ||
| name: this.getCountryName(currentCountry) | ||
| }; | ||
| } |
There was a problem hiding this comment.
If this.country() is undefined but defaultCountry() is defined (e.g., 'US'), currentCountry will be undefined at runtime. In this case, getCountryCodeForRegion will resolve the fallback country code successfully, but then this.getCountryName(currentCountry) will be called with undefined, throwing a runtime TypeError when calling toUpperCase() on it.
We should use a resolved fallback country variable for isoCode and getCountryName to prevent this crash.
if (!country) {
const resolvedCountry = currentCountry ?? this.defaultCountry() ?? 'XX';
const countryCode = this.phoneUtil.getCountryCodeForRegion(resolvedCountry);
if (countryCode) {
country = {
isoCode: resolvedCountry,
countryCode,
name: this.getCountryName(resolvedCountry)
};
}
}| } | ||
|
|
||
| if (!this.countryList().some(c => c.value.isoCode === this.selectedCountry!.isoCode)) { | ||
| if (!this.countryList().some(c => c.value.isoCode === this.selectedCountry()!.isoCode)) { |
There was a problem hiding this comment.
The non-null assertion this.selectedCountry()!.isoCode can cause a runtime TypeError if selectedCountry() is undefined (for example, when the country is unsupported or cannot be resolved). We should safely retrieve the signal value and check if it is defined before accessing its properties.
const selectedCountry = this.selectedCountry();
if (!selectedCountry || !this.countryList().some(c => c.value.isoCode === selectedCountry.isoCode)) {| if (this.selectedCountry()) { | ||
| this.placeholder = this.phoneUtil | ||
| .format( | ||
| this.phoneUtil.getExampleNumber(this.selectedCountry.isoCode), | ||
| this.phoneUtil.getExampleNumber(this.selectedCountry()!.isoCode), |
There was a problem hiding this comment.
To avoid multiple signal reads and eliminate the non-null assertion operator (!), we can assign this.selectedCountry() to a local variable.
| if (this.selectedCountry()) { | |
| this.placeholder = this.phoneUtil | |
| .format( | |
| this.phoneUtil.getExampleNumber(this.selectedCountry.isoCode), | |
| this.phoneUtil.getExampleNumber(this.selectedCountry()!.isoCode), | |
| const selectedCountry = this.selectedCountry(); | |
| if (selectedCountry) { | |
| this.placeholder = this.phoneUtil | |
| .format( | |
| this.phoneUtil.getExampleNumber(selectedCountry.isoCode), |
| @if (selectedCountry()) { | ||
| <span class="si-body ms-4" [id]="id() + '-value'"> | ||
| <span class="visually-hidden">{{ selectedCountry.name }}</span> | ||
| +{{ selectedCountry.countryCode }} | ||
| <span class="visually-hidden">{{ selectedCountry()!.name }}</span> | ||
| +{{ selectedCountry()!.countryCode }} | ||
| </span> | ||
| } |
There was a problem hiding this comment.
Using the local variable binding syntax in Angular's native @if control flow (@if (selectedCountry(); as selected)) allows us to avoid multiple signal reads and eliminates the need for non-null assertions (!) in the template.
| @if (selectedCountry()) { | |
| <span class="si-body ms-4" [id]="id() + '-value'"> | |
| <span class="visually-hidden">{{ selectedCountry.name }}</span> | |
| +{{ selectedCountry.countryCode }} | |
| <span class="visually-hidden">{{ selectedCountry()!.name }}</span> | |
| +{{ selectedCountry()!.countryCode }} | |
| </span> | |
| } | |
| @if (selectedCountry(); as selected) { | |
| <span class="si-body ms-4" [id]="id() + '-value'"> | |
| <span class="visually-hidden">{{ selected.name }}</span> | |
| +{{ selected.countryCode }} | |
| </span> | |
| } |
1 participant
Display a non-editable
(0)indicator between the country code and thephone number input for countries that use a droppable
0trunk prefix(e.g. Germany), producing numbers like
+49 (0) 176 .... The prefix isdetected via libphonenumber's national direct dialing prefix and is
omitted for countries without a
0trunk prefix (e.g. US, Italy).Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: