Conversation
|
@FayezBast is attempting to deploy a commit to the Elie Team on Vercel. A member of the Team first needs to authorize it. |
|
Thanks for the solid work on this, Fayez! The extracted utility + test coverage is a big improvement over the inline approach. A couple of requests before merging:
|
|
done |
There was a problem hiding this comment.
✅ What's Good
- Clean extraction:
map-locale.tsis 88 lines, single responsibility, well-structured - Fallback chain:
coalesce(localized → English → native)is the correct approach - RTL plugin: Self-hosted in
public/avoids CSP issues — good pattern for this repo - Test coverage: 416 lines, covers edge cases (null maps, empty layers, idempotency, RTL), uses both English and Arabic module instances
- Integration: Called at both
map.on('load')andstyle.load(theme switch) — covers all lifecycle points - Improvement over #1027: Uses
name:en(colon, correct for CARTO tiles) instead ofname_en(underscore). Also localizes ALL symbol layers, not justsource-layer: 'place'
⚠️ Action Items
[ACTION 1] — RTL plugin guard
if (!maplibregl.getRTLTextPluginStatus || maplibregl.getRTLTextPluginStatus() === 'unavailable') {The !maplibregl.getRTLTextPluginStatus check suggests guarding against older MapLibre versions that don't have the function. But package.json pins ^5.16.0 which definitely has it.
👉 Remove the unnecessary null check — it adds confusion about what versions are supported. Just use:
if (maplibregl.getRTLTextPluginStatus() === 'unavailable') {[ACTION 2] — Empty line at top of map-locale.ts
Line 1 is blank before the import statement.
👉 Remove the leading blank line.
[ACTION 3] — Vietnamese fallback documentation
Vietnamese (vi) silently falls back to English with only a code comment. Users switching to Vietnamese will see English map labels with no indication why.
[ACTION 4] — type Expression = any
Loose typing on the expression type.
👉 Optional but recommended: Replace with type Expression = unknown[] or type Expression = [string, ...unknown[]] for basic shape safety.
📋 Verified
- ✅ RTL plugin API:
setRTLTextPlugin(url, lazy)— confirmed correct for MapLibre v5.16.0 - ✅
name:enfield syntax — correct for CARTO vector tiles (OpenMapTiles schema) - ✅ No clock auto-translation churn interaction — PR #1034 (
translate="no") is about browser auto-translate, completely unrelated to this map localization - ✅ No infinite loop risk —
setLayoutPropertydoes NOT re-triggerstyle.load
@FayezBast thx
|
u need me to fix what u addressed ? or khalas done |
|
Pls do |
* feat/localize map(koala73#1017) * feat :map localization * fix: address map-localizatio
Summary
Localize MapLibre basemap labels to the user's UI language. Country names, cities, water bodies, and POI labels now render in the user's selected language (20 of 21 supported languages) with automatic fallback: localized name → English → native script.
Includes a self-hosted RTL text plugin to correctly render Arabic and other RTL scripts.
Closes #1017
CORS / CSP note
This PR self-hosts the Mapbox RTL text plugin under
public/(seepublic/mapbox-gl-rtl-text.min.js) to avoid CSP/CORS issues that can occur when loading the plugin from Mapbox’s CDN. The localization logic only changes symbol-layertext-fieldexpressions and does not introduce new tile/style origins.Type of change
Affected areas
Checklist
npm run typecheck)tests/map-locale.test.mts)Screenshots
Switch language in Settings → Language (e.g., Chinese, Arabic, French) and reload the page.
Map labels update automatically to the selected language.