[Better Expense Report View] Remove RBR logic from Preview & clean-up invoice, split, and self-DM previews#59811
Conversation
|
@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] |
|
Going to run a test build 🚀 |
|
🚧 @shawnborton has triggered a test app build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
Reviewer Checklist
Screenshots/VideosiOS: NativeScreenRecording_04-10-2025.17-00-37_1.MP4 |
allgandalf
left a comment
There was a problem hiding this comment.
overall great job!, just few comments and we should be good to go, I don't think so the functionality is changing, so i will go forward with the testing and in the meantime you can work on the changes
src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx
Show resolved
Hide resolved
src/components/ReportActionItem/MoneyRequestReportPreview/types.ts
Outdated
Show resolved
Hide resolved
|
@allgandalf Okay, the problem is that onPreviewPressed is used as an conditional to render styled PressableWithoutFeedback, so it can't be empty. This will be fixed in another PR by @Kicu (not yet created), but it doesn't affect the UI or other logic, so it's NAB. |
|
🚧 @mountiny has triggered a test app build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
|
@allgandalf @JakubKorytko Sorry to clarify, is this what you see on the current branch? #59811 (comment) lets not merge it with regression like this even if its behind beta |
|
thanks for clarifying, it was not clear to me from the comments @JakubKorytko What issue are you referring to by this comment? #59811 (comment) |
|
No issue raised for that, it's just the way this component works, if onPreviewPressed is not set then PressableWithoutFeedback (with styles applied) will not render, so we need to pass the empty function as a prop for it to work properly. This will be fixed in another PR by @Kicu |
mountiny
left a comment
There was a problem hiding this comment.
Waiting for the translation confirmation
|
Gonna move this one ahead and we can delve into the invoice report name later |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.1.27-0 🚀
|
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.1.28-15 🚀
|
| isBillSplit={isSplitBillActionReportActionsUtils(action)} | ||
| wrapperStyle={shouldUseNarrowLayout ? {...styles.w100, ...styles.mw100} : reportPreviewStyles.transactionPreviewStyle} | ||
| onPreviewPressed={() => {}} | ||
| isTrackExpense={isTrackExpenseActionReportActionsUtils(action)} |
There was a problem hiding this comment.
We need to pass shouldDisplayContextMenu here, otherwise it will cause this bug #60808 (comment)
| children = emptyHTML; | ||
| // Table Report View does not display these components as separate messages, except for self-DM | ||
| if (canUseTableReportView && report?.type === CONST.REPORT.TYPE.CHAT) { | ||
| if (report.chatType === CONST.REPORT.CHAT_TYPE.SELF_DM) { |
| so we don't need to know the width of the component to calculate it - works with percentages. | ||
| */} | ||
| <View style={[styles.r0, styles.b0, styles.p4, styles.mtn1, styles.pAbsolute, styles.mirror]}> | ||
| <SkeletonViewContentLoader |
There was a problem hiding this comment.
turns out this was loading in opposite directions, we fixed it in #67356









Explanation of Change
This PR fixes following issues from the Preview Followups:
Also fixed blank messages appearing in chats where previews/expenses/system messages should appear after new view changes.
Fixed Issues
$ #59452 (partial)
$ #59979 (skeleton layout)
PROPOSAL: N/A
Tests
[Remove RBR logic from Preview]
[Clean-up self-DM previews]
[Clean-up invoice previews]
[Clean-up split previews]
Also verify that there are no empty messages where the expense/preview/system message should be and that trips are working properly.
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))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.mov
iOS: mWeb Safari
MacOS: Chrome / Safari
Web.mov
MacOS: Desktop