-
Notifications
You must be signed in to change notification settings - Fork 13k
feat: Media Call History contextual bar #37739
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! 🎉 |
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR introduces UI components and translations for a call history contextual bar feature, including display components for internal and external contacts, a new Storybook story file, date formatting utilities, and translation keys for call-related labels. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
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 |
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: 3
🧹 Nitpick comments (5)
packages/i18n/src/locales/en.i18n.json (1)
914-914: Optional: unify “ID” acronym casing across call keysThis adds Call_ID while nearby we have Caller_Id and Client_ID. Consider standardizing on one style in a follow‑up (ID vs Id) to reduce drift. No change required for this PR.
packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryExternalUser.tsx (1)
7-19: LGTM! Component implementation is clean and straightforward.The phone number formatting logic (line 16) is simple and correct. If this pattern is reused elsewhere, consider extracting it to a utility function.
packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.tsx (3)
66-66: Clarify or remove the TODO comment.The comment mentions "Duration will also be displayed here," but duration is already shown at lines 123-125. If this TODO refers to integrating server-side block generation, consider updating the comment for clarity.
Do you want me to help generate a shared function for UiKit block generation that could be used on both client and server?
87-87: Remove commented-out code.The commented-out destructured variables should be removed. If these actions will be used in the future, they can be added back when needed.
Apply this diff:
- const { voiceCall, /* videoCall, jumpToMessage, */ directMessage, userInfo /* voiceCallExtension, direction */ } = actions; + const { voiceCall, directMessage, userInfo } = actions;
120-120: Consider alternative to negative margin.The negative margin
mbs={-8}might indicate a layout issue. Consider adjusting the MessageBlock or InfoPanelSection spacing instead of using negative margins.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
packages/i18n/src/locales/en.i18n.json(4 hunks)packages/ui-voip/package.json(2 hunks)packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.stories.tsx(1 hunks)packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.tsx(1 hunks)packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryExternalUser.tsx(1 hunks)packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryInternalUser.tsx(1 hunks)packages/ui-voip/src/views/CallHistoryContextualbar/useFullStartDate.ts(1 hunks)
🧰 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/CallHistoryContextualbar/CallHistoryContextualbar.tsxpackages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryInternalUser.tsxpackages/ui-voip/src/views/CallHistoryContextualbar/useFullStartDate.tspackages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.stories.tsxpackages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryExternalUser.tsx
🧠 Learnings (6)
📓 Common learnings
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.
📚 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:
packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.tsxpackages/i18n/src/locales/en.i18n.json
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.
Applied to files:
packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryInternalUser.tsx
📚 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/ui-voip/package.jsonpackages/i18n/src/locales/en.i18n.json
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
packages/ui-voip/package.json
📚 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:
packages/i18n/src/locales/en.i18n.json
🧬 Code graph analysis (3)
packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryInternalUser.tsx (2)
packages/ui-contexts/src/index.ts (2)
useUserAvatarPath(83-83)useUserPresence(99-99)packages/livechat/src/components/Avatar/index.tsx (1)
Avatar(21-51)
packages/ui-voip/src/views/CallHistoryContextualbar/useFullStartDate.ts (1)
packages/ui-contexts/src/index.ts (1)
useLanguage(38-38)
packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.stories.tsx (1)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (10)
packages/ui-voip/package.json (1)
37-37: LGTM! Dependency additions follow the established pattern.The addition of
@rocket.chat/fuselage-ui-kitto both devDependencies and peerDependencies is consistent with how other workspace UI packages are declared in this package (e.g.,@rocket.chat/ui-avatar,@rocket.chat/ui-client). This pattern correctly ensures the dependency is available for development/testing while requiring consuming applications to provide it at runtime.Also applies to: 80-80
packages/ui-voip/src/views/CallHistoryContextualbar/useFullStartDate.ts (2)
1-2: LGTM! Imports are correct.The imports are appropriate for this hook's functionality.
4-13: Implementation looks solid.The hook correctly memoizes the formatted date with appropriate dependencies, and the use of
Intl.DateTimeFormatwithdateStyle: 'full'andtimeStyle: 'medium'provides good internationalization support.packages/i18n/src/locales/en.i18n.json (3)
1807-1807: LGTM: generic label“Duration” fits existing patterns (e.g., Chat_Duration). No issues.
3970-3970: Key is properly wired in UI and StorybookThe new
Outgoing_voice_callkey is correctly referenced in the CallHistoryContextualbar component (packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.tsx:111) using the translation function, and the Storybook stories include it in mock translations. Implementation is complete.
2574-2574: Label is clear and actively used in call history UI; no duplication found.The new
Incoming_voice_callkey is already integrated inCallHistoryContextualbar.tsx(line 111) where it correctly displays inbound voice calls in the call history. It serves a distinct purpose from existing keys likeIncoming_call(generic widget headers) andIncoming_call_ellipsis(notifications), with no conflicts or unnecessary duplication.packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.tsx (2)
1-65: LGTM! Type definitions and setup are well-structured.The type guard
isInternalCallHistoryContactcorrectly uses theinoperator to distinguish contact types, and all type definitions are clear and complete.
84-154: Component logic is correct and handles both contact types properly.The conditional rendering for internal vs external contacts works well, and the UI structure is clear.
packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryInternalUser.tsx (1)
13-23: LGTM! Hook usage and memoization are correct.The avatar URL is properly memoized with correct dependencies, and all hooks are used appropriately.
packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.stories.tsx (1)
1-86: LGTM! Storybook stories are well-structured and comprehensive.The stories provide good coverage of both internal and external contact scenarios, and the translation mocks include all necessary keys referenced by the component.
packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.tsx
Outdated
Show resolved
Hide resolved
packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryInternalUser.tsx
Show resolved
Hide resolved
packages/ui-voip/src/views/CallHistoryContextualbar/useFullStartDate.ts
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37739 +/- ##
===========================================
+ Coverage 67.61% 67.69% +0.08%
===========================================
Files 3463 3463
Lines 113694 113694
Branches 20902 20902
===========================================
+ Hits 76875 76968 +93
+ Misses 34687 34589 -98
- Partials 2132 2137 +5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Proposed changes (including videos or screenshots)
Issue(s)
VGA-105
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Localization
Tests
✏️ Tip: You can customize this high-level summary in your review settings.