Conversation
…ages Change the email-comment whiteSpace from "normal" to "pre-wrap" to preserve newlines between HTML elements (like between </p> and <ul>) while still allowing text to wrap for long lines. This fixes the formatting issue where Concierge responses (which have source: "email") were displaying with collapsed whitespace, making paragraphs and lists run together. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@ikevin127 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] |
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c796864bab
ℹ️ 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".
| getMixedUAStyles: (tnode) => { | ||
| if (tnode.attributes.islarge === undefined) { | ||
| return {whiteSpace: 'normal'}; | ||
| return {whiteSpace: 'pre-wrap'}; | ||
| } | ||
| return {whiteSpace: 'normal', ...styles.onlyEmojisText}; | ||
| return {whiteSpace: 'pre-wrap', ...styles.onlyEmojisText}; |
There was a problem hiding this comment.
Setting whiteSpace: 'pre-wrap' on the email-comment wrapper makes all descendant text nodes preserve newlines and indentation from the raw HTML source. Many email templates are pretty‑printed with line breaks/indentation inside <p>/<li> tags for readability, which were previously collapsed by whiteSpace: 'normal'. With this change, those formatting newlines become visible line breaks or leading spaces in the rendered message, so emails with wrapped source lines will show unintended line breaks/extra whitespace inside paragraphs and list items.
Useful? React with 👍 / 👎.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Taking this into a separate PR |
2 participants
Explanation of Change
This PR fixes a formatting issue with Concierge responses by changing the CSS
whiteSpaceproperty foremail-commentelements from'normal'to'pre-wrap'.Root Cause: Messages with
source: 'email'(including Concierge auto-responses) get wrapped in<email-comment>tags. These tags were usingwhiteSpace: 'normal', which collapses whitespace between HTML elements (like newlines between</p>and<ul>tags). This caused paragraphs and lists to run together without proper spacing.The Fix: Change
whiteSpace: 'normal'towhiteSpace: 'pre-wrap':pre-wrappreserves newlines between HTML elements (fixing the formatting issue)pre-wrapstill wraps long lines (preventing horizontal overflow)This is consistent with the approach taken in PR #76009 which fixed similar whitespace issues for list elements.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/590655
Tests
Offline tests
N/A - This is a CSS styling change with no network dependency.
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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