-
Notifications
You must be signed in to change notification settings - Fork 47
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
feat(web): add time series czml example #1417
Conversation
WalkthroughThis pull request introduces three new timeline plugins into the PluginPlayground. The timeline category of the preset configuration is updated to include the plugins: Changes
Sequence Diagram(s)sequenceDiagram
participant Loader as Preset Loader
participant PC as playbackControl Plugin
participant TDF as timeDrivenFeatures Plugin
participant TDP as timeDrivenPath Plugin
participant Platform as Re:Earth Platform
Loader->>PC: Import & Initialize playbackControl
PC->>Platform: Setup playback functionality
Loader->>TDF: Import & Initialize timeDrivenFeatures
TDF->>Platform: Add time-based animated features
Loader->>TDP: Import & Initialize timeDrivenPath
TDP->>Platform: Animate 3D model along path
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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. |
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.
Actionable comments posted: 5
🧹 Nitpick comments (3)
web/src/beta/features/PluginPlayground/Plugins/presets/index.ts (1)
97-97
: Consider reordering timeline plugins for better user experience.The current order is
[playbackControl, timeDrivenFeatures, timeDrivenPath]
. Consider reordering to group related features:
playbackControl
(controls)timeDrivenPath
(path animation)timeDrivenFeatures
(feature animation)This ordering would create a more intuitive flow from basic controls to advanced features.
web/src/beta/features/PluginPlayground/Plugins/presets/timeline/timeDrivenPath.ts (1)
108-113
: Extract camera position configuration.The camera position coordinates are hard-coded magic numbers. Consider extracting these into a named configuration object for better maintainability.
+const INITIAL_CAMERA_POSITION = { + heading: 6.246954319760702, + height: 591887.4618586897, + lat: 29.62255491782384, + lng: 137.32567845678386, + pitch: -0.8072976015234672, + roll: 0.0006403173192017775, +}; -reearth.camera.flyTo( - { - heading: 6.246954319760702, - height: 591887.4618586897, - lat: 29.62255491782384, - lng: 137.32567845678386, - pitch: -0.8072976015234672, - roll: 0.0006403173192017775, - }, +reearth.camera.flyTo( + INITIAL_CAMERA_POSITION,web/src/beta/features/PluginPlayground/Plugins/presets/timeline/timeDrivenFeatures.ts (1)
51-51
: Define height constants for better maintainability.The extruded height values (100000, 200000, 300000, 400000) are magic numbers. Consider extracting these into named constants:
+const POLYGON_HEIGHTS = { + LEVEL_1: 100000, + LEVEL_2: 200000, + LEVEL_3: 300000, + LEVEL_4: 400000, +};Also applies to: 67-67, 83-83, 99-99
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
web/src/beta/features/PluginPlayground/Plugins/presets/index.ts
(2 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/timeline/playbackControl.ts
(1 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/timeline/timeDrivenFeatures.ts
(1 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/timeline/timeDrivenPath.ts
(1 hunks)
⏰ 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
sourceCode: `TBD | ||
` |
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.
Implementation missing in playbackControl widget.
The widget file contains a "TBD" placeholder instead of actual implementation. This needs to be completed before the PR can be merged.
Would you like me to help generate a basic implementation for the playback control widget? This could include standard playback features like play, pause, and speed control.
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.
This example will be made in next task
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.
🧩 Analysis chain
@ShogoHirasawa Understood, I'll mark this as planned for the next task. Would you like me to create a follow-up issue to track the implementation of the playback control widget?
🏁 Script executed:
#!/bin/bash
# Check if there's already an existing issue for this
gh issue list --search "playback control widget implementation" --json title,url
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Create a new issue for the playback control widget implementation
gh issue create \
--title "Implement playback control widget" \
--body "Follow-up task from PR #1417 to implement the playback control widget in the Plugin Playground.
This will complete the timeline examples by adding standard playback features.
Context: https://github.com/reearth/reearth-visualizer/pull/1417#discussion_r1952519488" \
--assignee "ShogoHirasawa"
Length of output: 418
@ShogoHirasawa I've created issue #1418 to track the playback control widget implementation. We can proceed with the current PR as planned.
🔥 Failed to add learnings
Learnings cannot be added due to server error. Please try again.
interval: "2024-02-12T06:00:00Z/2024-02-12T08:22:00Z", // Define time range(start and end time) | ||
currentTime: "2024-02-12T06:00:00Z", // The initial time | ||
multiplier: 600, // Playback speed (600 means 10 minutes of real time = 1 minute in the animation) | ||
range: "LOOP_STOP", // This will loop the animation until it stops |
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.
🛠️ Refactor suggestion
Use relative dates for better demo experience.
The current implementation uses fixed dates (February 2024). Consider using relative dates based on the current time to ensure the demo remains relevant:
- interval: "2024-02-12T06:00:00Z/2024-02-12T08:22:00Z",
- currentTime: "2024-02-12T06:00:00Z",
+ interval: "${startTime}/${endTime}",
+ currentTime: "${startTime}",
Add this before the CZML data to generate dynamic dates:
const now = new Date();
const startTime = new Date(now.getTime() + 24*60*60*1000).toISOString(); // tomorrow
const endTime = new Date(now.getTime() + 26*60*60*1000).toISOString(); // tomorrow + 2 hours
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.
This is sample script so it has be easy to read. I think current one is good for any user.
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.
@ShogoHirasawa Understood, you make a good point. Fixed dates do make the example clearer and easier to understand. I'll withdraw my suggestion.
(。•́︿•̀。)
🔥 Failed to add learnings
Learnings cannot be added due to server error. Please try again.
|
||
// Define the 3D model | ||
model: { | ||
gltf: "https://reearth.github.io/visualizer-plugin-sample-data/public/gltf/train.gltf", // URL of 3D model(gltf) |
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.
🛠️ Refactor suggestion
Add error handling for 3D model loading.
The code should handle potential failures when loading the external 3D model.
+// Add error handling for model loading
+reearth.on('layerload', ({ layerId, error }) => {
+ if (error) {
+ console.error(`Failed to load layer ${layerId}:`, error);
+ // Optionally show a user-friendly error message
+ }
+});
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
gltf: "https://reearth.github.io/visualizer-plugin-sample-data/public/gltf/train.gltf", // URL of 3D model(gltf) | |
gltf: "https://reearth.github.io/visualizer-plugin-sample-data/public/gltf/train.gltf", // URL of 3D model(gltf) | |
// Add error handling for model loading | |
reearth.on('layerload', ({ layerId, error }) => { | |
if (error) { | |
console.error(`Failed to load layer ${layerId}:`, error); | |
// Optionally show a user-friendly error message | |
} | |
}); |
cartographicDegrees: [ | ||
// Define lon,lat,height of the bottom of the feature | ||
-95.19644404998762, 39.121855688403606, 0, -95.86066795468007, | ||
30.135315689161864, 0, -84.60676001423798, 29.013591974308632, 0, | ||
-82.7088322952643, 37.87660535441917, 0, -95.19644404998762, | ||
39.121855688403606, 0, | ||
], | ||
}, | ||
extrudedHeight: 100000, // Defines the height at which the polygon is extruded | ||
}, | ||
}, | ||
{ | ||
id: "samplePolygon002", | ||
name: "samplePolygon002", | ||
availability: "2024-12-24T01:00:05Z/2024-12-24T01:00:10Z", | ||
polygon: { | ||
positions: { | ||
cartographicDegrees: [ | ||
-95.19644404998762, 39.121855688403606, 0, -95.86066795468007, | ||
30.135315689161864, 0, -84.60676001423798, 29.013591974308632, 0, | ||
-82.7088322952643, 37.87660535441917, 0, -95.19644404998762, | ||
39.121855688403606, 0, | ||
], | ||
}, | ||
extrudedHeight: 200000, | ||
}, | ||
}, | ||
{ | ||
id: "samplePolygon003", | ||
name: "samplePolygon003", | ||
availability: "2024-12-24T01:00:10Z/2024-12-24T01:00:15Z", | ||
polygon: { | ||
positions: { | ||
cartographicDegrees: [ | ||
-95.19644404998762, 39.121855688403606, 0, -95.86066795468007, | ||
30.135315689161864, 0, -84.60676001423798, 29.013591974308632, 0, | ||
-82.7088322952643, 37.87660535441917, 0, -95.19644404998762, | ||
39.121855688403606, 0, | ||
], | ||
}, | ||
extrudedHeight: 300000, | ||
}, | ||
}, | ||
{ | ||
id: "samplePolygon004", | ||
name: "samplePolygon004", | ||
availability: "2024-12-24T01:00:15Z/2024-12-24T01:00:20Z", | ||
polygon: { | ||
positions: { | ||
cartographicDegrees: [ | ||
-95.19644404998762, 39.121855688403606, 0, -95.86066795468007, | ||
30.135315689161864, 0, -84.60676001423798, 29.013591974308632, 0, | ||
-82.7088322952643, 37.87660535441917, 0, -95.19644404998762, | ||
39.121855688403606, 0, | ||
], | ||
}, | ||
extrudedHeight: 400000, | ||
}, | ||
}, |
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.
🛠️ Refactor suggestion
Reduce code duplication in polygon definitions.
The polygon coordinates are duplicated across all four time intervals. Extract these into a reusable constant:
+const POLYGON_COORDINATES = [
+ -95.19644404998762, 39.121855688403606, 0,
+ -95.86066795468007, 30.135315689161864, 0,
+ -84.60676001423798, 29.013591974308632, 0,
+ -82.7088322952643, 37.87660535441917, 0,
+ -95.19644404998762, 39.121855688403606, 0,
+];
+const createPolygon = (id, startTime, endTime, height) => ({
+ id: `samplePolygon${id}`,
+ name: `samplePolygon${id}`,
+ availability: `${startTime}/${endTime}`,
+ polygon: {
+ positions: {
+ cartographicDegrees: POLYGON_COORDINATES,
+ },
+ extrudedHeight: height,
+ },
+});
const czmlData = [
{
id: "document",
// ... clock configuration ...
},
+ createPolygon("001", "${startTime}", "${addSeconds(startTime, 5)}", 100000),
+ createPolygon("002", "${addSeconds(startTime, 5)}", "${addSeconds(startTime, 10)}", 200000),
+ createPolygon("003", "${addSeconds(startTime, 10)}", "${addSeconds(startTime, 15)}", 300000),
+ createPolygon("004", "${addSeconds(startTime, 15)}", "${endTime}", 400000),
- // ... remove existing polygon definitions ...
];
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
cartographicDegrees: [ | |
// Define lon,lat,height of the bottom of the feature | |
-95.19644404998762, 39.121855688403606, 0, -95.86066795468007, | |
30.135315689161864, 0, -84.60676001423798, 29.013591974308632, 0, | |
-82.7088322952643, 37.87660535441917, 0, -95.19644404998762, | |
39.121855688403606, 0, | |
], | |
}, | |
extrudedHeight: 100000, // Defines the height at which the polygon is extruded | |
}, | |
}, | |
{ | |
id: "samplePolygon002", | |
name: "samplePolygon002", | |
availability: "2024-12-24T01:00:05Z/2024-12-24T01:00:10Z", | |
polygon: { | |
positions: { | |
cartographicDegrees: [ | |
-95.19644404998762, 39.121855688403606, 0, -95.86066795468007, | |
30.135315689161864, 0, -84.60676001423798, 29.013591974308632, 0, | |
-82.7088322952643, 37.87660535441917, 0, -95.19644404998762, | |
39.121855688403606, 0, | |
], | |
}, | |
extrudedHeight: 200000, | |
}, | |
}, | |
{ | |
id: "samplePolygon003", | |
name: "samplePolygon003", | |
availability: "2024-12-24T01:00:10Z/2024-12-24T01:00:15Z", | |
polygon: { | |
positions: { | |
cartographicDegrees: [ | |
-95.19644404998762, 39.121855688403606, 0, -95.86066795468007, | |
30.135315689161864, 0, -84.60676001423798, 29.013591974308632, 0, | |
-82.7088322952643, 37.87660535441917, 0, -95.19644404998762, | |
39.121855688403606, 0, | |
], | |
}, | |
extrudedHeight: 300000, | |
}, | |
}, | |
{ | |
id: "samplePolygon004", | |
name: "samplePolygon004", | |
availability: "2024-12-24T01:00:15Z/2024-12-24T01:00:20Z", | |
polygon: { | |
positions: { | |
cartographicDegrees: [ | |
-95.19644404998762, 39.121855688403606, 0, -95.86066795468007, | |
30.135315689161864, 0, -84.60676001423798, 29.013591974308632, 0, | |
-82.7088322952643, 37.87660535441917, 0, -95.19644404998762, | |
39.121855688403606, 0, | |
], | |
}, | |
extrudedHeight: 400000, | |
}, | |
}, | |
const POLYGON_COORDINATES = [ | |
-95.19644404998762, 39.121855688403606, 0, | |
-95.86066795468007, 30.135315689161864, 0, | |
-84.60676001423798, 29.013591974308632, 0, | |
-82.7088322952643, 37.87660535441917, 0, | |
-95.19644404998762, 39.121855688403606, 0, | |
]; | |
const createPolygon = (id, startTime, endTime, height) => ({ | |
id: `samplePolygon${id}`, | |
name: `samplePolygon${id}`, | |
availability: `${startTime}/${endTime}`, | |
polygon: { | |
positions: { | |
cartographicDegrees: POLYGON_COORDINATES, | |
}, | |
extrudedHeight: height, | |
}, | |
}); | |
const czmlData = [ | |
{ | |
id: "document", | |
// ... clock configuration ... | |
}, | |
createPolygon("001", "${startTime}", "${addSeconds(startTime, 5)}", 100000), | |
createPolygon("002", "${addSeconds(startTime, 5)}", "${addSeconds(startTime, 10)}", 200000), | |
createPolygon("003", "${addSeconds(startTime, 10)}", "${addSeconds(startTime, 15)}", 300000), | |
createPolygon("004", "${addSeconds(startTime, 15)}", "${endTime}", 400000), | |
]; |
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.
yes, but current script is simple. this is better to read for beginer.
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.
@ShogoHirasawa Understood! You're right - for educational purposes, having explicit code can be more beneficial than abstracted code. I'll withdraw the suggestion.
🔥 Failed to add learnings
Learnings cannot be added due to server error. Please try again.
interval: "2024-12-24T01:00:00Z/2024-12-24T01:00:20Z", // Define time range | ||
currentTime: "2024-12-24T01:00:00Z", // Define current (start) time | ||
multiplier: 5, // Playback speed (5 means 20 secounds of real time = 4 secounds in the animation) |
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.
🛠️ Refactor suggestion
Use relative dates for better demo experience.
Similar to the timeDrivenPath plugin, this uses fixed dates (December 2024). Consider using relative dates:
+const now = new Date();
+const startTime = new Date(now.getTime() + 24*60*60*1000).toISOString(); // tomorrow
+const endTime = new Date(now.getTime() + 24*60*60*1000 + 20000).toISOString(); // tomorrow + 20 seconds
- interval: "2024-12-24T01:00:00Z/2024-12-24T01:00:20Z",
- currentTime: "2024-12-24T01:00:00Z",
+ interval: "${startTime}/${endTime}",
+ currentTime: "${startTime}",
Committable suggestion skipped: line range outside the PR's diff.
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.
yes, but current script is simple. this is better to read for beginner
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.
@ShogoHirasawa I understand your point about keeping the example code simple and beginner-friendly. You're right that fixed dates are more straightforward to read and understand. I'll withdraw my suggestion.
🔥 Failed to add learnings
Learnings cannot be added due to server error. Please try again.
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/src/beta/features/PluginPlayground/Plugins/presets/index.ts (1)
103-103
: LGTM! Consider adding spaces after commas for better readability.The timeline plugins are correctly configured. Consider adding spaces after commas for better readability:
- plugins: [playbackControl,timeDrivenFeatures,timeDrivenPath] + plugins: [playbackControl, timeDrivenFeatures, timeDrivenPath]
📜 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/timeline/timeDrivenPath.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/beta/features/PluginPlayground/Plugins/presets/timeline/timeDrivenPath.ts
🧰 Additional context used
🪛 Biome (1.9.4)
web/src/beta/features/PluginPlayground/Plugins/presets/index.ts
[error] 27-27: Shouldn't redeclare 'zoomInOut'. Consider to delete it or rename it.
'zoomInOut' is defined here:
(lint/suspicious/noRedeclare)
[error] 28-28: Shouldn't redeclare 'cameraRotation'. Consider to delete it or rename it.
'cameraRotation' is defined here:
(lint/suspicious/noRedeclare)
[error] 29-29: Shouldn't redeclare 'cameraPosition'. Consider to delete it or rename it.
'cameraPosition' is defined here:
(lint/suspicious/noRedeclare)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: ci-server / ci-server-lint
import { zoomInOut } from "./camera/zoomInOut"; | ||
import { cameraRotation } from "./camera/cameraRotation"; | ||
import { cameraPosition } from "./camera/cameraPosition"; |
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.
Remove duplicate imports.
These camera-related imports are already defined at lines 3-5. Remove the duplicate imports to maintain clean code.
Apply this diff to remove the duplicates:
-import { zoomInOut } from "./camera/zoomInOut";
-import { cameraRotation } from "./camera/cameraRotation";
-import { cameraPosition } from "./camera/cameraPosition";
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import { zoomInOut } from "./camera/zoomInOut"; | |
import { cameraRotation } from "./camera/cameraRotation"; | |
import { cameraPosition } from "./camera/cameraPosition"; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 27-27: Shouldn't redeclare 'zoomInOut'. Consider to delete it or rename it.
'zoomInOut' is defined here:
(lint/suspicious/noRedeclare)
[error] 28-28: Shouldn't redeclare 'cameraRotation'. Consider to delete it or rename it.
'cameraRotation' is defined here:
(lint/suspicious/noRedeclare)
[error] 29-29: Shouldn't redeclare 'cameraPosition'. Consider to delete it or rename it.
'cameraPosition' is defined here:
(lint/suspicious/noRedeclare)
import { FileType, PluginType } from "../../constants"; | ||
|
||
const yamlFile: FileType = { | ||
id: "timeDrivenFeatures-reearth-yml", |
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.
lets use kebase case here.
time-driven-features-reearth-yml
clock: { | ||
interval: "2024-12-24T01:00:00Z/2024-12-24T01:00:20Z", // Define time range | ||
currentTime: "2024-12-24T01:00:00Z", // Define current (start) time | ||
multiplier: 5, // Playback speed (5 means 20 secounds of real time = 4 secounds in the animation) |
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.
Small typo here. Should be seconds
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.
Looks great! Great work! Added small comments. I'll let Shaun have a review.
Overview
This is PR for adding timeline examples in Plugin Playground.
What I've done
Time Driven Features


Time Driven Path
What I haven't done
If it will be approved, I will finish all task in timeline examples
How I tested
My local environment
Which point I want you to review particularly
Memo
Summary by CodeRabbit