refactor(web): add toggle-button to plugin and rename plugin name #1504
Conversation
WalkthroughThis pull request standardizes plugin identifiers and names and updates related translations. It refactors the UI messenger plugin by renaming its identifiers and corresponding keys in title mappings and translation files. Additionally, a deprecated plugin is removed from the preset list. The user interface for toggling shadow and terrain functionality is updated by replacing button controls with toggle switches, accompanied by revised event handling and error logging. Minor adjustments include adding a documentation comment for mouse events and correcting a button label typo. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Toggle
participant Plugin
participant Parent
User->>Toggle: Toggle switch changed
Toggle->>Plugin: Emit change event
Plugin->>Plugin: Update status display ("ON"/"OFF")
Plugin->>Parent: Send message to update shadow/terrain state
Parent-->>Plugin: Acknowledge change
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 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
🔭 Outside diff range comments (2)
web/src/beta/features/PluginPlayground/Plugins/presets/data/uiExtensionMessenger.ts (2)
26-28:⚠️ Potential issueFile identifiers need to be updated
The widget file identifiers still use the old naming convention, which could cause reference issues.
Update the file identifiers to match the new naming scheme:
- id: "ui-extension-messenger-widget", - title: "ui-extension-messenger.js", + id: "messenger-between-extension-and-visualizer-widget", + title: "messenger-between-extension-and-visualizer.js",
160-163:⚠️ Potential issueExported plugin type ID needs updating
The exported constant still uses the old ID, which will cause inconsistencies when the plugin is referenced elsewhere in the codebase.
Update the exported type ID:
export const uiExtensionMessenger: PluginType = { - id: "ui-extension-messenger", + id: "messenger-between-extension-and-visualizer", files: [yamlFile, widgetFile] };You may also want to consider renaming the exported constant itself for consistency:
-export const uiExtensionMessenger: PluginType = { +export const messengerBetweenExtensionAndVisualizer: PluginType = {However, this would require updating all import references elsewhere in the codebase.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
web/src/beta/features/PluginPlayground/Plugins/presets/data/uiExtensionMessenger.ts(1 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/index.ts(2 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/useTitles.ts(1 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/viewer/enableShadowStyle.ts(1 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/viewer/enableTerrain.ts(1 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/viewer/mouseEvent.ts(1 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/viewer/takeScreenshot.ts(1 hunks)web/src/services/i18n/translations/en.yml(1 hunks)web/src/services/i18n/translations/ja.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: ci-server / ci-server-lint
- GitHub Check: ci-web / ci
- GitHub Check: ci-server / ci-server-test
🔇 Additional comments (13)
web/src/beta/features/PluginPlayground/Plugins/presets/viewer/takeScreenshot.ts (1)
56-56: Corrected capitalization in button labelFixed the capitalization of "Screenshot" in the button label for consistency with standard capitalization practices.
web/src/services/i18n/translations/en.yml (1)
343-343: Updated plugin name for clarityThe translation key has been changed from "UI Extension Messenger" to "Messenger between Extension and Visualizer", which better describes the plugin's purpose and functionality.
web/src/beta/features/PluginPlayground/Plugins/presets/viewer/mouseEvent.ts (1)
78-78: Added helpful documentation linkAdded a reference link to the official documentation for mouse events, which improves developer experience by providing easy access to relevant API documentation.
web/src/services/i18n/translations/ja.yml (1)
346-346: Updated Japanese translation key for consistencyThe translation key has been updated to match the new English key "Messenger between Extension and Visualizer" while maintaining the same Japanese translation text.
web/src/beta/features/PluginPlayground/Plugins/presets/index.ts (2)
39-39: Commented out unused importThe import for
showLabelhas been commented out as part of removing this plugin from the available options.
71-71: Removed redundant plugin with explanatory commentThe
showLabelplugin has been removed from the preset plugins list with a clear comment explaining the rationale: it could cause conflicts when clients request the same functionality in another plugin.web/src/beta/features/PluginPlayground/Plugins/presets/useTitles.ts (1)
28-30: Plugin name updated correctly in title mappingsThe change from "UI Extension Messenger" to "Messenger between Extension and Visualizer" aligns with the PR objectives to improve clarity. The updated name better describes the plugin's functionality as a communication bridge.
web/src/beta/features/PluginPlayground/Plugins/presets/viewer/enableShadowStyle.ts (3)
32-91: Good UI enhancement with toggle switch implementationThe replacement of a button with a modern toggle switch provides a clearer visual indication of the current state (on/off). The CSS styling follows best practices for toggle switches with appropriate transitions and visual feedback.
93-99: Well-structured toggle container with clear status indicatorThe HTML structure provides a clean layout with the toggle switch and a status indicator that clearly shows the current state of the shadow.
100-124: Improved error handling and event managementThe implementation now includes proper error handling with console error logging if required elements aren't found. The event listener is appropriately set up for the toggle switch and includes clear status updates.
web/src/beta/features/PluginPlayground/Plugins/presets/viewer/enableTerrain.ts (3)
32-91: Consistent UI approach with shadow toggle implementationThe terrain toggle uses the same UI pattern as the shadow toggle, maintaining consistency across the application. The CSS styling is well-structured and follows modern toggle switch design patterns.
93-99: Well-structured toggle container with clear status indicatorThe HTML structure provides a clean layout with the toggle switch and a status indicator that clearly shows the current state of the terrain feature.
100-124: Good error handling and event managementSimilar to the shadow toggle implementation, this includes proper error handling and clear status updates. The code is well-organized and follows best practices for event handling.
web/src/beta/features/PluginPlayground/Plugins/presets/data/uiExtensionMessenger.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/src/beta/features/PluginPlayground/Plugins/presets/data/uiExtensionMessenger.ts (1)
1-3: Consider renaming the file to match new plugin nameWhile all internal identifiers have been updated, the filename itself (
uiExtensionMessenger.ts) still reflects the old naming convention. Consider renaming the file tomessengerBetweenExtensionAndVisualizer.tsfor complete consistency, though this would require updating imports in other files.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/beta/features/PluginPlayground/Plugins/presets/data/uiExtensionMessenger.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: Pages changed - reearth-web
- GitHub Check: ci-server / ci-server-test
- GitHub Check: ci-web / ci
- GitHub Check: ci-server / ci-server-lint
🔇 Additional comments (3)
web/src/beta/features/PluginPlayground/Plugins/presets/data/uiExtensionMessenger.ts (3)
4-23: YAML file definition consistently renamedThe renaming from "UI Extension Messenger" to "Messenger between Extension and Visualizer" has been applied consistently throughout the YAML file definition, including the file ID, plugin ID, name, and extension properties. This change aligns with the PR objective to improve clarity about the plugin's functionality.
25-28: Widget file identifiers properly updatedThe widget file identifiers have been correctly updated to match the new naming convention, addressing the inconsistencies pointed out in the previous review. Both the ID and title now use the "messenger-between-extension-and-visualizer" naming pattern.
160-163: Exported constant ID updatedThe exported constant's ID has been updated to "messenger-between-extension-and-visualizer", maintaining consistency with the renaming throughout the file and addressing the previous review feedback.
mulengawilfred
changed the title
mulengawilfred
changed the title
There was a problem hiding this comment.
Thank you for your contribution! I have one idea of UI.
Looking at other examples, the plugin UI features rounded corners, while this plugin has square corners. I think it might be better to standardize the design to one style. What do you think?
There was a problem hiding this comment.
Thanks. Unifying the design across plugins is correctly in progress in another task. So okay to have this one as is.
There was a problem hiding this comment.
Same as above.
Looking at other examples, the plugin UI features rounded corners, while this plugin has square corners. I think it might be better to standardize the design to one style. What do you think?
There was a problem hiding this comment.
Thanks. Unifying the design across plugins is correctly in progress in another task. So okay to have this one as is.
| @@ -343,7 +343,7 @@ Sidebar: サイドバー | |||
| Header: ヘッダー | |||
| Popup: ポップアップ | |||
| Modal Window: モーダルウィンドウ | |||
| UI Extension Messenger: エクステンションとコアとの通信 | |||
| Messenger between Extension and Visualizer: エクステンションとコアとの通信 | |||
There was a problem hiding this comment.
I think below Japanese is more natural and suitable.
Messenger between Extension and Visualizer: エクステンションとVisualizerとの通信
There was a problem hiding this comment.
Thanks for confirming this translation. I have updated it.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/src/beta/features/PluginPlayground/Plugins/presets/data/messengerBetweenExtensions.ts (1)
8-9: Consider updating plugin display name for consistencyWhile the IDs have been updated to "messenger-between-extensions", the display name on line 9 still shows "Extension To Extension Messenger". Consider updating this for full naming consistency.
-name: Extension To Extension Messenger +name: Messenger Between Extensions
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
web/src/beta/features/PluginPlayground/Plugins/presets/data/messengerBetweenExtensions.ts(2 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/index.ts(4 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/useTitles.ts(1 hunks)web/src/services/i18n/translations/en.yml(2 hunks)web/src/services/i18n/translations/ja.yml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- web/src/beta/features/PluginPlayground/Plugins/presets/useTitles.ts
- web/src/services/i18n/translations/en.yml
- web/src/services/i18n/translations/ja.yml
- 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-test
- GitHub Check: ci-server / ci-server-lint
- GitHub Check: ci-web / ci
🔇 Additional comments (3)
web/src/beta/features/PluginPlayground/Plugins/presets/data/messengerBetweenExtensions.ts (3)
6-8: YAML file identifiers consistently renamedThe renaming from "extension-to-extension-messenger" to "messenger-between-extensions" in the YAML file identifiers improves clarity and aligns with the PR objective to use more descriptive naming.
231-238: Extension file identifiers consistently renamedBoth extension file identifiers have been appropriately updated to match the new naming convention, maintaining consistency throughout the plugin.
243-245:❓ Verification inconclusive
Exported constant renamed for better clarity
The exported constant has been renamed from
extensionExtensionMessengertomessengerBetweenExtensions, which is more descriptive of its purpose and consistent with the new naming pattern throughout the codebase.
🏁 Script executed:
#!/bin/bash # Verify that all imports of the old constant name have been updated # to use the new name throughout the codebase echo "Checking for any remaining references to the old constant name..." rg "extensionExtensionMessenger" --type tsLength of output: 183
Renamed constant applied consistently—please verify manually
The constant has been successfully renamed from
extensionExtensionMessengertomessengerBetweenExtensionsin
web/src/beta/features/PluginPlayground/Plugins/presets/data/messengerBetweenExtensions.ts(lines 243-245). The grep check did not return any lingering references to the old name, suggesting the renaming is complete. Please manually verify that the integration and runtime behavior remain unaffected.
- File affected:
web/src/beta/features/PluginPlayground/Plugins/presets/data/messengerBetweenExtensions.ts- Change: Renamed constant to better reflect its purpose and align with the current naming pattern.
- Next step: Manual verification is encouraged to ensure no indirect dependencies or side-effects remain.
Overview
This PR addresses several issues documented in the Plugin playground general Notion page:
Improvements
TerrainandShadow Stylesplugin to use a toggle button.Naming and Organization
UI Extension Messengerplugin to the more descriptiveMessenger between Extension and VisualizerWhat I've done
What I haven't done
Would need to confirm the translation for
Messenger between Extension and VisualizerHow I tested
Locally
Which point I want you to review particularly
None
Memo
Summary by CodeRabbit