-
Notifications
You must be signed in to change notification settings - Fork 2
CLIM-1322 (4): Popup Title Display Discrepancy When InteractiveRegionOption.GRIDDED_DATA #685
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
e0ea993
CLIM-1322 (4): BEGIN
renoirb 94b2f94
CLIM-1322 (2): Atom1 Alias province_fr as province
renoirb 9790fff
CLIM-1322 (2): Atom3 .title for Popup not .geo_id
renoirb 851abd9
CLIM-1322 (3): Atom14 Use autocomplete row for gridded popup title
renoirb d50d085
CLIM-1322 (3): Atom12 Use short-form title for UC-Search popup; expos…
renoirb 1a716dd
CLIM-1322 (4): Atom15 dispatchMapClick is only about dispatching DOM …
renoirb f9bcbaf
CLIM-1322 (4): Comments to add clarity
renoirb 3b6bc0b
CLIM-1322 (4): Atom16 UC-RawCoordPaste GRIDDED pin precision (DoD-2)
renoirb d5f0fd3
CLIM-1322 (4): Comments to add clarity
renoirb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ import { | |
| useCallback, | ||
| useEffect, | ||
| useMemo, | ||
| useRef, | ||
| useState, | ||
| } from 'react'; | ||
| import { __ } from '@/context/locale-provider'; | ||
|
|
@@ -20,13 +21,15 @@ import 'leaflet-search/dist/leaflet-search.min.css'; | |
| import 'leaflet-search'; | ||
| import mapPinIcon from '@/assets/map-pin.svg'; | ||
|
|
||
| import { useClimateVariable } from '@/hooks/use-climate-variable'; | ||
| import { cn, parseLatLon } from '@/lib/utils'; | ||
| import { dispatchMapClick } from '@/lib/dispatch-map-click'; | ||
| import { fetchLocationByCoords } from '@/services/services'; | ||
| import { | ||
| type SearchControlLocationItem, | ||
| type SearchControlResponse, | ||
| } from '@/types/types'; | ||
| import { InteractiveRegionOption } from '@/types/climate-variable-interface'; | ||
| import { | ||
| LOCATION_SEARCH_ENDPOINT, | ||
| SEARCH_DEFAULT_ZOOM, | ||
|
|
@@ -57,6 +60,14 @@ function convertSearchLatLng(inputLatLng: SearchLatLng): L.LatLng { | |
| */ | ||
| export interface SearchControlProps { | ||
| className?: string; | ||
| /** | ||
| * Set selected location directly from an autocomplete pick when the | ||
| * climate variable's interactive region is GRIDDED_DATA. Bypasses the | ||
| * synthetic-click + reverse-coords-lookup path. | ||
| */ | ||
| onSelectGriddedLocation?: ( | ||
| input: { latlng: L.LatLng; title: string }, | ||
| ) => void; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -71,7 +82,16 @@ export interface SearchControlProps { | |
| */ | ||
| const SearchControl = ({ | ||
| className, | ||
| onSelectGriddedLocation, | ||
| }: SearchControlProps): ReactElement => { | ||
| const { climateVariable } = useClimateVariable(); | ||
| // Captured at the point formatData() builds entries, keyed by the same | ||
| // title string leaflet-search later passes back to moveToLocation(). | ||
| // A closure-scoped Map keeps lookup local to the search lifecycle | ||
| // and avoids any cross-render leakage. | ||
| const formattedItemsRef = useRef<Map<string, SearchControlLocationItem>>( | ||
| new Map(), | ||
| ); | ||
| const [isGeolocationEnabled, setIsGeolocationEnabled] = | ||
| useState<boolean>(false); | ||
| const [isTracking, setIsTracking] = useState<boolean>(false); | ||
|
|
@@ -95,7 +115,10 @@ const SearchControl = ({ | |
| const map = useMap(); | ||
|
|
||
| const handleLocationChange = useCallback( | ||
| async (inputLatlng: SearchLatLng) => { | ||
| async ( | ||
| inputLatlng: SearchLatLng, | ||
| autocompleteItem?: { item: SearchControlLocationItem; title: string }, | ||
| ) => { | ||
| const latlng = convertSearchLatLng(inputLatlng); | ||
| // clear all existing markers from the map | ||
| map.eachLayer(layer => { | ||
|
|
@@ -104,10 +127,32 @@ const SearchControl = ({ | |
| } | ||
| }); | ||
| map.setView(latlng, SEARCH_DEFAULT_ZOOM); | ||
| await dispatchMapClick(map, latlng); | ||
|
|
||
| const interactiveRegion = climateVariable?.getInteractiveRegion() | ||
| ?? InteractiveRegionOption.GRIDDED_DATA; | ||
|
|
||
| // The autocomplete row already has a usable title — set the selected location directly. | ||
| if ( | ||
| autocompleteItem | ||
| && interactiveRegion === InteractiveRegionOption.GRIDDED_DATA | ||
| && onSelectGriddedLocation | ||
| ) { | ||
| onSelectGriddedLocation({ | ||
| latlng, | ||
| title: autocompleteItem.title, | ||
| }); | ||
| // Skip the synthetic-click reverse-lookup path that the other modes need. | ||
| return; | ||
| } | ||
|
|
||
| // UC-LocateMe, UC-RawCoordPaste, and all polygon modes: | ||
| // fall through to existing behaviour from main. | ||
| await dispatchMapClick(map); | ||
| }, | ||
| [ | ||
| climateVariable, | ||
| map, | ||
| onSelectGriddedLocation, | ||
| ], | ||
| ); | ||
|
|
||
|
|
@@ -205,6 +250,11 @@ const SearchControl = ({ | |
| SearchControlLocationItem & { loc: number[]; lng: string; } | ||
| > = {}; | ||
|
|
||
| // Reset and rebuild the title→item index for the current | ||
| // suggestion set, so moveToLocation can look the row up by | ||
| // title without round-tripping through the server. | ||
| formattedItemsRef.current.clear(); | ||
|
|
||
| response.items.forEach((item: SearchControlLocationItem) => { | ||
| const title = buildLocationTitle(item); | ||
| const loc = [parseFloat(item.lat), parseFloat(item.lon)]; | ||
|
|
@@ -213,6 +263,7 @@ const SearchControl = ({ | |
| lng: item.lon, | ||
| loc, | ||
| }; | ||
| formattedItemsRef.current.set(title, item); | ||
| }); | ||
|
|
||
| return formattedData; | ||
|
|
@@ -221,21 +272,59 @@ const SearchControl = ({ | |
| void _; // intentionally ignore to suppress typescript error | ||
| return `<div>${buildLocationTitle(item)}</div>`; | ||
| }, | ||
|
|
||
| /** | ||
| * Search by raw "lat,lon" coordinates or anything else entry point. | ||
| * | ||
| * The autocomplete-hit path (UC-Search) never reaches here; it lands | ||
| * in `moveToLocation` above with a matching row in formattedItemsRef. | ||
| */ | ||
| locationNotFound: async function() { | ||
| // If the list of suggestions is still shown, no error message is shown. | ||
| // See #86862 | ||
| if (Object.keys(this._recordsCache).length > 0) { | ||
| this.showTooltip(this._recordsCache) | ||
| return; | ||
| } | ||
| // Check if the coordinates are valid if the location is empty. | ||
| const latLng = parseLatLon(this._input.value); | ||
|
|
||
| // parseLatLon is both PARSER and DETECTOR: a non-partial result means | ||
| // the input was a valid "lat, lon" string. | ||
| const parsedSearchControlInput = 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 (parsedSearchControlInput && !parsedSearchControlInput.isPartial) { | ||
| const latLng = new L.LatLng(parsedSearchControlInput.lat, parsedSearchControlInput.lon); | ||
| const locationByCoords = await fetchLocationByCoords(latLng); | ||
|
|
||
| // Bypass leaflet-search's showLocation pipeline for GRIDDED mode so the | ||
| // marker lands at the typed coordinates rather than dispatchMapClick | ||
| // calculated center (container-center reprojection). | ||
| // Other `InteractiveRegionOption.` modes (CENSUS/HEALTH/WATERSHED) | ||
| // fall through 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: latLng, | ||
| title: locationByCoords.title, | ||
| }); | ||
| return; | ||
| } | ||
|
Comment on lines
+298
to
+318
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Basically the same pattern as when selecting one of the autocomplete suggestion, but this time for when it's raw coordinates. |
||
|
|
||
| this.showLocation( | ||
| { | ||
| ...locationByCoords, | ||
| lat: latLng.lat, | ||
| lng: latLng.lng, | ||
| }, | ||
| locationByCoords.title, | ||
| ); | ||
| } | ||
| else { | ||
| // Show error alert. | ||
|
|
@@ -250,8 +339,24 @@ const SearchControl = ({ | |
| popupAnchor: [0, -41], // Popup position relative to the icon | ||
| }), | ||
| }), | ||
| moveToLocation: (latlng: SearchLatLng) => { | ||
| handleLocationChange(latlng); | ||
| moveToLocation: (latlng: SearchLatLng, title: string) => { | ||
| // leaflet-search invokes this with (latlng, title, map); | ||
| // `title` is the same key formatData() used in formattedData, | ||
| // (the verbose buildLocationTitle output, kept for the | ||
| // dropdown display), so we can recover the original | ||
| // autocomplete row from it. | ||
| const item = formattedItemsRef.current.get(title); | ||
| if (item) { | ||
| // UC-Search popup title uses the short server-resolved | ||
| // shape ("Montréal, QC"), matching cdc_get_location_by_coords, | ||
| // not the verbose dropdown display. | ||
| const popupTitle = `${item.text}, ${item.province_short}`; | ||
| handleLocationChange(latlng, { item, title: popupTitle }); | ||
| } else { | ||
| // No matching row (e.g. raw lat/lon paste branch from | ||
| // locationNotFound) — fall through to existing behaviour. | ||
| handleLocationChange(latlng); | ||
| } | ||
| }, | ||
| }); | ||
|
|
||
|
|
||
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
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have the impression that's what opens up the PopUp thing. And the lat,lon at.