feat(#35): ✨ Change to offline rendering #36
Conversation
Add geoJSON file to replace the online OSM. Use this for offline rendering.
WalkthroughAdds offline globe rendering with a semi‑transparent base color, a loader to add GeoJSON country borders to the Cesium viewer (stored as Changes
Sequence Diagram(s)sequenceDiagram
participant Main as main.js
participant CV as cesiumViewer.js
participant Cesium as Cesium Viewer
participant Geo as /data/geo/world_simplified.geojson
Main->>CV: createCesiumViewer()
Main->>CV: await loadCountryBorders(window.cesiumViewer)
activate CV
CV->>Geo: fetch GeoJSON
Geo-->>CV: return GeoJSON
CV->>Cesium: dataSources.add(GeoJsonDataSource) & apply style
CV->>Cesium: viewer._countryBorders = datasource
CV-->>Main: resolve datasource
deactivate CV
Note over Main,Cesium: Later — camera sync updates visibility
Main->>Cesium: compute showGlobe
Main->>Cesium: set globe.show / skyAtmosphere.show
alt country borders present
Main->>Cesium: set viewer._countryBorders.show = showGlobe
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
![coderabbit[bot]](https://avatars.git.ntnu.no/u/70329?s=60){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/cesiumViewer.js (2)
24-24: Globe base color set appropriately.The semi-transparent pastel blue (
#D6EBF2at 50% alpha) provides a clean background for the offline country borders. Consider moving this toconfig.jsif the color scheme might change in the future.
31-36: Consider extracting hardcoded values to configuration.The GeoJSON file path and styling options (colors, stroke width) are hardcoded. While this works for the current implementation, moving these to
config.jswould improve maintainability if adjustments are needed.Example configuration in
config.js:export const COUNTRY_BORDERS_CONFIG = { path: '/data/geo/world_simplified.geojson', stroke: { color: '#000000', alpha: 0.2 }, fill: { color: '#F2EFE9' }, strokeWidth: 1.0 }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/cameraSync.js(1 hunks)src/cesiumViewer.js(1 hunks)src/main.js(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/cesiumViewer.js (1)
src/potreeViewer.js (1)
viewer(14-16)
src/main.js (1)
src/cesiumViewer.js (1)
loadCountryBorders(29-45)
⏰ 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). (1)
- GitHub Check: prettier
🔇 Additional comments (3)
src/cameraSync.js (1)
36-38: LGTM: Visibility synchronization implemented correctly.The guard pattern appropriately handles the optional
_countryBordersproperty and ensures visibility stays in sync with the globe. Note that using_countryBordersas a pseudo-private property on the viewer object works but deviates from strict encapsulation—consider whether a more formal state management approach might benefit future extensions.src/cesiumViewer.js (1)
20-20: LGTM: Imagery provider correctly disabled for offline rendering.Setting
imageryProvider: falseappropriately disables the default online OSM tiles, which aligns with the PR objective to implement offline rendering.src/main.js (1)
13-13: The GeoJSON file exists and will be accessible at runtime.The file
public/data/geo/world_simplified.geojsonexists and is correctly referenced in the code. Vite (the project's build tool) serves files from thepublic/directory at the root path/by default, so the path/data/geo/world_simplified.geojsonused inloadCountryBorders()will correctly resolve at runtime. No issues found.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main.js (1)
13-17: Error handling successfully addresses previous review feedback.The try-catch block properly handles potential GeoJSON loading failures and logs meaningful error information. The application gracefully continues with degraded functionality if borders fail to load.
Optional enhancement: Consider adding a user-facing notification (e.g., a toast or alert) when country borders fail to load, so users understand why the globe may appear different than expected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/cesiumViewer.js(1 hunks)src/main.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/cesiumViewer.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/main.js (1)
src/cesiumViewer.js (1)
loadCountryBorders(38-54)
⏰ 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). (1)
- GitHub Check: prettier
🔇 Additional comments (1)
src/main.js (1)
2-2: LGTM! Import statement is correct.The addition of
loadCountryBordersto the import list is properly structured and necessary for the offline rendering feature.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/cesiumViewer.js (1)
38-54: Function implementation is correct and well-documented.The function properly loads and configures the GeoJSON data source with appropriate styling for offline rendering. The JSDoc is clear and helpful.
Two optional refinements to consider:
Hard-coded file path (line 40): Consider accepting the path as an optional parameter with a default value. This would improve testability and flexibility.
Mutating viewer object (line 51): Storing
_countryBordersdirectly on the Cesium viewer instance is a code smell—you're adding custom properties to a library-owned object. While this pattern appears consistent with your codebase (used in cameraSync.js), consider maintaining a separate application state object for these references instead.Example for making the path configurable:
-export async function loadCountryBorders(viewer) { +export async function loadCountryBorders(viewer, geoJsonPath = '/data/geo/world_simplified.geojson') { const dataSource = await Cesium.GeoJsonDataSource.load( - '/data/geo/world_simplified.geojson', + geoJsonPath, {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/cesiumViewer.js(1 hunks)src/potreeViewer.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/potreeViewer.js
⏰ 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). (1)
- GitHub Check: prettier
🔇 Additional comments (1)
src/cesiumViewer.js (1)
20-25: Offline rendering configuration looks good.The changes correctly disable the online imagery provider and set an appropriate ocean-like base color. The semi-transparent alpha (0.5) allows the GeoJSON land masses to layer nicely over the globe.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cypress/e2e/globeRendering.cy.js (1)
3-3: Use Cypress baseUrl instead of hard-coded URL.The hard-coded
localhost:5173URL may fail in different environments (CI/CD, different port configurations). ConfigurebaseUrlincypress.config.jsand use a relative path.- cy.visit('http://localhost:5173/#') + cy.visit('/#')Then ensure
cypress.config.jscontains:{ e2e: { baseUrl: 'http://localhost:5173' } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cypress/e2e/globeRendering.cy.js(1 hunks)
⏰ 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). (1)
- GitHub Check: prettier
Add geoJSON file to replace the online OSM. Use this for offline rendering.
Issue number
Closes #35
Description
Change to offline rendering with a geoJSON map. Map downloaded from: https://www.naturalearthdata.com/downloads/50m-cultural-vectors/
Testing steps
Screenshots
Summary by CodeRabbit
New Features
UI/Visualization
Enhancement
Tests