Skip to content

fix: multiple bolds with </em> in between are not rendered correctly#59147

Merged
Julesssss merged 1 commit intoExpensify:mainfrom
mkzie2:mkzie2-issue/58125
Mar 31, 2025
Merged

fix: multiple bolds with </em> in between are not rendered correctly#59147
Julesssss merged 1 commit intoExpensify:mainfrom
mkzie2:mkzie2-issue/58125

Conversation

@mkzie2
Copy link
Contributor

@mkzie2 mkzie2 commented Mar 26, 2025

Explanation of Change

Fixed Issues

$ #58215
PROPOSAL: #58215 (comment)

Tests

  1. Paste this message into the composer in App:
~_**Zoom**_~
*Test*
  1. Verify *Zoom* is formatted with bold, italic and strikethrough
  2. Verify Test is formatted with bold
  3. Send the message
  4. Verify *Zoom* and Test are formatted with bold
  • Verify that no errors appear in the JS console

Offline tests

None

QA Steps

  1. Paste this message into the composer in App:
~_**Zoom**_~
*Test*
  1. Verify *Zoom* is formatted with bold, italic and strikethrough
  2. Verify Test is formatted with bold
  3. Send the message
  4. Verify *Zoom* and Test are formatted with bold
  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
    • MacOS: Desktop
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
      • If any non-english text was added/modified, I used JaimeGPT to get English > Spanish translation. I then posted it in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
  • I verified that if a function's arguments changed that all usages have also been updated correctly
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

Screenshot_1742993987
Screenshot_1742993983

Android: mWeb Chrome

Screenshot_1742993959
Screenshot_1742993957

iOS: Native

Simulator Screenshot - iPhone 16 - 2025-03-26 at 19 53 50
Simulator Screenshot - iPhone 16 - 2025-03-26 at 19 53 46

iOS: mWeb Safari

Screenshot 2025-03-26 at 19 55 36
Screenshot 2025-03-26 at 19 55 33

MacOS: Chrome / Safari

Screenshot 2025-03-26 at 19 52 46
Screenshot 2025-03-26 at 19 52 44

MacOS: Desktop

Screenshot 2025-03-26 at 19 54 59
Screenshot 2025-03-26 at 19 54 57

@github-actions
Copy link
Contributor

⚠️ This PR is possibly changing native code and/or updating libraries, it may cause problems with HybridApp. Please check if any patch updates are required in the HybridApp repo and run an AdHoc build to verify that HybridApp will not break. Ask Contributor Plus for help if you are not sure how to handle this. ⚠️

@mkzie2 mkzie2 marked this pull request as ready for review March 26, 2025 13:00
@mkzie2 mkzie2 requested a review from a team as a code owner March 26, 2025 13:00
@melvin-bot melvin-bot bot requested review from ishpaul777 and removed request for a team March 26, 2025 13:00
@melvin-bot
Copy link

melvin-bot bot commented Mar 26, 2025

@ishpaul777 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]

@mkzie2
Copy link
Contributor Author

mkzie2 commented Mar 26, 2025

⚠️ This PR is possibly changing native code and/or updating libraries, it may cause problems with HybridApp. Please check if any patch updates are required in the HybridApp repo and run an AdHoc build to verify that HybridApp will not break. Ask Contributor Plus for help if you are not sure how to handle this. ⚠️

@ishpaul777 Do you know what is causing this?

@ishpaul777
Copy link
Contributor

ishpaul777 commented Mar 28, 2025

