feat(google-maps): add ScriptGoogleMapsGeoJson component#656
feat(google-maps): add ScriptGoogleMapsGeoJson component#656
Conversation
- Add Data layer mock (createMockDataLayer) with GeoJSON methods - Add DATA_MOUSE_EVENTS and DATA_FEATURE_EVENTS constants - Add simulateGeoJsonLifecycle helper and geoJson test options - Add unit tests covering creation, loading, events, cleanup, lifecycle - Add GeoJson to playground sfcs page
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Vue 3 component Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 (2)
src/runtime/components/GoogleMaps/ScriptGoogleMapsGeoJson.vue (2)
70-73: Style watcher doesn't reset styling whenstylebecomesundefined.If a consumer removes the style prop (sets it to
undefined), the watcher conditionstylebeing truthy preventssetStylefrom being called. The layer retains its previous styling. If this is intentional, consider documenting it; otherwise, you may want to reset to default styling.💡 Optional: Reset to default when style is removed
watch(() => props.style, (style) => { - if (dataLayer.value && style) - dataLayer.value.setStyle(style) + if (dataLayer.value) + dataLayer.value.setStyle(style ?? {}) }, { deep: true })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/GoogleMaps/ScriptGoogleMapsGeoJson.vue` around lines 70 - 73, The watcher on props.style currently only calls dataLayer.value.setStyle(style) when style is truthy, so when the prop is removed the layer keeps previous styles; update the watcher (watch(() => props.style, ...)) to handle falsy/undefined by calling dataLayer.value.setStyle(null) (or undefined per the Maps API) to reset to default styling, e.g. check for dataLayer.value existence and call setStyle(style) when style is provided and setStyle(null) when style is undefined/removed; touch the watcher logic and add a short comment documenting the intended reset behavior.
63-68: Consider addingdeep: trueto thesrcwatcher for object mutations.When
srcis a GeoJSON object, consumers might mutate the object's features array rather than replacing the entire object reference. Withoutdeep: true, such mutations won't trigger a reload.💡 Suggested change
-watch(() => props.src, (src) => { +watch(() => props.src, (src) => { if (!dataLayer.value) return dataLayer.value.forEach(feature => dataLayer.value!.remove(feature)) loadGeoJson(src, dataLayer.value) -}) +}, { deep: true })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/GoogleMaps/ScriptGoogleMapsGeoJson.vue` around lines 63 - 68, The watcher on props.src should observe deep mutations so in the watcher defined for props.src (the callback that clears dataLayer.value and calls loadGeoJson) add the watcher option { deep: true } to ensure modifications inside a GeoJSON object (e.g., changes to features array) trigger the handler; update the watcher invocation where dataLayer.value is referenced and loadGeoJson(src, dataLayer.value) is called to include the deep: true option.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/runtime/components/GoogleMaps/ScriptGoogleMapsGeoJson.vue`:
- Around line 70-73: The watcher on props.style currently only calls
dataLayer.value.setStyle(style) when style is truthy, so when the prop is
removed the layer keeps previous styles; update the watcher (watch(() =>
props.style, ...)) to handle falsy/undefined by calling
dataLayer.value.setStyle(null) (or undefined per the Maps API) to reset to
default styling, e.g. check for dataLayer.value existence and call
setStyle(style) when style is provided and setStyle(null) when style is
undefined/removed; touch the watcher logic and add a short comment documenting
the intended reset behavior.
- Around line 63-68: The watcher on props.src should observe deep mutations so
in the watcher defined for props.src (the callback that clears dataLayer.value
and calls loadGeoJson) add the watcher option { deep: true } to ensure
modifications inside a GeoJSON object (e.g., changes to features array) trigger
the handler; update the watcher invocation where dataLayer.value is referenced
and loadGeoJson(src, dataLayer.value) is called to include the deep: true
option.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1df12fdd-b757-49a2-ac00-26f75c1c4681
📒 Files selected for processing (5)
playground/pages/third-parties/google-maps/sfcs.vuesrc/runtime/components/GoogleMaps/ScriptGoogleMapsGeoJson.vuetest/unit/__helpers__/google-maps-test-utils.tstest/unit/__mocks__/google-maps-api.tstest/unit/google-maps-geojson.test.ts
|
@DamianGlowala please review 🙏 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/content/scripts/google-maps.md (1)
587-589: Good callout, but the surrounding “all components use options” wording is now inaccurate.The new callout is helpful, but readers can still be misled by the earlier absolute statement that all SFC components use
options. Consider changing that sentence to “most” to keep docs internally consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/scripts/google-maps.md` around lines 587 - 589, The docs state that "all SFC components use options" which conflicts with the new callout about ScriptGoogleMapsGeoJson; update the earlier absolute sentence to a qualified one such as "most SFC components use the options pattern" and add a brief parenthetical noting the exception (ScriptGoogleMapsGeoJson uses src and style props) so the callout and surrounding text are consistent; search for the sentence referencing "all SFC components use options" and replace it, keeping the existing ::callout and the mention of ScriptGoogleMapsGeoJson, src, style, and options intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/scripts/google-maps.md`:
- Around line 534-545: The script block is using TypeScript (e.g., the typed
parameter in handleFeatureClick and Google types referenced) but the <script
setup> tag lacks lang="ts"; update the <script setup> tag to <script setup
lang="ts"> so TypeScript is enabled for the geoJsonStyle constant and the
handleFeatureClick(event: google.maps.Data.MouseEvent) function, ensuring proper
type checking and editor support.
- Line 605: The docs line for ScriptGoogleMapsGeoJson incorrectly lists the
style prop as only google.maps.Data.StyleOptions; update that table row (the row
for ScriptGoogleMapsGeoJson) so the style column shows the full union type
google.maps.Data.StylingFunction | google.maps.Data.StyleOptions, preserving the
existing formatting/markdown and backticks used for other types.
---
Nitpick comments:
In `@docs/content/scripts/google-maps.md`:
- Around line 587-589: The docs state that "all SFC components use options"
which conflicts with the new callout about ScriptGoogleMapsGeoJson; update the
earlier absolute sentence to a qualified one such as "most SFC components use
the options pattern" and add a brief parenthetical noting the exception
(ScriptGoogleMapsGeoJson uses src and style props) so the callout and
surrounding text are consistent; search for the sentence referencing "all SFC
components use options" and replace it, keeping the existing ::callout and the
mention of ScriptGoogleMapsGeoJson, src, style, and options intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3d59bdd1-a911-4a7d-bcde-69cb48a90cd7
📒 Files selected for processing (1)
docs/content/scripts/google-maps.md
- Add `lang="ts"` to GeoJSON example script tag - Update style prop type to include StylingFunction union in docs - Regenerate lockfile for catalog: specifiers
commit: |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/content/scripts/google-maps.md (1)
587-589: Consider softening the “all components use options” wording for consistency.This callout is helpful, but it now conflicts with the sentence at Line 585 (“All SFC components…”). Consider changing that sentence to “Most SFC components…” so the exception reads cleanly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/scripts/google-maps.md` around lines 587 - 589, Update the sentence that currently reads "All SFC components..." so it softens that absolute claim to "Most SFC components..." (or similar phrasing) so it aligns with the callout about ScriptGoogleMapsGeoJson using src and style props; locate the paragraph referencing "All SFC components" and change it to "Most SFC components" to make the exception for ScriptGoogleMapsGeoJson clear and consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/content/scripts/google-maps.md`:
- Around line 587-589: Update the sentence that currently reads "All SFC
components..." so it softens that absolute claim to "Most SFC components..." (or
similar phrasing) so it aligns with the callout about ScriptGoogleMapsGeoJson
using src and style props; locate the paragraph referencing "All SFC components"
and change it to "Most SFC components" to make the exception for
ScriptGoogleMapsGeoJson clear and consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9b9a6357-1355-4a9f-936c-b8728c1c23e0
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
docs/content/scripts/google-maps.md
| features: [{ | ||
| type: 'Feature', | ||
| geometry: { type: 'Point', coordinates: [150.644, -34.397] }, | ||
| properties: { name: 'My Point' }, |
There was a problem hiding this comment.
We could (but we don't need to) repeat :style and @click attrs for explicitness.
docs/content/scripts/google-maps.md
Outdated
| icon: i-simple-icons-github | ||
| to: https://github.com/nuxt/scripts/blob/main/src/runtime/components/ScriptGoogleMaps.vue | ||
| size: xs | ||
| - label: "<ScriptGoogleMapsGeoJson>" |
There was a problem hiding this comment.
Not sure where it is used, and, whether this is needed?
Co-authored-by: Damian Głowala <damian.glowala.rebkow@gmail.com>
- Remove GeoJson frontmatter link (not needed) - "All SFC components" → "Most" with inline exception note - Reword GeoJson description per reviewer suggestion - Style watcher resets to default when prop removed
2 participants
🔗 Linked issue
Resolves #655
❓ Type of change
📚 Description
Adds a
ScriptGoogleMapsGeoJsoncomponent that wrapsgoogle.maps.Datato load and style GeoJSON data on the map. Acceptssrc(string URL forloadGeoJsonor object foraddGeoJson) andstyleprops, emits mouse events (click, contextmenu, etc.) and feature lifecycle events (addfeature, removefeature, setgeometry, setproperty, removeproperty). Includes unit tests, mock infrastructure, and playground integration.