-
-
Notifications
You must be signed in to change notification settings - Fork 38
Remove the google maps 3D #381
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
Conversation
It doesn't work any more as a basemap.
Something like this would work
this.map = new Map({
basemap: this.basemaps.Satellite,
ground: { layers: [this.elevationLayer] },
layers: [google3DTilesLayer]
});
Disabling for now
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRemoves the now-broken Google 3D basemap configuration from the 3D map component, leaving other basemaps intact. Class diagram for Map3dElement basemap configuration changesclassDiagram
class Map3dElement {
- airspace Airspace3dElement
- basemaps Record_string_BasemapOrStringOrNull_
- createBasemaps()
}
class Basemap {
+baseLayers IntegratedMesh3DTilesLayer[*]
+referenceLayers Layer[*]
+title string
+id string
}
class IntegratedMesh3DTilesLayer {
+url string
+title string
+customParameters Record_string_string_
}
Map3dElement --> Basemap : uses
Basemap --> IntegratedMesh3DTilesLayer : baseLayers
%% Before change
class BasemapsBefore {
+Satellite BasemapOrStringOrNull
+Google BasemapOrStringOrNull
+OpenTopoMap BasemapOrStringOrNull
+IGN_France BasemapOrStringOrNull
+Topo BasemapOrStringOrNull
}
%% After change
class BasemapsAfter {
+Satellite BasemapOrStringOrNull
+OpenTopoMap BasemapOrStringOrNull
+IGN_France BasemapOrStringOrNull
+Topo BasemapOrStringOrNull
}
Map3dElement --> BasemapsBefore : basemaps (removed)
Map3dElement --> BasemapsAfter : basemaps (new)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThe change removes Google basemap support from a 3D map component by eliminating the basemap option from configuration, its construction logic, and associated imports for IntegratedMesh3DTilesLayer and API key retrieval utilities. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- Now that the
Googlebasemap is removed from thebasemapsmap, double-check any UI or configuration logic that referencedbasemaps.Google(e.g., basemap selectors or defaults) so they don’t try to use a non-existent key. - If
IntegratedMesh3DTilesLayeris no longer used anywhere after this removal, consider deleting its import and any related configuration to avoid carrying unused dependencies.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Now that the `Google` basemap is removed from the `basemaps` map, double-check any UI or configuration logic that referenced `basemaps.Google` (e.g., basemap selectors or defaults) so they don’t try to use a non-existent key.
- If `IntegratedMesh3DTilesLayer` is no longer used anywhere after this removal, consider deleting its import and any related configuration to avoid carrying unused dependencies.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Deploying flyxc with
|
| Latest commit: |
72398fd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://21f22284.flyxc.pages.dev |
| Branch Preview URL: | https://vicb-gmap-3d.flyxc.pages.dev |
It doesn't work any more as a basemap.
Something like this would work
Disabling for now
Summary by Sourcery
Bug Fixes:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.