feat(web): plugin playground presets extension to extension messenger #1367
Conversation
WalkthroughThis pull request introduces a new messaging extension plugin called Changes
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. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1367 +/- ##
==========================================
- Coverage 22.74% 22.60% -0.15%
==========================================
Files 1047 1054 +7
Lines 109304 110058 +754
Branches 669 676 +7
==========================================
+ Hits 24865 24877 +12
- Misses 83178 83920 +742
Partials 1261 1261
Flags with carried forward coverage won't be shown. Click here to find out more.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
web/src/beta/features/PluginPlayground/Plugins/presets/communication/extensionExtensionMessenger.ts (1)
80-90: Improve accessibility support.The UI components lack proper accessibility attributes and keyboard navigation support.
Add accessibility improvements:
<div id="wrapper"> - <h2>Messenger 1</h2> + <h2 id="messenger-title">Messenger 1</h2> <div class="flex-center"> <input id="messageInput" type="text" + aria-label="Message input" + aria-describedby="messenger-title" placeholder="Enter message"/> - <button id="sendButton">Send</button> + <button id="sendButton" + aria-label="Send message" + role="button" + tabindex="0">Send</button> </div> - <h3>Received Messages</h3> + <h3 id="received-title">Received Messages</h3> <div class="received"> - <div id="messagesContainer" class="messages-container"></div> + <div id="messagesContainer" + class="messages-container" + role="log" + aria-labelledby="received-title"></div> </div> </div>Also applies to: 182-192
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/beta/features/PluginPlayground/Plugins/presets/communication/extensionExtensionMessenger.ts(1 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/index.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: ci-server / ci-server-lint
- GitHub Check: ci-web / ci
- GitHub Check: ci-server / ci-server-test
- 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/index.ts (1)
3-4: LGTM! Clean integration of the new extension messenger.The changes properly integrate the new extension messenger while maintaining the existing UI messenger functionality.
Also applies to: 39-39
web/src/beta/features/PluginPlayground/Plugins/presets/communication/extensionExtensionMessenger.ts (1)
4-33: LGTM! Well-structured YAML configuration.The YAML configuration properly defines two widget extensions with appropriate metadata and layout settings.
.../beta/features/PluginPlayground/Plugins/presets/communication/extensionExtensionMessenger.ts
Show resolved
Hide resolved
.../beta/features/PluginPlayground/Plugins/presets/communication/extensionExtensionMessenger.ts
Show resolved
Hide resolved
.../beta/features/PluginPlayground/Plugins/presets/communication/extensionExtensionMessenger.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
web/src/beta/features/PluginPlayground/Plugins/presets/communication/extensionExtensionMessenger.ts (2)
84-92:⚠️ Potential issueAdd security measures for message handling.
The current implementation lacks several security measures:
- No origin validation in postMessage
- No input validation or sanitization
- No message size limits
Also applies to: 185-192
109-117:⚠️ Potential issueAdd error handling for extension communication.
The current implementation lacks error handling for extension operations and message posting.
Also applies to: 208-216
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/src/beta/features/PluginPlayground/Code/hook.ts(2 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/communication/extensionExtensionMessenger.ts(1 hunks)web/src/beta/features/Visualizer/Crust/Plugins/Plugin/hooks/usePluginAPI.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Redirect rules - reearth-web
- GitHub Check: ci-web / ci
- GitHub Check: Header rules - reearth-web
- GitHub Check: ci-server / ci-server-lint
- GitHub Check: Pages changed - reearth-web
- GitHub Check: ci-server / ci-server-test
🔇 Additional comments (4)
web/src/beta/features/PluginPlayground/Code/hook.ts (1)
125-128: LGTM! Enhanced widget identification.The changes improve widget identification and traceability by:
- Using UUID v4 for unique widget IDs
- Adding descriptive metadata (name, extensionId, pluginId)
web/src/beta/features/Visualizer/Crust/Plugins/Plugin/hooks/usePluginAPI.ts (1)
119-119: LGTM! Proper extension event handling.The addition of extensionEvents properly integrates extension-specific event handling with initialization and cleanup.
web/src/beta/features/PluginPlayground/Plugins/presets/communication/extensionExtensionMessenger.ts (2)
29-68: Reduce code duplication in styles.The CSS styles are duplicated between both messengers. Consider extracting them to a shared constant.
Also applies to: 131-170
240-244: LGTM! Well-structured plugin definition.The plugin is well-organized with clear separation of concerns between YAML configuration and widget implementations.
.../beta/features/PluginPlayground/Plugins/presets/communication/extensionExtensionMessenger.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
web/src/beta/features/PluginPlayground/Plugins/presets/communication/uiExtensionMessenger.ts (2)
83-91: Enhance accessibility with ARIA attributes.The info section should include ARIA attributes to improve accessibility for screen readers.
- <button id="info-toggle">Show Info</button> - <div id="info-content" style="display: none;"> + <button id="info-toggle" aria-expanded="false" aria-controls="info-content">Show Info</button> + <div id="info-content" style="display: none;" role="region" aria-labelledby="info-toggle">
126-132: Update ARIA attributes with toggle state.The toggle functionality should update ARIA attributes to maintain accessibility.
document.getElementById("info-toggle").addEventListener("click", () => { const infoContent = document.getElementById("info-content"); const isHidden = infoContent.style.display === "none"; infoContent.style.display = isHidden ? "block" : "none"; - document.getElementById("info-toggle").textContent = isHidden ? "Hide Info" : "Show Info"; + const toggleButton = document.getElementById("info-toggle"); + toggleButton.textContent = isHidden ? "Hide Info" : "Show Info"; + toggleButton.setAttribute("aria-expanded", isHidden ? "true" : "false"); });web/src/beta/features/PluginPlayground/Plugins/presets/communication/extensionExtensionMessenger.ts (1)
207-209: Enhance implementation documentation.While the note about Re:Earth visualizer requirements is helpful, consider adding more comprehensive documentation:
- Add JSDoc comments for each function
- Document the message format and types
- Include examples of valid messages
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/src/beta/features/PluginPlayground/Plugins/presets/communication/extensionExtensionMessenger.ts(1 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/communication/uiExtensionMessenger.ts(2 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/index.ts(2 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-web / ci
- GitHub Check: Header rules - reearth-web
- GitHub Check: ci-server / ci-server-lint
- GitHub Check: Pages changed - reearth-web
- GitHub Check: ci-server / ci-server-test
🔇 Additional comments (4)
web/src/beta/features/PluginPlayground/Plugins/presets/communication/uiExtensionMessenger.ts (1)
56-78: LGTM! Well-structured CSS additions.The new CSS styles for the info section are well-organized and follow consistent styling patterns.
web/src/beta/features/PluginPlayground/Plugins/presets/communication/extensionExtensionMessenger.ts (3)
84-92: Add security measures for message handling.The implementation needs security measures for safe message handling.
Also applies to: 95-104
109-125: Add error handling for extension communication.The extension communication logic needs proper error handling.
29-68: Reduce code duplication in styles.The CSS styles are duplicated between both extensions.
Also applies to: 132-170
.../beta/features/PluginPlayground/Plugins/presets/communication/extensionExtensionMessenger.ts
Show resolved
Hide resolved
|
|
||
| // YAML File Definition | ||
| const yamlFile: FileType = { | ||
| id: "extension-extension-messenger-reearth-yml", |
There was a problem hiding this comment.
I think we can call this extension-to-extension-messenger-reearth-yml
| name: Extension Extension Messenger | ||
| version: 1.0.0 | ||
| extensions: | ||
| - id: extension1 |
| @@ -89,6 +122,14 @@ const widgetFile: FileType = { | |||
| alt: 1000 // Fixed camera height for better viewing | |||
| }, "*"); | |||
| }); | |||
| // Toggle info section | |||
| document.getElementById("info-toggle").addEventListener("click", () => { | |||
There was a problem hiding this comment.
When I click the show info button in the ui-extension-messenger plugin, the widget disappears for me. Does the same happen for you @OpeDada ?
There was a problem hiding this comment.
I guess it's still the iframe auto resize bug. We could fix that in another PR i think.
There was a problem hiding this comment.
No , it just extends downwards for me. Maybe you can show me?
Overview
Created an extension to extension messenger plugin based on these requirements:
What I've done
What I haven't done
How I tested
Locally
Which point I want you to review particularly
Memo
Summary by CodeRabbit