feat(web): plugin playground presets - create layer styling example with point marker #1378
Conversation
WalkthroughThis pull request updates the plugin configuration by modifying import statements and enhancing the Changes
Sequence Diagram(s)sequenceDiagram
participant Loader as PluginLoader
participant Plugin as layerStylingExamples Plugin
participant Env as Reearth Environment
Loader->>Plugin: Request plugin configuration
Plugin->>Env: Initialize GeoJSON layer
Plugin->>Env: Initialize CZML layer
Plugin->>Env: Initialize KML layer and encode data
Env-->>Plugin: Layers added confirmation
Plugin->>Env: Adjust camera to encompass layers
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
✅ Deploy Preview for reearth-web ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/layerStyling.ts (3)
4-18: Consider enhancing the plugin metadata.While the basic configuration is good, consider adding more metadata fields for better documentation:
- author
- repository
- license
name: Layer Styling Manager version: 1.0.0 +author: reearth +repository: https://github.com/reearth/reearth-visualizer +license: Apache-2.0 extensions:
25-44: Consider making coordinates configurable.The coordinates are currently hardcoded, which limits reusability. Consider making them configurable through the UI or as plugin parameters.
57-85: Move styles to a separate CSS file.Inline styles make maintenance difficult. Consider moving styles to a separate CSS file for better maintainability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/beta/features/PluginPlayground/Plugins/presets/index.ts(2 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/layerStyling.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: ci-server / ci-server-lint
- GitHub Check: ci-server / ci-server-test
- GitHub Check: ci-web / ci
- GitHub Check: Redirect rules - reearth-web
- GitHub Check: Header rules - reearth-web
- GitHub Check: Pages changed - reearth-web
🔇 Additional comments (3)
web/src/beta/features/PluginPlayground/Plugins/presets/index.ts (1)
15-15: LGTM! Plugin integration looks good.The layerStylingManager is correctly imported and added to the appropriate category in presetPlugins.
Also applies to: 67-67
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/layerStyling.ts (2)
1-146: Add comprehensive testing.The plugin implementation would benefit from automated tests covering:
- Layer creation and management
- UI interactions and validation
- Message handling and security
- Error scenarios
Would you like me to help create a test suite for this plugin?
47-54: 🛠️ Refactor suggestionAdd error handling for layer creation and camera movement.
The code assumes layer creation always succeeds. Consider adding error handling:
-const currentLayerId = reearth.layers.add({ +let currentLayerId; +try { + currentLayerId = reearth.layers.add({ // ... layer config ... -}); + }); + + // Only move camera if layer was created successfully + reearth.camera.flyTo({ + lat: 35.74642872517698, + lng: 139.97422779688281, + height: 2000, + heading: 0, + pitch: -45, + roll: 0 + }); +} catch (error) { + console.error("Failed to create layer:", error); + reearth.ui.show("<div>Error: Failed to create layer</div>"); +}Likely invalid or redundant comment.
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/layerStyling.ts
Outdated
Show resolved
Hide resolved
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/layerStyling.ts
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1378 +/- ##
==========================================
- Coverage 22.79% 22.52% -0.28%
==========================================
Files 1052 1063 +11
Lines 109347 110870 +1523
Branches 669 679 +10
==========================================
+ Hits 24924 24968 +44
- Misses 83161 84636 +1475
- Partials 1262 1266 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/geojsonStyling.ts (6)
3-17: Add documentation for plugin configuration options.Consider adding comments to document the purpose and impact of the configuration options, especially for fields like
disableEditanddisableDelete. This will help other developers understand the configuration constraints.const yamlFile: FileType = { + // Unique identifier for the YAML configuration file id: "geojson-styling-reearth-yml", title: "reearth.yml", + // Plugin configuration in YAML format sourceCode: `id: geojson-styling-plugin name: GeoJSON Styling Examples version: 1.0.0 extensions: - id: geojson-styling type: widget name: GeoJSON Styling description: Examples of GeoJSON features with different styling options `, + // Prevent users from modifying the configuration disableEdit: true, + // Prevent users from removing the configuration disableDelete: true };
24-51: Consider extracting configuration into constants.The point marker configuration uses hardcoded values. Consider extracting these into named constants for better maintainability and reusability.
+// Point marker configuration +const POINT_MARKER_CONFIG = { + coordinates: [139.97422779688281, 35.74642872517698], + style: { + pointColor: "red", + pointSize: 12, + pointOutlineColor: "white", + pointOutlineWidth: 1, + height: 100, + heightReference: "relative" + } +}; const pointLayer = reearth.layers.add({ type: "simple", data: { type: "geojson", value: { type: "FeatureCollection", features: [ { type: "Feature", properties: {}, geometry: { - coordinates: [139.97422779688281, 35.74642872517698], + coordinates: POINT_MARKER_CONFIG.coordinates, type: "Point", }, }, ], }, }, marker: { style: "point", - pointColor: "red", - pointSize: 12, - pointOutlineColor: "white", - pointOutlineWidth: 1, - height: 100, - heightReference: "relative" + ...POINT_MARKER_CONFIG.style } });
54-82: Remove redundant property and extract configuration.
- The
show: trueproperty is redundant as it's the default value.- Consider extracting the configuration similar to the point marker suggestion above.
+// Polyline configuration +const POLYLINE_CONFIG = { + coordinates: [ + [139.93007825346956, 35.81332779614391], + [139.8105822019014, 35.730789521095986] + ], + style: { + clampToGround: true, + classificationType: "terrain", + strokeColor: "red", + strokeWidth: 3 + } +}; const polylineLayer = reearth.layers.add({ type: "simple", data: { type: "geojson", value: { type: "FeatureCollection", features: [ { type: "Feature", properties: {}, geometry: { - coordinates: [ - [139.93007825346956, 35.81332779614391], - [139.8105822019014, 35.730789521095986], - ], + coordinates: POLYLINE_CONFIG.coordinates, type: "LineString", }, }, ], }, }, polyline: { - clampToGround: true, - classificationType: "terrain", - show: true, - strokeColor: "red", - strokeWidth: 3 + ...POLYLINE_CONFIG.style } });
85-123: Simplify color expression and remove redundant properties.
- The
show: trueandfill: trueproperties are redundant as they're default values.- The color expression could be simplified using rgba.
- Consider extracting the configuration similar to previous suggestions.
+// Polygon configuration +const POLYGON_CONFIG = { + coordinates: [ + [ + [139.56560369329821, 35.859787461762906], + [139.56560369329821, 35.586320662892106], + [139.73648312259508, 35.586320662892106], + [139.73648312259508, 35.859787461762906], + [139.56560369329821, 35.859787461762906], + ], + ], + style: { + clampToGround: true, + classificationType: "terrain", + fillColor: { + expression: "rgba(237, 2, 151, 0.5)" + }, + stroke: true, + strokeColor: "blue", + strokeWidth: 3 + } +}; const polygonLayer = reearth.layers.add({ type: "simple", data: { type: "geojson", value: { type: "FeatureCollection", features: [ { type: "Feature", properties: {}, geometry: { - coordinates: [ - [ - [139.56560369329821, 35.859787461762906], - [139.56560369329821, 35.586320662892106], - [139.73648312259508, 35.586320662892106], - [139.73648312259508, 35.859787461762906], - [139.56560369329821, 35.859787461762906], - ], - ], + coordinates: POLYGON_CONFIG.coordinates, type: "Polygon", }, }, ], }, }, polygon: { - clampToGround: true, - classificationType: "terrain", - fill: true, - fillColor: { - expression: "color('#ed0297',0.5)" - }, - show: true, - stroke: true, - strokeColor: "blue", - strokeWidth: 3 + ...POLYGON_CONFIG.style } });
126-133: Consider dynamic camera positioning based on layer bounds.Instead of hardcoded camera position, consider calculating the bounding box of all layers and positioning the camera to fit this view. This would make the example more resilient to coordinate changes.
136-140: Consider adding TypeScript interfaces for better type safety.Define interfaces for the plugin configuration to improve type safety and documentation.
+interface GeoJSONPluginConfig { + id: string; + title: string; + files: FileType[]; +} -export const geojsonStyling: PluginType = { +export const geojsonStyling: GeoJSONPluginConfig = { id: "geojson-styling", title: "GeoJSON Styling", files: [widgetFile, yamlFile] };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/beta/features/PluginPlayground/Plugins/presets/index.ts(2 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/geojsonStyling.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/beta/features/PluginPlayground/Plugins/presets/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Redirect rules - reearth-web
- GitHub Check: Header rules - reearth-web
- GitHub Check: Pages changed - reearth-web
- GitHub Check: ci-server / ci-server-lint
- GitHub Check: ci-web / ci
- GitHub Check: ci-server / ci-server-test
|
@OpeDada
|
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/csvStyling.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/csvStyling.ts (2)
3-17: Enhance plugin documentation and configuration.Consider the following improvements to the YAML configuration:
- Add more detailed description about available styling options
- Include UI-specific configuration for the widget extension
- Follow semantic versioning (e.g., start with 0.1.0 for initial development)
const yamlFile: FileType = { id: "csv-styling-reearth-yml", title: "reearth.yml", sourceCode: `id: csv-styling-plugin name: CSV Styling Examples -version: 1.0.0 +version: 0.1.0 extensions: - id: csv-styling type: widget name: CSV Styling - description: Examples of CSV features with different styling options + description: | + Examples of CSV features with different styling options: + - Point markers with customizable colors and sizes + - Height-based visualization + - Outline styling options + ui: + panel: + title: CSV Layer Styling + type: panel `, disableEdit: true, disableDelete: true };
37-45: Extract marker styling configuration to constants.The marker styling uses hardcoded values which reduces reusability and makes maintenance harder.
+// Define default marker styles +const DEFAULT_MARKER_STYLES = { + POINT_COLOR: "red", + POINT_SIZE: 12, + OUTLINE_COLOR: "white", + OUTLINE_WIDTH: 1, + HEIGHT: 100, + HEIGHT_REFERENCE: "relative" +}; + marker: { style: "point", - pointColor: "red", - pointSize: 12, - pointOutlineColor: "white", - pointOutlineWidth: 1, - height: 100, - heightReference: "relative" + pointColor: DEFAULT_MARKER_STYLES.POINT_COLOR, + pointSize: DEFAULT_MARKER_STYLES.POINT_SIZE, + pointOutlineColor: DEFAULT_MARKER_STYLES.OUTLINE_COLOR, + pointOutlineWidth: DEFAULT_MARKER_STYLES.OUTLINE_WIDTH, + height: DEFAULT_MARKER_STYLES.HEIGHT, + heightReference: DEFAULT_MARKER_STYLES.HEIGHT_REFERENCE },web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/czmlStyling.ts (2)
24-70: Consider terrain-relative positioning and data organization improvements.The CZML data structure could be enhanced in several ways:
- Consider using terrain-relative positioning for better ground clamping
- Add height references for 3D visualization
- Consider organizing related features into a FeatureCollection
Example improvement for the polygon:
polygon: { positions: { cartographicDegrees: [ -110.0, 40.0, 0, -110.5, 35.0, 0, -100.5, 35.0, 0, -100.0, 40.0, 0, -110.0, 40.0, 0 ], }, extrudedHeight: 300000, + heightReference: "RELATIVE_TO_GROUND", },
77-108: Consider making styles configurable via plugin properties.The current implementation uses hardcoded values for colors, sizes, and other styling properties. Consider making these configurable through plugin properties to enhance reusability.
Example refactor:
const layerCzmlEncoded = { type: "simple", data: { type: "czml", url: encodedCzml, }, marker: { style: "point", show: true, - pointColor: "red", - pointSize: 12, + pointColor: reearth.widget.property.pointColor ?? "red", + pointSize: reearth.widget.property.pointSize ?? 12, pointOutlineColor: "white", pointOutlineWidth: 1, height: 100, heightReference: "relative", },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/src/beta/features/PluginPlayground/Plugins/presets/index.ts(2 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/csvStyling.ts(1 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/czmlStyling.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/beta/features/PluginPlayground/Plugins/presets/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Redirect rules - reearth-web
- GitHub Check: ci-server / ci-server-lint
- GitHub Check: Header rules - reearth-web
- GitHub Check: ci-web / ci
- GitHub Check: Pages changed - reearth-web
- GitHub Check: ci-server / ci-server-test
🔇 Additional comments (4)
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/csvStyling.ts (2)
53-57: LGTM!The plugin export is well-structured with consistent IDs and includes all necessary files.
27-35: Consider using GeoJSON as suggested in PR comments.As per ShogoHirasawa's suggestion, consider storing the marker data in a GeoJSON file instead of CSV. This would provide a more standardized approach and allow for easier integration of additional features like polylines and polygons in the future.
Here's a script to check if GeoJSON support is already available in the codebase:
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/czmlStyling.ts (2)
1-17: LGTM! Well-structured plugin configuration.The YAML configuration properly defines the plugin metadata with appropriate security constraints (disableEdit and disableDelete).
114-125: Adjust camera position as per review feedback.As suggested in the PR comments by ShogoHirasawa, the camera position should be adjusted to better capture the entire dataset. Consider:
- Increasing the pitch angle for a better overview
- Adjusting the height based on the dataset extent
- Centering the view on the dataset's centroid
Example adjustment:
{ lng: -95.0, lat: 34.0, height: 3000000, heading: 5.949278757227463, - pitch: -1.45, + pitch: -45, roll: 0, },✅ Verification successful
Confirmed: Camera pitch adjustment is needed for better visualization
The suggested camera adjustment is valid and aligns with other CZML visualizations in the codebase. The current pitch of -1.45° is unusually shallow, while other 3D views typically use steeper angles for better overview. The height and heading values are consistent with other CZML examples.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Search for CZML dataset files and camera configurations echo "=== Searching for CZML files ===" fd -e czml echo -e "\n=== Searching for camera configurations in TypeScript files ===" rg "camera\.flyTo|camera\.position" -t ts -A 5 -B 5 echo -e "\n=== Searching for camera-related constants or configurations ===" rg "const.*camera|camera.*config" -t tsLength of output: 12664
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/csvStyling.ts
Outdated
Show resolved
Hide resolved
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/csvStyling.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/kmlStyling.ts (3)
85-85: Add a descriptive name for the point marker.The point marker's name is empty, which may affect usability in the UI. Consider adding a descriptive name like "Sample Point" for consistency with other features.
- <name></name> + <name>Sample Point</name>
60-63: Consider extracting coordinates as configuration.The coordinates are hardcoded in the KML data. Consider extracting these as configuration variables to make the example more reusable across different locations.
Also applies to: 77-78, 88-88
98-121: Consider using GeoJSON as suggested in PR comments.As per ShogoHirasawa's suggestion, consider using a single GeoJSON file referenced via URL instead of embedded KML. This would simplify data management and allow for easier updates.
Example GeoJSON structure:
{ "type": "FeatureCollection", "features": [ { "type": "Feature", "properties": { "fillColor": "rgba(255, 0, 0, 0.5)", "strokeColor": "blue" }, "geometry": { "type": "Polygon", "coordinates": [...] } } ] }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/src/beta/features/PluginPlayground/Plugins/presets/index.ts(2 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/czmlStyling.ts(1 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/kmlStyling.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/czmlStyling.ts
- web/src/beta/features/PluginPlayground/Plugins/presets/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - reearth-web
- GitHub Check: Header rules - reearth-web
- GitHub Check: Pages changed - reearth-web
🔇 Additional comments (3)
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/kmlStyling.ts (3)
3-17: LGTM! Plugin configuration is well-structured.The YAML configuration provides clear metadata and appropriate read-only protection.
137-141: LGTM! Plugin export is well-structured.The export maintains consistent IDs and includes all necessary files.
127-134: Adjust camera position as suggested in PR comments.As per ShogoHirasawa's feedback, consider adjusting the camera position to better capture the entire dataset. The current height and pitch might not provide the optimal view.
Try these adjusted values for better visibility:
reearth.camera.flyTo({ lng: 139.7373068, lat: 35.7709591, - height: 25000, + height: 30000, heading: 0, - pitch: -1.55, + pitch: -0.5, roll: 0, });✅ Verification successful
Adjust camera flyTo settings as per PR feedback.
The suggested modifications—increasing the height from 25000 to 30000 and adjusting the pitch from -1.55 to -0.5—directly follow ShogoHirasawa’s recommendations to capture the entire dataset more effectively. Please ensure that these new values improve the dataset’s visibility as intended.
- File to update:
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/kmlStyling.ts- Modify camera settings to:
height: 30000pitch: -0.5🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/layerStylingExamples.ts (4)
26-86: Consider externalizing geographic data to a separate GeoJSON file.As suggested in the PR comments, consider storing the geographic data (markers, polylines, and polygons) in a single GeoJSON file and referencing it via URL. This would improve maintainability and make it easier to update the data without modifying the code.
Example structure:
const geojsonLayer = { type: "simple", data: { type: "geojson", url: "https://example.com/path/to/data.geojson" }, marker: { // ... styling properties remain the same }, polyline: { // ... styling properties remain the same }, polygon: { // ... styling properties remain the same } };
92-165: Consider performance implications of data URLs for CZML data.While using data URLs works, it might not be optimal for larger datasets. Consider:
- Moving the CZML data to a separate file
- Using a CDN or static file hosting for better performance
Example structure:
const czmlLayer = { type: "simple", data: { type: "czml", url: "https://example.com/path/to/data.czml" }, // ... styling properties remain the same };
170-267: Consider simplifying KML styling approach.The current implementation defines styles both in KML and in the layer properties. Consider:
- Moving the KML data to a separate file
- Relying on layer styling properties instead of KML styles for consistency with other layers
Example structure:
const kmlLayer = { type: "simple", data: { type: "kml", url: "https://example.com/path/to/data.kml" }, // ... styling properties remain the same };
274-281: Adjust camera position for better dataset visibility.As suggested in the PR comments, consider adjusting the camera position to capture the entire dataset more effectively. The current position might not provide the best view of all features.
Consider these adjustments:
reearth.camera.flyTo({ lng: 139.700, - lat: 35.7609591, - height: 40000, + lat: 35.7509591, // Adjusted for better center position + height: 45000, // Increased for wider view heading: 0, - pitch: -1.55, + pitch: -1.2, // Adjusted for better perspective roll: 0 });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/beta/features/PluginPlayground/Plugins/presets/index.ts(2 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/layerStylingExamples.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/beta/features/PluginPlayground/Plugins/presets/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - reearth-web
- GitHub Check: Header rules - reearth-web
- GitHub Check: Pages changed - reearth-web
🔇 Additional comments (2)
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/layerStylingExamples.ts (2)
3-17: LGTM! Plugin configuration is well-structured.The YAML configuration properly defines the plugin metadata with appropriate settings for id, name, version, and extensions.
284-288: LGTM! Plugin export is correctly structured.The plugin is properly exported with all necessary files and follows the expected PluginType structure.
web/src/beta/features/PluginPlayground/Plugins/presets/layerStyles/layerStylingExamples.ts
Outdated
Show resolved
Hide resolved
…yles/layerStylingExamples.ts Co-authored-by: lby <[email protected]>
Overview
What I've done
What I haven't done
How I tested
Which point I want you to review particularly
Memo
Summary by CodeRabbit