Conversation
Co-authored-by: Rory Abraham <roryabraham@users.noreply.github.com>
melvin-bot
Bot
added
Melvin-Test-Android
Melvin-Test-Web
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
This comment was marked as low quality.
This comment was marked as low quality.
This comment was marked as low quality.
This comment was marked as low quality.
|
🚧 roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
@MelvinBot This doesn’t work on web, and it looks like the change only supports two languages. |
|
You're right on both counts. I reproduced the web failure and traced why. 1. "Doesn't work on web" — confirmed. I tested on web: switched the app to Japanese, and the map's place/road labels stayed entirely English while the rest of the UI localized. So the Root cause: In mapbox-gl v3, To confirm the last step, open the map on web and check Fix options (roughly increasing effort):
2. "Only supports two languages" — it's a bit different, but there is a real gap. I'm not pushing a fix here since the most likely real fix is a Mapbox Studio style change outside this repo, and I'd want Test result: fail — map labels did not localize on web after switching to Japanese.
Evidence (2)Distance map with app language English — all place/road labels in English
Same route after switching app language to Japanese — UI is Japanese but map labels remain English
|
|
@linhvovan29546 @MelvinBot I can confirm I see toggles to have language and worldview "Configured in SDK": 
But I don't want to enable that without confirming first that we are configuring Mapbox language and worldview in the Expensify app in production. Do we need to merge and deploy this PR first, then enable those settings?
Regarding this: I understand it could be a larger refactor, but would switching to Google Maps provide more complete localization? I don't recall the context for why we used Mapbox in the first place, but I recall that it being closed-source comes with some extra config burden in this repo. Is there a ready-to-go Google Maps library for React Native? |
|
Short answer: No — production configures neither today. Deploy this PR first, then flip Language only. Do not flip Worldview. And no, we don't need Google Maps: I was wrong about the 3 missing locales. 1. Are we configuring language/worldview in production today?No, neither. 2. Ordering: deploy first, then flip LanguageDeploy first is the safe order. Merging is a no-op today: the style pins labels to English, so Flipping first is the risky order. The style is server-side, so it takes effect instantly for every client — current production, old native builds, cached web sessions. Those clients set no language, and Mapbox's documented default is the browser/device locale, not English. A user with a German browser and an English app would suddenly get German map labels, with no deploy to roll back through. Leave Worldview pinned. It controls disputed-boundary rendering, defaults to 3. Google Maps? No — and I owe you a correctionMy "el/nl/pl will never localize" claim was wrong. That was the old The real gap is native, not Mapbox. Web and native use different mechanisms:
So after the flip, web gets all 11 locales and native still misses 3. The fix is moving native to the same server-side path (Mapbox's Why Google Maps would cost far more than it returns
Verifying before you publish the styleStudio keeps a draft version of the style. If the token scope allows it, point a local build at Style URL is at Recommended sequence: merge + deploy → flip Language to "Configured in SDK" (leave Worldview) → verify on web with a locale like Greek or Polish, which only work through the server-side path → file a follow-up for native server-side language. Sources: Mapbox maps internationalization, Android localization plugin |
|
Ok:
|
|
@MelvinBot Please check #99304 (comment) |
Mapbox draws disputed borders from the perspective of the worldview it is given. The map never set one, so every user saw the style's default. Web now sends the worldview matching the user's country alongside the label language. Co-authored-by: Rory Abraham <roryabraham@users.noreply.github.com> Co-authored-by: Linh Vo <linhvovan29546@users.noreply.github.com>
Co-authored-by: Rory Abraham <roryabraham@users.noreply.github.com>
|
Fixed the failing check — pushed
The other failing check, Verification
|
Mapbox falls back to the style's default worldview for country codes it has no worldview for, so an allowlist only adds a list that goes stale as Mapbox adds worldviews. Only values that aren't country codes at all still need dropping, since those raise an error. Co-authored-by: Rory Abraham <roryabraham@users.noreply.github.com> Co-authored-by: Linh Vo <linhvovan29546@users.noreply.github.com>
…ude-mapboxMapLocale Co-authored-by: Linh Vo <linhvovan29546@users.noreply.github.com>
|
Worldview is wired up on web and pushed ( 1. WorldviewWeb now sends worldview alongside language, from the user's country by IP:
Two things worth your call, The country source. Worldview is a country, not a language, so it can't come from This is a policy change, not just plumbing. Today every user sees the style's default worldview. After this, an Indian user sees Indian borders and a Chinese user sees Chinese borders. That's the point of the feature, but it's a decision about disputed territory, so flag it if you'd rather pin it. I pass any well-formed country code straight through rather than allowlisting Heads-up:
|
|
MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
This branch has not been deployed
3 participants
Explanation of Change
The Mapbox map never told Mapbox which language to render, so map labels (place names, roads) stayed in the default language and ignored the app locale. This wires the user's
preferredLocale(fromuseLocalize()) into the map on both platforms:MapViewImpl.web.tsx): calls the underlyingmapbox-glmap.setLanguage(...)once the style is loaded, reapplying whenever the map or locale changes.MapView.tsx): passes thelocalizeLabels={{locale}}prop to<Mapbox.MapView>(supported by@rnmapbox/maps).getMapboxLanguagehelper inutils.tsmaps app locales to the BCP-47 codes Mapbox expects (pt-BR→pt,zh-hans→zh-Hans; the rest pass through). Unsupported codes fall back to each label's local language on the Mapbox side.Unit tests were added for
getMapboxLanguage.Fixed Issues
$ #97752
PROPOSAL: #97752 (comment)
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari