-
Notifications
You must be signed in to change notification settings - Fork 13k
feat: Add "Direct Message" button to Media Call widget #38169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: afcbedd The changes in this PR will be included in the next version bump. This PR includes changesets to release 40 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 |
WalkthroughAdds a DM quick-link from the call widget: introduces a new Changes
Sequence DiagramsequenceDiagram
participant Room as Room Component
participant Tracker as useMediaCallOpenRoomTracker
participant Context as MediaCallContext
participant Provider as MediaCallProvider
participant Widget as OngoingCall Widget
participant User as User
participant Router as Router
Room->>Tracker: invoke with room._id
Tracker->>Context: read setOpenRoomId
Tracker->>Context: setOpenRoomId(room._id)
Tracker-->>Context: on unmount setOpenRoomId(undefined)
Widget->>Context: useMediaCallContext()
Context-->>Widget: { onClickDirectMessage, ... }
User->>Widget: click Direct Message button
Widget->>Context: call onClickDirectMessage()
Context->>Router: navigate to direct message route (rid)
Router-->>Room: updates current room
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
c24ee18 to
813677a
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #38169 +/- ##
===========================================
- Coverage 70.71% 70.67% -0.05%
===========================================
Files 3141 3142 +1
Lines 108799 108820 +21
Branches 19587 19644 +57
===========================================
- Hits 76942 76912 -30
- Misses 29857 29911 +54
+ Partials 2000 1997 -3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 19 files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.changeset/eighty-weeks-rush.md:
- Line 7: Update the changelog sentence that currently reads "Introduces in the
call widget a quick link that redirects to the participants direct message" to
use the correct possessive form by changing "participants direct message" to
"participant's direct message", ensuring the message now reads "Introduces in
the call widget a quick link that redirects to the participant's direct
message".
In `@packages/ui-client/src/hooks/useGoToDirectMessage.ts`:
- Around line 20-27: The hook creates an unnecessary in-memory subscription by
passing an empty string; instead pass the actual username (possibly undefined)
and make useUserSubscriptionByName guard against falsy input so it returns
undefined early without creating watchers. Update useGoToDirectMessage to call
useUserSubscriptionByName(targetUser.username) (not ''), and change the
useUserSubscriptionByName implementation/signature to accept string | undefined
and immediately return undefined when username is falsy, ensuring no
subscription/predicate is created and types are updated accordingly.
🧹 Nitpick comments (3)
packages/ui-voip/src/views/MediaCallWidget/OngoingCall.stories.tsx (1)
66-72: LGTM! Story demonstrates the DM button feature.The story correctly wraps
OngoingCallwithMockedMediaCallProviderpassingonClickDirectMessageto enable the DM button rendering.Consider using
action('onClickDirectMessage')from@storybook/addon-actionsinstead of() => undefinedto log interactions in the Storybook Actions panel, which aids visual testing.packages/ui-voip/src/context/MediaCallProvider.tsx (1)
13-13: Remove commented-out import.Per coding guidelines, avoid code comments in implementation files. This commented import should be removed.
Suggested fix
- // useRouteParameter,apps/meteor/client/views/mediaCallHistory/useMediaCallInternalHistoryActions.ts (1)
50-50: Consider passingundefinedinstead of empty string.While this works correctly (empty string is falsy, so the
alreadyOpencheck behaves as expected), passingundefinedis semantically clearer when no room is open.Suggested fix
- const goToDirectMessage = useGoToDirectMessage({ username: contact.username }, openRoomId ?? ''); + const goToDirectMessage = useGoToDirectMessage({ username: contact.username }, openRoomId);
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Douglas Fabris <devfabris@gmail.com>
Proposed changes (including videos or screenshots)
Added a quick link to go back to the callers DM.
Issue(s)
VGA-85
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.