CLIM-1322 (4): Popup Title Display Discrepancy When InteractiveRegionOption.GRIDDED_DATA#685
Merged
Merged
Conversation
This was referenced May 9, 2026
Draft
b1dc7e7 to
c6a07ac
Compare
The function selected "province_fr" (or "province") via $term_append but the result row was always read as $result['province']. In FR this produced "Undefined array key" warnings and a null province_short, which in turn yielded titles like "Sainte-Adèle, " (trailing comma). Aliases the column back to the "province" key, matching the pattern already used by cdc_get_location_by_id() at the same file.
In the locationNotFound branch (raw lat,lng paste), showLocation()'s title argument was the opaque 5-char geo_id (e.g. "EPYTF", "EJK8O"), producing a popup title users could not read. The fetched response already carries a human-readable title field built server-side; pass that instead.
c039d6b to
42582e8
Compare
42582e8 to
672e7ef
Compare
renoirb
commented
May 12, 2026
Comment on lines
282
to
300
| // Check if the coordinates are valid if the location is empty. | ||
| const latLng = parseLatLon(this._input.value); | ||
| const latLon = parseLatLon(this._input.value); | ||
| // If the coordinates are valid, move to that location. | ||
| if (latLng && !latLng.isPartial) { | ||
| // Fetch location data | ||
| const locationByCoords = await fetchLocationByCoords({ lat: latLng.lat, lng: latLng.lon }); | ||
| // Trigger show location. | ||
| this.showLocation(locationByCoords, locationByCoords.geo_id); | ||
| if (latLon && !latLon.isPartial) { | ||
| const latLng = { | ||
| lat: latLon.lat, | ||
| lng: latLon.lon, | ||
| } | ||
| const locationByCoords = await fetchLocationByCoords(latLng); | ||
| const locationAtTypedCoords = { | ||
| ...locationByCoords, | ||
| lat: latLng.lat, | ||
| lng: latLng.lng, | ||
| }; | ||
| this.showLocation( | ||
| locationAtTypedCoords, | ||
| locationByCoords.title, | ||
| ); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Make the map move to the entered coordinates.
renoirb
commented
May 12, 2026
Comment on lines
+140
to
+143
| onSelectGriddedLocation({ | ||
| latlng, | ||
| title: autocompleteItem.title, | ||
| }); |
Contributor
Author
There was a problem hiding this comment.
I have the impression that's what opens up the PopUp thing. And the lat,lon at.
…e province_short in autocomplete
Atom 9-equivalent's buildLocationTitle() produced the verbose autocomplete
dropdown display ("Montréal, (Ville), Montréal; Montréal, Québec") in the
popup title; make UC-Search's popup match the server-resolved short shape
("Montréal, QC") that cdc_get_location_by_coords already returns.
Server-side cdc_location_search() now adds province_short to each item,
derived via the existing short_province() helper from fw-child/functions.php
(already used by cdc_get_location_by_coords and cdc_get_location_by_id), so
the client doesn't need its own province-name mapping. buildLocationTitle()
is unchanged — leaflet-search still uses it as the dropdown display key.
…PointerEvent AFTER setView
Empirical drift in UC-RawCoordPaste was traced to leaflet-search's showLocation synchronously invoking moveToLocation -> handleLocationChange -> dispatchMapClick (container-center synthetic click) -> drifted ~80m re-projection in handleClick's addMarker. This commit bypasses that chain in GRIDDED mode by calling setView + onSelectGriddedLocation directly with the typed lat/lng, mirroring the same-intent-same-pattern path UC-Search GRIDDED already uses. Polygon modes (CENSUS/HEALTH/WATERSHED) preserve the existing showLocation path because their popup title comes from layer.properties.label_* via handleClick's override and the ~80m drift is benign at polygon scale.
672e7ef to
3b6bc0b
Compare
renoirb
commented
May 12, 2026
Comment on lines
+292
to
+311
| // Bypass leaflet-search's showLocation pipeline for GRIDDED mode so the | ||
| // marker lands at the typed coordinates rather than the synthetic-click | ||
| // container-center reprojection. Polygon modes (CENSUS/HEALTH/WATERSHED) | ||
| // fall through to the existing showLocation path because their popup | ||
| // title comes from layer.properties.label_* and the small drift is | ||
| // benign at polygon scale. | ||
| const interactiveRegion = climateVariable?.getInteractiveRegion() | ||
| ?? InteractiveRegionOption.GRIDDED_DATA; | ||
|
|
||
| if ( | ||
| interactiveRegion === InteractiveRegionOption.GRIDDED_DATA | ||
| && onSelectGriddedLocation | ||
| ) { | ||
| map.setView(latLng, SEARCH_DEFAULT_ZOOM); | ||
| onSelectGriddedLocation({ | ||
| latlng: L.latLng(latLng.lat, latLng.lng), | ||
| title: locationByCoords.title, | ||
| }); | ||
| return; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Basically the same pattern as when selecting one of the autocomplete suggestion, but this time for when it's raw coordinates.
57d10a9 to
d5f0fd3
Compare
ChaamC
approved these changes
May 13, 2026
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.
Popup Title Display Discrepancy When InteractiveRegionOption.GRIDDED_DATA
Summary
Pass 4 minimal hot-fix synthesis for CLIM-1322. Cherry-picks the load-bearing fixes from Passes 2 and 3, adds two native commits (Atoms 15 & 16) to close DoD-1 (name parity EN/FR for UC-Search) and DoD-2 (raw lat/lng pin precision for UC-RawCoordPaste) within
InteractiveRegionOption.GRIDDED_DATA. Polygon modes (CENSUS / HEALTH / WATERSHED) untouched. No SQL filter rewrite — that work remains parked onCLIM-1322_3_Popup-Title-Name-Resolutionfor a future ticket.Branched from
origin/mainat2044c88d. Diff vs main: 9 files, ~140 added / ~21 removed.Use cases addressed (GRIDDED_DATA)
${item.text}, ${item.province_short}(e.g."Montréal, QC"). One API call:/wp-json/cdc/v2/location_search?q=…. No second/get_location_by_coordsround-trip. Closes DoD-1.handleClickflow.lat, lng)showLocation → moveToLocation → handleLocationChange → dispatchMapClick → containerPointToLatLngchain that previously produced ~80m drift. Polygon modes preserve existing behavior (drift benign at polygon scale; title fromlayer.properties.label_*regardless). Closes DoD-2.Atoms
cdc_get_location_by_coordsSELECT aliasesprovince_frasprovince. Eliminates FRUndefined array key "province"warning and the trailing-comma title ("Sainte-Adèle, "→"Sainte-Adèle, QC").locationNotFoundpasses the readable.titletoshowLocation's second arg instead of the opaque 5-char.geo_id.CLIM-1322_3_…selectGriddedLocation({latlng, title})exported fromuseMapInteractions; prop-chained asonSelectGriddedLocationthroughMap → MapContainer → SearchControl. UC-Search GRIDDED invokes it directly with the autocomplete row's data — no synthetic-click round-trip.CLIM-1322_3_…cdc_location_searchadditively returnsprovince_shortvia the sameshort_province()helper used bycdc_get_location_by_coords. Client:SearchControlLocationItem.province_shorttyped; UC-Search popup title becomes the short-form${item.text}, ${item.province_short}.dispatchMapClickdrops itslatlngparameter — it was always discarded (void latlng;). Clarifies design intent: the function is solely about dispatching a real-browser-equivalentPointerEvent('click')at the map container's geometric center aftersetViewsettles. Updates two call sites (search-control.tsx,recent-locations.tsx).locationNotFound: branches oninteractiveRegion === GRIDDED_DATA && onSelectGriddedLocationto callsetView+onSelectGriddedLocation({latlng: typed, title: server-resolved})+ return. Bypasses leaflet-search'sshowLocationchain. Polygon-modeelsebranch preserves the existingshowLocation+ spread-override path.Definition of Done
province_short→ same short-form popup title regardless of UI language. Example: Laval rowEGYIC→"Laval, QC"whether the user typed in EN or FR autocomplete.latLngdirectly toselectGriddedLocation, bypassing the synthetic-click reprojection chain that produced ~80m drift viacontainerPointToLatLngon container-center pixel coords.Out of scope (parked for follow-up)
gen_termtier preferences — full design and probe-verification onCLIM-1322_3_Popup-Title-Name-Resolution(not merged). Would address T28 (Park-class defects: McMasterville → "Parc Ensoleillé", Bedford → "Parc des Bouts-de-Choux") for UC-MapClick and UC-LocateMe. UC-Search T28 dissolves structurally in this PR via Atom 14 because the autocomplete row never round-trips throughcdc_get_location_by_coords.dispatchMapClick → moveAndPointAtrename + per-map WeakMap precise-latlng channel — Iteration 2's Atoms 6/8/9. Out of scope here because UC-Search bypasses the synthetic-click pipeline (no consumer in scope) and UC-RawCoordPaste GRIDDED takes the early-return path in Atom 16. Logically a single follow-up unit if a future ticket needs precise pin placement under the synthetic-click path.MapFeaturePropertiestyping migration — parked on a separate local branchstashed/MapFeatureProperties(single pure-typing commit). Pure refactor; pick up in a typing-only PR.Verification
./dev.sh typecheck-appsclean../dev.sh lint-appsclean on touched files (one pre-existing warning on an untouched line inMap'suseEffectdeps).dev-enanddev-frfor the 16-anchor reference table (Vancouver / Montréal / Halifax / Sainte-Adèle / Toronto / Edmonton / Calgary / Victoria / Ottawa / Québec / Charlottetown / St. John's / Saskatoon / Banff / Bedford / McMasterville) confirm short-form popup titles for UC-Search GRIDDED.lat,lonraw coord tuples (UC-RawCoordPaste) at five precision levels (2 to 6 decimal places) confirmed marker matches typed coord after Atom16 — the empirical signature that previously identified the ~80m drift (identical post-setViewviewport-center across coords differing only in low-order decimals) is gone.Related
Ticket: CLIM-1322
Parked branches preserved as research:
CLIM-1322_3_Popup-Title-Name-Resolutionstashed/MapFeaturePropertiesSupersedes earlier Passes:
Pass 1 CLIM-1322: Map search popup title (Illustration Keep As-Is, or Rework?) #674 — Illustrative of a solution path.
Pass 2 full SQL filter rewrite, kept as parked research
CLIM-1322_2_Popup-Title-Name-ResolutionPass 3 CLIM-1322 (3): Popup Title Display Discrepancy When InteractiveRegionOption.GRIDDED_DATA #684 — continuation of Pass 2 but introduced
ORDER BYranking and this change would solve other problems identified such as clicking on the map to very close-by location (pixel on the screen at a high zoom level) with wildly different popup titles