Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -234,11 +234,15 @@ function makeOverlayClass(mapsApi: typeof google.maps, map: google.maps.Map) {
if (blockMapInteraction)
mapsApi.OverlayView.preventMapHitsAndGesturesFrom(el)
}
if (panOnOpen) {
// Wait for draw() to position the element, then pan
if (panOnOpen && open.value !== false) {
// Wait for draw() to position the element, then pan. Re-check inside
// the rAF callback so we don't pan when:
// - the controlled `open` prop flipped to false during the frame
// - draw() never resolved a position (closed/missing position)
// - the anchor element is gone (component unmounted mid-frame)
const padding = typeof panOnOpen === 'number' ? panOnOpen : 40
requestAnimationFrame(() => {
Comment on lines +237 to 244
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟑 Minor

panOnOpen: 0 is currently treated as disabled.

Line 237 uses a truthy guard (if (panOnOpen && ...)), so panOnOpen={0} won’t schedule panning even though numeric values are documented as valid custom padding. Please switch to an explicit boolean check.

Suggested fix
-      if (panOnOpen && open.value !== false) {
+      if (panOnOpen !== false && open.value !== false) {
         // Wait for draw() to position the element, then pan. Re-check inside
         // the rAF callback so we don't pan when:
         //   - the controlled `open` prop flipped to false during the frame
         //   - draw() never resolved a position (closed/missing position)
         //   - the anchor element is gone (component unmounted mid-frame)
         const padding = typeof panOnOpen === 'number' ? panOnOpen : 40
πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (panOnOpen && open.value !== false) {
// Wait for draw() to position the element, then pan. Re-check inside
// the rAF callback so we don't pan when:
// - the controlled `open` prop flipped to false during the frame
// - draw() never resolved a position (closed/missing position)
// - the anchor element is gone (component unmounted mid-frame)
const padding = typeof panOnOpen === 'number' ? panOnOpen : 40
requestAnimationFrame(() => {
if (panOnOpen !== false && open.value !== false) {
// Wait for draw() to position the element, then pan. Re-check inside
// the rAF callback so we don't pan when:
// - the controlled `open` prop flipped to false during the frame
// - draw() never resolved a position (closed/missing position)
// - the anchor element is gone (component unmounted mid-frame)
const padding = typeof panOnOpen === 'number' ? panOnOpen : 40
requestAnimationFrame(() => {
πŸ€– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/script/src/runtime/components/GoogleMaps/ScriptGoogleMapsOverlayView.vue`
around lines 237 - 244, The guard currently treats panOnOpen falsy values (like
0) as disabled; update the conditional around panOnOpen in the block that
schedules requestAnimationFrame (the if checking panOnOpen && open.value !==
false) to explicitly check for non-null/undefined or boolean trueβ€”e.g. test
panOnOpen !== false && panOnOpen != null (or typeof panOnOpen === 'number' ||
panOnOpen === true) so numeric 0 is accepted; keep the rest of the logic
(calculating padding, calling draw(), and the requestAnimationFrame callback)
unchanged.

if (overlayAnchor.value)
if (open.value !== false && overlayAnchor.value && overlayPosition.value)
panMapToFitOverlay(overlayAnchor.value, map, padding)
})
}
Expand Down
61 changes: 60 additions & 1 deletion test/nuxt-runtime/google-maps-overlay-view.nuxt.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/// <reference types="google.maps" />
import { mountSuspended } from '@nuxt/test-utils/runtime'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { defineComponent, h, nextTick, provide, shallowRef } from 'vue'
import ScriptGoogleMapsOverlayView from '../../packages/script/src/runtime/components/GoogleMaps/ScriptGoogleMapsOverlayView.vue'
import { MAP_INJECTION_KEY, normalizeLatLng } from '../../packages/script/src/runtime/components/GoogleMaps/useGoogleMapsResource'
Expand Down Expand Up @@ -347,4 +347,63 @@ describe('scriptGoogleMapsOverlayView', () => {
expect(content.dataset.state).toBe('open')
})
})

describe('panOnOpen guard for closed/unpositioned overlays', () => {
// Regression: `onAdd()` previously scheduled `panMapToFitOverlay` whenever
// `panOnOpen` was enabled, even if the overlay started closed or never
// resolved a position. That caused unexpected map panning on initial mount
// and remount. The fix gates the rAF scheduling on `open !== false` and
// re-checks `open` + `overlayPosition` inside the callback before panning.

let rafSpy: ReturnType<typeof vi.spyOn>

beforeEach(() => {
rafSpy = vi.spyOn(globalThis, 'requestAnimationFrame')
})

afterEach(() => {
rafSpy.mockRestore()
})

it('does not schedule pan-on-open when overlay starts closed (defaultOpen=false)', async () => {
const mocks = createOverlayMocks()
await mountOverlay(
{ position: { lat: 10, lng: 20 }, defaultOpen: false },
mocks,
)

expect(rafSpy).not.toHaveBeenCalled()
expect(mocks.mockMap.panBy).not.toHaveBeenCalled()
})

it('does not schedule pan-on-open when controlled :open is false on mount', async () => {
const mocks = createOverlayMocks()
await mountOverlay(
{ position: { lat: 10, lng: 20 }, open: false },
mocks,
)

expect(rafSpy).not.toHaveBeenCalled()
expect(mocks.mockMap.panBy).not.toHaveBeenCalled()
})

it('schedules pan-on-open when overlay starts open with a position', async () => {
const mocks = createOverlayMocks()
await mountOverlay({ position: { lat: 10, lng: 20 } }, mocks)

// The guard allows the rAF to be scheduled when the happy path applies
expect(rafSpy).toHaveBeenCalled()
})

it('respects panOnOpen=false even when overlay is open and positioned', async () => {
const mocks = createOverlayMocks()
await mountOverlay(
{ position: { lat: 10, lng: 20 }, panOnOpen: false },
mocks,
)

expect(rafSpy).not.toHaveBeenCalled()
expect(mocks.mockMap.panBy).not.toHaveBeenCalled()
})
})
})
Loading