Add ability to cycle via reports v2#69221
Conversation
sumo-slonik
force-pushed
the
feature/kuba_nowakowski/add_ability_to_cycle_via_reports_v2
branch
from
5c4d8c1 to
6b9a22d
Compare
…nto feature/kuba_nowakowski/add_ability_to_cycle_via_reports_v2 # Conflicts: # src/pages/home/sidebar/FloatingActionButtonAndPopover.tsx
…_via_reports_v2 # Conflicts: # src/components/MoneyRequestReportView/MoneyRequestReportView.tsx # src/pages/Search/SearchMoneyRequestReportPage.tsx
SzymczakJ
left a comment
SzymczakJ
left a comment
There was a problem hiding this comment.
Left few minor comments. LGTM
src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| const goToPrevReport = () => { | ||
| if (currentIndex === -1 || allReports.length === 0) { | ||
| return ''; |
There was a problem hiding this comment.
| return ''; | |
| return; |
src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx
Outdated
Show resolved
Hide resolved
|
We can probably start the second round of testing, since we managed to fix everything that we agreed should be delivered within this PR. |
|
@JmillsExpensify pinging so we can merge this ASAP |
|
@JmillsExpensify Can you please take a look at this PR when you have a chance? Thanks |
|
Looking now! Sorry for the delay. |
|
Works really great! Thanks for keeping at this one. |
|
@sumo-slonik can you please merge main to resolve the conflicts? Then we can merge cc @mjasikowski |
|
Merged and ready 👍 |
|
Awesome job everyone, merging! |
|
✋ 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/mjasikowski in version: 9.2.17-0 🚀
|
|
Bug found on staging: #71117 I'm not sure if it's critical enough to revert |
|
Another bug found from this: #71137 |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.2.17-6 🚀
|
| ); | ||
|
|
||
| if (!!(isLoadingInitialReportActions && reportActions.length === 0 && !isOffline) || shouldWaitForTransactions) { | ||
| if (!!(isLoadingInitialReportActions && reportActions.length === 0 && !isOffline) || shouldWaitForTransactions || shouldWaitForReportSync) { |
There was a problem hiding this comment.
Coming from #71268:
Regarding || shouldWaitForReportSync condition:
@sumo-slonik do we need to show skeleton while loading report from Onyx? This affects only android. I don't see skeleton in other platforms (maybe due to fast Onyx loading).
There was a problem hiding this comment.
I think the absence of this condition was causing some GUI elements to flicker.
| {isMoreContentShown && <View style={[styles.dFlex, styles.flexColumn, shouldAddGapToContents && styles.gap3, styles.pb3, styles.ph5]}>{moreContent}</View>} | ||
| {shouldDisplayNarrowVersion && | ||
| (shouldShowSelectedTransactionsButton ? ( | ||
| <View style={[styles.dFlex, styles.w100, styles.ph5]}> |
There was a problem hiding this comment.
Lack of spacing for the view component caused padding missing between next steps message and selected button for medium screen here
16 participants
Explanation of Change
Fixed Issues
main one:
$ #65283
prev deploy blokers:
$ #69045
$ #69098
$ #69052
$ #69104
$ #69086
PROPOSAL:
Tests
groupByReports.Offline tests
QA Steps
// 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))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
Screen.Recording.2025-08-29.at.13.16.43.mov
Android: mWeb Chrome
Screen.Recording.2025-08-29.at.13.26.46.mov
iOS: Native
iosNative.mov
iOS: mWeb Safari
Screen.Recording.2025-08-29.at.13.52.12.mov
MacOS: Chrome / Safari
Screen.Recording.2025-08-29.at.12.53.57.mov
MacOS: Desktop
Screen.Recording.2025-08-29.at.13.55.35.mov