feat/add button to reset map to initial viewport#52
feat/add button to reset map to initial viewport#52henriwoodcock wants to merge 1 commit intoAnmolSaini16:mainfrom
Conversation
|
@henriwoodcock is attempting to deploy a commit to the mapcn oss program Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe changes add a new optional Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
|
@AnmolSaini16 please review |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/registry/map.tsx`:
- Around line 784-788: The effect currently captures the initial viewport as
soon as `map` exists; change the destructuring from `useMap()` in `MapControls`
to also pull `isLoaded` and update the `useEffect` to only set
`initialViewportRef.current = getViewport(map)` when `map` is defined AND
`isLoaded` is true, using the same pattern as `MapRoute` (i.e., check `isLoaded`
before calling `getViewport`), so the initial camera is captured after the
MapLibre `load` event finalizes the camera state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c36320f2-392c-4302-ab14-c894226a724f
📒 Files selected for processing (3)
src/app/docs/_components/examples/map-controls-example.tsxsrc/app/docs/api-reference/page.tsxsrc/registry/map.tsx
| useEffect(() => { | ||
| if (map && !initialViewportRef.current) { | ||
| initialViewportRef.current = getViewport(map); | ||
| } | ||
| }, [map]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -n 900 src/registry/map.tsx | tail -n 130 | cat -nRepository: AnmolSaini16/mapcn
Length of output: 4641
🏁 Script executed:
sed -n '1,50p' src/registry/map.tsx | cat -nRepository: AnmolSaini16/mapcn
Length of output: 1825
🏁 Script executed:
sed -n '710,790p' src/registry/map.tsx | cat -nRepository: AnmolSaini16/mapcn
Length of output: 2993
🏁 Script executed:
sed -n '830,850p' src/registry/map.tsx | cat -nRepository: AnmolSaini16/mapcn
Length of output: 728
🏁 Script executed:
# Search for the Map component definition and its initialization
rg -n "function Map|const Map" src/registry/map.tsx | head -20Repository: AnmolSaini16/mapcn
Length of output: 319
🏁 Script executed:
# Find the getViewport function definition
rg -n "function getViewport|const getViewport" src/registry/map.tsxRepository: AnmolSaini16/mapcn
Length of output: 122
🏁 Script executed:
# Search for MapViewport type definition
rg -n "type MapViewport|interface MapViewport" src/registry/map.tsxRepository: AnmolSaini16/mapcn
Length of output: 86
🏁 Script executed:
# Check if there's any 'load' or 'idle' event handling in the file
rg -n "\.on\(|load|idle" src/registry/map.tsx | head -30Repository: AnmolSaini16/mapcn
Length of output: 1298
🏁 Script executed:
# Get more context around the Map component initialization
sed -n '80,250p' src/registry/map.tsx | cat -nRepository: AnmolSaini16/mapcn
Length of output: 6362
🏁 Script executed:
# Check more context around Map component initialization and load event
sed -n '164,280p' src/registry/map.tsx | cat -nRepository: AnmolSaini16/mapcn
Length of output: 4341
🏁 Script executed:
# Check how isLoaded is provided to MapControls through context
sed -n '300,360p' src/registry/map.tsx | cat -nRepository: AnmolSaini16/mapcn
Length of output: 2170
🏁 Script executed:
# Check what MapControls receives from useMap hook
rg -n "useMap\(\)" src/registry/map.tsxRepository: AnmolSaini16/mapcn
Length of output: 298
🏁 Script executed:
# Check MapLibre documentation reference about defaults and load event
# Also verify that isLoaded is reliably set before camera is resolved
sed -n '164,165p' src/registry/map.tsx | cat -nRepository: AnmolSaini16/mapcn
Length of output: 82
🏁 Script executed:
# Verify the exact MapLibre version to confirm load event behavior
rg -n "maplibre" package.jsonRepository: AnmolSaini16/mapcn
Length of output: 94
🏁 Script executed:
# Verify MapLibre 5.15.0 behavior by checking if there's documentation about the load event
# and when camera properties are finalized
web_search "maplibre-gl 5.15 load event camera initialization"Repository: AnmolSaini16/mapcn
Length of output: 112
Wait for isLoaded before capturing the initial viewport.
The useEffect at lines 775–779 captures initialViewportRef.current = getViewport(map) as soon as the map instance becomes available, but MapLibre may not have finished resolving the initial camera state. The load event (registered in the Map component at line 88) signals when the camera is finalized.
Instead of moving the capture logic, MapControls already has access to the isLoaded flag via useMap() (see MapRoute at line 1091 for the pattern). Update the destructuring to include isLoaded and only capture the viewport when isLoaded is true.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/registry/map.tsx` around lines 784 - 788, The effect currently captures
the initial viewport as soon as `map` exists; change the destructuring from
`useMap()` in `MapControls` to also pull `isLoaded` and update the `useEffect`
to only set `initialViewportRef.current = getViewport(map)` when `map` is
defined AND `isLoaded` is true, using the same pattern as `MapRoute` (i.e.,
check `isLoaded` before calling `getViewport`), so the initial camera is
captured after the MapLibre `load` event finalizes the camera state.
Summary by CodeRabbit
New Features
Documentation
showResetprop for MapControls component.