-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[$250] Reports - No spinner on receipt thumbnail #62036
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
Comments
Triggered auto assignment to @jliexpensify ( |
🚨 Edited by proposal-police: This proposal was edited at 2025-05-14 22:45:33 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Reports - No spinner on receipt thumbnail What is the root cause of that problem?no mechanism for showing spinner while loading : App/src/components/Image/index.tsx Lines 138 to 156 in f65da19
What changes do you think we should make in order to solve the problem?add check for this case :
If source is undefined we could show nothing or just the spinner like current approach... styling could be dealt with later What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?UI issue What alternative solutions did you explore? (Optional)N/A |
cc @Expensify/design to confirm the expected result:
|
Job added to Upwork: https://www.upwork.com/jobs/~021922808396648832547 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @sobitneupane ( |
I think we're in the process of fixing this one on another issue. @shawnborton is involved in it, so I'll let him comment. |
Yeah, we do indeed have another issue for this: #61073 cc @Valforte @nkdengineer @dominictb for extra eyes |
Oh cool, should we close this one? |
Let me chim in to see if it's a regression from my PR. Because I cannot see the spinner on production as well. |
I think it's not related to #61073 since we only change the style of the spinner. |
Yeah that's what I thought, the mentioned issue is about styling this one is about spinner not showing on image loading. |
@jliexpensify Sorry if this is off-topic, but maybe it'd be worth checking with the design team if we should use our Image component instead of RN's in other places too such as here:
Because I'm pretty sure RN's doesn't have a spinner. Maybe a linter rule to avoid using third-party |
Got it, then we can proceed with this issue to try to figure out why the spinner isn't showing. Strange that it was working at some point but suddenly no longer is though... cc @mountiny @trjExpensify in case you think this has anything to do with TradReports changes. |
Mhm, I don't think we've touched those search result rows. |
also not remembering a specific change that could touch this @Kicu @sumo-slonik @Guccio163 maybe you remember I think we can keep this external |
Thanks Vit - still open for proposals everyone! cc @sobitneupane |
🚨 Edited by proposal-police: This proposal was edited at 2025-05-15 23:25:30 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.
What is the root cause of that problem?
What changes do you think we should make in order to solve the problem?
<ImageWithSizeCalculation
url={source ?? ''}
onMeasure={() => {}}
style={[style ?? [styles.w100, styles.h100], styles.overflowHidden]}
isAuthTokenRequired={!!isAuthTokenRequired}
loadingIconSize={"small"}
/>
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?N/A What alternative solutions did you explore? (Optional)
ResultMonosnap.screencast.2025-05-16.04-55-34.mp4 |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 9.1.45-15
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: #61764
Issue reported by: Applause - Internal Team
Action Performed:
Precondition: account has several expenses with receipts
2 Make sure you are in Expenses or Expense reports
Expected Result:
The loading spinner displayed for receipt thumbnails.
Default BG color fill for the thumbnail area, and use a 20x20 spinner in the middle of the thumbnail area for these
Actual Result:
No loading spinner displayed for receipt thumbnails
Workaround:
Unknown
Platforms:
Select the officially supported platforms where the issue was reproduced:
Platforms Tested:
On which of our officially supported platforms was this issue tested:Screenshots/Videos
Add any screenshot/video evidence
Bug6831198_1747239274940.bandicam_2025-05-14_18-50-12-819.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @sobitneupaneThe text was updated successfully, but these errors were encountered: