fix: description with url is shown as text in preview#83422
fix: description with url is shown as text in preview#83422daledah wants to merge 16 commits intoExpensify:mainfrom
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@DylanDylann Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
src/components/ReportActionItem/TransactionPreview/TransactionPreviewContent.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6241a5ce28
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/components/ReportActionItem/TransactionPreview/TransactionPreviewContent.tsx
Outdated
Show resolved
Hide resolved
src/components/ReportActionItem/TransactionPreview/TransactionPreviewContent.tsx
Outdated
Show resolved
Hide resolved
JmillsExpensify
left a comment
There was a problem hiding this comment.
All looks good for product.
|
|
@daledah please resolve all the above comments from codex and github-actions |
|
@DylanDylann looks like we allow the Multi-Line row wrapping in Expense Previews in this PR so i updated my solution to match with this expected. Please check again |
looks like the PR is reverted, i updated to previous version |
|
@daledah Is this ready? |
|
@DylanDylann yes, i updated. Please move forward |
|
@daledah Please check the failed test |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de99380163
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/components/ReportActionItem/TransactionPreview/TransactionPreviewContent.tsx
Outdated
Show resolved
Hide resolved
@daledah Since the width of the transaction preview can change, I'm not sure we should use a fixed max character limit. For normal descriptions, the text auto-truncates when there's not enough space. Could you check if we can do the same with HTML rendering? Otherwise, I think we need to confirm this new behavior with the design team first. |
From https://github.com/Expensify/App/blob/main/contributingGuides/CodeCov.md
TransactionPreviewContent is already covered (likely nested in a parent component), so I think we should maintain the coverage of this file. For an easier approach, we can create a new test for TransactionPreviewContent. Wdyt? |
|
@daledah This is my draft try, I believe it will help us in this case to maintain code coverage |
i updated
I’m not sure what the expected behavior is here—whether it should be one line or two lines. We previously had a PR that allowed multi-line row wrapping in Expense Previews, but it was reverted. Could you tag the design team here and ask what the expected behavior should be?
thanks @DylanDylann, I tried your test and it worked. I added |
@daledah It looks like another screen. Could you please ask the author of that PR to confirm? |
@TaduJR could you please confirm that we should show the description and the merchant on two lines, as implemented in this PR? |
No we should not show two line for Expense Preview, two line feature is only intended for Reports Page view. For more context please check out this thread https://expensify.slack.com/archives/C01GTK53T8Q/p1772543905741479 LMK if you need anything else. Thanks! |
thanks @TaduJR |
|
@DylanDylann i updated, please check again |
@daledah With the current approach, we would change the original behavior. We shouldn’t hard-code the max length because the available space may vary depending on the screen. |
@DylanDylann I've updated. Please check again |
|
Looks like the failing check is not related to this PR. i'll merge main to resolve it soon |
|
|
||
| const description = truncate(StringUtils.lineBreaksToSpaces(Parser.htmlToText(requestComment ?? '')), {length: CONST.REQUEST_PREVIEW.MAX_LENGTH}); | ||
| const isDescriptionHTML = !!requestComment && Parser.isHTML(requestComment); | ||
| const requestDescription = truncate(requestComment ?? '', {length: CONST.REQUEST_PREVIEW.MAX_LENGTH}); |
There was a problem hiding this comment.
@MelvinBot Could you check whether removing lineBreaksToSpaces is safe?
|
@codex review |
|
Removing Why it was there (old flow)New flowWhat's safe
Potential risks
SummaryThe removal is safe if transaction descriptions only contain inline HTML (which appears to be the common case). The main concern is risk #2 — losing custom renderers may affect how links behave/look in the preview. It would be worth verifying that link press handling and styling work as expected with the limited renderer set. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1fb2e6ff22
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| <RenderHTMLConfigProvider | ||
| defaultTextProps={{selectable: isSelectable ?? true, allowFontScaling: false, numberOfLines}} | ||
| renderersProps={renderersProps} | ||
| renderers={renderers} |
There was a problem hiding this comment.
Preserve custom HTML renderers in the new truncation path
When numberOfLines is set, this wraps the source in a fresh RenderHTMLConfigProvider that only registers bullet-item and sparkles-icon. I checked react-native-render-html's RenderHTMLConfigProvider/RenderRegistryProvider for v6.3.1, and nested providers rebuild the renderer registry from the passed renderers instead of merging the ambient one. That means transaction previews rendered via numberOfLines no longer use our a, deleted-action, mention-*, etc. renderers from BaseHTMLEngineProvider, so links in descriptions will bypass AnchorRenderer's internal-link/deleted-state handling.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@MelvinBot Could you explain the comment above in more detail?
There was a problem hiding this comment.
The Codex comment is about a custom renderer loss issue. Here's the detailed explanation:
How HTML rendering works in the App
The app has a global BaseHTMLEngineProvider (BaseHTMLEngineProvider.tsx:251) that registers 22 custom renderers for tags like <a>, <code>, <img>, <mention-user>, <deleted-action>, etc. These custom renderers handle Expensify-specific behavior — for example, AnchorRenderer handles internal link navigation, hover styles, deleted-state strikethrough on links, and right-click context menus.
What happens when numberOfLines is passed
In RenderHTML.tsx:67-78, when numberOfLines is set, the component creates a new nested RenderHTMLConfigProvider:
if (onLinkPress !== undefined || numberOfLines !== undefined) {
return (
<RenderHTMLConfigProvider
defaultTextProps={{selectable: isSelectable ?? true, allowFontScaling: false, numberOfLines}}
renderersProps={renderersProps}
renderers={renderers} // ← only has bullet-item and sparkles-icon!
>
{htmlSource}
</RenderHTMLConfigProvider>
);
}The renderers object (RenderHTML.tsx:57-61) only includes 2 of the 22 custom renderers:
bullet-itemsparkles-icon
Why inner providers replace rather than merge
react-native-render-html's RenderHTMLConfigProvider creates a new RenderRegistryProvider that sets a fresh context value. In React, the innermost context provider wins — it doesn't merge with the outer one. So the inner provider with 2 renderers completely shadows the outer BaseHTMLEngineProvider that has all 22.
What's lost (20 renderers)
a (AnchorRenderer), code, img, video, edited, pre, task-title, rbr, mention-user, mention-report, mention-here, mention-short, user-details, copy-text, emoji, next-step-email, deleted-action, concierge-link, transaction-history-link, account-manager-link
Why this matters for the PR
The most critical loss is AnchorRenderer (a tag). The whole point of this PR is to make URL descriptions clickable in transaction previews. Without the custom AnchorRenderer, links will still render (the library has a built-in default <a> handler), but they'll lose:
- Internal link navigation — Expensify URLs won't use
openLinkfor in-app navigation - Deleted-state styling — strikethrough won't apply to links inside deleted actions
- Context menu — no right-click menu via
AnchorForCommentsOnly - Hover styles — no custom hover behavior
Note: this is a pre-existing issue
The same 2-renderer limitation existed before this PR for the onLinkPress code path. This PR just expanded the trigger to also include numberOfLines, exposing it in a new context where <a> tags matter.
Possible fix
The RenderHTML component should either:
- Import and spread
htmlRenderersfromHTMLEngineProvider/HTMLRenderersinto its localrenderersobject - Or read the current renderers from context and merge with local overrides
- Or avoid creating a separate
RenderHTMLConfigProviderand passnumberOfLinesthrough a different mechanism
| <RenderHTML | ||
| html={isDeleted ? `<del>${description}</del>` : description} | ||
| numberOfLines={1} |
There was a problem hiding this comment.
Keep non-HTML descriptions on the plain Text path
isDescriptionHTML is computed here, but the new render branch ignores it and sends every description through <RenderHTML>. For expense descriptions that are just arbitrary user input, strings containing literal tag-like text such as <draft> or 5 < 10 can now be interpreted as markup instead of being shown verbatim, whereas the previous <Text> path displayed the raw text. This only affects previews without a merchant, but it is a user-visible regression for plain descriptions that merely happen to contain angle brackets.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. @daledah Could you check this comment?
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeScreen.Recording.2026-03-18.at.14.32.50.moviOS: HybridAppScreen.Recording.2026-03-18.at.14.34.27.moviOS: mWeb SafariScreen.Recording.2026-03-18.at.14.32.25.movMacOS: Chrome / SafariScreen.Recording.2026-03-18.at.14.30.53.mov |
|
@daledah It doesn't work on Android
|
|
still find the solution for the android bug here |
| <RenderHTML | ||
| html={isDeleted ? `<del>${description}</del>` : description} | ||
| numberOfLines={1} |
There was a problem hiding this comment.
@MelvinBot Could you investigate why the truncated logic doesn't work on the Android app?
🔍 Investigation: Why truncation doesn't work on AndroidSummary: The Root Cause: When On iOS/Web, the parent On Android, React Native's text layout engine does not properly propagate Recommended Fix Options (from simplest to most involved):
Detailed InvestigationEvidence Chain
Additional NoteThe inner Next Steps: |

Explanation of Change
Fixed Issues
$ #82301
PROPOSAL: #82301 (comment)
Tests
Test case 1
Test case 2
Offline tests
Same as tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari