fix: Display missing mobile image captions on desktop UI#40875
fix: Display missing mobile image captions on desktop UI#40875NihalShinde4933 wants to merge 2 commits into
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 |
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (2)📚 Learning: 2026-03-27T14:52:56.865ZApplied to files:
📚 Learning: 2026-05-06T12:21:44.083ZApplied to files:
🛑 Comments failed to post (1)
🔇 Additional comments (1)
WalkthroughImageAttachment.tsx now imports ChangesImage Description Caption Display
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx (1)
41-41: ⚡ Quick winRemove inline JSX comment from implementation.
Please drop the
{/*Box for caption rendering*/}line and keep the JSX self-explanatory.As per coding guidelines: for
**/*.{ts,tsx,js}, “Avoid code comments in the implementation”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx` at line 41, The ImageAttachment component contains an inline JSX comment ("{/*Box for caption rendering*/}") that violates the guideline to avoid implementation comments; remove that JSX comment from the render return in ImageAttachment and ensure the caption area remains self-explanatory via clear element names/props (e.g., keep the surrounding JSX structure and any accessible attributes like aria-labels or descriptive classNames) so no inline comment is needed.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx`:
- Line 41: The ImageAttachment component contains an inline JSX comment ("{/*Box
for caption rendering*/}") that violates the guideline to avoid implementation
comments; remove that JSX comment from the render return in ImageAttachment and
ensure the caption area remains self-explanatory via clear element names/props
(e.g., keep the surrounding JSX structure and any accessible attributes like
aria-labels or descriptive classNames) so no inline comment is needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dc2e3ab5-2bea-4775-b80e-cf579902d497
📒 Files selected for processing (1)
apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx
📜 Review details
⏰ 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: cubic · AI code reviewer
🧰 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/content/attachments/file/ImageAttachment.tsx
🧠 Learnings (2)
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx (2)
45-45:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove implementation comment.
Per coding guidelines for TypeScript files: "Avoid code comments in the implementation."
🧹 Proposed fix
-{/*Box for caption rendering*/} {description && (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx` at line 45, Remove the implementation comment inside the JSX of the ImageAttachment component: delete the inline JSX comment "{/*Box for caption rendering*/}" within the ImageAttachment render/return so the component's markup contains only elements and no implementation comments; ensure no other TypeScript implementation comments remain in the ImageAttachment function/class.Source: Coding guidelines
46-50:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
MarkdownTextto preserve emoji and markdown formatting in captions.The caption renders
descriptionas plain text, but line 32 demonstrates thatdescriptionshould be processed withMarkdownText parseEmojito handle emoji (:smile:) and potentially markdown. Without this, captions will display raw emoji codes and unformatted markdown instead of rendering them properly.♻️ Proposed fix to support emoji and markdown
-{description && ( - <Box color='hint' fontScale='p2' style={{ wordBreak: 'break-word' }}> - {description} - </Box> -)} +{description && ( + <Box color='hint' fontScale='p2' style={{ wordBreak: 'break-word' }}> + <MarkdownText parseEmoji content={description} /> + </Box> +)}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx` around lines 46 - 50, The caption currently renders raw text in ImageAttachment.tsx (the Box wrapping description), so replace that plain-text rendering with the MarkdownText component and enable emoji/markdown parsing (use MarkdownText with parseEmoji) to preserve emoji like :smile: and any markdown formatting; update the JSX where description is used (the conditional block that renders description) to render <MarkdownText ...> with appropriate props instead of the Box content, keeping existing styling/wordBreak behavior via className or style props.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx`:
- Line 32: The description is rendered twice when descriptionMd is falsy because
MarkdownText is rendered at the top-level and the same plain description is also
rendered as a caption inside MessageCollapsible; update the rendering logic in
ImageAttachment.tsx so only one location shows the plain description: either
remove the top-level MarkdownText and leave caption rendering inside
MessageCollapsible, or change the top-level render to {descriptionMd &&
<MessageContentBody md={descriptionMd} />} so only descriptionMd uses the top
spot while the plain description is shown below the image; touch the JSX around
descriptionMd, MessageContentBody, MarkdownText and the caption block in
MessageCollapsible to implement the chosen fix.
---
Outside diff comments:
In
`@apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx`:
- Line 45: Remove the implementation comment inside the JSX of the
ImageAttachment component: delete the inline JSX comment "{/*Box for caption
rendering*/}" within the ImageAttachment render/return so the component's markup
contains only elements and no implementation comments; ensure no other
TypeScript implementation comments remain in the ImageAttachment function/class.
- Around line 46-50: The caption currently renders raw text in
ImageAttachment.tsx (the Box wrapping description), so replace that plain-text
rendering with the MarkdownText component and enable emoji/markdown parsing (use
MarkdownText with parseEmoji) to preserve emoji like :smile: and any markdown
formatting; update the JSX where description is used (the conditional block that
renders description) to render <MarkdownText ...> with appropriate props instead
of the Box content, keeping existing styling/wordBreak behavior via className or
style props.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3f30f006-7f99-408b-ac7d-4439c147e532
📒 Files selected for processing (1)
apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx
📜 Review details
🧰 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/content/attachments/file/ImageAttachment.tsx
🧠 Learnings (2)
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx
🔇 Additional comments (1)
apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx (1)
2-2: LGTM!Also applies to: 6-6, 8-8
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx (2)
45-45:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove implementation comment.
Per coding guidelines for TypeScript files: "Avoid code comments in the implementation."
🧹 Proposed fix
-{/*Box for caption rendering*/} {description && (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx` at line 45, Remove the implementation comment inside the JSX of the ImageAttachment component: delete the inline JSX comment "{/*Box for caption rendering*/}" within the ImageAttachment render/return so the component's markup contains only elements and no implementation comments; ensure no other TypeScript implementation comments remain in the ImageAttachment function/class.Source: Coding guidelines
46-50:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
MarkdownTextto preserve emoji and markdown formatting in captions.The caption renders
descriptionas plain text, but line 32 demonstrates thatdescriptionshould be processed withMarkdownText parseEmojito handle emoji (:smile:) and potentially markdown. Without this, captions will display raw emoji codes and unformatted markdown instead of rendering them properly.♻️ Proposed fix to support emoji and markdown
-{description && ( - <Box color='hint' fontScale='p2' style={{ wordBreak: 'break-word' }}> - {description} - </Box> -)} +{description && ( + <Box color='hint' fontScale='p2' style={{ wordBreak: 'break-word' }}> + <MarkdownText parseEmoji content={description} /> + </Box> +)}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx` around lines 46 - 50, The caption currently renders raw text in ImageAttachment.tsx (the Box wrapping description), so replace that plain-text rendering with the MarkdownText component and enable emoji/markdown parsing (use MarkdownText with parseEmoji) to preserve emoji like :smile: and any markdown formatting; update the JSX where description is used (the conditional block that renders description) to render <MarkdownText ...> with appropriate props instead of the Box content, keeping existing styling/wordBreak behavior via className or style props.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx`:
- Line 32: The description is rendered twice when descriptionMd is falsy because
MarkdownText is rendered at the top-level and the same plain description is also
rendered as a caption inside MessageCollapsible; update the rendering logic in
ImageAttachment.tsx so only one location shows the plain description: either
remove the top-level MarkdownText and leave caption rendering inside
MessageCollapsible, or change the top-level render to {descriptionMd &&
<MessageContentBody md={descriptionMd} />} so only descriptionMd uses the top
spot while the plain description is shown below the image; touch the JSX around
descriptionMd, MessageContentBody, MarkdownText and the caption block in
MessageCollapsible to implement the chosen fix.
---
Outside diff comments:
In
`@apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx`:
- Line 45: Remove the implementation comment inside the JSX of the
ImageAttachment component: delete the inline JSX comment "{/*Box for caption
rendering*/}" within the ImageAttachment render/return so the component's markup
contains only elements and no implementation comments; ensure no other
TypeScript implementation comments remain in the ImageAttachment function/class.
- Around line 46-50: The caption currently renders raw text in
ImageAttachment.tsx (the Box wrapping description), so replace that plain-text
rendering with the MarkdownText component and enable emoji/markdown parsing (use
MarkdownText with parseEmoji) to preserve emoji like :smile: and any markdown
formatting; update the JSX where description is used (the conditional block that
renders description) to render <MarkdownText ...> with appropriate props instead
of the Box content, keeping existing styling/wordBreak behavior via className or
style props.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3f30f006-7f99-408b-ac7d-4439c147e532
📒 Files selected for processing (1)
apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx
📜 Review details
🔇 Additional comments (1)
apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx (1)
2-2: LGTM!Also applies to: 6-6, 8-8
🛑 Comments failed to post (1)
apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx (1)
32-32:
⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Description renders twice when
descriptionMdis absent.When
descriptionMdis not provided, line 32 rendersdescriptionviaMarkdownText(above the collapsible), and lines 46-50 render the samedescriptionas a caption (below the image inside the collapsible). This causes the caption to appear twice in the UI.Per the PR objective, the caption should appear "underneath the image." The caption rendering inside
MessageCollapsible(lines 46-50) achieves this goal. Line 32 should either:
- Be removed entirely if descriptions should only appear as captions below images, or
- Be guarded to prevent duplication:
{descriptionMd && <MessageContentBody md={descriptionMd} />}🐛 Proposed fix to prevent duplication
If descriptions should only render as captions below images:
-{descriptionMd ? <MessageContentBody md={descriptionMd} /> : <MarkdownText parseEmoji content={description} />} +{descriptionMd && <MessageContentBody md={descriptionMd} />}Or if both rendering locations are intentional, add a condition to prevent overlap:
-{descriptionMd ? <MessageContentBody md={descriptionMd} /> : <MarkdownText parseEmoji content={description} />} +{descriptionMd && <MessageContentBody md={descriptionMd} />}and keep lines 46-50 for plain
description.📝 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.{descriptionMd && <MessageContentBody md={descriptionMd} />}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx` at line 32, The description is rendered twice when descriptionMd is falsy because MarkdownText is rendered at the top-level and the same plain description is also rendered as a caption inside MessageCollapsible; update the rendering logic in ImageAttachment.tsx so only one location shows the plain description: either remove the top-level MarkdownText and leave caption rendering inside MessageCollapsible, or change the top-level render to {descriptionMd && <MessageContentBody md={descriptionMd} />} so only descriptionMd uses the top spot while the plain description is shown below the image; touch the JSX around descriptionMd, MessageContentBody, MarkdownText and the caption block in MessageCollapsible to implement the chosen fix.
|
Hi team! The implementation is fully complete, locally verified, and passes all strict ESLint checks cleanly. |
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx">
<violation number="1" location="apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx:46">
P1: `description` is rendered twice: once via `MarkdownText` above `MessageCollapsible` and again via the new `<Box>` inside it, causing duplicate captions for payloads with `description` but no `descriptionMd` (the mobile case this PR targets).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| /> | ||
|
|
||
| {/*Box for caption rendering*/} | ||
| {description && ( |
There was a problem hiding this comment.
P1: description is rendered twice: once via MarkdownText above MessageCollapsible and again via the new <Box> inside it, causing duplicate captions for payloads with description but no descriptionMd (the mobile case this PR targets).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx, line 46:
<comment>`description` is rendered twice: once via `MarkdownText` above `MessageCollapsible` and again via the new `<Box>` inside it, causing duplicate captions for payloads with `description` but no `descriptionMd` (the mobile case this PR targets).</comment>
<file context>
@@ -40,6 +41,13 @@ const ImageAttachment = ({
/>
+
+ {/*Box for caption rendering*/}
+ {description && (
+ <Box color='hint' fontScale='p2' style={{ wordBreak: 'break-word' }}>
+ {description}
</file context>
Proposed changes
This PR fixes a cross-platform layout rendering bug where text captions sent alongside images from mobile applications (iOS/Android) fail to display on desktop and web clients (Windows/Linux/Web).
Root Cause
When the mobile client sends an image with a caption, it populates the
descriptionfield within theattachmentsarray payload while leaving the root message string (msg) empty. On desktop, theImageAttachmentcomponent was mapping this string variable exclusively to thealtproperty of the rendering canvas element. As a result, the caption remained hidden inside the HTML markup instead of being rendered to the viewport.Solution
Updated
ImageAttachment.tsxto conditionally render thedescriptiontext string directly underneath the image canvas using standard Fuselage<Box>components and layout styling (fontScale='p2', andcolor='hint').Issue(s)
#40730
Steps to test or reproduce
Emulating Mobile Payload via REST API
chat.sendMessageendpoint.X-Auth-TokenandX-User-Id) in the request headers.msgblank and places the text string inside the attachment metadata object, exactly matching the React Native output):{ "message": { "rid": "GENERAL", "msg": "", "attachments": [ { "title": "MobileUserIMG_6827.jpg", "description": "This is the mobile caption text that disappears on desktop viewports!", "image_url": "[https://raw.githubusercontent.com/RocketChat/Rocket.Chat/develop/public/images/logo/logo.png](https://raw.githubusercontent.com/RocketChat/Rocket.Chat/develop/public/images/logo/logo.png)", "type": "file" } ] } } <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Image attachments now show plain descriptions inside collapsible message views as styled hint captions with improved text wrapping, ensuring descriptions are readable and consistently displayed alongside existing formatted description support. <!-- end of auto-generated comment: release notes by coderabbit.ai -->