-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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: update how we show pending and scanning status #53112
Conversation
I am a little stuck on the Android and iOS Native builds, will add later |
src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx
Outdated
Show resolved
Hide resolved
What's the latest on this one? Seems like we hit a slowdown. Let's keep the momentum going so we can get this merged! |
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.
Code LGTM, I like your suggestion to change the name, could you push that? @nkdengineer
@ntdiary i fixed lint and updated |
@ntdiary i updated, please check again |
src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx
Outdated
Show resolved
Hide resolved
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.
Let me know when this is ready for a re-review and I'll trigger a final test build
@nkdengineer, Can you please post the translation on Slack to request internal review? I'm preparing the test videos. :) |
asking here |
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.
I've uploaded many test screenshots in the check list, will approve it for now. And once the translation above is confirmed, I think it's fine to .
BTW, I noticed that on Android platform, the Cash • Pending match • Review required
wraps to the next line, and I think that's normal behavior (case 9, or could be Card • Pending match • Review required
). :D
@nkdengineer, PR has conflicts. Additionally, could someone help confirm the translation in the PR so we can get this merged, we have posted it on slack :) cc @grgia @shawnborton |
Bump on conflicts @nkdengineer. Let's not hold on the translations, they are easy to update |
![]() @grgia, not sure why the ![]() |
@nkdengineer, typecheck is failed. :) |
@grgia, friendly bump for the review. :) |
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.
Code changes LGTM
@shawnborton @dannymcclain do you wanna do a final review? |
Nice, gonna run a test build! |
🚧 @shawnborton has triggered a test 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! 🧪🧪
|
Feeling pretty good to me, let's get this into final review? |
@nkdengineer, please resolve the conflicts again when you have time. 😂 |
@grgia, I think once the translation is confirmed, it's fine to merge it. :) |
✋ 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/grgia in version: 9.0.99-0 🚀
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.0.99-2 🚀
|
Explanation of Change
Fixed Issues
$ #52921
PROPOSAL: #52921 (comment)
Tests
Offline tests
Same as tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
web.mov
MacOS: Desktop
desktop.mov