fix(google-maps): prevent zoom/pan reset when overlay toggles#685
fix(google-maps): prevent zoom/pan reset when overlay toggles#685
Conversation
The options watcher called setOptions with initial zoom and center values on every re-evaluation, resetting user interactions when parent components re-rendered (e.g. overlay open/close toggling state). Split zoom and center into dedicated watchers that only fire on actual prop value changes.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
There was a problem hiding this comment.
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📝 WalkthroughWalkthroughThis pull request modifies the Google Maps component to separately handle zoom updates from other map options. The component's options watcher is updated to exclude Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes 🚥 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 docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/unit/google-maps-regressions.test.ts (1)
320-400: Add one regression for the new dedicated zoom watcher path.This suite verifies
setOptionsno longer carrieszoom/center, but it doesn’t assert the newsetZoomwatcher behavior itself (zoom change vs unrelated re-render). Adding that would lock in the full fix contract.✅ Suggested additional test
describe('map setOptions should not reset zoom or center', () => { + it('fixed behavior: zoom watcher updates zoom only when zoom value changes', () => { + const map = createMockMap() + let prevZoom: number | undefined + + function applyZoomWatcher(zoom: number | undefined) { + if (zoom !== prevZoom && zoom != null) + map.setZoom(zoom) + prevZoom = zoom + } + + applyZoomWatcher(12) // initial controlled value + applyZoomWatcher(12) // unrelated re-render, no zoom change + applyZoomWatcher(13) // actual prop change + + expect(map.setZoom).toHaveBeenCalledTimes(2) + expect(map.setZoom).toHaveBeenNthCalledWith(1, 12) + expect(map.setZoom).toHaveBeenNthCalledWith(2, 13) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/google-maps-regressions.test.ts` around lines 320 - 400, Add a new test that verifies the dedicated zoom watcher calls map.setZoom only when the zoom value actually changes and not on unrelated re-renders: create a mock map via createMockMap(), call applyOptionsFixed(map, options) with the same zoom repeatedly and assert map.setZoom was not called, then call applyOptionsFixed(map, { ...options, zoom: newZoom }) and assert map.setZoom was called once with newZoom (and similarly ensure setOptions never received center/zoom). Reference applyOptionsFixed, createMockMap, map.setZoom and map.setOptions to locate where to add the assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/unit/google-maps-regressions.test.ts`:
- Around line 320-400: Add a new test that verifies the dedicated zoom watcher
calls map.setZoom only when the zoom value actually changes and not on unrelated
re-renders: create a mock map via createMockMap(), call applyOptionsFixed(map,
options) with the same zoom repeatedly and assert map.setZoom was not called,
then call applyOptionsFixed(map, { ...options, zoom: newZoom }) and assert
map.setZoom was called once with newZoom (and similarly ensure setOptions never
received center/zoom). Reference applyOptionsFixed, createMockMap, map.setZoom
and map.setOptions to locate where to add the assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e3fad9c7-0577-4b79-8cdc-0cbc29717c08
📒 Files selected for processing (3)
packages/script/src/runtime/components/GoogleMaps/ScriptGoogleMaps.vuetest/unit/__mocks__/google-maps-api.tstest/unit/google-maps-regressions.test.ts
🔗 Linked issue
N/A (user report)
❓ Type of change
📚 Description
The
watch(options)watcher inScriptGoogleMapscalledmap.setOptions()with the full options object (including initialzoomandcentervalues) whenever the computed re-evaluated. Sincedefureturns a new object reference each time, any parent re-render (e.g. from toggling an overlay's open state) would trigger this watcher and reset user pan/zoom interactions back to the initial prop values.The fix excludes
centerandzoomfrom the genericsetOptionscall and adds a dedicatedsetZoomwatcher that only fires when the zoom value actually changes. Center already had its own dedicated watcher.Includes regression tests that reproduce both the old (broken) and new (fixed) behavior.