Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 92
fix(google-maps): re-init on color-mode change for cloud-styled mapIds#727
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
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
16 changes: 16 additions & 0 deletions
16 docs/content/scripts/google-maps/1.guides/2.map-styling.md
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
59 changes: 55 additions & 4 deletions
59 packages/script/src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue
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 |
|---|---|---|
| @@ -329,9 +329,11 @@ describe('google Maps Regressions', () => { | ||
| map.setOptions(options) | ||
| } | ||
| // Simulate the fixed watcher: strips center and zoom before calling setOptions | ||
| // Simulate the fixed watcher: strips center, zoom, mapId, and colorScheme | ||
| // before calling setOptions. mapId/colorScheme are init-only in Google Maps | ||
| // and are handled by a dedicated re-init watcher (see #726 regression suite). | ||
| function applyOptionsFixed(map: ReturnType<typeof createMockMap>, options: Record<string, any>) { | ||
| const { center: _, zoom: __, ...rest } = options | ||
| const { center: _, zoom: __, mapId: ___, colorScheme: ____, ...rest } = options | ||
| map.setOptions(rest) | ||
| } | ||
| @@ -348,19 +350,22 @@ describe('google Maps Regressions', () => { | ||
| ) | ||
| }) | ||
| it('fixed behavior: setOptions excludes zoomand center', () => { | ||
| it('fixed behavior: setOptions excludes zoom, center, mapId, and colorScheme', () => { | ||
| const map = createMockMap() | ||
| const options = { center: { lat: 40, lng: -74 }, zoom: 12, mapId: 'abc' } | ||
| const options = { center: { lat: 40, lng: -74 }, zoom: 12, mapId: 'abc', disableDefaultUI: true } | ||
| applyOptionsFixed(map, options) | ||
| expect(map.setOptions).toHaveBeenCalledWith({ mapId: 'abc' }) | ||
| expect(map.setOptions).toHaveBeenCalledWith({ disableDefaultUI: true }) | ||
| expect(map.setOptions).not.toHaveBeenCalledWith( | ||
| expect.objectContaining({ center: expect.anything() }), | ||
| ) | ||
| expect(map.setOptions).not.toHaveBeenCalledWith( | ||
| expect.objectContaining({ zoom: expect.anything() }), | ||
| ) | ||
| expect(map.setOptions).not.toHaveBeenCalledWith( | ||
| expect.objectContaining({ mapId: expect.anything() }), | ||
| ) | ||
| }) | ||
| it('old behavior: repeated overlay toggles reset zoom/center every time', () => { | ||
| @@ -502,4 +507,118 @@ describe('google Maps Regressions', () => { | ||
| expect(iw.close).not.toHaveBeenCalled() | ||
| }) | ||
| }) | ||
| describe('color-mode reactivity for cloud-based map IDs (#726)', () => { | ||
| // Regression: toggling color mode with `mapIds` set (or with cloud-based | ||
| // styling on a single mapId) did not update the map. The old code passed | ||
| // the resolved mapId via `setOptions`, which Google Maps refuses | ||
| // ("A Map's mapId property cannot be changed after initial Map render"). | ||
| // Both `mapId` and `colorScheme` are init-only; the fix excludes them | ||
| // from the generic setOptions call and re-initialises the Map instance | ||
| // when either changes. | ||
| function resolveMapId(props: { | ||
| mapIds?: { light?: string, dark?: string } | ||
| mapOptions?: { mapId?: string } | ||
| }, colorMode: 'light' | 'dark') { | ||
| if (!props.mapIds) | ||
| return props.mapOptions?.mapId | ||
| return props.mapIds[colorMode] || props.mapIds.light || props.mapOptions?.mapId | ||
| } | ||
| function resolveColorScheme(props: { | ||
| mapIds?: { light?: string, dark?: string } | ||
| colorMode?: 'light' | 'dark' | ||
| hasNuxtColorMode?: boolean | ||
| }, currentColorMode: 'light' | 'dark') { | ||
| if (!props.mapIds && !props.colorMode && !props.hasNuxtColorMode) | ||
| return undefined | ||
| return currentColorMode === 'dark' ? 'DARK' : 'LIGHT' | ||
| } | ||
| function applyOptionsFixed(map: ReturnType<typeof createMockMap>, options: Record<string, any>) { | ||
| const { center: _, zoom: __, mapId: ___, colorScheme: ____, ...rest } = options | ||
| map.setOptions(rest) | ||
| } | ||
| it('strips mapId and colorScheme from setOptions to avoid the init-only warning', () => { | ||
| const map = createMockMap() | ||
| const options = { | ||
| center: { lat: 40, lng: -74 }, | ||
| zoom: 12, | ||
| mapId: 'abc', | ||
| colorScheme: 'DARK', | ||
| disableDefaultUI: true, | ||
| } | ||
| applyOptionsFixed(map, options) | ||
| expect(map.setOptions).toHaveBeenCalledWith({ disableDefaultUI: true }) | ||
| expect(map.setOptions).not.toHaveBeenCalledWith( | ||
| expect.objectContaining({ mapId: expect.anything() }), | ||
| ) | ||
| expect(map.setOptions).not.toHaveBeenCalledWith( | ||
| expect.objectContaining({ colorScheme: expect.anything() }), | ||
| ) | ||
| }) | ||
coderabbitai[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| it('resolves a different mapId per color mode when both light and dark are provided', () => { | ||
| const props = { mapIds: { light: 'LIGHT_ID', dark: 'DARK_ID' } } | ||
| expect(resolveMapId(props, 'light')).toBe('LIGHT_ID') | ||
| expect(resolveMapId(props, 'dark')).toBe('DARK_ID') | ||
| }) | ||
| it('emits a colorScheme so a single mapId with cloud-based light/dark styling can re-init', () => { | ||
| // User configured one mapId in Cloud Console with both Light and Dark | ||
| // schemes. mapIds resolves to the same id in both modes, so the only | ||
| // signal that triggers re-init is the colorScheme value. | ||
| const props = { mapIds: { light: 'SAME_ID', dark: 'SAME_ID' } } | ||
| expect(resolveMapId(props, 'light')).toBe('SAME_ID') | ||
| expect(resolveMapId(props, 'dark')).toBe('SAME_ID') | ||
| expect(resolveColorScheme(props, 'light')).toBe('LIGHT') | ||
| expect(resolveColorScheme(props, 'dark')).toBe('DARK') | ||
| }) | ||
| it('does not emit a colorScheme when no color-mode props or @nuxtjs/color-mode are present', () => { | ||
| // Avoid forcing a LIGHT scheme on existing maps that never opted in to | ||
| // color-mode reactivity β would otherwise needlessly re-init on first | ||
| // mount or accidentally override mapOptions.colorScheme. | ||
| expect(resolveColorScheme({}, 'light')).toBeUndefined() | ||
| }) | ||
| it('emits a colorScheme when @nuxtjs/color-mode is detected even without explicit mapIds', () => { | ||
| expect(resolveColorScheme({ hasNuxtColorMode: true }, 'dark')).toBe('DARK') | ||
| }) | ||
| it('triggers re-init only when the resolved mapId or colorScheme actually changes', () => { | ||
| // Mirrors the dedup guard in the recreate watcher. | ||
| function shouldReinit( | ||
| prev: { mapId: string | undefined, scheme: string | undefined }, | ||
| next: { mapId: string | undefined, scheme: string | undefined }, | ||
| ) { | ||
| return prev.mapId !== next.mapId || prev.scheme !== next.scheme | ||
| } | ||
| // Identical β no re-init (covers e.g. unrelated re-renders that re-evaluate the options computed). | ||
| expect(shouldReinit( | ||
| { mapId: 'abc', scheme: 'LIGHT' }, | ||
| { mapId: 'abc', scheme: 'LIGHT' }, | ||
| )).toBe(false) | ||
| // mapId changes (two-id light/dark setup). | ||
| expect(shouldReinit( | ||
| { mapId: 'LIGHT_ID', scheme: 'LIGHT' }, | ||
| { mapId: 'DARK_ID', scheme: 'DARK' }, | ||
| )).toBe(true) | ||
| // Single mapId, only colorScheme changes (cloud styling on one id). | ||
| expect(shouldReinit( | ||
| { mapId: 'SAME_ID', scheme: 'LIGHT' }, | ||
| { mapId: 'SAME_ID', scheme: 'DARK' }, | ||
| )).toBe(true) | ||
| }) | ||
| }) | ||
| }) | ||
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.