feat: New ui-data-getter and refactor of ui-data's #23314
Conversation
|
""" WalkthroughThis set of changes updates the import paths for several constants and types from the Sequence Diagram(s)PresentationMenu: Slide Content Extraction and Plugin Event DispatchsequenceDiagram
participant User
participant PresentationMenu
participant WhiteboardAPI
participant PluginConsumer
User->>PresentationMenu: Clicks snapshot menu item
PresentationMenu->>WhiteboardAPI: Get slide shapes and poll shapes
WhiteboardAPI-->>PresentationMenu: Returns shapes and poll data
PresentationMenu->>PresentationMenu: extractSlideContentToImage()
PresentationMenu->>PresentationMenu: Generate image (SVG/PNG)
PresentationMenu->>User: Initiates file download
PluginConsumer->>PresentationMenu: Subscribes to UI_DATA_GETTER_SUBSCRIBED event
PresentationMenu->>PresentationMenu: extractSlideContentToImage()
PresentationMenu->>PluginConsumer: Dispatches image data as custom event
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![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 (1)
bigbluebutton-html5/imports/ui/components/presentation/presentation-menu/component.jsx (1)
201-234: Fix misleading function name.The function name
updateUiDataHookPCurrentWhiteboardSVGWithAnnotationsForPlugincontains "SVG" but it actually dispatches PNG data. This is confusing and should be corrected.- const updateUiDataHookPCurrentWhiteboardSVGWithAnnotationsForPlugin = async () => { + const updateUiDataHookCurrentWhiteboardPNGWithAnnotationsForPlugin = async () => {Also update the references to this function in lines 225 and 230.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bigbluebutton-html5/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
bigbluebutton-html5/imports/startup/client/intlAdapter.tsx(1 hunks)bigbluebutton-html5/imports/ui/components/chat/chat-graphql/chat-message-form/component.tsx(1 hunks)bigbluebutton-html5/imports/ui/components/external-video-player/external-video-player-graphql/component.tsx(1 hunks)bigbluebutton-html5/imports/ui/components/plugins-engine/ui-data-hooks/layout/presentation-area/utils.ts(1 hunks)bigbluebutton-html5/imports/ui/components/presentation/presentation-menu/component.jsx(3 hunks)bigbluebutton-html5/imports/ui/components/user-list/user-list-content/user-participants/user-list-participants/component.tsx(1 hunks)bigbluebutton-html5/package.json(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Plugin SDK - Match version of html5 package.json with bigbluebutton.properties
bigbluebutton-html5/package.json
[error] 1-1: The bigbluebutton-html-plugin-sdk dependency is pointing to a commit in the SDK's repository, which is not allowed.
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: install-and-run-bbb-tests (8)
- GitHub Check: install-and-run-bbb-tests (7)
- GitHub Check: install-and-run-bbb-tests (6)
- GitHub Check: install-and-run-bbb-tests (5)
- GitHub Check: install-and-run-bbb-tests (4)
- GitHub Check: install-and-run-bbb-tests (3)
- GitHub Check: install-and-run-bbb-tests (2)
- GitHub Check: install-and-run-bbb-tests (1)
- GitHub Check: install-and-run-plugin-tests
🔇 Additional comments (8)
bigbluebutton-html5/imports/startup/client/intlAdapter.tsx (1)
9-9: Import path update aligns with reorganized SDK structure
The path forUI_DATA_LISTENER_SUBSCRIBEDhas been updated fromui-data-hooks/conststoui-data/hooks/consts, matching the new module layout in the plugin SDK. This change is consistent with other files and should resolve import errors.bigbluebutton-html5/imports/ui/components/chat/chat-graphql/chat-message-form/component.tsx (1)
14-15: Consistent SDK import path adjustments
BothUI_DATA_LISTENER_SUBSCRIBEDandChatFormUiDataPayloadsimports have been moved to their new locations underui-data/hooks/constsandui-data/domain/chat/form/typesrespectively. This aligns with the SDK restructuring and maintains consistency across components.bigbluebutton-html5/imports/ui/components/user-list/user-list-content/user-participants/user-list-participants/component.tsx (1)
3-4: Updated SDK import paths for user-list UI data hooks
The imports forUI_DATA_LISTENER_SUBSCRIBEDandUserListUiDataPayloadsnow point to the reorganized directories (ui-data/hooks/constsandui-data/domain/user-list/types). This mirrors the changes in other UI-data consumers and should ensure correct resolution.bigbluebutton-html5/imports/ui/components/plugins-engine/ui-data-hooks/layout/presentation-area/utils.ts (1)
4-5: Align presentation-area hook imports with new SDK layout
LayoutPresentationAreaUiDataPayloadsandUI_DATA_LISTENER_SUBSCRIBEDhave been relocated toui-data/domain/layout/presentation-area/typesandui-data/hooks/constsrespectively. These paths reflect the updated plugin SDK structure and maintain parity with other UI-data hooks.bigbluebutton-html5/imports/ui/components/external-video-player/external-video-player-graphql/component.tsx (1)
17-19: Import path updates look good.The import paths have been correctly updated to reflect the new SDK structure, changing from
ui-data-hooksto the newui-dataorganization.bigbluebutton-html5/imports/ui/components/presentation/presentation-menu/component.jsx (3)
6-7: New imports for ui-data-getter functionality.The imports correctly support the new on-demand data fetching feature as described in the PR objectives.
158-200: Well-implemented image extraction helper function.The
extractSlideContentToImagefunction properly centralizes the slide content extraction logic with good platform-specific handling for iOS/Safari (SVG) vs other platforms (PNG). The implementation correctly processes both whiteboard shapes and poll shapes.
397-408: Good refactoring using the new helper function.The snapshot handler has been nicely simplified by using the new
extractSlideContentToImagehelper function, improving code reusability and maintainability while preserving all the original functionality.
…tton pattern of working with the whiteboard
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bigbluebutton-html5/imports/ui/components/presentation/presentation-menu/component.jsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build-package (others)
- GitHub Check: build-package (bbb-html5)
- GitHub Check: build-package (bbb-graphql-server)
- GitHub Check: build-package (bbb-playback-record)
🔇 Additional comments (3)
bigbluebutton-html5/imports/ui/components/presentation/presentation-menu/component.jsx (3)
6-7: Import path updates look consistent with SDK restructuring.The import paths have been updated to reflect the new module organization in the
bigbluebutton-html-plugin-sdk. This aligns with the AI summary indicating SDK restructuring.
201-233: Plugin integration implementation looks solid.The useEffect hook properly sets up and cleans up the event listener for plugin SDK integration. The error handling with specific log codes and the async event handler implementation are well done.
The event listener name construction using template literals and the proper cleanup in the return function demonstrate good React practices.
397-407: Excellent refactoring of snapshot download logic.The simplified onClick handler that leverages the new
extractSlideContentToImagefunction is much cleaner and more maintainable than the previous inline implementation. The error handling flow is preserved while eliminating code duplication.This change significantly improves code maintainability by centralizing the image extraction logic.
bigbluebutton-html5/imports/ui/components/presentation/presentation-menu/component.jsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/docs/plugins.md (1)
850-850: Use hyphen in “well-defined”
For grammatical accuracy, change “well defined” to “well-defined”:- As seen for the `useUiData`, the return type is well defined by the enum chosen... + As seen for the `useUiData`, the return type is well-defined by the enum chosen...🧰 Tools
🪛 LanguageTool
[misspelling] ~850-~850: This word is normally spelled with a hyphen.
Context: ...for theuseUiData, the return type is well defined by the enum chosen (in other words, nam...(EN_COMPOUNDS_WELL_DEFINED)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/docs/plugins.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/docs/plugins.md
[misspelling] ~850-~850: This word is normally spelled with a hyphen.
Context: ...for the useUiData, the return type is well defined by the enum chosen (in other words, nam...
(EN_COMPOUNDS_WELL_DEFINED)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: build-package (others)
- GitHub Check: build-package (bbb-freeswitch)
- GitHub Check: build-package (bbb-fsesl-akka)
- GitHub Check: build-package (bbb-graphql-server)
- GitHub Check: build-package (bbb-playback-record)
- GitHub Check: build-package (bbb-learning-dashboard)
|
Do we need bigbluebutton/bigbluebutton-html-plugin-sdk#188 merged (and released) first? |
|
@antobinary, we do, actually. Once that's merged and published, I'll bump the version here. |
|
Automated tests Summary✅ All the CI tests have passed! |
4 participants
What does this PR do?
This PR introduces two main changes:
Adds a new
ui-data-getterA utility to fetch UI data on demand — unlike existing reactive hooks, this does not automatically update when data changes. It fetches the current data only when explicitly requested.
Refactors existing
ui-datalogicUpdates the previous
ui-data-hooksstructure to integrate seamlessly with the newui-data-getter. Both now align under a shared goal: providing access to UI-related information, but with different update strategies (on-demand vs. reactive).Why is this needed?
A key use case is retrieving a snapshot of the current slide with annotations. Doing this reactively (i.e., every time a new annotation is added) would be inefficient — especially in scenarios like a live class with multiple students making changes simultaneously.
This new approach avoids unnecessary computation by allowing a one-time fetch when needed.
Additionally, this design lays the groundwork for flexibility: if reactivity is later desired for specific data (e.g.,
presentation-whiteboard-current-slide), we can easily expose it as a hook with minimal changes to the core and SDK.Motivation
As part of the La Suite Numérique docs integration, we need to append a base64-encoded PNG snapshot of the current slide at the end of the document. This feature supports that requirement cleanly and efficiently.
More
Closely related to bigbluebutton/bigbluebutton-html-plugin-sdk#188
Updates on the plugin itself: iMDT/bbb-paris-hackdays#5