refactor(web): plugin playground presets - camera position #1415
Conversation
WalkthroughThis change refactors the camera position plugin in the web application. The YAML-defined widget layout is removed and replaced with an HTML info-section that provides user instructions. CSS adjustments introduce a sticky info-section, reduced padding for camera controls, and new styles for the info-toggle and info-content. JavaScript logic now adds a checkInputs function with event listeners to control the apply button state and refines message handling by verifying camera position changes before triggering a fly-to action. New variables (yamlFile and widgetFile) have been introduced, and the exported cameraPosition entity is updated. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant IF as Input Field
participant CI as checkInputs Function
participant AB as Apply Button
U->>IF: Enter camera position
IF->>CI: Trigger input event
CI->>AB: Enable/disable based on input values
sequenceDiagram
participant E as Extension
participant CP as Camera Plugin
participant IF as Input Field
participant FT as Fly-to Action
E->>CP: Send new camera position message
CP->>IF: Update input fields (if manual input inactive)
CP->>CP: Validate position change
CP->>FT: Trigger fly-to action (if changed)
Possibly related PRs
Suggested labels
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: 1
🧹 Nitpick comments (4)
web/src/beta/features/PluginPlayground/Plugins/presets/camera/cameraPosition.ts (4)
34-39: Consider improving info section contrast.The info section uses a light gray background (#eee) which might not provide sufficient contrast for optimal readability.
#info-section { position: sticky; top: 0; - background: #eee; + background: #e0e0e0; text-align: left; }
100-105: Improve status message flexibility.The fixed height of 4px might truncate longer status messages. Consider using min-height instead.
.status-message { text-align: center; color: #666; - height: 4px; + min-height: 4px; padding: 4px; }
297-300: Extract position change threshold as a constant.The floating-point comparison threshold should be defined as a named constant for better maintainability.
+ const POSITION_CHANGE_THRESHOLD = 0.0001; const isPositionChanged = Object.keys(params).some(key => //Use a tolerance threshold to compare floating-point values - Math.abs((currentPosition[key] || 0) - (params[key] || 0)) > 0.0001 + Math.abs((currentPosition[key] || 0) - (params[key] || 0)) > POSITION_CHANGE_THRESHOLD
302-309: Consider adding a small transition duration.A duration of 0 results in an instant camera movement which might be disorienting. Consider adding a small duration for smoother transitions.
if (isPositionChanged) { reearth.camera.flyTo( params, { - duration: 0, + duration: 0.3, // Add a short duration for smoother transition easing: (t) => t * t } );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/beta/features/PluginPlayground/Plugins/presets/camera/cameraPosition.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Redirect rules - reearth-web
- GitHub Check: Header rules - reearth-web
- GitHub Check: ci-server / ci-server-test
- GitHub Check: Pages changed - reearth-web
- GitHub Check: ci-server / ci-server-lint
- GitHub Check: ci-web / ci
🔇 Additional comments (2)
web/src/beta/features/PluginPlayground/Plugins/presets/camera/cameraPosition.ts (2)
1-2: LGTM! Well-structured plugin configuration.The file structure follows TypeScript best practices with proper typing and clear organization of plugin components.
Also applies to: 4-18, 20-24, 320-324
7-15: LGTM! Clear and concise plugin metadata.The YAML configuration provides essential plugin information with appropriate versioning and description.
| // Collect ALL input values | ||
| Object.keys(inputs).forEach(key => { | ||
| const value = inputs[key].value.trim(); | ||
| if (value !== '') { | ||
| cameraParams[key] = Number(value); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for numeric input values.
The code should validate that the parsed numbers are finite and within acceptable ranges.
// Collect ALL input values
Object.keys(inputs).forEach(key => {
const value = inputs[key].value.trim();
if (value !== '') {
- cameraParams[key] = Number(value);
+ const num = Number(value);
+ if (isFinite(num)) {
+ cameraParams[key] = num;
+ } else {
+ statusMessage.textContent = `Invalid value for ${key}`;
+ return;
+ }
}
});📝 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.
| // Collect ALL input values | |
| Object.keys(inputs).forEach(key => { | |
| const value = inputs[key].value.trim(); | |
| if (value !== '') { | |
| cameraParams[key] = Number(value); | |
| } | |
| }); | |
| // Collect ALL input values | |
| Object.keys(inputs).forEach(key => { | |
| const value = inputs[key].value.trim(); | |
| if (value !== '') { | |
| const num = Number(value); | |
| if (isFinite(num)) { | |
| cameraParams[key] = num; | |
| } else { | |
| statusMessage.textContent = `Invalid value for ${key}`; | |
| return; | |
| } | |
| } | |
| }); |
OpeDada
deleted the
refactor/plugin-playground-presets-camera-position
branch
Overview
I added a show Info section to this plugin to guide user on how to use the plugin
What I've done
What I haven't done
How I tested
Locally
Which point I want you to review particularly
If this feature is necessary or not
Memo
Summary by CodeRabbit
New Features
- Introduced an enhanced information panel that provides clear usage instructions and offers toggle functionality for a more intuitive experience.
Style
- Updated the visual layout with a sticky information section and refined control styling, including optimized spacing, to improve overall usability.
Functionality
- Improved interaction behavior, ensuring action buttons are enabled only when appropriate inputs are provided.