feat: default APM phone dialing code from browser locale#272
Draft
roshan-gorasia-cko wants to merge 2 commits into
Draft
feat: default APM phone dialing code from browser locale#272roshan-gorasia-cko wants to merge 2 commits into
roshan-gorasia-cko wants to merge 2 commits into
Conversation
The new embedded APM (client.apm.authorization / apm.tokenization) phone field previously defaulted its dialing code to dialing_codes[0], which is just the first country alphabetically (~Afghanistan) after the API sorts the list by name. Merchants could already prefill both the number and the dialing code via initialData.phone_number, but there was no sensible default when nothing is prefilled. Derive the default dialing code from the browser locale's region subtag (e.g. en-GB -> +44), falling back to dialing_codes[0] when the locale has no region or no matching code. Explicit initialData still takes precedence. - add getBrowserRegion / getDefaultDialingCode helpers in src/apm/utils.ts - use them in the Phone component (the authoritative displayed/emitted default) and in the NextSteps initial form seed Also introduce the repo's first test harness: - Vitest, with tests under top-level test/ (kept out of every SDK build glob, so no test code ships in dist/processout.js) - a loader that transpiles and evaluates the real namespace source with an injected navigator, so the shipped logic is tested without adding any test-only hooks to production files - wire `yarn test` into the existing build-lint-test CI workflow Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the embedded APM phone field behavior to choose a default dialing code based on the shopper’s browser locale (region), rather than always picking the first alphabetically-sorted country entry. It also introduces a small Vitest-based unit test setup to validate the new locale helpers.
Changes:
- Added
getBrowserRegion()andgetDefaultDialingCode()helpers insrc/apm/utils.tsand used them when initializing phone values. - Updated APM form seeding (
NextSteps) and thePhoneelement to use the locale-based default dialing code. - Added Vitest config + unit tests, and wired tests into CI and
yarn verify.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Adds lockfile entries for Vitest (and its dependency graph). |
| vitest.config.ts | Introduces Vitest configuration for running TypeScript unit tests in Node. |
| test/support/loadNamespace.ts | Adds a helper to transpile/evaluate src/apm/utils.ts for testing internal-namespace exports. |
| test/apm/locale.test.ts | Adds unit tests for getBrowserRegion and getDefaultDialingCode. |
| src/apm/views/NextSteps.ts | Uses the new helper when seeding default phone dialing code. |
| src/apm/utils.ts | Adds the locale parsing + dialing code selection helpers. |
| src/apm/elements/phone.ts | Uses the new helper to choose an initial/default dialing code. |
| package.json | Adds test scripts, adds Vitest dependency, and extends verify to run tests. |
| eslint.config.mjs | Marks tests and Vitest config as Node-environment files for ESLint globals. |
| .github/workflows/build-lint-test.yml | Adds a CI step to run yarn test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+42
to
+44
| const region = getBrowserRegion(); | ||
| const match = region && dialing_codes.filter(c => c.region_code && c.region_code.toUpperCase() === region)[0]; | ||
| return (match || dialing_codes[0]).value; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The embedded APM (
client.apm.authorization/apm.tokenization) phone field now defaults its dialing code from the browser locale instead of always using the first country in the list.Previously the default was
dialing_codes[0], which — after the list is sorted alphabetically by country name — is the same country for everyone regardless of where the shopper is. Merchants can already prefill the number and dialing code viainitialData.phone_number; this only changes the default when nothing is prefilled.How
src/apm/utils.ts:getBrowserRegion()— extracts the region subtag fromnavigator.languages[0] || navigator.language(e.g.en-GB→GB).getDefaultDialingCode(dialing_codes)— returns the code whoseregion_codematches that region, else falls back todialing_codes[0].Phonecomponent (src/apm/elements/phone.ts) and the initial form seed (src/apm/views/NextSteps.ts).initialData.phone_number.dialing_codestill takes precedence; the shopper can still change the country.Tests
test/apm/locale.test.ts).test/directory, outside the SDK build, so no test code ships in the bundle.yarn testis wired into the existing lint/build CI workflow.