fix unreported expense skeletons#63042
Conversation
|
@jayeshmangwani 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] |
|
@sumo-slonik Changes look good here! Just a couple of small things - not related to this PR directly, but since it's on the Unreported expenses page, we might want to address them here or in a follow-up PR:
ios-scroll.mov
|
|
@jayeshmangwani I think we might want to merge this PR earlier. Once I finish working on another bug, I’ll address the issues you mentioned. If the PR is still open by then, I’ll include the changes here. If it has already been merged, I’ll make a follow-up. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid.movAndroid: mWeb Chromemweb-chrome.moviOS: HybridAppiOS.moviOS: mWeb Safarimweb-safari.movMacOS: Chrome / Safariweb.movMacOS: Desktopdesktop.mov |
jayeshmangwani
left a comment
There was a problem hiding this comment.
Changes work well, looks good to me 🚀
@shawnborton What do you think about this? Should we add some spacing between the button and the list? I feel like the way it looks now is consistent with other parts of the app |
I fixed it and added the key. |
Yeah, I would think the bottom of the view that contains the expenses should have some extra bottom padding so that it can be scrolled up and above the fixed area where the button sits. |
| const [hasMoreUnreportedTransactionsResults] = useOnyx(ONYXKEYS.HAS_MORE_UNREPORTED_TRANSACTIONS_RESULTS, {canBeMissing: true}); | ||
| const [isLoadingUnreportedTransactions] = useOnyx(ONYXKEYS.IS_LOADING_UNREPORTED_TRANSACTIONS, {canBeMissing: true}); | ||
|
|
||
| const showUnreportedTransactionsSkeletons = isLoadingUnreportedTransactions && hasMoreUnreportedTransactionsResults; |
There was a problem hiding this comment.
Should we also check that the user is online here to prevent showing the skeleton when they're offline?
There was a problem hiding this comment.
We should. Good catch.
| const [hasMoreUnreportedTransactionsResults] = useOnyx(ONYXKEYS.HAS_MORE_UNREPORTED_TRANSACTIONS_RESULTS, {canBeMissing: true}); | ||
| const [isLoadingUnreportedTransactions] = useOnyx(ONYXKEYS.IS_LOADING_UNREPORTED_TRANSACTIONS, {canBeMissing: true}); | ||
|
|
||
| const showUnreportedTransactionsSkeletons = isLoadingUnreportedTransactions && hasMoreUnreportedTransactionsResults && !isOffline; |
There was a problem hiding this comment.
NAB - just caught this a bit late, but we should've used shouldShowUnreportedTransactionsSkeletons var name instead of showUnreportedTransactionsSkeletons
|
✋ 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/luacmartins in version: 9.1.55-0 🚀
|
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 9.1.56-2 🚀
|
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 9.1.58-0 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.1.58-4 🚀
|

Explanation of Change
Fixed Issues
$ #62954
PROPOSAL:
Tests
On iOS native and Android native:
Expected behavior:
Skeletons should only appear while something is loading — there should be no infinite loading of skeletons.
Offline tests
Unnesesary
QA Steps
Same as test
// 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
iOS: Native
Screen.Recording.2025-05-29.at.16.49.16.mov