-
-
Notifications
You must be signed in to change notification settings - Fork 7
Coordinate Rendering and Map Panning #434
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
base: main
Are you sure you want to change the base?
Changes from all commits
424fbcd
2dd6df7
9ae874f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| 'use client'; | ||
|
|
||
| import React, { useEffect } from 'react'; | ||
| import { useMapData } from './map-data-context'; | ||
|
|
||
| interface CoordinateLinkProps { | ||
| lat: number; | ||
| lng: number; | ||
| label?: string; | ||
| } | ||
|
|
||
| export const CoordinateLink: React.FC<CoordinateLinkProps> = ({ lat, lng, label }) => { | ||
| const { setMapData } = useMapData(); | ||
| const id = `${lat},${lng}`; | ||
|
|
||
| useEffect(() => { | ||
| // Automatically add marker and pan to it when the component mounts | ||
| setMapData(prev => { | ||
| const exists = prev.markers?.some(m => m.id === id); | ||
| const newMarkers = exists ? prev.markers : [...(prev.markers || []), { id, latitude: lat, longitude: lng, title: label || id }]; | ||
|
|
||
| return { | ||
| ...prev, | ||
| targetPosition: { lat, lng }, | ||
| markers: newMarkers | ||
| }; | ||
| }); | ||
| }, [lat, lng, id, label, setMapData]); | ||
|
Comment on lines
+16
to
+28
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. The This is especially problematic because SuggestionConsider not panning on mount. Split responsibilities:
Example: useEffect(() => {
setMapData(prev => {
const exists = prev.markers?.some(m => m.id === id)
if (exists) return prev
return {
...prev,
markers: [
...(prev.markers || []),
{ id, latitude: lat, longitude: lng, title: label || id }
]
}
})
}, [id, lat, lng, label, setMapData])
const handleClick = (e: React.MouseEvent) => {
e.preventDefault()
setMapData(prev => ({ ...prev, targetPosition: { lat, lng } }))
}If you still want auto-pan, gate it behind a flag (e.g. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion. |
||
|
|
||
| const handleClick = (e: React.MouseEvent) => { | ||
| e.preventDefault(); | ||
| setMapData(prev => ({ | ||
| ...prev, | ||
| targetPosition: { lat, lng } | ||
| })); | ||
| }; | ||
|
|
||
| return ( | ||
| <a | ||
| href={`https://www.google.com/maps/search/?api=1&query=${lat},${lng}`} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| onClick={handleClick} | ||
| className="text-primary font-semibold underline decoration-solid hover:text-primary/80 transition-colors cursor-pointer inline-flex items-center gap-1" | ||
| > | ||
| <span className="bg-primary/10 px-1 rounded">{label || `${lat.toFixed(4)}, ${lng.toFixed(4)}`}</span> | ||
| </a> | ||
| ); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,16 +36,23 @@ export const MapQueryHandler: React.FC<MapQueryHandlerProps> = ({ toolOutput }) | |
|
|
||
| if (typeof latitude === 'number' && typeof longitude === 'number') { | ||
| console.log(`MapQueryHandler: Received data from geospatialTool. Place: ${place_name}, Lat: ${latitude}, Lng: ${longitude}`); | ||
| setMapData(prevData => ({ | ||
| ...prevData, | ||
| targetPosition: { lat: latitude, lng: longitude }, | ||
| // Optionally store more info from mcp_response if needed by MapboxMap component later | ||
| mapFeature: { | ||
| place_name, | ||
| // Potentially add mapUrl or other details from toolOutput.mcp_response | ||
| mapUrl: toolOutput.mcp_response?.mapUrl | ||
| } | ||
| })); | ||
| const id = `${latitude},${longitude}`; | ||
| setMapData(prevData => { | ||
| const exists = prevData.markers?.some(m => m.id === id); | ||
| const newMarkers = exists ? prevData.markers : [...(prevData.markers || []), { id, latitude, longitude, title: place_name || id }]; | ||
|
|
||
| return { | ||
| ...prevData, | ||
| targetPosition: { lat: latitude, lng: longitude }, | ||
| markers: newMarkers, | ||
| // Optionally store more info from mcp_response if needed by MapboxMap component later | ||
| mapFeature: { | ||
| place_name, | ||
| // Potentially add mapUrl or other details from toolOutput.mcp_response | ||
| mapUrl: toolOutput.mcp_response?.mapUrl | ||
| } | ||
| }; | ||
| }); | ||
|
Comment on lines
+39
to
+55
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.
Also, SuggestionIntroduce and reuse a shared helper to generate marker IDs with fixed precision, e.g. const id = coordId(latitude, longitude)That keeps deduping stable and avoids near-duplicate markers. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion. |
||
| } else { | ||
| console.warn("MapQueryHandler: Invalid latitude/longitude in toolOutput.mcp_response:", toolOutput.mcp_response.location); | ||
| // Clear target position if data is invalid | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,35 @@ | ||
| $ next dev --turbo | ||
| ⚠ Port 3000 is in use, using available port 3003 instead. | ||
|
|
||
| > QCX@0.1.0 dev | ||
| > next dev --turbo | ||
|
|
||
| ▲ Next.js 15.3.6 (Turbopack) | ||
| - Local: http://localhost:3003 | ||
| - Network: http://192.168.0.2:3003 | ||
| - Environments: .env.local, .env | ||
| - Local: http://localhost:3000 | ||
| - Network: http://192.168.0.2:3000 | ||
| - Environments: .env | ||
|
|
||
| ✓ Starting... | ||
| ○ Compiling middleware ... | ||
| ✓ Compiled middleware in 648ms | ||
| ✓ Ready in 2.5s | ||
| ✓ Compiled middleware in 359ms | ||
| ✓ Ready in 1716ms | ||
| ○ Compiling / ... | ||
| ✓ Compiled / in 27.5s | ||
| Chat DB actions loaded. Ensure getCurrentUserId() is correctly implemented for server-side usage if applicable. | ||
| [Upstash Redis] The 'url' property is missing or undefined in your Redis config. | ||
| [Upstash Redis] The 'token' property is missing or undefined in your Redis config. | ||
| GET / 200 in 31388ms | ||
| GET / 200 in 934ms | ||
| ○ Compiling /search/[id] ... | ||
| ✓ Compiled /search/[id] in 12.1s | ||
| Chat DB actions loaded. Ensure getCurrentUserId() is correctly implemented for server-side usage if applicable. | ||
| [Upstash Redis] The 'url' property is missing or undefined in your Redis config. | ||
| [Upstash Redis] The 'token' property is missing or undefined in your Redis config. | ||
| ⨯ [TypeError: action is not a function] { digest: '2341665709' } | ||
| POST /search/4hlC3SIEkZPxlRUV5KUmP 500 in 15389ms | ||
| [DEBUG] getSelectedModel - Reading from path: "/app/config/model.json" | ||
| [DEBUG] getSelectedModel - Raw file content: "{ | ||
| "selectedModel": null | ||
| } | ||
| " | ||
| POST /search/4hlC3SIEkZPxlRUV5KUmP 200 in 945ms | ||
| Chat DB actions loaded. Ensure getCurrentUserId() is correctly implemented for server-side usage if applicable. | ||
| [Upstash Redis] The 'url' property is missing or undefined in your Redis config. | ||
| [Upstash Redis] The 'token' property is missing or undefined in your Redis config. |
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.
The
idis derived from rawlat,lngnumbers. The same real-world coordinate can appear with different precision (e.g.37.77vs37.7700) and become duplicate markers. Also, floating-point stringification can be inconsistent across sources.A stable ID strategy (rounding to a fixed precision for IDs, or using an explicit unique ID from the tool when available) will prevent marker spam and make dedupe reliable.
Suggestion
Normalize coordinate IDs to a fixed precision (and consider including a namespace) so dedupe is consistent:
Then use the same normalization in
MapQueryHandlerwhen generating IDs.Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.