Report the pointer coordinate in DD, DMS, DDM or UTM (#1814) - #1824
Conversation
📝 WalkthroughWalkthroughThe PR adds coordinate-format preferences for DD, DMS, DDM, and UTM. It adds UTM conversion exports, clickable status-bar format switching, pointer elevation display, preference normalization, tests, and localized strings. ChangesCoordinate format readouts
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MapSettings
participant Preferences
participant StatusBar
participant CoordinateFormat
MapSettings->>Preferences: Save coordinateFormat
StatusBar->>Preferences: Cycle coordinateFormat
Preferences-->>StatusBar: Return selected format
StatusBar->>CoordinateFormat: Format pointer coordinates
CoordinateFormat-->>StatusBar: Return coordinate readout
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 Cloudflare PR preview
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Code reviewBugs: None found. Traced the DD/DMS/DDM/UTM code paths, the polar/antimeridian/southern-hemisphere fallback logic in Security: None found. No new user input handling, no injection surface; Performance: None found. Quality:
CLAUDE.md: No violations found. The new |
🔍 GitHub Pages PR preview
Note GitHub Pages built this preview successfully, but its serving edge returned HTTP 403 when checked. The links may still be propagating. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Code reviewBugs: None found. The core logic ( Security: None found. The free-string Performance: None found. The subpath import ( Quality:
CLAUDE.md: No violations found. New user-facing strings go through |
Code reviewBugs: None found. The Security: None found. No new input surfaces; the Performance: None found. Quality:
CLAUDE.md: No violations. UI strings go through Overall this is a clean, well-tested, low-risk feature addition with a single cosmetic nit. |
0b99d65 to
1527f6a
Compare
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/geolibre-desktop/src/components/layout/TopToolbar.tsx`:
- Line 1915: Add a command-palette entry named control.pointer-elevation
alongside the existing Controls-menu command definitions, wiring its action to
consent.handleTogglePointerElevation. Keep the command behavior and labeling
consistent with the other control.* toggle entries.
In `@apps/geolibre-desktop/src/hooks/useConsentGatedActions.ts`:
- Around line 85-91: Rename the local setPointerElevation helper in
useConsentGatedActions to distinguish it from the store action, and update both
call sites in handleTogglePointerElevation and confirmEnablePointerElevation.
Preserve its existing behavior of updating preferences.map.showPointerElevation.
In `@apps/geolibre-desktop/src/i18n/locales/fa.json`:
- Line 1238: Update the readinessReasonCors translations so the data host is
explicitly named as the subject of the missing CORS headers: use «این میزبان» in
apps/geolibre-desktop/src/i18n/locales/fa.json lines 1238-1238, replace «il»
with «cet hôte» in apps/geolibre-desktop/src/i18n/locales/fr.json lines
1238-1238, and replace «यह» with «यह होस्ट» in
apps/geolibre-desktop/src/i18n/locales/hi.json lines 1238-1238.
In `@apps/geolibre-desktop/src/i18n/locales/fr.json`:
- Line 1232: Update the readinessTruncated_one translation to use the idiomatic
singular wording “Seulement {{count}} source a été vérifiée.” Preserve the
existing {{count}} interpolation placeholder exactly.
In `@apps/geolibre-desktop/src/i18n/locales/hi.json`:
- Line 193: Update the Hindi “dd” translation value to use the established term
“दशमलव डिग्री”, matching the existing DD label, while leaving the translation
key and surrounding entries unchanged.
- Line 1842: Update the coordinateFormatHint translation in hi.json to use the
provided statement wording, preserving the instruction about changing the
notation by clicking the status-bar coordinates.
- Line 2667: Update the pointerElevationNoticeTitle translation to use the
wording “ऊँचाई सार्वजनिक सेवा से प्राप्त की जाती है”, matching the established
elevation-source meaning used by the related translation.
In `@apps/geolibre-desktop/src/i18n/locales/nl.json`:
- Line 2666: Update the storymapEllipsis locale entry to use the Dutch
translation "Verhaalkaart..." instead of "Story Map...", while preserving the
ellipsis and surrounding localization structure.
In `@apps/geolibre-desktop/src/i18n/locales/pt.json`:
- Line 2545: Update the Portuguese wording for pointerElevationHint and the
corresponding privacy-notice translation so it explicitly states that GeoLibre
performs the remote elevation lookup, replacing the passive “é consultado/é
consultada” phrasing while preserving the existing meaning.
- Line 1231: Update the Portuguese readinessNote translation to replace the
ambiguous phrase “diferente do que para você” with “diferente do que no seu
dispositivo,” preserving the rest of the message unchanged.
- Line 1238: Update the Portuguese readinessReasonCors translation so the
server/host is the subject of the CORS explanation, stating that it does not
return the required cross-origin headers or is inaccessible; remove the wording
that attributes missing headers to the browser.
In `@apps/geolibre-desktop/src/lib/share-readiness.ts`:
- Around line 201-212: Update hasCredentialField so its depth >=
MAX_REDACT_DEPTH branch returns true, matching redactConfigurationValue’s
fail-closed behavior and ensuring prepareShareSources detects potentially
removed credential subtrees in layer.source and layer.metadata.
In `@docs/features.md`:
- Line 203: Update the Share-readiness check description to avoid stating that
every source is probed or confirmed inaccessible. Describe classifying each
referenced source, anonymously probing eligible remote targets, and listing
sources that may fail or could not be checked, while preserving the existing
examples and non-blocking behavior.
In `@tests/pointer-elevation.test.ts`:
- Around line 252-281: Reduce the wall-time cost of the eviction test around
createPointerElevationResolver by avoiding a real 4 ms wait for every cache-fill
iteration. Prefer injecting a small cache limit for this test while preserving
the same least-recently-used eviction behavior and assertion that the refreshed
first cell remains cached.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 48276ee8-f430-40d6-95d3-c8ec0836a0e9
📒 Files selected for processing (47)
apps/geolibre-desktop/src/components/layout/DesktopShell.tsxapps/geolibre-desktop/src/components/layout/SettingsDialog.tsxapps/geolibre-desktop/src/components/layout/ShareProjectDialog.tsxapps/geolibre-desktop/src/components/layout/StatusBar.tsxapps/geolibre-desktop/src/components/layout/TopToolbar.tsxapps/geolibre-desktop/src/components/layout/toolbar/ConsentNoticeDialogs.tsxapps/geolibre-desktop/src/components/layout/toolbar/ControlsMenu.tsxapps/geolibre-desktop/src/hooks/useConsentGatedActions.tsapps/geolibre-desktop/src/i18n/locales/ar.jsonapps/geolibre-desktop/src/i18n/locales/de.jsonapps/geolibre-desktop/src/i18n/locales/en.jsonapps/geolibre-desktop/src/i18n/locales/es.jsonapps/geolibre-desktop/src/i18n/locales/fa.jsonapps/geolibre-desktop/src/i18n/locales/fr.jsonapps/geolibre-desktop/src/i18n/locales/hi.jsonapps/geolibre-desktop/src/i18n/locales/id.jsonapps/geolibre-desktop/src/i18n/locales/it.jsonapps/geolibre-desktop/src/i18n/locales/ja.jsonapps/geolibre-desktop/src/i18n/locales/ka.jsonapps/geolibre-desktop/src/i18n/locales/ko.jsonapps/geolibre-desktop/src/i18n/locales/nl.jsonapps/geolibre-desktop/src/i18n/locales/pt.jsonapps/geolibre-desktop/src/i18n/locales/ru.jsonapps/geolibre-desktop/src/i18n/locales/th.jsonapps/geolibre-desktop/src/i18n/locales/tr.jsonapps/geolibre-desktop/src/i18n/locales/zh.jsonapps/geolibre-desktop/src/lib/coordinate-format.tsapps/geolibre-desktop/src/lib/elevation-consent.tsapps/geolibre-desktop/src/lib/share-readiness.tsapps/geolibre-desktop/src/lib/ui-profile.tsdocs/features.mddocs/user-guide/projects.mdpackages/core/src/credentials.tspackages/core/src/elevation.tspackages/core/src/index.tspackages/core/src/project.tspackages/core/src/store.tspackages/core/src/types.tspackages/map/src/MapCanvas.tsxpackages/plugins/package.jsonpackages/plugins/src/index.tspackages/plugins/src/plugins/elevation-profile/elevation/client.tspackages/plugins/src/plugins/maplibre-graticule.tspackages/plugins/src/plugins/terrain-measure.tstests/coordinate-format.test.tstests/pointer-elevation.test.tstests/share-readiness.test.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/geolibre-desktop/src/components/layout/TopToolbar.tsx`:
- Line 1915: Add a command-palette entry named control.pointer-elevation
alongside the existing Controls-menu command definitions, wiring its action to
consent.handleTogglePointerElevation. Keep the command behavior and labeling
consistent with the other control.* toggle entries.
In `@apps/geolibre-desktop/src/hooks/useConsentGatedActions.ts`:
- Around line 85-91: Rename the local setPointerElevation helper in
useConsentGatedActions to distinguish it from the store action, and update both
call sites in handleTogglePointerElevation and confirmEnablePointerElevation.
Preserve its existing behavior of updating preferences.map.showPointerElevation.
In `@apps/geolibre-desktop/src/i18n/locales/fa.json`:
- Line 1238: Update the readinessReasonCors translations so the data host is
explicitly named as the subject of the missing CORS headers: use «این میزبان» in
apps/geolibre-desktop/src/i18n/locales/fa.json lines 1238-1238, replace «il»
with «cet hôte» in apps/geolibre-desktop/src/i18n/locales/fr.json lines
1238-1238, and replace «यह» with «यह होस्ट» in
apps/geolibre-desktop/src/i18n/locales/hi.json lines 1238-1238.
In `@apps/geolibre-desktop/src/i18n/locales/fr.json`:
- Line 1232: Update the readinessTruncated_one translation to use the idiomatic
singular wording “Seulement {{count}} source a été vérifiée.” Preserve the
existing {{count}} interpolation placeholder exactly.
In `@apps/geolibre-desktop/src/i18n/locales/hi.json`:
- Line 193: Update the Hindi “dd” translation value to use the established term
“दशमलव डिग्री”, matching the existing DD label, while leaving the translation
key and surrounding entries unchanged.
- Line 1842: Update the coordinateFormatHint translation in hi.json to use the
provided statement wording, preserving the instruction about changing the
notation by clicking the status-bar coordinates.
- Line 2667: Update the pointerElevationNoticeTitle translation to use the
wording “ऊँचाई सार्वजनिक सेवा से प्राप्त की जाती है”, matching the established
elevation-source meaning used by the related translation.
In `@apps/geolibre-desktop/src/i18n/locales/nl.json`:
- Line 2666: Update the storymapEllipsis locale entry to use the Dutch
translation "Verhaalkaart..." instead of "Story Map...", while preserving the
ellipsis and surrounding localization structure.
In `@apps/geolibre-desktop/src/i18n/locales/pt.json`:
- Line 2545: Update the Portuguese wording for pointerElevationHint and the
corresponding privacy-notice translation so it explicitly states that GeoLibre
performs the remote elevation lookup, replacing the passive “é consultado/é
consultada” phrasing while preserving the existing meaning.
- Line 1231: Update the Portuguese readinessNote translation to replace the
ambiguous phrase “diferente do que para você” with “diferente do que no seu
dispositivo,” preserving the rest of the message unchanged.
- Line 1238: Update the Portuguese readinessReasonCors translation so the
server/host is the subject of the CORS explanation, stating that it does not
return the required cross-origin headers or is inaccessible; remove the wording
that attributes missing headers to the browser.
In `@apps/geolibre-desktop/src/lib/share-readiness.ts`:
- Around line 201-212: Update hasCredentialField so its depth >=
MAX_REDACT_DEPTH branch returns true, matching redactConfigurationValue’s
fail-closed behavior and ensuring prepareShareSources detects potentially
removed credential subtrees in layer.source and layer.metadata.
In `@docs/features.md`:
- Line 203: Update the Share-readiness check description to avoid stating that
every source is probed or confirmed inaccessible. Describe classifying each
referenced source, anonymously probing eligible remote targets, and listing
sources that may fail or could not be checked, while preserving the existing
examples and non-blocking behavior.
In `@tests/pointer-elevation.test.ts`:
- Around line 252-281: Reduce the wall-time cost of the eviction test around
createPointerElevationResolver by avoiding a real 4 ms wait for every cache-fill
iteration. Prefer injecting a small cache limit for this test while preserving
the same least-recently-used eviction behavior and assertion that the refreshed
first cell remains cached.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 48276ee8-f430-40d6-95d3-c8ec0836a0e9
📒 Files selected for processing (47)
apps/geolibre-desktop/src/components/layout/DesktopShell.tsxapps/geolibre-desktop/src/components/layout/SettingsDialog.tsxapps/geolibre-desktop/src/components/layout/ShareProjectDialog.tsxapps/geolibre-desktop/src/components/layout/StatusBar.tsxapps/geolibre-desktop/src/components/layout/TopToolbar.tsxapps/geolibre-desktop/src/components/layout/toolbar/ConsentNoticeDialogs.tsxapps/geolibre-desktop/src/components/layout/toolbar/ControlsMenu.tsxapps/geolibre-desktop/src/hooks/useConsentGatedActions.tsapps/geolibre-desktop/src/i18n/locales/ar.jsonapps/geolibre-desktop/src/i18n/locales/de.jsonapps/geolibre-desktop/src/i18n/locales/en.jsonapps/geolibre-desktop/src/i18n/locales/es.jsonapps/geolibre-desktop/src/i18n/locales/fa.jsonapps/geolibre-desktop/src/i18n/locales/fr.jsonapps/geolibre-desktop/src/i18n/locales/hi.jsonapps/geolibre-desktop/src/i18n/locales/id.jsonapps/geolibre-desktop/src/i18n/locales/it.jsonapps/geolibre-desktop/src/i18n/locales/ja.jsonapps/geolibre-desktop/src/i18n/locales/ka.jsonapps/geolibre-desktop/src/i18n/locales/ko.jsonapps/geolibre-desktop/src/i18n/locales/nl.jsonapps/geolibre-desktop/src/i18n/locales/pt.jsonapps/geolibre-desktop/src/i18n/locales/ru.jsonapps/geolibre-desktop/src/i18n/locales/th.jsonapps/geolibre-desktop/src/i18n/locales/tr.jsonapps/geolibre-desktop/src/i18n/locales/zh.jsonapps/geolibre-desktop/src/lib/coordinate-format.tsapps/geolibre-desktop/src/lib/elevation-consent.tsapps/geolibre-desktop/src/lib/share-readiness.tsapps/geolibre-desktop/src/lib/ui-profile.tsdocs/features.mddocs/user-guide/projects.mdpackages/core/src/credentials.tspackages/core/src/elevation.tspackages/core/src/index.tspackages/core/src/project.tspackages/core/src/store.tspackages/core/src/types.tspackages/map/src/MapCanvas.tsxpackages/plugins/package.jsonpackages/plugins/src/index.tspackages/plugins/src/plugins/elevation-profile/elevation/client.tspackages/plugins/src/plugins/maplibre-graticule.tspackages/plugins/src/plugins/terrain-measure.tstests/coordinate-format.test.tstests/pointer-elevation.test.tstests/share-readiness.test.ts
🛑 Comments failed to post (6)
apps/geolibre-desktop/src/components/layout/TopToolbar.tsx (1)
1915-1915: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Consider adding a matching command-palette entry.
Every other Controls-menu toggle also has a
control.*command (for examplecontrol.directions,control.graticuleis absent butcontrol.effectsexists). The pointer-elevation toggle is reachable from the menu only. Add acontrol.pointer-elevationcommand that runsconsent.handleTogglePointerElevationso the palette stays in parity with the menu.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/geolibre-desktop/src/components/layout/TopToolbar.tsx` at line 1915, Add a command-palette entry named control.pointer-elevation alongside the existing Controls-menu command definitions, wiring its action to consent.handleTogglePointerElevation. Keep the command behavior and labeling consistent with the other control.* toggle entries.apps/geolibre-desktop/src/hooks/useConsentGatedActions.ts (1)
85-91: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Rename the local helper to avoid confusion with the store action.
setPointerElevationinpackages/core/src/store.tssets the resolved elevation value. This local helper sets theshowPointerElevationpreference. The identical name makes the two easy to confuse in this file, which also callsuseAppStore.♻️ Proposed rename
- const setPointerElevation = (enabled: boolean): void => { + const setShowPointerElevation = (enabled: boolean): void => { const current = useAppStore.getState().preferences; useAppStore.getState().setPreferences({ ...current, map: { ...current.map, showPointerElevation: enabled }, }); };Update the two call sites in
handleTogglePointerElevationandconfirmEnablePointerElevation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/geolibre-desktop/src/hooks/useConsentGatedActions.ts` around lines 85 - 91, Rename the local setPointerElevation helper in useConsentGatedActions to distinguish it from the store action, and update both call sites in handleTogglePointerElevation and confirmEnablePointerElevation. Preserve its existing behavior of updating preferences.map.showPointerElevation.apps/geolibre-desktop/src/i18n/locales/hi.json (1)
2667-2667: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use wording that identifies the elevation source.
Line 2667 says that elevation uses a public service. The notice explains that GeoLibre obtains elevation from the service. Use
ऊँचाई सार्वजनिक सेवा से प्राप्त की जाती हैto match the meaning at Line 2545.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/geolibre-desktop/src/i18n/locales/hi.json` at line 2667, Update the pointerElevationNoticeTitle translation to use the wording “ऊँचाई सार्वजनिक सेवा से प्राप्त की जाती है”, matching the established elevation-source meaning used by the related translation.apps/geolibre-desktop/src/i18n/locales/nl.json (1)
2666-2666: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail python - <<'PY' import json from pathlib import Path for path in ( Path("apps/geolibre-desktop/src/i18n/locales/en.json"), Path("apps/geolibre-desktop/src/i18n/locales/nl.json"), ): data = json.loads(path.read_text()) print( path, "storymapEllipsis=", data.get("toolbar", {}).get("item", {}).get("storymapEllipsis"), "storymap.title=", data.get("storymap", {}).get("title"), ) PYRepository: opengeos/GeoLibre
Length of output: 366
Use a Dutch translation for
toolbar.item.storymapEllipsis.The Dutch label should match the localized feature title: change
"Story Map..."to"Verhaalkaart...".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/geolibre-desktop/src/i18n/locales/nl.json` at line 2666, Update the storymapEllipsis locale entry to use the Dutch translation "Verhaalkaart..." instead of "Story Map...", while preserving the ellipsis and surrounding localization structure.Source: Coding guidelines
apps/geolibre-desktop/src/i18n/locales/pt.json (1)
2545-2545: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Make the remote elevation actor explicit.
The passive forms
"é consultado"and"é consultada"are awkward and obscure that GeoLibre performs the remote lookup. This is especially important in the privacy notice at Line 2668.Proposed wording
- "pointerElevationHint": "Mostrar a elevação do terreno sob o ponteiro na barra de estado. Sem relevo 3D é consultado um serviço público de elevação.", + "pointerElevationHint": "Mostrar a elevação do terreno sob o ponteiro na barra de estado. Sem relevo 3D, o aplicativo consulta um serviço público de elevação.", - "pointerElevationNoticeDesc": "A elevação é obtida do relevo 3D do mapa quando este está ativo, sem enviar nada. Sem relevo 3D é consultada a API pública Open-Meteo e as coordenadas sob o ponteiro saem do seu dispositivo." + "pointerElevationNoticeDesc": "A elevação é obtida do relevo 3D do mapa quando este está ativo, sem enviar nada. Sem relevo 3D, o aplicativo consulta a API pública Open-Meteo, e as coordenadas sob o ponteiro saem do seu dispositivo."Also applies to: 2668-2668
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/geolibre-desktop/src/i18n/locales/pt.json` at line 2545, Update the Portuguese wording for pointerElevationHint and the corresponding privacy-notice translation so it explicitly states that GeoLibre performs the remote elevation lookup, replacing the passive “é consultado/é consultada” phrasing while preserving the existing meaning.tests/pointer-elevation.test.ts (1)
252-281: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Consider lowering the loop cost of the eviction test.
The loop runs
POINTER_ELEVATION_CACHE_LIMIT + 5iterations and awaits a real 4 ms timer in each one, so this single test costs roughly two seconds of wall time. Accept an injectable cache limit, or reduce the awaited tick, to keep the suite fast.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/pointer-elevation.test.ts` around lines 252 - 281, Reduce the wall-time cost of the eviction test around createPointerElevationResolver by avoiding a real 4 ms wait for every cache-fill iteration. Prefer injecting a small cache limit for this test while preserving the same least-recently-used eviction behavior and assertion that the refreshed first cell remains cached.
Code reviewNote on scope: this diff bundles two unrelated features — the coordinate-format readout (DD/DMS/DDM/UTM) described in the PR title/body, and a ~2,100-line Share-readiness pre-flight check ( Bugs
Security
Performance
Quality
CLAUDE.md
|
326a50d to
c5567e1
Compare
fd764fd to
41d7b0f
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/geolibre-desktop/src/i18n/locales/hi.json (1)
1842-1842: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRewrite the coordinate-format hint as a statement.
The first sentence still uses the interrogative form
किस नोटेशन में दिखाता हैand ends with a full stop. Use a noun phrase for a clear settings description.Suggested wording
- "coordinateFormatHint": "स्टेटस बार पॉइंटर निर्देशांक किस नोटेशन में दिखाता है। नोटेशन बदलने के लिए स्टेटस बार में निर्देशांक पर क्लिक करें।" + "coordinateFormatHint": "स्टेटस बार में पॉइंटर निर्देशांकों के लिए उपयोग किया जाने वाला नोटेशन। नोटेशन बदलने के लिए स्टेटस बार में निर्देशांकों पर क्लिक करें।"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/geolibre-desktop/src/i18n/locales/hi.json` at line 1842, Update the coordinateFormatHint translation to use a noun-phrase settings description rather than the interrogative wording “किस नोटेशन में दिखाता है,” while retaining the instruction to click the status-bar coordinates to change the notation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@apps/geolibre-desktop/src/i18n/locales/hi.json`:
- Line 1842: Update the coordinateFormatHint translation to use a noun-phrase
settings description rather than the interrogative wording “किस नोटेशन में
दिखाता है,” while retaining the instruction to click the status-bar coordinates
to change the notation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cf88a44b-afa0-47b8-a373-c759d982e229
📒 Files selected for processing (3)
apps/geolibre-desktop/src/i18n/locales/hi.jsonapps/geolibre-desktop/src/lib/coordinate-format.tstests/coordinate-format.test.ts
GeoLibre could already parse DD/DMS/DDM on input (#719, Set View) and draw a UTM grid (#1170, Gridlines), but it could only ever report a coordinate in decimal degrees. A user could see a UTM grid over the map and type a DMS coordinate to fly somewhere, yet had no way to point at a feature and read its coordinate in either. This is the missing third side. Neither conversion is reimplemented. DMS/DDM come from the app's dms.ts, which the Set View dialog already uses. UTM comes from a new lngLatToUtm in the Gridlines plugin -- the same proj4 projection that draws the grid lines -- so the numbers in the status bar always agree with the grid on screen. The formatter lives in the app rather than @geolibre/core because the UTM projection needs proj4, and pulling a projection library into the package everything depends on, for one readout, is the wrong trade. The app already depends on @geolibre/plugins, so there is still exactly one implementation of each conversion. It imports the Gridlines plugin through a new package subpath rather than the barrel, matching the existing local-netcdf/raster-symbology pattern: the barrel pulls in every plugin (Earth Engine among them), which a small formatter has no business loading -- and which made the module unimportable from a Node test. Switchable from Settings -> Map, or by clicking the coordinates in the status bar to cycle. UTM falls back to decimal degrees outside its valid latitude range (-80 to 84), since the poles have no UTM coordinate and printing one anyway would be a confident lie.
The UTM branch re-typed the rounding and mE/mN suffixes inline, which is the duplication this PR claims to avoid: maplibre-graticule already exports formatEasting/formatNorthing, and the grid labels use them. Importing them here means the readout and the grid cannot round or suffix differently.
- Explicit "dd" case in the switch, so a fifth format added without a branch reads as a gap rather than as intended behaviour. - Indonesian UTM label translates easting/northing, matching every other locale this PR added.
Rebased onto #1820 so the two status-bar PRs no longer conflict on merge. The coordinate readout becomes the format-cycling button from this PR, with the elevation span from #1820 following it; both preferences sit side by side in MapPreferences and its normalizer. The locale catalogs auto-merged badly -- 1824's statusBar object replaced 1820's keys rather than joining them -- so the elevation labels are restored alongside the coordinate-format ones in all 18.
MapLibre does not wrap lngLat.lng after the user pans past the antimeridian, so it can arrive as 190 or -190. Decimal degrees tolerate that, but DMS rendered 190 degrees east and UTM resolved a zone that does not exist. Normalised once in formatCoordinate rather than in each branch. Also uses the established Hindi term for decimal degrees, and rewrites the Hindi coordinate-format hint as a statement.
f1fdc64 to
686746a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/geolibre-desktop/src/i18n/locales/hi.json`:
- Around line 198-199: Update the Hindi `dms` and `ddm` locale labels to use
`डिग्री` instead of `अंश`, matching the established `dd` label terminology while
preserving the existing minute and second wording.
In `@apps/geolibre-desktop/src/i18n/locales/id.json`:
- Line 1803: Update the Indonesian `coordinateFormatHint` translation to use
grammatically complete wording, including the agent marker “oleh” and the
appropriate display verb in the first sentence; preserve the existing
instruction about clicking the status-bar coordinate to change the notation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4996b631-698e-424a-b766-c85661593538
📒 Files selected for processing (19)
apps/geolibre-desktop/src/components/layout/StatusBar.tsxapps/geolibre-desktop/src/i18n/locales/ar.jsonapps/geolibre-desktop/src/i18n/locales/de.jsonapps/geolibre-desktop/src/i18n/locales/en.jsonapps/geolibre-desktop/src/i18n/locales/es.jsonapps/geolibre-desktop/src/i18n/locales/fa.jsonapps/geolibre-desktop/src/i18n/locales/fr.jsonapps/geolibre-desktop/src/i18n/locales/hi.jsonapps/geolibre-desktop/src/i18n/locales/id.jsonapps/geolibre-desktop/src/i18n/locales/it.jsonapps/geolibre-desktop/src/i18n/locales/ja.jsonapps/geolibre-desktop/src/i18n/locales/ka.jsonapps/geolibre-desktop/src/i18n/locales/ko.jsonapps/geolibre-desktop/src/i18n/locales/nl.jsonapps/geolibre-desktop/src/i18n/locales/pt.jsonapps/geolibre-desktop/src/i18n/locales/ru.jsonapps/geolibre-desktop/src/i18n/locales/th.jsonapps/geolibre-desktop/src/i18n/locales/tr.jsonapps/geolibre-desktop/src/i18n/locales/zh.json
| "dms": "अंश, मिनट, सेकंड", | ||
| "ddm": "अंश, दशमलव मिनट", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use डिग्री consistently in the DMS and DDM labels.
The DD label uses the established term दशमलव डिग्री, but the DMS and DDM labels use अंश. Use डिग्री for all coordinate-format labels.
Proposed wording
- "dms": "अंश, मिनट, सेकंड",
- "ddm": "अंश, दशमलव मिनट",
+ "dms": "डिग्री, मिनट, सेकंड",
+ "ddm": "डिग्री, दशमलव मिनट",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "dms": "अंश, मिनट, सेकंड", | |
| "ddm": "अंश, दशमलव मिनट", | |
| "dms": "डिग्री, मिनट, सेकंड", | |
| "ddm": "डिग्री, दशमलव मिनट", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/geolibre-desktop/src/i18n/locales/hi.json` around lines 198 - 199,
Update the Hindi `dms` and `ddm` locale labels to use `डिग्री` instead of `अंश`,
matching the established `dd` label terminology while preserving the existing
minute and second wording.
| "errorBoundsUnavailable": "Batas peta belum tersedia." | ||
| "errorBoundsUnavailable": "Batas peta belum tersedia.", | ||
| "coordinateFormat": "Format koordinat", | ||
| "coordinateFormatHint": "Notasi yang digunakan bilah status untuk koordinat penunjuk. Klik koordinat di bilah status untuk mengganti notasi." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the Indonesian grammar in the map-setting hint.
The first sentence omits the agent marker oleh and the display verb. Use a complete sentence so the setting clearly describes the notation used by the status bar.
Proposed wording
- "coordinateFormatHint": "Notasi yang digunakan bilah status untuk koordinat penunjuk. Klik koordinat di bilah status untuk mengganti notasi."
+ "coordinateFormatHint": "Notasi yang digunakan oleh bilah status untuk menampilkan koordinat penunjuk. Klik koordinat di bilah status untuk mengganti notasi."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "coordinateFormatHint": "Notasi yang digunakan bilah status untuk koordinat penunjuk. Klik koordinat di bilah status untuk mengganti notasi." | |
| "coordinateFormatHint": "Notasi yang digunakan oleh bilah status untuk menampilkan koordinat penunjuk. Klik koordinat di bilah status untuk mengganti notasi." |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/geolibre-desktop/src/i18n/locales/id.json` at line 1803, Update the
Indonesian `coordinateFormatHint` translation to use grammatically complete
wording, including the agent marker “oleh” and the appropriate display verb in
the first sentence; preserve the existing instruction about clicking the
status-bar coordinate to change the notation.
Closes #1814.
GeoLibre could already parse DD/DMS/DDM on input (#719, Set View) and draw a UTM grid (#1170, Gridlines) — but it could only ever report a coordinate in decimal degrees. A user could see a UTM grid over the map and type a DMS coordinate to fly somewhere, yet had no way to point at a feature and read its coordinate in either notation. This is the missing third side.
One implementation of each conversion
Neither conversion is reimplemented:
dms.ts, which the Set View dialog already uses.lngLatToUtmin the Gridlines plugin — the same proj4 projection that draws the grid lines. So the numbers in the status bar can never disagree with the grid on screen, which was the specific risk in having two implementations.Why the formatter lives in the app, not core
The issue suggested lifting the UTM conversion into a shared helper. It ended up in the plugin rather than
@geolibre/core, because the projection needs proj4 and core has four small dependencies today — pulling a projection library into the package everything depends on, for one readout, is the wrong trade. The app already depends on@geolibre/plugins, so there is still exactly one implementation.It imports through a new package subpath (
@geolibre/plugins/maplibre-graticule) rather than the barrel, matching the existinglocal-netcdf/raster-symbologypattern. The barrel pulls in every plugin, Earth Engine included, which a small formatter has no business loading — and which made the module unimportable from a Node test until I switched.Interaction
Two ways to switch, because they suit different moments:
Ordering follows each notation's own convention rather than being forced to match: decimal degrees stay lng/lat (as the rest of the app and every GeoJSON do), while DMS and DDM lead with latitude, which is how they are conventionally written.
The edge case worth reviewing
UTM is undefined at the poles (it covers −80° to 84°). Rather than print a meaningless zone,
formatCoordinatefalls back to decimal degrees there, and on any proj4 failure. There is a test for both hemispheres, the antimeridian, and the polar fallback.The preference is typed as a plain
stringinMapPreferencesso core does not need to know the format union; the app normalises anything unrecognised todd, so a hand-edited project cannot break the readout.Tests
tests/coordinate-format.test.ts, 10 cases: normalisation of junk values, a full cycle through the formats, each notation's shape, southern-hemisphere and antimeridian zones, and the polar fallback.Full suite: 5632 pass, 0 fail.
npm run typecheckand scopedpre-commitclean.Note on overlap
This branches from
mainindependently of #1820. Both touchStatusBar.tsx, so whichever merges second will need a small import merge — the changes themselves do not overlap (that one adds spans, this one changes the coordinate span into a button).i18n
Format names and hints in all 18 catalogs, with the acronyms kept and the expansions translated —
Grados, minutos, segundos(es),度分秒(ja),Десятичные градусы(ru).Summary by CodeRabbit