-
Notifications
You must be signed in to change notification settings - Fork 13k
feat: Separate video call and voice call room actions #35700
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: 118602b The changes in this PR will be included in the next version bump. This PR includes changesets to release 36 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 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #35700 +/- ##
===========================================
- Coverage 60.98% 60.98% -0.01%
===========================================
Files 2956 2955 -1
Lines 70539 70526 -13
Branches 16174 16203 +29
===========================================
- Hits 43017 43007 -10
Misses 24571 24571
+ Partials 2951 2948 -3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
MarcosSpessatto
left a comment
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.
Please add a changeset and a description :)
dougfabris
left a comment
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.
can we avoid this data-qa-id please?
dougfabris
left a comment
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.
LGTM!
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
| dispatchWarning(); | ||
| }); |
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.
if (typeof remoteUser?.freeSwitchExtension === 'string') {
return makeCall(remoteUser.freeSwitchExtension);
}
dispatchWarning();The remoteUser?.freeSwitchExtension is being cast to a string without ensuring it is a valid string, which could lead to runtime errors.
This issue appears in multiple locations:
- apps/meteor/client/hooks/roomActions/useVoiceCallRoomAction.tsx: Lines 53-54
- apps/meteor/client/hooks/roomActions/useVoiceCallRoomAction.tsx: Lines 53-54
Please add a type check to ensurefreeSwitchExtensionis a valid string before making the call to prevent potential runtime errors.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Proposed changes (including videos or screenshots)
The phone icon does not intuitively represent a video call. Users expect separate, clearly distinguishable icons for Video Call and Voice Call, similar to other communication platforms. This PR separates the call items on room header
Before
After
Issue(s)
Steps to test or reproduce
Further comments
CORE-999
This pull request introduces a feature to separate video call and voice call room actions in the Rocket.Chat application. The changes include:
useVideoCallRoomActionhook by removing unused imports, simplifying the return structure, and consolidating the logic for video call action configuration.useVoiceCallRoomAction, which replaces the previoususeVoipMenuOptionsfunction, improving the structure for better maintainability and clarity.ui.tsfile to include new imports and modify the list of room action hooks by addinguseVideoCallRoomActionanduseVoiceCallRoomAction, while removinguseStartCallRoomAction.