fix: hide message reactions when "Use Emojis" is disabled in user preferences#38863
fix: hide message reactions when "Use Emojis" is disabled in user preferences#38863ishanmitra wants to merge 5 commits intoRocketChat:developfrom
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughAdds a user-preference guard ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsx (1)
73-73:tagNameassertion is brittle.
expect(emoji.tagName).toBe('SPAN')couples the test to the internal DOM element choice. Therole="img"query on line 72 already confirms the accessible semantic; the tagName check adds no meaningful contract and will fail if the emoji renderer ever switches to a different element.♻️ Proposed simplification
const emoji = screen.getByRole('img', { name: ':emoji:' }); -expect(emoji.tagName).toBe('SPAN'); expect(emoji).toBeInTheDocument();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsx` at line 73, The test's assertion on DOM internals is brittle: remove the tagName check (expect(emoji.tagName).toBe('SPAN')) and rely on the existing accessible query (getByRole('img') / the emoji variable) and any semantic/attribute checks you need (e.g., aria-label, role, textContent) instead; update the test in RoomMessageEmoji.spec.tsx to delete the tagName assertion so it only asserts accessible behavior such as that the element with role="img" exists and has the expected aria-label or content.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/meteor/client/components/message/toolbar/items/DefaultItems.tsxapps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsxapps/meteor/client/components/message/variants/room/RoomMessageContent.tsx
🧰 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:
apps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsxapps/meteor/client/components/message/toolbar/items/DefaultItems.tsx
🧠 Learnings (8)
📚 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:
apps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/components/message/toolbar/items/DefaultItems.tsx
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsx
🧬 Code graph analysis (2)
apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx (1)
packages/ui-contexts/src/index.ts (1)
useUserPreference(86-86)
apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsx (2)
apps/meteor/client/components/message/list/MessageListContext.tsx (2)
MessageListContext(79-79)messageListContextDefaultValue(48-77)packages/message-parser/src/utils.ts (1)
emoji(143-147)
⏰ 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). (4)
- GitHub Check: check
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/client/components/message/toolbar/items/DefaultItems.tsx`:
- Around line 13-17: The DefaultItemsProps type includes an unused enableEmojis
field; remove that property from the DefaultItemsProps declaration and export so
the props type only contains message, room, and subscription, and update any
related imports/uses if referenced; confirm DefaultItems component
(DefaultItems) continues to derive enableEmojis via
useUserPreference<boolean>('useEmojis') and no callers pass enableEmojis.
In `@apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx`:
- Around line 92-94: The JSX condition in RoomMessageContent renders a stray "0"
when normalizedMessage.reactions is an empty object because
Object.keys(...).length is a number; change the conditional to explicitly
produce a boolean (e.g., use
Boolean(Object.keys(normalizedMessage.reactions).length) or
Object.keys(normalizedMessage.reactions).length > 0) so the expression
enableEmojis && <boolean> && <Reactions .../> does not ever evaluate to 0;
update the check around normalizedMessage.reactions that decides whether to
render <Reactions message={normalizedMessage} /> accordingly.
In `@apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsx`:
- Line 43: The test currently mocks modules to undefined which breaks named
imports (e.g., useGetMessageByID used by ReactionTooltip rendered via the
Reactions component, and actionLinks used by MessageActions); update the
jest.mock calls in RoomMessageEmoji.spec.tsx to return an object (e.g., () =>
({})) or provide the specific named export stubs so imports like
useGetMessageByID and actionLinks resolve to defined values instead of
undefined, ensuring ReactionTooltip/Reactions/MessageActions can import their
named exports at runtime.
- Line 100: Replace the fragile toolbar assertion in RoomMessageEmoji.spec.tsx
with a direct check for the reaction button to assert emojis are disabled:
instead of asserting on the toolbar role (which can be missing due to mock
deps), update the test that renders DefaultItems/ReactionMessageAction to use
screen.queryByRole('button', { name: /add reaction/i }) and assert
.not.toBeInTheDocument() so the test explicitly verifies the absence of the
reaction action when enableEmojis=false.
---
Nitpick comments:
In `@apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsx`:
- Line 73: The test's assertion on DOM internals is brittle: remove the tagName
check (expect(emoji.tagName).toBe('SPAN')) and rely on the existing accessible
query (getByRole('img') / the emoji variable) and any semantic/attribute checks
you need (e.g., aria-label, role, textContent) instead; update the test in
RoomMessageEmoji.spec.tsx to delete the tagName assertion so it only asserts
accessible behavior such as that the element with role="img" exists and has the
expected aria-label or content.
apps/meteor/client/components/message/toolbar/items/DefaultItems.tsx
Outdated
Show resolved
Hide resolved
| {enableEmojis && normalizedMessage.reactions && Object.keys(normalizedMessage.reactions).length && ( | ||
| <Reactions message={normalizedMessage} /> | ||
| )} |
There was a problem hiding this comment.
Stray 0 rendered when reactions is an empty object.
Object.keys(normalizedMessage.reactions).length is a number. If the left side of && is 0, the whole JSX expression gets that value and React will happily render 0 rather than nothing. If normalizedMessage.reactions is ever {}, this renders a visible 0 text node.
🐛 Proposed fix
- {enableEmojis && normalizedMessage.reactions && Object.keys(normalizedMessage.reactions).length && (
+ {enableEmojis && normalizedMessage.reactions && Object.keys(normalizedMessage.reactions).length > 0 && (
<Reactions message={normalizedMessage} />
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {enableEmojis && normalizedMessage.reactions && Object.keys(normalizedMessage.reactions).length && ( | |
| <Reactions message={normalizedMessage} /> | |
| )} | |
| {enableEmojis && normalizedMessage.reactions && Object.keys(normalizedMessage.reactions).length > 0 && ( | |
| <Reactions message={normalizedMessage} /> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx`
around lines 92 - 94, The JSX condition in RoomMessageContent renders a stray
"0" when normalizedMessage.reactions is an empty object because
Object.keys(...).length is a number; change the conditional to explicitly
produce a boolean (e.g., use
Boolean(Object.keys(normalizedMessage.reactions).length) or
Object.keys(normalizedMessage.reactions).length > 0) so the expression
enableEmojis && <boolean> && <Reactions .../> does not ever evaluate to 0;
update the check around normalizedMessage.reactions that decides whether to
render <Reactions message={normalizedMessage} /> accordingly.
| })); | ||
| jest.mock('../../../lib/utils/fireGlobalEvent', () => ({ fireGlobalEvent: () => undefined })); | ||
| jest.mock('../../../views/room/hooks/useGoToRoom', () => ({ useGoToRoom: () => undefined })); | ||
| jest.mock('../../../views/room/contextualBar/Threads/hooks/useGetMessageByID', () => undefined); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsx | head -60Repository: RocketChat/Rocket.Chat
Length of output: 2177
🏁 Script executed:
rg -n "from.*useGetMessageByID|require.*useGetMessageByID" --type=ts --type=tsx -g "!*.spec.*" -A2 -B2Repository: RocketChat/Rocket.Chat
Length of output: 93
🏁 Script executed:
rg -n "from.*actionLinks|require.*actionLinks" --type=ts --type=tsx -g "!*.spec.*" -A2 -B2Repository: RocketChat/Rocket.Chat
Length of output: 93
🏁 Script executed:
rg "from.*useGetMessageByID|require.*useGetMessageByID" -g "!*.spec.*" -A2 -B2Repository: RocketChat/Rocket.Chat
Length of output: 1370
🏁 Script executed:
rg "from.*actionLinks|require.*actionLinks" -g "!*.spec.*" -A2 -B2Repository: RocketChat/Rocket.Chat
Length of output: 531
🏁 Script executed:
# Also check the actual imports in the main RoomMessage.tsx file
fd -e tsx -e ts "RoomMessage\.tsx" | grep -v spec | head -5Repository: RocketChat/Rocket.Chat
Length of output: 128
🏁 Script executed:
cat -n apps/meteor/client/components/message/variants/RoomMessage.tsx | head -50Repository: RocketChat/Rocket.Chat
Length of output: 2128
🏁 Script executed:
rg "useGetMessageByID|actionLinks" apps/meteor/client/components/message/variants/RoomMessage.tsx -A2 -B2Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Check dependencies of RoomMessage
rg "import.*from" apps/meteor/client/components/message/variants/RoomMessage.tsx | head -20Repository: RocketChat/Rocket.Chat
Length of output: 1099
🏁 Script executed:
fd RoomMessageEmoji -e tsx -e tsRepository: RocketChat/Rocket.Chat
Length of output: 138
🏁 Script executed:
rg "useGetMessageByID|actionLinks" apps/meteor/client/components/message/ -R --max-depth=5Repository: RocketChat/Rocket.Chat
Length of output: 470
🏁 Script executed:
# Check RoomMessageContent
cat -n apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx | head -60Repository: RocketChat/Rocket.Chat
Length of output: 3376
🏁 Script executed:
# Check if MessageActions is used/rendered in RoomMessageContent
cat -n apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx | grep -A 80 "return"Repository: RocketChat/Rocket.Chat
Length of output: 3097
🏁 Script executed:
# Check what MessageActions imports
head -30 apps/meteor/client/components/message/content/MessageActions.tsxRepository: RocketChat/Rocket.Chat
Length of output: 1057
🏁 Script executed:
# Check what's in the actual test - does it render the component tree that uses these modules?
cat -n apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsxRepository: RocketChat/Rocket.Chat
Length of output: 3696
🏁 Script executed:
# Check Reactions component
head -40 apps/meteor/client/components/message/content/Reactions.tsxRepository: RocketChat/Rocket.Chat
Length of output: 1771
🏁 Script executed:
# Check ReactionTooltip imports
head -20 apps/meteor/client/components/message/content/reactions/ReactionTooltip.tsxRepository: RocketChat/Rocket.Chat
Length of output: 723
🏁 Script executed:
# Check if Reactions renders ReactionTooltip
rg "ReactionTooltip" apps/meteor/client/components/message/content/Reactions.tsx -A3 -B3Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Check if Reaction component uses ReactionTooltip
head -50 apps/meteor/client/components/message/content/reactions/Reaction.tsxRepository: RocketChat/Rocket.Chat
Length of output: 1937
🏁 Script executed:
# Verify the Reaction component imports
rg "import.*ReactionTooltip|ReactionTooltip" apps/meteor/client/components/message/content/reactions/Reaction.tsx -B5 -A5Repository: RocketChat/Rocket.Chat
Length of output: 876
Mocking modules to undefined is non-idiomatic and causes runtime errors when code imports named exports.
jest.mock('...', () => undefined) makes the module itself undefined. Code that imports named exports from it (e.g. import { useGetMessageByID } from '...') will fail at runtime.
useGetMessageByID is imported in ReactionTooltip.tsx, which is rendered by this test via the Reactions component (present in the message fixture). Mocking it to undefined will cause the component to fail during rendering.
Similarly, actionLinks is imported as a named export in MessageActions.tsx. While not triggered in this specific test (message lacks actionLinks), it's still non-idiomatic.
Use () => ({}) instead:
Proposed fix
-jest.mock('../../../views/room/contextualBar/Threads/hooks/useGetMessageByID', () => undefined);
+jest.mock('../../../views/room/contextualBar/Threads/hooks/useGetMessageByID', () => ({}));-jest.mock('../../../lib/actionLinks', () => undefined);
+jest.mock('../../../lib/actionLinks', () => ({}));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jest.mock('../../../views/room/contextualBar/Threads/hooks/useGetMessageByID', () => undefined); | |
| jest.mock('../../../views/room/contextualBar/Threads/hooks/useGetMessageByID', () => ({})); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsx` at
line 43, The test currently mocks modules to undefined which breaks named
imports (e.g., useGetMessageByID used by ReactionTooltip rendered via the
Reactions component, and actionLinks used by MessageActions); update the
jest.mock calls in RoomMessageEmoji.spec.tsx to return an object (e.g., () =>
({})) or provide the specific named export stubs so imports like
useGetMessageByID and actionLinks resolve to defined values instead of
undefined, ensuring ReactionTooltip/Reactions/MessageActions can import their
named exports at runtime.
|
|
||
| expect(screen.queryByRole('img')).not.toBeInTheDocument(); | ||
| expect(screen.getByText('message body :emoji:')).toBeInTheDocument(); | ||
| expect(screen.queryByRole('toolbar')).not.toBeInTheDocument(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the test file around line 100
fd -e spec.tsx "RoomMessageEmoji" --exec wc -l {} \;Repository: RocketChat/Rocket.Chat
Length of output: 144
🏁 Script executed:
# Read the test file to understand the context
fd -e spec.tsx "RoomMessageEmoji" --exec cat -n {} \; | head -150Repository: RocketChat/Rocket.Chat
Length of output: 3696
🏁 Script executed:
# Search for ReactionMessageAction implementation
rg "ReactionMessageAction" --type=tsx -B 3 -A 10 | head -100Repository: RocketChat/Rocket.Chat
Length of output: 93
🏁 Script executed:
# Search for DefaultItems to understand what it renders
rg "DefaultItems" --type=tsx -B 2 -A 5 | head -100Repository: RocketChat/Rocket.Chat
Length of output: 93
🏁 Script executed:
# Find RoomMessage component
fd -e "RoomMessage" -type f | grep -v spec | grep -v node_modules | head -20Repository: RocketChat/Rocket.Chat
Length of output: 236
🏁 Script executed:
# Search for ReactionMessageAction without type restriction
rg "ReactionMessageAction" -B 3 -A 10 | head -80Repository: RocketChat/Rocket.Chat
Length of output: 8374
🏁 Script executed:
# Search for toolbar role in the codebase to understand what creates it
rg "toolbar" --type=ts -B 2 -A 2 | head -100Repository: RocketChat/Rocket.Chat
Length of output: 7664
🏁 Script executed:
# Check if RoomMessage imports or renders DefaultItems
rg "DefaultItems" -B 2 -A 5 | head -80Repository: RocketChat/Rocket.Chat
Length of output: 3580
🏁 Script executed:
# Get full DefaultItems implementation
rg "DefaultItems = \({" -A 30 apps/meteor/client/components/message/toolbar/items/DefaultItems.tsxRepository: RocketChat/Rocket.Chat
Length of output: 165
🏁 Script executed:
# Get full ReactionMessageAction implementation to see what it renders
rg "const ReactionMessageAction = " -A 40 apps/meteor/client/components/message/toolbar/items/actions/ReactionMessageAction.tsxRepository: RocketChat/Rocket.Chat
Length of output: 1113
🏁 Script executed:
# Check MessageToolbar to understand how DefaultItems is used and how toolbar is rendered
rg "MessageToolbar" -A 50 apps/meteor/client/components/message/toolbar/MessageToolbar.tsx | head -100Repository: RocketChat/Rocket.Chat
Length of output: 3187
🏁 Script executed:
# Get the complete DefaultItems implementation
cat -n apps/meteor/client/components/message/toolbar/items/DefaultItems.tsxRepository: RocketChat/Rocket.Chat
Length of output: 1350
🏁 Script executed:
# Check QuoteMessageAction implementation to see if it can render null
rg "const QuoteMessageAction" -A 30 apps/meteor/client/components/message/toolbar/items/actions/Repository: RocketChat/Rocket.Chat
Length of output: 3529
🏁 Script executed:
# Check ForwardMessageAction
rg "const ForwardMessageAction" -A 30 apps/meteor/client/components/message/toolbar/items/actions/Repository: RocketChat/Rocket.Chat
Length of output: 3593
🏁 Script executed:
# Check ReplyInThreadMessageAction
rg "const ReplyInThreadMessageAction" -A 30 apps/meteor/client/components/message/toolbar/items/actions/Repository: RocketChat/Rocket.Chat
Length of output: 3750
🏁 Script executed:
# Get complete ForwardMessageAction implementation
cat -n apps/meteor/client/components/message/toolbar/items/actions/ForwardMessageAction.tsx | head -50Repository: RocketChat/Rocket.Chat
Length of output: 1904
🏁 Script executed:
# Check MessageToolbarItem to understand what it renders
rg "const MessageToolbarItem" -A 20 apps/meteor/client/components/message/toolbar/Repository: RocketChat/Rocket.Chat
Length of output: 3690
🏁 Script executed:
# Get complete ReactionMessageAction to see what it returns
cat -n apps/meteor/client/components/message/toolbar/items/actions/ReactionMessageAction.tsx | tail -30Repository: RocketChat/Rocket.Chat
Length of output: 977
🏁 Script executed:
# Check if MessageToolbar conditionally renders based on empty children
rg "FuselageMessageToolbar" -B 5 -A 10 apps/meteor/client/components/message/toolbar/MessageToolbar.tsxRepository: RocketChat/Rocket.Chat
Length of output: 2043
🏁 Script executed:
# Check RoomMessage implementation to understand if it conditionally renders MessageToolbar
rg "RoomMessage" -B 5 -A 30 apps/meteor/client/components/message/variants/RoomMessage.tsx | head -80Repository: RocketChat/Rocket.Chat
Length of output: 2349
🏁 Script executed:
# Check if there's logic preventing toolbar rendering in specific test contexts
fd "RoomMessage.tsx" -type f | grep -v spec | head -1 | xargs wc -lRepository: RocketChat/Rocket.Chat
Length of output: 255
🏁 Script executed:
# Get more context around how MessageToolbar is used in RoomMessage
fd "RoomMessage.tsx" -type f | grep -v spec | head -1 | xargs cat | head -100Repository: RocketChat/Rocket.Chat
Length of output: 236
🏁 Script executed:
# Check MessageToolbarHolder implementation
fd "MessageToolbarHolder" -type f | grep -v spec | xargs catRepository: RocketChat/Rocket.Chat
Length of output: 236
🏁 Script executed:
# Check if FuselageMessageToolbar from fuselage actually creates a toolbar role
rg "MessageToolbar as FuselageMessageToolbar" -B 5 -A 5 apps/meteor/client/components/message/toolbar/MessageToolbar.tsxRepository: RocketChat/Rocket.Chat
Length of output: 633
🏁 Script executed:
# Look for FuselageMessageToolbar in fuselage package
rg "MessageToolbar" apps/meteor/node_modules/@rocket.chat/fuselage/dist/ 2>/dev/null | head -5 || echo "Checking fuselage source..."Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Check test to see if toolbar actually appears when useEmojis is true
rg "should render emoji and show reactions if enabled" -A 30 apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsxRepository: RocketChat/Rocket.Chat
Length of output: 943
🏁 Script executed:
# Find and read MessageToolbarHolder
find apps/meteor/client/components/message -name "MessageToolbarHolder*" | head -1 | xargs catRepository: RocketChat/Rocket.Chat
Length of output: 1826
🏁 Script executed:
# Check what FuselageMessageToolbar actually is from fuselage
find . -path "*/node_modules/@rocket.chat/fuselage*" -name "*.d.ts" -o -name "*.tsx" -o -name "*.ts" 2>/dev/null | xargs grep -l "export.*MessageToolbar" | head -3Repository: RocketChat/Rocket.Chat
Length of output: 473
🏁 Script executed:
# Let's check the actual test result by searching for how toolbar is used in similar tests
rg "queryByRole.*toolbar" apps/meteor/client/components/message/ -B 5 -A 2Repository: RocketChat/Rocket.Chat
Length of output: 785
🏁 Script executed:
# Check if MessageToolbarStarsActionMenu or MessageToolbarActionMenu might return null
rg "MessageToolbarStarsActionMenu\|MessageToolbarActionMenu" -A 20 apps/meteor/client/components/message/toolbar/ | head -60Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Check what happens in the test - see if the toolbar actually renders or if room is missing
# Look at how useIsVisible works - it might prevent toolbar from rendering
rg "useIsVisible" -B 2 -A 10 apps/meteor/client/views/room/hooks/useIsVisible.tsxRepository: RocketChat/Rocket.Chat
Length of output: 158
🏁 Script executed:
# Check what MessageToolbarWrapper does
rg "MessageToolbarWrapper" -B 2 -A 10Repository: RocketChat/Rocket.Chat
Length of output: 11787
🏁 Script executed:
# Double-check: when useEmojis=false, what actually prevents the toolbar from appearing?
# Let's trace through: Does ReactionMessageAction render a toolbar role?
rg "role.*toolbar\|aria-label.*Message_actions" apps/meteor/client/components/message/toolbar/Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Most importantly: Let's check if FuselageMessageToolbar is what creates the toolbar role
# by looking at what it actually renders
python3 << 'EOF'
import json
# Search for MessageToolbar export from fuselage
import subprocess
result = subprocess.run(['rg', 'export.*MessageToolbar', '-g', '*.ts', '-g', '*.tsx', '-g', '*.d.ts'],
capture_output=True, text=True, cwd='.')
for line in result.stdout.split('\n')[:20]:
if 'fuselage' in line.lower() and 'node_modules' not in line:
print(line)
EOFRepository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Find useIsVisible hook
find apps/meteor/client -name "useIsVisible*" -o -type f -path "*hooks*" | xargs grep -l "useIsVisible" 2>/dev/null | head -5Repository: RocketChat/Rocket.Chat
Length of output: 117
🏁 Script executed:
# Search for useIsVisible definition anywhere in the codebase
rg "export.*useIsVisible\|const useIsVisible" -B 2 -A 10 | head -40Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Most importantly: verify MessageToolbarHolder condition - does it prevent toolbar rendering when room is null?
rg "depsQueryResult.data.room" -B 5 -A 5 apps/meteor/client/components/message/MessageToolbarHolder.tsxRepository: RocketChat/Rocket.Chat
Length of output: 531
🏁 Script executed:
# Read the useIsVisible hook to understand the complete flow
cat apps/meteor/client/views/room/hooks/useIsVisible.tsRepository: RocketChat/Rocket.Chat
Length of output: 762
🏁 Script executed:
# Let's verify: In the test, what is the state of depsQueryResult.data.room?
# The test uses mockAppRoot() - let's check what ChatContext provides
rg "mockAppRoot" -B 5 -A 20 apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsx | head -50Repository: RocketChat/Rocket.Chat
Length of output: 1443
🏁 Script executed:
# Check what mockAppRoot actually provides for chat context
rg "mockAppRoot" --type=ts -B 3 -A 15 | grep -A 15 "export.*mockAppRoot" | head -40Repository: RocketChat/Rocket.Chat
Length of output: 1233
🏁 Script executed:
# Check MockedAppRootBuilder to see what it provides for chat context
rg "MockedAppRootBuilder\|class.*AppRoot" -B 5 -A 30 packages/mock-providers/src/ | head -80Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Check if the mock provides a default room/subscription for chat context
rg "ChatContext\|chat.*data.*findRoom" packages/mock-providers/src/ -B 3 -A 10Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Let's verify: does the toolbar actually appear in the first test (useEmojis=true)?
# If so, what's different about the mock setup?
rg "should render emoji and show reactions if enabled" -A 28 apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsxRepository: RocketChat/Rocket.Chat
Length of output: 900
🏁 Script executed:
# Find and read MockedAppRootBuilder implementation
fd "MockedAppRootBuilder" packages/mock-providers/src/ | head -1 | xargs cat | head -100Repository: RocketChat/Rocket.Chat
Length of output: 2972
🏁 Script executed:
# Search more broadly for the AppRoot builder
fd -e "ts" -e "tsx" packages/mock-providers/src | xargs grep -l "class.*AppRootBuilder\|export.*mockAppRoot" | head -3Repository: RocketChat/Rocket.Chat
Length of output: 479
🏁 Script executed:
# Let's check if there's something conditional in MessageToolbarHolder based on useEmojis
rg "useEmojis\|enableEmojis" apps/meteor/client/components/message/ -B 3 -A 3Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Key question: does the toolbar actually get rendered to the DOM in test 2?
# Let's check what FuselageMessageToolbar is - it might conditionally render children
rg "FuselageMessageToolbar\|as FuselageMessageToolbar" -B 2 -A 10 packages/ui-client/src/ 2>/dev/null | head -60Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Let's check the MockedAppRootBuilder more carefully - look at the full file
cat packages/mock-providers/src/MockedAppRootBuilder.tsx | head -200Repository: RocketChat/Rocket.Chat
Length of output: 6623
🏁 Script executed:
# Check if there's a ChatContext provider in the builder
rg "ChatContext\|chat.*provider" packages/mock-providers/src/ -B 2 -A 5Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Key question: In the test, when using mockAppRoot(), does chat?.data.findRoom() return a room?
# Let's check if there are any other tests that check toolbar behavior
rg "toolbar\|MessageToolbar" apps/meteor/client/components/message/ --type=tsx -l | grep -E "spec|test"Repository: RocketChat/Rocket.Chat
Length of output: 93
🏁 Script executed:
# Let's check what actually happens in the test - verify the mock setup provides or doesn't provide a room
cat packages/mock-providers/src/MockedAppRootBuilder.tsx | grep -A 20 "build()"Repository: RocketChat/Rocket.Chat
Length of output: 731
🏁 Script executed:
# Look for test utilities to understand what mockAppRoot() provides
rg "class MockedAppRootBuilder" -A 300 packages/mock-providers/src/MockedAppRootBuilder.tsx | tail -150Repository: RocketChat/Rocket.Chat
Length of output: 5050
🏁 Script executed:
# Critical: Check what this.room is by default in MockedAppRootBuilder
rg "private room\|this.room =" packages/mock-providers/src/MockedAppRootBuilder.tsx -B 2 -A 2Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Check the full user context queryRoom implementation
rg "queryRoom:" packages/mock-providers/src/MockedAppRootBuilder.tsx -B 2 -A 3Repository: RocketChat/Rocket.Chat
Length of output: 361
🏁 Script executed:
# So the question is: does mockAppRoot() without explicit room setup provide a room to chat context?
# Let's look at what happens in MessageToolbarHolder when room query fails
cat apps/meteor/client/components/message/MessageToolbarHolder.tsx | grep -A 15 "showToolbar"Repository: RocketChat/Rocket.Chat
Length of output: 930
Assertion tests incidental mock behavior; verify reaction button absence instead.
expect(screen.queryByRole('toolbar')).not.toBeInTheDocument() is fragile because its absence may result from mock setup (missing room/subscription in depsQueryResult) rather than enableEmojis=false logic. Since DefaultItems conditionally renders ReactionMessageAction only when enableEmojis=true, a more resilient and intent-revealing assertion would directly verify the reaction button is absent:
expect(screen.queryByRole('button', { name: /add reaction/i })).not.toBeInTheDocument();This tests the actual intent—that emoji reactions are disabled—rather than relying on the entire toolbar's presence, which depends on mock ChatContext and query state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/client/components/message/variants/RoomMessageEmoji.spec.tsx` at
line 100, Replace the fragile toolbar assertion in RoomMessageEmoji.spec.tsx
with a direct check for the reaction button to assert emojis are disabled:
instead of asserting on the toolbar role (which can be missing due to mock
deps), update the test that renders DefaultItems/ReactionMessageAction to use
screen.queryByRole('button', { name: /add reaction/i }) and assert
.not.toBeInTheDocument() so the test explicitly verifies the absence of the
reaction action when enableEmojis=false.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38863 +/- ##
===========================================
+ Coverage 70.60% 70.65% +0.05%
===========================================
Files 3189 3189
Lines 112703 112706 +3
Branches 20436 20410 -26
===========================================
+ Hits 79573 79633 +60
+ Misses 31069 31011 -58
- Partials 2061 2062 +1
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.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/meteor/client/components/message/toolbar/items/DefaultItems.tsx
🧰 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:
apps/meteor/client/components/message/toolbar/items/DefaultItems.tsx
🧠 Learnings (3)
📚 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:
apps/meteor/client/components/message/toolbar/items/DefaultItems.tsx
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/client/components/message/toolbar/items/DefaultItems.tsx
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
apps/meteor/client/components/message/toolbar/items/DefaultItems.tsx
🧬 Code graph analysis (1)
apps/meteor/client/components/message/toolbar/items/DefaultItems.tsx (1)
packages/ui-contexts/src/index.ts (1)
useUserPreference(86-86)
⏰ 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: cubic · AI code reviewer
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
apps/meteor/client/components/message/toolbar/items/DefaultItems.tsx (1)
9-26: LGTM — past prop issue resolved, conditional guard is logically correct.
enableEmojis: booleanhas been correctly removed fromDefaultItemsProps(addressed from the previous review), the hook call is the single source of truth, and all other actions remain unaffected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/client/components/message/toolbar/items/DefaultItems.tsx`:
- Around line 16-20: The optional return from
useUserPreference<boolean>('useEmojis') can be undefined and currently hides
ReactionMessageAction; change the call to provide a safe default or coalesce the
result so undefined becomes the expected default (e.g. pass a defaultValue true
to useUserPreference or replace the render condition with (enableEmojis ?? true)
&& <ReactionMessageAction .../>), updating the enableEmojis usage in
DefaultItems.tsx to ensure the reaction button shows for users without an
explicit preference.
- Around line 16-20: The enableEmojis variable can be undefined because
useUserPreference<boolean>('useEmojis') returns T | undefined; update the
initialization of enableEmojis to provide a default true using the nullish
coalescing operator (e.g., replace the current assignment to
useUserPreference<boolean>('useEmojis') with
useUserPreference<boolean>('useEmojis') ?? true) so that ReactionMessageAction
is shown by default while the preference is loading.
| const enableEmojis = useUserPreference<boolean>('useEmojis'); | ||
|
|
||
| return ( | ||
| <> | ||
| <ReactionMessageAction message={message} room={room} subscription={subscription} /> | ||
| {enableEmojis && <ReactionMessageAction message={message} room={room} subscription={subscription} />} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the useUserPreference hook definition
rg -n "useUserPreference" packages/ui-contexts/src/ -A 5 -B 2Repository: RocketChat/Rocket.Chat
Length of output: 3012
🏁 Script executed:
#!/bin/bash
# Also check the DefaultItems.tsx file to see the current state
cat -n apps/meteor/client/components/message/toolbar/items/DefaultItems.tsx | head -30Repository: RocketChat/Rocket.Chat
Length of output: 1319
🏁 Script executed:
#!/bin/bash
# Search for useUserPreference export/definition
fd -e ts -e tsx "ui-contexts" --exec grep -l "useUserPreference" {}Repository: RocketChat/Rocket.Chat
Length of output: 48
useUserPreference<boolean> returns undefined, silently hiding reactions when preference is uninitialized.
The hook signature is useUserPreference = <T>(key: string, defaultValue?: T): T | undefined. Since useUserPreference<boolean>('useEmojis') is called without a defaultValue, the result can be boolean | undefined at runtime. The conditional render {enableEmojis && <ReactionMessageAction …/>} treats undefined as falsy, hiding the reaction button for users who have never set the preference, breaking the expected default behavior.
Provide a safe default that coalesces undefined to the expected value:
Fix
- const enableEmojis = useUserPreference<boolean>('useEmojis');
+ const enableEmojis = useUserPreference<boolean>('useEmojis') ?? true;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const enableEmojis = useUserPreference<boolean>('useEmojis'); | |
| return ( | |
| <> | |
| <ReactionMessageAction message={message} room={room} subscription={subscription} /> | |
| {enableEmojis && <ReactionMessageAction message={message} room={room} subscription={subscription} />} | |
| const enableEmojis = useUserPreference<boolean>('useEmojis') ?? true; | |
| return ( | |
| <> | |
| {enableEmojis && <ReactionMessageAction message={message} room={room} subscription={subscription} />} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/client/components/message/toolbar/items/DefaultItems.tsx` around
lines 16 - 20, The optional return from useUserPreference<boolean>('useEmojis')
can be undefined and currently hides ReactionMessageAction; change the call to
provide a safe default or coalesce the result so undefined becomes the expected
default (e.g. pass a defaultValue true to useUserPreference or replace the
render condition with (enableEmojis ?? true) && <ReactionMessageAction .../>),
updating the enableEmojis usage in DefaultItems.tsx to ensure the reaction
button shows for users without an explicit preference.
🧩 Analysis chain
🏁 Script executed:
# Find the useUserPreference hook definition
fd -e "ts" -e "tsx" -e "js" -e "jsx" | xargs rg "export.*useUserPreference" | head -20Repository: RocketChat/Rocket.Chat
Length of output: 798
🏁 Script executed:
# Search in `@rocket.chat/ui-contexts` or similar packages
fd -type f -path "*/ui-contexts*" \( -name "*.ts" -o -name "*.tsx" \) | head -20Repository: RocketChat/Rocket.Chat
Length of output: 236
🏁 Script executed:
# Look for the hook implementation more broadly
rg "function useUserPreference|const useUserPreference" -A 10 | head -50Repository: RocketChat/Rocket.Chat
Length of output: 647
🏁 Script executed:
# Check the DefaultItems.tsx file to see the import and context
cat -n apps/meteor/client/components/message/toolbar/items/DefaultItems.tsx | head -30Repository: RocketChat/Rocket.Chat
Length of output: 1319
🏁 Script executed:
# Check other usages of useUserPreference in the codebase to understand common patterns
rg "useUserPreference\<boolean\>" -A 1 | head -40Repository: RocketChat/Rocket.Chat
Length of output: 177
🏁 Script executed:
# Check if there are usages with defaultValue to understand pattern
rg "useUserPreference.*??|useUserPreference.*\|\|" -B 1 -A 1 | head -30Repository: RocketChat/Rocket.Chat
Length of output: 2636
Use nullish coalescing to provide default value for emoji preference.
The useUserPreference<T> hook returns T | undefined, and the call to useUserPreference<boolean>('useEmojis') on line 16 provides no default value. This means enableEmojis will be undefined on initial render or until the preference loads, causing the conditional on line 20 to hide ReactionMessageAction unintentionally. Since emoji reactions should be enabled by default, add a nullish coalescing operator:
Fix
- const enableEmojis = useUserPreference<boolean>('useEmojis');
+ const enableEmojis = useUserPreference<boolean>('useEmojis') ?? true;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const enableEmojis = useUserPreference<boolean>('useEmojis'); | |
| return ( | |
| <> | |
| <ReactionMessageAction message={message} room={room} subscription={subscription} /> | |
| {enableEmojis && <ReactionMessageAction message={message} room={room} subscription={subscription} />} | |
| const enableEmojis = useUserPreference<boolean>('useEmojis') ?? true; | |
| return ( | |
| <> | |
| {enableEmojis && <ReactionMessageAction message={message} room={room} subscription={subscription} />} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/client/components/message/toolbar/items/DefaultItems.tsx` around
lines 16 - 20, The enableEmojis variable can be undefined because
useUserPreference<boolean>('useEmojis') returns T | undefined; update the
initialization of enableEmojis to provide a default true using the nullish
coalescing operator (e.g., replace the current assignment to
useUserPreference<boolean>('useEmojis') with
useUserPreference<boolean>('useEmojis') ?? true) so that ReactionMessageAction
is shown by default while the preference is loading.
1 participant
Proposed changes (including videos or screenshots)
The screenshot reflects the before and after of disabling Use Emojis from the settings:
Use Emojis enabled:
Use Emojis disabled:
Note that the emoji rendering as shortcodes was already implemented. Only the quick reactions and reactions below the message gets hidden.
Issue(s)
A bug was identified where users were able to react to messages with emojis, even after disabling the "Use Emojis" option in their personal preferences. The expected behavior is that when this setting is turned off, users should no longer be able to add emoji reactions to any messages.
Steps to test or reproduce
Go to Admin > Accounts > Default User Preferences and turn off both "User Emojis" options.
Further comments
Added tests that assert the correct behavior.
Summary by CodeRabbit