-
Notifications
You must be signed in to change notification settings - Fork 13.1k
regression: fix reactions on federated rooms #37173
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! 🎉 |
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds username normalization for reaction tooltips to remove duplicated “@” and extends the Reactions message action visibility to include federated contexts. No public API changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Message UI
participant Tooltip as ReactionTooltip
User->>UI: Hover reaction
UI->>UI: normalizeUsernames(names) // strip leading "@"
UI->>Tooltip: Render tooltip with normalized usernames
Tooltip-->>User: Show single-@ usernames
sequenceDiagram
autonumber
actor User
participant UI as Message UI
participant Actions as Message Actions
User->>UI: Hover message (federated)
UI->>Actions: Evaluate visibility contexts
Actions-->>UI: Include "Reactions" action
UI-->>User: Display Reactions action
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (2)
🔇 Additional comments (2)
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release-7.11.0 #37173 +/- ##
==================================================
- Coverage 67.45% 67.44% -0.01%
==================================================
Files 3329 3329
Lines 113381 113383 +2
Branches 20578 20571 -7
==================================================
Hits 76476 76476
- Misses 34305 34314 +9
+ Partials 2600 2593 -7
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.
Pull Request Overview
This pull request fixes reaction functionality in federated rooms by addressing two key issues: removing double @ symbols from reaction usernames and ensuring the reactions menu is available in federated room contexts.
- Remove double
@prefix from usernames in reaction tooltips - Enable reactions menu in federated room message contexts
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| useShowMessageReactionsAction.tsx | Adds 'federated' to the list of contexts where the reactions action is available |
| Reaction.tsx | Introduces username normalization to remove double @ prefixes from reaction usernames |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -9,6 +9,8 @@ import ReactionTooltip from './ReactionTooltip'; | |||
| import { getEmojiClassNameAndDataTitle } from '../../../../lib/utils/renderEmoji'; | |||
| import { MessageListContext } from '../../list/MessageListContext'; | |||
|
|
|||
Copilot
AI
Oct 9, 2025
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.
The normalizeUsernames function lacks documentation explaining its purpose and the specific issue it addresses. Consider adding a JSDoc comment explaining why username normalization is needed for federated rooms.
| /** | |
| * Normalizes usernames by removing a leading '@' character if present. | |
| * In federated rooms, usernames may be prefixed with '@' to distinguish remote users. | |
| * This function ensures usernames are displayed consistently in the UI without the prefix. | |
| * @param names Array of usernames, possibly prefixed with '@' | |
| * @returns Array of normalized usernames without leading '@' | |
| */ |
FDR-211
Proposed changes (including videos or screenshots)
@from reaction's usernamesIssue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Bug Fixes