Feature: [P2P Distance] Enable P2P/splits in App#37185
Feature: [P2P Distance] Enable P2P/splits in App#37185neil-marcellini merged 17 commits intoExpensify:mainfrom
Conversation
|
Pending confirmation in #36984 (comment) before marking PR ready. |
|
@alitoshmatov 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] |
neil-marcellini
left a comment
There was a problem hiding this comment.
Looking pretty good, suggested some tweaks.
|
I've resolved all the feedbacks! |
|
One thing to confirm: For group chat, do we allow distance split? @neil-marcellini |
Yes, let's make that work too. |
neil-marcellini
left a comment
There was a problem hiding this comment.
Looking really good now, thanks for the updates! There's one thing I would like to see changed, and it sounds like we also need to enable this for group chats.
After that we can probably get this merged. The backend is almost done, but it will be a little while longer to finish up. Since we have the beta, I think it's ok if this goes out before the backend is done.
src/components/MoneyTemporaryForRefactorRequestConfirmationList.js
Outdated
Show resolved
Hide resolved
@neil-marcellini @koko57 I've enabled distance request for both group chats and DMs. I also think we should reflect this in design doc. |
Sounds good. Please go ahead and make a suggestion there for how you would like to see it updated. I'll then review/accept it. |
neil-marcellini
left a comment
There was a problem hiding this comment.
Good to go, pending C+ testing 🙂
|
Reviewing |
Reviewer Checklist
Screenshots/VideosAndroid: Nativep2p-android.movAndroid: mWeb Chromep2p-mweb.moviOS: Nativehttps://github.com/Expensify/App/assets/59907218/3c5a5f45-d308-4518-9759-7f9d5b8a66dciOS: mWeb Safarip2p-safari.mp4MacOS: Chrome / Safarip2p-web.movMacOS: Desktopcompressed-p2p-desktop.mov |
alitoshmatov
left a comment
There was a problem hiding this comment.
Changes looks good to me
neil-marcellini
left a comment
There was a problem hiding this comment.
Mostly looks good, but I don't understand why we are showing "Route pending..." for the distance field on P2P distance requests. It should display with the proper rate and distance text just like for a workspace.
Oh I see, it's because we need getRateForDisplay from this issue [$500] [P2P Distance] Create a new Rate field.
I'm fine if it doesn't fully work at this point, because it's under beta and we want to keep the PRs small 👍. I also can't think of any other reasonable way to do it, so looks good.
❗❗❗ Note to QA/bug reporters ❗❗❗Please refrain from creating deploy blockers related to this PR/issue. The feature is under a beta that is only available to expensify accounts while we build the feature. It's expected to be incomplete right now. |
|
✋ 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/neil-marcellini in version: 1.4.48-0 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 1.4.48-0 🚀
|
Details
Enable P2P/Splits in App following this plan.
Fixed Issues
$ #36984
PROPOSAL: #36984 (comment)
Tests
+) > Request moneyRoute pending...for P2P request, or73.45 miles @ $0.66 / milefor workspace)+) >> Request money+) >> Split bill+) >> Request moneyOffline tests
NA
QA Steps
+) > Request moneyRoute pending...for P2P request, or73.45 miles @ $0.66 / milefor workspace)+) >> Request money+) >> Split bill+) >> Request moneyPR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.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 so 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.2024-02-26.at.15.26.25-compressed.mov
Android: mWeb Chrome
Screen.Recording.2024-02-26.at.15.31.02-compressed.mov
iOS: Native
1.mov
2.mov
iOS: mWeb Safari
Screen.Recording.2024-02-26.at.15.21.55-compressed.mov
MacOS: Chrome / Safari
Screen.Recording.2024-03-04.at.15.45.43-compressed.mov
Screen.Recording.2024-03-04.at.15.44.46-compressed.mov
Screen.Recording.2024-03-04.at.15.47.11-compressed.mov
MacOS: Desktop
Screen.Recording.2024-03-04.at.15.45.43-compressed.mov
Screen.Recording.2024-03-04.at.15.44.46-compressed.mov
Screen.Recording.2024-03-04.at.15.47.11-compressed.mov