fix: render Attachment file name in Thread Message preview when body is empty#7323
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📜 Recent 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)
WalkthroughDerive thread-reply preview text from the first attachment when the message body is empty (translation-aware); add ChangesAttachment Preview for Thread Messages
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labelstype: bug 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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: 1
🤖 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 `@app/containers/message/Content.tsx`:
- Around line 56-61: The memo comparator for this component (which decides
whether to re-render the preview) currently ignores props.attachments and
props.autoTranslateLanguage while the preview value uses props.msg or
getPreviewMessageFromAttachment(props.attachments[0],
props.autoTranslateLanguage); update the equality check used by React.memo (or
the custom comparator) to also compare attachments (at least length and first
relevant attachment fields used by getPreviewMessageFromAttachment) and
autoTranslateLanguage so changes to attachments/title/translation force a
re-render of MarkdownPreview; apply the same change to the other memoized
preview block that mirrors this logic.
🪄 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: 4341c8c5-38d4-4c2a-8a33-2d58d62b3f9c
⛔ Files ignored due to path filters (1)
app/containers/message/__snapshots__/Message.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
app/containers/message/Content.test.tsxapp/containers/message/Content.tsxapp/containers/message/Message.stories.tsxapp/containers/message/interfaces.tsapp/containers/message/utils.test.tsapp/containers/message/utils.ts
📜 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). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/containers/message/utils.test.tsapp/containers/message/Content.tsxapp/containers/message/Content.test.tsxapp/containers/message/interfaces.tsapp/containers/message/Message.stories.tsxapp/containers/message/utils.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
Files:
app/containers/message/utils.test.tsapp/containers/message/Content.tsxapp/containers/message/Content.test.tsxapp/containers/message/interfaces.tsapp/containers/message/Message.stories.tsxapp/containers/message/utils.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character width, no trailing commas, avoid arrow parens, and bracket same line
Files:
app/containers/message/utils.test.tsapp/containers/message/Content.tsxapp/containers/message/Content.test.tsxapp/containers/message/interfaces.tsapp/containers/message/Message.stories.tsxapp/containers/message/utils.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use ESLint with
@rocket.chat/eslint-configbase including React, React Native, TypeScript, and Jest plugins
Files:
app/containers/message/utils.test.tsapp/containers/message/Content.tsxapp/containers/message/Content.test.tsxapp/containers/message/interfaces.tsapp/containers/message/Message.stories.tsxapp/containers/message/utils.ts
app/containers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place reusable UI components in 'app/containers/' directory
Files:
app/containers/message/utils.test.tsapp/containers/message/Content.tsxapp/containers/message/Content.test.tsxapp/containers/message/interfaces.tsapp/containers/message/Message.stories.tsxapp/containers/message/utils.ts
🧠 Learnings (2)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/containers/message/utils.test.tsapp/containers/message/Content.tsxapp/containers/message/Content.test.tsxapp/containers/message/interfaces.tsapp/containers/message/Message.stories.tsxapp/containers/message/utils.ts
📚 Learning: 2026-03-15T13:55:42.038Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6911
File: app/containers/markdown/Markdown.stories.tsx:104-104
Timestamp: 2026-03-15T13:55:42.038Z
Learning: In Rocket.Chat React Native, the markdown parser requires a space between the underscore wrapping italic text and a mention sigil (_ mention _ instead of _mention_). Ensure stories and tests that include italic-wrapped mentions follow this form to guarantee proper parsing. Specifically, for files like app/containers/markdown/Markdown.stories.tsx, and any test/content strings that exercise italic-mentions, use the pattern _ mention _ (with spaces) to prevent the mention from being treated as plain text. Validate any test strings or story content accordingly.
Applied to files:
app/containers/message/Message.stories.tsx
🔇 Additional comments (5)
app/containers/message/interfaces.ts (1)
74-75: LGTM!app/containers/message/utils.ts (1)
215-223: LGTM!app/containers/message/utils.test.ts (1)
1-34: LGTM!app/containers/message/Content.test.tsx (1)
1-133: LGTM!app/containers/message/Message.stories.tsx (1)
1555-1585: LGTM!Also applies to: 1609-1639
|
Android Build Available Rocket.Chat Experimental 4.73.0.108873 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNR3CvUte4zrFxtY9NE_lf2cXQZIyfdHz-VJtEafPomakv27_pbIWG-tO1jkdyp3l2R8_S3CI9NBEcWRh01r |
|
iOS Build Available Rocket.Chat Experimental 4.73.0.108874 |
|
Android Build Available Rocket.Chat Experimental 4.73.0.108875 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNROJlGZfldeUe-M_ZdYWIZIb1N5qiQmVcB63qs6KZfllc8GgSBUvFWaxdDeLSVXycvBEyKRRCZ9PJudECrK |
…is empty Inline Thread Message previews with Attachments but no text body rendered as empty avatar rows in the parent Room view. Add a `getPreviewMessageFromAttachment` helper (translation > description > title) and use it in `Content.tsx`'s preview branch so the file name (or caption) of the first Attachment appears via MarkdownPreview. The encrypted branch still fires first, so no file-name leak. `getMessageFromAttachment` is unchanged to preserve full Attachment / Quote caption behavior.
The new preview fallback reads attachments[0] and autoTranslateLanguage but the React.memo comparator ignored them, so a late-arriving attachment or language switch could keep rendering a stale preview when msg is empty.
405baf1 to
e95b489
Compare
|
iOS Build Available Rocket.Chat Experimental 4.73.0.108876 |
* feat: alt text * action: organized translations * fix: move alt text accessibility to button level and fix gallery height * chore: format code and fix lint issues * ix: stable gallery keys and forward alt text accessibility to gallery items * fix: i18n fallbacks, alt text a11y label, stale altText on remove, and 8.4.0 boundary test * action: organized translations * fix: i18n * action: organized translations * fix: preserve caption on send, correct version gate for share extension, and translated a11y labels in gallery * fix: alt text label * feat: queue composer attachments inline * feat: open attachment alt text in action sheet * fix: remove image gallery * action: organized translations * feat: edit message alt text * chore: format code and fix lint issues * feat: backward compatibilities * fix: edit * remove image gallery * fix: conflicts * code improvements * fix: composer * code improvements * fix: tests * fix(a11y): improve image attachment accessibility labels and order * action: organized translations * fix: unit tests * fix: focus * fix: a11y labels * chore: format code and fix lint issues * test * fix: merge conflicts * feat: improve a11y experience * fix: unlabelled on android * fix: action sheet input keyboard * fix: announce GIFs as interactive in ImageViewer for screen readers * fix: ignore whitespace-only alt text in message a11y label * refactor: extract sendAttachments helper from composer and share view * action: organized translations * feat: code improvements * code improvements * refactor: extract useImageDescriptionLabel hook and inline useAltTextSupported in Image * fix: i18n * action: organized translations * refactor: stabilize FlatList renderers and harden altText checks * fix(a11y): omit empty segments in message accessibilityLabel * refactor: extract normalizeAttachment and preserve handler order in useChooseMedia * refactor: extract normalizeAttachment and preserve handler order in useChooseMedia * rollback prettier changes * feat: attachment action sheet stories and test * feat: attachment actionsheet stories and tests * feat: announce images without description to screen readers * action: organized translations * fix: render Attachment file name in Thread Message preview when body is empty (#7323) * fix: preference value changes causing reset to other option (#7313) * fix: do not encrypt messages when workspace E2E is disabled (#7324) * fix: snapshot test * fix: snapshot test * feat: unify thumbs * efactor: rename handlePickedAttachments to handleSelectedAttachments * fix: pass altText and isAnimated to ImageViewer in ShareView Preview * refactor: make AltTextLabel altText optional with early return * fix: stop leaking attachment.altText into caption rendering * fix: if no alt text its rendering an empty absolute view * test: cover useMessageAccessibilityLabel and keep suffix on translated * refactor: prefer title_link/message_link over index for Reply attachment key * refactor: unify ShareView Thumbs and ComposerAttachments under shared AttachmentThumbs * feat: unify Thumbs * remove memo of useImageDescriptionLabel * fix: test and lint * fix: test * fix: i18n missing translation * fix: fallback accessibility label when alt text is empty * fix: remove unused code * rollback prettier changes * refactor: colocate useImageDescriptionLabel with message hooks * refactor: simplify message accessibility label composition * feat: add missing keys * feat: use thumb as children instead of add it again * refactor: tighten types and stable keys in message/share views * fix: useTheme on AltTextInput * action: organized translations * chore: code organization * chore: type improvement * chore: code organization * fix: image improvement * code improvements * fix: avoid undefined a11y label * fix: altText trim and improvements * fix: tests * fix: use memo composer attachments * chore: remove editAltText (not available) * feat: standardize shareView composer --------- Co-authored-by: OtavioStasiak <OtavioStasiak@users.noreply.github.com> Co-authored-by: Diego Mello <diegolmello@gmail.com> Co-authored-by: Rohit Bansal <40559587+Rohit3523@users.noreply.github.com>
Proposed changes
When a Thread Message with no text body (
msgempty) but one or more Attachments is rendered inline in the parent Room view (the abbreviated thread-reply preview row beneath the parent Message), the row showed only a small avatar with no body content — file name, caption, and image preview were all missing. This affected every Attachment type (.pptx,.png, audio, video, generic files).This PR fixes the inline preview branch only. The dedicated Thread view continues to render the full Attachment.
Changes:
getPreviewMessageFromAttachment(attachment, translateLanguage)inapp/containers/message/utils.tsreturns the best single-line preview string for one Attachment: translated caption > description > title > undefined.Content.tsx'sisPreviewbranch now falls back to the helper's result viaMarkdownPreviewwhenmsgis empty andattachmentshas at least one entry. The encrypted branch fires first, so "Encrypted message" continues to render with no file-name leak.IMessageContentwidened with optionalattachments?: IAttachment[]andautoTranslateLanguage?: string.Message.tsxalready forwards{...props}toContent, so no rewiring is needed at the call site.getMessageFromAttachmentis left untouched — Full Attachment and Quote renderers depend on its existing "description / translation only" semantics.Issue(s)
https://rocketchat.atlassian.net/browse/SUP-1041
How to test or reproduce
presentation.pptx,example.png, an audio clip) and check "also send to channel" so the Thread reply renders inline in the parent Room.Storybook coverage was extended (
MessageWithThread,MessageWithThreadLargeFont) with.png,.pptx, and multi-Attachment cases alongside the existing audio case.Screenshots
Types of changes
Checklist
Further comments
Content.tsx's preview rendering (image-title, file-title, description wins, translation wins, body wins over Attachment, encrypted no leak, non-preview no fallback, multi-Attachment first-only).removeQuotefilter inAttachments.tsx(which can drop bare file Attachments outside threads) is an adjacent gap that is intentionally out of scope for this fix.Summary by CodeRabbit
New Features
Tests
Documentation