Reviewer Checklist

  • I have verified the author checklist is complete (all boxes are checked off).
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the steps for local testing are in the Tests section
    • I verified the steps for Staging and/or Production testing are in the QA steps section
    • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I checked that screenshots or videos are included for tests on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
  • I verified tests pass on all platforms & I tested again on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
    • MacOS: Desktop
  • If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
  • I verified proper code patterns were followed (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I verified that this PR follows the guidelines as stated in the Review Guidelines
  • I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately and each prop has a /** comment above it */
    • The file is named correctly
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
    • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
    • All JSX used for rendering exists in the render method
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native
Screen.Recording.2025-03-29.at.5.09.22.PM.mov
Android: mWeb Chrome
Screen.Recording.2025-03-29.at.6.28.17.PM.mov
iOS: Native
Screen.Recording.2025-03-29.at.5.59.50.PM.mov
iOS: mWeb Safari
Screen.Recording.2025-03-29.at.5.20.06.PM.mov
MacOS: Chrome / Safari
Screen.Recording.2025-03-29.at.12.51.50.AM.mov
MacOS: Desktop
Screen.Recording.2025-03-29.at.6.32.07.PM.mov

@ishpaul777
Copy link
Contributor

@ishpaul777 Do you know what is causing this?

just a warning to test this PR on hybrid app since dependency is change, since you don't have access I'll do it myself

Copy link
Contributor

@ishpaul777 ishpaul777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! found a unrelated bug reported here

@melvin-bot melvin-bot bot requested a review from Julesssss March 29, 2025 13:22
@Julesssss Julesssss merged commit 2ad1860 into Expensify:main Mar 31, 2025
28 checks passed
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@github-actions github-actions bot added the DeployBlockerCash This issue or pull request should block deployment label Mar 31, 2025
@github-actions
Copy link
Contributor

Performance Comparison Report 📊⚠️ Some tests did not pass successfully, so some results are omitted from final report: Linking

Significant Changes To Duration

Name Duration
App start time contentAppeared_To_screenTTI 976.298 ms → 1154.810 ms (+178.512 ms, +18.3%) 🔴
App start time TTI 1666.498 ms → 1832.710 ms (+166.212 ms, +10.0%) 🔴
Show details
Name Duration
App start time contentAppeared_To_screenTTI Baseline
Mean: 976.298 ms
Stdev: 252.628 ms (25.9%)
Runs: 519.4785239994526 569.2924290001392 594.6399349998683 601.4857400003821 614.2358780000359 629.2554189991206 630.8823130000383 634.3641630001366 636.7242360003293 638.251978000626 643.5596990007907 647.3912519998848 647.6974689997733 657.497289000079 659.9580889996141 663.2524960003793 668.8589900005609 682.6050680000335 705.5550400000066 719.784679999575 750.1726769991219 945.9206780008972 1021.47597300075 1030.650482000783 1045.4532379992306 1056.8059570007026 1057.395704999566 1070.901093000546 1081.4571150001138 1099.914123000577 1102.9906050004065 1121.3889700006694 1126.7362249996513 1139.5195269994438 1146.836730999872 1151.8217060007155 1157.246357999742 1159.3738059997559 1167.1158700007945 1176.0526960007846 1178.9340250007808 1179.327343000099 1182.6634330004454 1183.8235560003668 1188.5724960006773 1188.8044390007854 1191.284096000716 1198.3379149995744 1198.73129699938 1201.171181999147 1212.9987969994545 1219.0981489997357 1220.8752759993076 1220.9484449997544 1223.7667169999331 1226.3841099999845 1233.5365309994668 1237.9904500003904 1251.6806690003723 1264.9531420003623

Current
Mean: 1154.810 ms
Stdev: 61.164 ms (5.3%)
Runs: 997.6099260002375 1024.4972980003804 1037.3917520008981 1053.5482100006193 1061.216677999124 1065.570392999798 1085.6573189999908 1086.581706000492 1102.3637520000339 1105.5435739997774 1113.5643279999495 1117.8294409997761 1119.6065849997103 1123.4511169996113 1125.39553800039 1132.0819099992514 1143.3526259995997 1143.6429910007864 1143.7514789998531 1154.6008369997144 1154.621650999412 1161.2253230009228 1165.0983199998736 1168.2201949991286 1169.8442689999938 1175.2632909994572 1178.1739649996161 1182.705189999193 1184.1000030003488 1191.5706009995192 1195.090936999768 1197.459865000099 1198.7970610000193 1199.9522239994258 1204.7237470000982 1207.0843199994415 1207.6248339992017 1209.8492920007557 1214.002078000456 1219.0765950009227 1221.1305950004607 1227.88569400087 1228.1771350000054 1233.7208249997348 1243.4034850001335 1245.1925140004605
App start time TTI Baseline
Mean: 1666.498 ms
Stdev: 277.069 ms (16.6%)
Runs: 1182.6399349998683 1205.2924290001392 1209.5596990007907 1249.9580889996141 1261.485740000382 1262.251978000626 1270.2524960003793 1286.4785239994526 1288.3641630001366 1297.2554189991206 1304.3912519998848 1307.2358780000359 1317.8823130000383 1319.8589900005609 1324.7242360003293 1327.6050680000335 1340.6974689997733 1352.784679999575 1392.497289000079 1405.5550400000066 1459.172676999122 1666.47597300075 1671.9206780008972 1684.9906050004065 1693.650482000783 1747.8059570007026 1773.7362249996513 1801.246357999742 1807.395704999566 1813.0526960007846 1815.4532379992306 1815.836730999872 1836.4571150001138 1850.1158700007945 1856.5195269994438 1868.3738059997559 1873.6634330004454 1874.9340250007808 1876.8217060007155 1881.284096000716 1883.8235560003668 1885.914123000577 1888.8044390007854 1890.0981489997357 1892.7667169999331 1893.5724960006773 1910.901093000546 1912.171181999147 1912.3889700006694 1919.9987969994545 1920.3841099999845 1923.73129699938 1932.327343000099 1937.3379149995744 1943.9904500003904 1948.5365309994668 1949.6806690003723 1952.9484449997544 1956.9531420003623 1957.8752759993076

Current
Mean: 1832.710 ms
Stdev: 118.718 ms (6.5%)
Runs: 1399.1249780002981 1426.7116679996252 1633.6099260002375 1693.6573189999908 1700.216677999124 1735.570392999798 1736.3526259995997 1737.39553800039 1747.0819099992514 1747.5643279999495 1757.3917520008981 1784.5435739997774 1789.5482100006193 1790.6429910007864 1792.4511169996113 1797.6065849997103 1803.1739649996161 1818.581706000492 1830.2253230009228 1834.4972980003804 1848.2201949991286 1848.8442689999938 1857.8294409997761 1859.0983199998736 1873.7237470000982 1875.3637520000339 1882.7514789998531 1891.1000030003488 1893.9522239994258 1894.7970610000193 1898.1771350000054 1900.459865000099 1903.8492920007557 1909.0843199994415 1909.1305950004607 1910.1925140004605 1910.6008369997144 1917.2632909994572 1920.5706009995192 1929.6248339992017 1931.002078000456 1932.705189999193 1932.88569400087 1933.7208249997348 1934.621650999412 1957.090936999768 1976.4034850001335 1981.0765950009227

Meaningless Changes To Duration

Show entries
Name Duration
App start time appCreation 69.932 ms → 71.898 ms (+1.966 ms, +2.8%)
App start time nativeLaunch 25.967 ms → 24.814 ms (-1.153 ms, -4.4%)
App start time appCreationEnd_To_contentAppeared 512.000 ms → 516.508 ms (+4.508 ms, +0.9%)
App start time nativeLaunchEnd_To_appCreationStart 79.583 ms → 77.583 ms (-2.000 ms, -2.5%)
App start time runJsBundle 313.800 ms → 312.700 ms (-1.100 ms, ±0.0%)
App start time regularAppStart 0.020 ms → 0.020 ms (+0.000 ms, +1.8%)
App start time (CPU) 148.178 % → 147.053 % (-1.125 %, -0.8%)
App start time (FPS) 60.000 FPS → 60.000 FPS
App start time (RAM) 380.513 MB → 381.621 MB (+1.108 MB, ±0.0%)
App start time (CPU/JS) 0.000 % → 0.000 %
App start time (CPU/UI) 24.543 % → 23.905 % (-0.638 %, -2.6%)
Open search router TTI Load Search Options 166.780 ms → 166.025 ms (-0.754 ms, ±0.0%)
Open search router TTI Open Search Router TTI 1346.453 ms → 1333.324 ms (-13.129 ms, -1.0%)
Open search router TTI (CPU) 148.454 % → 147.671 % (-0.782 %, -0.5%)
Open search router TTI (FPS) 60.000 FPS → 60.000 FPS
Open search router TTI (RAM) 413.223 MB → 414.310 MB (+1.087 MB, ±0.0%)
Open search router TTI (CPU/JS) 0.000 % → 0.000 %
Open search router TTI (CPU/UI) 24.425 % → 24.523 % (+0.098 %, ±0.0%)
Report typing Composer typing rerender count 1.000 renders → 1.000 renders
Report typing Message sent 485.446 ms → 482.777 ms (-2.670 ms, -0.5%)
Report typing (CPU) 100.773 % → 101.416 % (+0.643 %, +0.6%)
Report typing (FPS) 60.000 FPS → 60.000 FPS
Report typing (RAM) 504.219 MB → 504.213 MB (-0.006 MB, ±0.0%)
Report typing (CPU/JS) 0.000 % → 0.000 %
Report typing (CPU/UI) 20.448 % → 20.630 % (+0.182 %, +0.9%)
Chat opening Chat TTI 1053.443 ms → 1042.308 ms (-11.136 ms, -1.1%)
Chat opening (CPU) 157.272 % → 157.288 % (+0.016 %, ±0.0%)
Chat opening (FPS) 60.000 FPS → 60.000 FPS
Chat opening (RAM) 421.868 MB → 425.937 MB (+4.069 MB, +1.0%)
Chat opening (CPU/JS) 0.000 % → 0.000 %
Chat opening (CPU/UI) 32.816 % → 33.302 % (+0.486 %, +1.5%)

undefined

@github-actions
Copy link
Contributor

@Expensify/mobile-deployers 📣 Please look into this performance regression as it's a deploy blocker.

@Julesssss
Copy link
Contributor

I'm not sure how much we trust this check? I'm skeptical that this regex change led to 10-18% slowdown.

Screenshot 2025-03-31 at 11 48 41

@github-actions
Copy link
Contributor

github-actions bot commented Apr 1, 2025

🚀 Deployed to staging by https://github.com/Julesssss in version: 9.1.22-0 🚀

platform result
🖥 desktop 🖥 success ✅
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@github-actions
Copy link
Contributor

github-actions bot commented Apr 3, 2025

🚀 Deployed to production by https://github.com/grgia in version: 9.1.22-10 🚀

platform result
🖥 desktop 🖥 success ✅
🕸 web 🕸 success ✅
🤖 android 🤖 failure ❌
🍎 iOS 🍎 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DeployBlockerCash This issue or pull request should block deployment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants