-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Fix - Search - When plenty of expenses are created already, haven't created expenses message shown #52059
base: main
Are you sure you want to change the base?
Conversation
…mpty search results
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.
@shawnborton Do we want similar behavior for the trip?
@FitseTLT Is the implementation will be different for the trip case?
I think the Trip message is more generic, it doesn't mention the user hasn't created a trip yet, so we can leave it as it is. |
@shawnborton WDYT about the above comments. Looks like you missed this convo. |
Ah apologies. Yeah I agree, let's leave the Trips empty state as-is. |
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
@FitseTLT Could you help resolve the conflict? Thank you! |
Good to go @mollfpr |
I agree with this because invoices are part of the expense. Just want to clarify what we should show when the invoice is empty but the expense is not and vice versa. |
My thinking is that if you have never sent or received an invoice, we'd show the "You haven't created any invoices yet" text. |
How do you like this? 2024-11-13.01-19-30.mp4 |
What do you think about the translation?
|
The translations seem good to me, but we'll want to follow our standard process there for making sure they get checked by our team. |
asked on slack |
Ready for review @mollfpr |
LGTM! However, I found an issue with the Android. On pressing cancel, it's briefly showing haven't created expense error after the nothing to show message. Screen.Recording.2024-11-14.at.22.26.52.mp4 |
Not reproducible in main ?? |
@FitseTLT It's not reproduced in the main. I think it's because the message already hasn't created expense. |
Details
This pr change the
No expense create yet
message displayed in empty search view for expenses type of search to be displayed only when there are not transactions created yet. I haven't used queryJSON to determine when to display it because when user change the status to any other value thanall
without any filter and the result is empty, we can't know the result is empty due to no expense created yet or no expenses in that statusFixed Issues
$ #51168
PROPOSAL: #51168 (comment)
Tests
Nothing to show
message is displayed (not haven't created yet any expenses message)Offline tests
Same as above
QA Steps
Same as above
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
2024-11-05.19-43-43.mp4
Android: mWeb Chrome
aw.mp4
iOS: Native
i.mp4
iOS: mWeb Safari
iw.mp4
MacOS: Chrome / Safari
w.mp4
MacOS: Desktop
d.mp4