-
Notifications
You must be signed in to change notification settings - Fork 3.5k
implemented HTML parsing for getReportNames()/ Conditional Parsing for Displaynames #74928
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@QichenZhu should you be reviewer here since you reviewed the first try PR? #74254 |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good from a product perspective 👍
|
I was also going to request that @QichenZhu be assigned for that very reason |
QichenZhu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@whiletrace, the previous PR was reverted, so could you reapply it here? Thanks!
Yes, Ill get the original commits reinstated and on the diff by EOD 11/13/2025, sorry for the delay. |
left over from refactor comment is no longer accurate Co-authored-by: Qichen Zhu <[email protected]>
refactoring based upon correct pattern usage Co-authored-by: Qichen Zhu <[email protected]>
…g github action processes
…ParseHTMLTest.tsx
0777dbe to
eb05c5c
Compare
|
@QichenZhu |
QichenZhu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick fix. Have a good weekend!
no problem @QichenZhu you as well! thanks again for the support its been |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
@whiletrace Seems like we have a lint error here
|
|
@samranahm I can look at it this afternoon about 15:00 Pacific Standard Time. |
@QichenZhu @carlosmiceli since this is merged, What is recommended process, can I fix and push? |
|
Yes, submit a new PR and link to the original issue. Thank you! |
@carlosmiceli for a lint-only fix, do you need me to go through the full testing checklist again, or can I just fix the lint error and note that no functional changes were made? |
|
We should still test just in case. |
|
@carlosmiceli okay I'll get on it I can put my new m1 through its paces. |
|
@carlosmiceli okay I pulled from Expensify main rebased unto my dev branch and created a new branch for the PR trying to recreate Lint error now. |
|
All sorted here: #75347 |
Thank you @QichenZhu as well as @roryabraham |
|
🚀 Deployed to staging by https://github.com/carlosmiceli in version: 9.2.60-0 🚀
|
|
🚀 Deployed to staging by https://github.com/carlosmiceli in version: 9.2.60-0 🚀
|
|
🚀 Deployed to staging by https://github.com/carlosmiceli in version: 9.2.60-0 🚀
|
|
@carlosmiceli @QichenZhu |
|
@whiletrace, you can use these credentials: https://expensify.slack.com/archives/C01GTK53T8Q/p1711049879962529 |
|
update created draft PR here |
[CP Staging] [No QA] Revert #74928 "implemented HTML parsing for getReportNames()/ Conditional Parsing for Displaynames"
|
🚀 Deployed to production by https://github.com/grgia in version: 9.2.60-2 🚀
|



…from issue 74713
Explanation of Change
This change Parses HTML of
unReportedTransactionMessagesin thegetreportNames()solving the regression that pull request from #68721 (comment) created as well as implementing the original changes of the PR: conditional Parsing of HTML at thedisplayNamescomponentFixed Issues
$ #68721 (comment)
$ #74713
PROPOSAL: #68721 (comment)
Tests
Additional steps to cover Regression bug
Offline tests
Additional steps to cover Regression bug
QA Steps
Same as tests
// TODO: These must be filled out, or the issue title must include "[No QA]."
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.mp4
Android: mWeb Chrome
compressed.mp4
iOS: Native
ioscomressed.mp4
iOS: mWeb Safari
safari-mobileweb.mp4
MacOS: Chrome / Safari
chrome.mp4
MacOS: Desktop
macappcompressed.mp4