fix: Missing "Open in room" button on widget's shared screen#40331
Merged
Conversation
Contributor
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 23ddc7b The changes in this PR will be included in the next version bump. This PR includes changesets to release 43 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
WalkthroughAdds an "Open in room" action to the shared screen StreamCard: new i18n key, a new StreamCardOpenInRoomButton component, StreamCard accepts an optional onClickOpenInRoom prop and renders the button when provided, and the MediaCallWidget wires that handler to the existing direct-message action. ChangesOpen-in-Room Stream Card
Sequence DiagramsequenceDiagram
participant User as User
participant StreamCard as StreamCard UI
participant OpenBtn as StreamCardOpenInRoomButton
participant Widget as MediaCallWidget (handler)
User->>StreamCard: Hover / view shared screen
StreamCard->>OpenBtn: Render button (showOnHover = true)
User->>OpenBtn: Click "Open in room"
OpenBtn->>Widget: call onClickOpenInRoom()
Widget->>Widget: invoke onClickDirectMessage logic (open room)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Git: Failed to clone repository. Please run the 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui-voip/src/views/MediaCallWidget/OngoingCallWithScreen.tsx`:
- Line 69: The StreamCard's onClickOpenInRoom prop is wired to
onClickDirectMessage (opening a DM) instead of the room navigation handler;
update the prop passed to StreamCard so onClickOpenInRoom uses the room-opening
function (e.g., onClickOpenInRoom or a new handler that navigates to the
room/channel context) rather than onClickDirectMessage, ensuring the
StreamCardOpenInRoom button triggers room navigation; locate the StreamCard
instantiation in OngoingCallWithScreen.tsx and replace the
onClickOpenInRoom={onClickDirectMessage} binding with the correct room
navigation handler.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: af1b04d0-d07c-4f0e-bf47-2d631e975ba8
📒 Files selected for processing (5)
.changeset/little-cars-love.mdpackages/i18n/src/locales/en.i18n.jsonpackages/ui-voip/src/components/Cards/StreamCard/StreamCard.tsxpackages/ui-voip/src/components/Cards/StreamCard/StreamCardOpenInRoom.tsxpackages/ui-voip/src/views/MediaCallWidget/OngoingCallWithScreen.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/ui-voip/src/views/MediaCallWidget/OngoingCallWithScreen.tsxpackages/ui-voip/src/components/Cards/StreamCard/StreamCardOpenInRoom.tsxpackages/ui-voip/src/components/Cards/StreamCard/StreamCard.tsx
🧠 Learnings (10)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39858
File: apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts:123-151
Timestamp: 2026-04-17T18:33:27.211Z
Learning: In RocketChat/Rocket.Chat (`apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts`), `executeBlockActionHandler` invocations originating from a **modal** surface intentionally do NOT include a `block_action_room` (room property) in the interaction payload. Modals are not scoped to a room, so no room id is available in that context. Do not flag the absence of a room assertion in the modal block-action test as a missing coverage bug; instead, document it explicitly with a `test.step` asserting the room entry is `undefined`.
📚 Learning: 2026-02-26T19:22:29.385Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryActions.tsx:40-40
Timestamp: 2026-02-26T19:22:29.385Z
Learning: For TSX files in the UI VOIP package, ensure that when a media session state is 'unavailable', the voiceCall action is excluded from the actions object passed to CallHistoryActions so it does not render in the menu. This filtering should occur upstream (before getItems is called) to avoid tooltips or UI hints for unavailable actions. If there are multiple actions with availability states, implement a centralized helper to filter actions based on session state.
Applied to files:
packages/ui-voip/src/views/MediaCallWidget/OngoingCallWithScreen.tsxpackages/ui-voip/src/components/Cards/StreamCard/StreamCardOpenInRoom.tsxpackages/ui-voip/src/components/Cards/StreamCard/StreamCard.tsx
📚 Learning: 2026-03-26T16:42:35.978Z
Learnt from: pierre-lehnen-rc
Repo: RocketChat/Rocket.Chat PR: 39870
File: packages/ui-voip/src/providers/useScreenShareStreams.ts:13-21
Timestamp: 2026-03-26T16:42:35.978Z
Learning: In `packages/ui-voip/src/providers/useScreenShareStreams.ts` (RocketChat/Rocket.Chat), the early return on `!instanceState.confirmed` in `getStreamWrappers` is intentional design: screen-share streams (both local and remote) are not used during unconfirmed/pending calls, so returning `null` for the whole unconfirmed state is correct. Do not suggest splitting the guard to expose the local participant stream before call confirmation.
Applied to files:
packages/ui-voip/src/views/MediaCallWidget/OngoingCallWithScreen.tsx
📚 Learning: 2025-12-18T15:18:31.688Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37773
File: apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx:24-34
Timestamp: 2025-12-18T15:18:31.688Z
Learning: In apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx, for internal call history items, the item.contactId is guaranteed to always match either the caller.id or callee.id in the call data, so the contact resolution in getContact will never result in undefined.
Applied to files:
packages/ui-voip/src/views/MediaCallWidget/OngoingCallWithScreen.tsx
📚 Learning: 2026-04-17T17:38:15.994Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39858
File: packages/ui-kit/src/interactions/UserInteraction.ts:33-33
Timestamp: 2026-04-17T17:38:15.994Z
Learning: In RocketChat/Rocket.Chat (`packages/ui-kit/src/interactions/UserInteraction.ts`), `ViewSubmitUserInteraction` and `ViewClosedUserInteraction` intentionally do NOT include `rid` when the interaction originates from a **modal** surface. Modals are not scoped to a room, so no room id is available in that context. The `rid?: string` field is optional to support the contextual bar surface (where room context exists) while remaining absent for modals. Do not flag the absence of `rid` in modal viewSubmit/viewClosed interactions as a missing-context bug.
Applied to files:
packages/ui-voip/src/views/MediaCallWidget/OngoingCallWithScreen.tsx
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
packages/ui-voip/src/views/MediaCallWidget/OngoingCallWithScreen.tsxpackages/ui-voip/src/components/Cards/StreamCard/StreamCardOpenInRoom.tsxpackages/ui-voip/src/components/Cards/StreamCard/StreamCard.tsx
📚 Learning: 2025-11-17T22:38:48.631Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37505
File: packages/i18n/src/locales/en.i18n.json:3765-3765
Timestamp: 2025-11-17T22:38:48.631Z
Learning: Rocket.Chat i18n copy: Keep sentence case for the value of "Notification_Desktop_show_voice_calls" in packages/i18n/src/locales/en.i18n.json (“Show desktop notifications for voice calls”) per design directive; do not change to Title Case even if nearby labels differ.
Applied to files:
.changeset/little-cars-love.md
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
.changeset/little-cars-love.md
📚 Learning: 2026-03-16T21:50:37.589Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:37.589Z
Learning: For changes related to OpenAPI migrations in Rocket.Chat/OpenAPI, when removing endpoint types and validators from rocket.chat/rest-typings (e.g., UserRegisterParamsPOST, /v1/users.register) document this as a minor changeset (not breaking) per RocketChat/Rocket.Chat-Open-API#150 Rule 7. Note that the endpoint type is re-exposed via a module augmentation .d.ts in the consuming package (e.g., packages/web-ui-registration/src/users-register.d.ts). In reviews, ensure the changeset clearly states: this is a non-breaking change, the major version should not be bumped, and the changeset reflects a minor version bump. Do not treat this as a breaking change during OpenAPI migrations.
Applied to files:
.changeset/little-cars-love.md
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
packages/i18n/src/locales/en.i18n.json
🔇 Additional comments (4)
.changeset/little-cars-love.md (1)
1-6: Looks consistent.The changeset metadata matches the PR scope and the minor bumps look appropriate for this UI addition.
packages/i18n/src/locales/en.i18n.json (1)
4043-4043: Looks good — translation key is added correctly.
Open_in_roomis well-formed and appropriately placed inen.i18n.jsonfor this change.Based on learnings: In this repository, new translation keys should be added to
packages/i18n/src/locales/en.i18n.jsononly; other locale files are handled by the translation pipeline/fallback.packages/ui-voip/src/components/Cards/StreamCard/StreamCard.tsx (1)
4-48: Looks good.The new
onClickOpenInRoomprop is threaded through cleanly and the button is conditionally rendered in the right place. No concerns here.packages/ui-voip/src/components/Cards/StreamCard/StreamCardOpenInRoom.tsx (1)
1-22: Looks good.The component is minimal, typed cleanly, and follows the existing card-slot hover pattern. No concerns here.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #40331 +/- ##
===========================================
- Coverage 69.68% 69.66% -0.03%
===========================================
Files 3322 3323 +1
Lines 122827 122843 +16
Branches 21921 21920 -1
===========================================
- Hits 85598 85579 -19
- Misses 33887 33899 +12
- Partials 3342 3365 +23
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
MartinSchoeler
approved these changes
May 5, 2026
gabriellsh
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
gabriellsh
removed
the
stat: QA assured
dionisio-bot
Bot
removed
the
stat: ready to merge
gabriellsh
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
gabriellsh
removed
the
stat: QA assured
dionisio-bot
Bot
removed
the
stat: ready to merge
gabriellsh
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
gabriellsh
removed
the
stat: QA assured
dionisio-bot
Bot
removed
the
stat: ready to merge
gabriellsh
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
gabriellsh
removed
the
stat: QA assured
dionisio-bot
Bot
removed
the
stat: ready to merge
gabriellsh
added
stat: ready to merge
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
scuciatto
removed
the
stat: QA assured
dionisio-bot
Bot
removed
the
stat: ready to merge
scuciatto
added
stat: ready to merge
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
Issue(s)
SSGA-13
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Localization