-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: pass tmid to show typing status in threads #6894
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
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request extends the typing notification system to support thread context. A new Changes
Sequence DiagramsequenceDiagram
participant UI as ComposerInput
participant Action as userTyping Action
participant Saga as watchUserTyping Saga
participant Service as emitTyping Service
participant SDK as Rocket.Chat SDK
UI->>Action: userTyping(rid, status, {tmid})
activate Action
Action->>Action: Create IUserTyping with args
deactivate Action
Saga->>Saga: Intercept IUserTyping action
Note over Saga: Extract args from action
Saga->>Service: emitTyping(room, typing, args)
activate Service
alt Server version >= 4.0.0
Service->>SDK: methodCall('stream-notify-room', ..., ['user-typing'], args)
Note over SDK: Includes thread context
else Server version < 4.0.0
Service->>SDK: methodCall('stream-notify-room', ...)
end
deactivate Service
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/sagas/room.js (1)
36-47: Critical:clearUserTypingmust forwardargsto clear typing in the correct context.The
clearUserTypingfunction doesn't accept or forward theargsparameter (includingtmid) when callingemitTyping(rid, false)on line 39. This means when a user stops typing in a thread, the server won't know which context to clear the typing indicator from, potentially leaving the indicator stuck in the thread or clearing it from the wrong location.🐛 Proposed fix
-const clearUserTyping = function* clearUserTyping({ rid, status }) { +const clearUserTyping = function* clearUserTyping({ rid, status, args }) { try { if (!status) { - yield emitTyping(rid, false); + yield emitTyping(rid, false, args); if (inactiveTypingTask) { yield cancel(inactiveTypingTask); }
🧹 Nitpick comments (1)
app/containers/MessageComposer/components/ComposerInput.tsx (1)
362-365: Consider omitting the empty object whentmidis not present.Since the
argsparameter is optional in theuserTypingfunction signature, you can passundefinedor omit it entirely instead of passing an empty object{}whentmidis not present.♻️ Proposed refactor
const handleTyping = (isTyping: boolean) => { if (sharing || !rid) return; - dispatch(userTyping(rid, isTyping, tmid ? { tmid } : {})); + dispatch(userTyping(rid, isTyping, tmid ? { tmid } : undefined)); };
📜 Review details
Configuration used: Organization 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 selected for processing (4)
app/actions/room.tsapp/containers/MessageComposer/components/ComposerInput.tsxapp/lib/services/restApi.tsapp/sagas/room.js
🧰 Additional context used
🧬 Code graph analysis (4)
app/actions/room.ts (1)
app/actions/actionsTypes.ts (1)
ROOM(16-26)
app/lib/services/restApi.ts (2)
app/definitions/IRoom.ts (1)
IRoom(18-65)app/lib/methods/helpers/compareServerVersion.ts (1)
compareServerVersion(10-15)
app/sagas/room.js (1)
app/lib/services/restApi.ts (1)
emitTyping(953-963)
app/containers/MessageComposer/components/ComposerInput.tsx (1)
app/actions/room.ts (1)
userTyping(117-124)
⏰ 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: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🔇 Additional comments (4)
app/actions/room.ts (2)
42-50: LGTM! Clean type definition for thread typing context.The new
IUserTypingArgstype and its integration intoIUserTypingproperly enable passing thread context to typing events. The optional fields allow backward compatibility.
117-123: LGTM! Function signature correctly updated.The
userTypingaction creator properly accepts and forwards the optionalargsparameter through the Redux action payload.app/sagas/room.js (1)
54-66: LGTM! Typing start correctly forwards thread context.The
watchUserTypingsaga properly extracts and forwards theargsparameter toemitTypingwhen typing begins.app/lib/services/restApi.ts (1)
953-963: Verify server API support forargsparameter in version 4.0.0+.The client-side implementation correctly gates the new
argsparameter anduser-activityendpoint behind a server version check (>= 4.0.0). However, verification of actual server-side support for theargsparameter (includingtmid) in thestream-notify-roomendpoint requires access to the Rocket.Chat server repository or official API documentation, which is outside this client codebase.
Proposed changes
Previously, typing events did not include the thread ID in the user activity, which caused the typing indicator to appear in the parent room instead of the thread on web clients. By passing the tmid, the typing status is now correctly scoped to the active thread, ensuring the indicator is displayed in the thread view and aligning the behavior with the expected user experience.
Issue(s)
https://rocketchat.atlassian.net/browse/CORE-1644
How to test or reproduce
Screenshots
Screen.Recording.2026-01-07.at.10.30.01.PM.mov
Types of changes
Checklist
Further comments
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.