fix: render mentions, emojis, and inline elements inside headings - #6911
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (4)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughInline markdown components (Bold, Italic, Strike) now consume MarkdownContext and render Changes
Sequence Diagram(s)sequenceDiagram
participant Parser as Markdown Parser
participant Inline as Inline Component
participant Context as MarkdownContext
participant AtMention as AtMention
participant Hashtag as Hashtag
participant UI as Rendered UI
Parser->>Inline: emit blocks (text, MENTION_USER, MENTION_CHANNEL, formatting)
Inline->>Context: read useRealName, username, navToRoomInfo, mentions, channels
Inline->>AtMention: render MENTION_USER with context props
Inline->>Hashtag: render MENTION_CHANNEL with channels + navToRoomInfo
AtMention->>UI: return interactive user mention node
Hashtag->>UI: return interactive channel mention node
Inline->>UI: return composed formatted text nodes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @app/containers/markdown/components/inline/Bold.tsx:
- Around line 22-63: The mapped elements inside the Bold component's value.map
(switch on block.type) are missing React key props; update each returned element
(Link, Plain, Strike, Italic, Hashtag, AtMention) to include a unique key
(prefer a stable id from block or block.value if available, otherwise derive a
stable string like `${block.type}-${index}`) so React can reconcile list items
properly; modify the JSX returns in Bold to add key={...} on each returned
component while keeping existing props.
In @app/containers/markdown/components/inline/Italic.tsx:
- Around line 21-62: The mapped children in the Italic component (inside
value.map) are missing key props; for every element returned (Link, Plain,
Strike, Bold, Hashtag, AtMention, etc.) add a unique key (preferably a stable id
on block like block.id or block.value.id, falling back to the map index only if
no stable id exists) so each returned element from value.map has a key prop to
satisfy React's list reconciliation.
In @app/containers/markdown/components/inline/Strike.tsx:
- Around line 21-62: The mapped elements inside Strike's value.map() (components
Link, Plain, Bold, Italic, Hashtag, AtMention returned from the switch) are
missing React key props; add a unique key prop to each returned element in the
value.map callback (prefer a stable identifier from the block object if
available, e.g., block.id or block.value.id, and fall back to the map index only
if no stable id exists) so every element rendered by Strike has a proper key for
reconciliation.
🧹 Nitpick comments (1)
app/containers/markdown/components/inline/Bold.tsx (1)
1-65: Consider extracting shared logic to reduce duplication.Bold.tsx, Italic.tsx, and Strike.tsx have nearly identical structure—they differ only in the style prop passed to Hashtag and AtMention. Extracting a shared helper function or higher-order component would reduce duplication and improve maintainability.
💡 Example refactoring approach
Create a shared helper:
// utils/renderInlineWithMentions.tsx const renderInlineBlock = ( block: any, index: number, context: MarkdownContextType, customStyle: any ) => { const { useRealName, username, navToRoomInfo, mentions, channels } = context; switch (block.type) { case 'LINK': return <Link key={index} value={block.value} />; case 'PLAIN_TEXT': return <Plain key={index} value={block.value} />; case 'STRIKE': return <Strike key={index} value={block.value} />; case 'ITALIC': return <Italic key={index} value={block.value} />; case 'BOLD': return <Bold key={index} value={block.value} />; case 'MENTION_CHANNEL': return ( <Hashtag key={index} hashtag={block.value.value} channels={channels} navToRoomInfo={navToRoomInfo} style={[customStyle]} /> ); case 'MENTION_USER': return ( <AtMention key={index} mention={block.value.value} username={username} navToRoomInfo={navToRoomInfo} style={[customStyle]} useRealName={useRealName} mentions={mentions} /> ); default: return null; } };Then simplify each component:
const Bold = ({ value }: IBoldProps) => { const context = useContext(MarkdownContext); return ( <Text style={styles.text}> {value.map((block, index) => renderInlineBlock(block, index, context, { fontWeight: 'bold' }) )} </Text> ); };
📜 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 ignored due to path filters (1)
app/containers/markdown/__snapshots__/Markdown.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
app/containers/markdown/Markdown.stories.tsxapp/containers/markdown/components/inline/Bold.tsxapp/containers/markdown/components/inline/Italic.tsxapp/containers/markdown/components/inline/Strike.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
app/containers/markdown/components/inline/Italic.tsx (1)
app/containers/markdown/Markdown.stories.tsx (1)
Hashtag(118-122)
app/containers/markdown/Markdown.stories.tsx (2)
app/containers/MessageComposer/components/Toolbar/Markdown.tsx (1)
Markdown(9-46)app/containers/markdown/components/mentions/index.tsx (1)
Hashtag(4-4)
app/containers/markdown/components/inline/Strike.tsx (2)
app/containers/markdown/components/inline/index.ts (4)
Strike(6-6)Link(6-6)Bold(6-6)Italic(6-6)app/containers/markdown/components/mentions/index.tsx (2)
Hashtag(4-4)AtMention(4-4)
app/containers/markdown/components/inline/Bold.tsx (2)
app/containers/markdown/components/inline/index.ts (4)
Bold(6-6)Link(6-6)Strike(6-6)Italic(6-6)app/containers/markdown/components/mentions/index.tsx (2)
Hashtag(4-4)AtMention(4-4)
🪛 Biome (2.1.2)
app/containers/markdown/components/inline/Italic.tsx
[error] 29-29: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 31-31: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 33-33: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 35-35: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 38-43: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 47-54: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
app/containers/markdown/components/inline/Strike.tsx
[error] 29-29: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 31-31: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 33-33: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 35-35: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 38-43: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 47-54: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
app/containers/markdown/components/inline/Bold.tsx
[error] 30-30: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 32-32: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 34-34: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 36-36: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 39-44: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 48-55: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ 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). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🔇 Additional comments (2)
app/containers/markdown/Markdown.stories.tsx (2)
80-80: LGTM!The extracted
channelsconstant improves reusability across stories.
89-116: Good test coverage for mention formatting combinations.The new
MentionsWithFormattingstory comprehensively demonstrates how mentions render with various formatting styles (bold, strikethrough, italic combinations), which aligns well with the context-driven rendering changes in the inline components.
diegolmello
left a comment
There was a problem hiding this comment.
Besides my comments, make sure to test (and update screenshots) for both iOS and Android, since it's handled differently.
Proposed changes
Users and channels mentioned inside bold, italic, strike, and headings were rendered as raw text (e.g.
@userinside**bold**showed as plain@userinstead of a tappable mention). Headings only supported plain text — all other inline elements were silently dropped.This PR fixes the markdown renderer to properly parse and display all inline elements inside formatting wrappers and headings.
Issue(s)
https://rocketchat.atlassian.net/browse/CORE-1667
https://rocketchat.atlassian.net/browse/SUP-1071
How to test or reproduce
Send a message with mixed formatting, e.g.:
**@user \*hello\***— bold mention_@user hello_— italic mention~@user hello~— strikethrough mention# H1 **bold** _italic_ ~strike~ :rocket: @user #channel https://rocket.chatVerify:
Screenshots
Types of changes
Checklist
Further comments
This PR is inspired by the earlier work in #5367 by @brf153.
That PR is outdated and has conflicts, so I reimplemented the changes from scratch
and added additional improvements to match the current codebase.
All credit for the original idea goes to @brf153.
Summary by CodeRabbit
New Features
Improvements