Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Decline/cancel IOU feedback. #3498

Merged
merged 25 commits into from
Jun 17, 2021
Merged

Decline/cancel IOU feedback. #3498

merged 25 commits into from
Jun 17, 2021

Conversation

rushatgabhane
Copy link
Member

@rushatgabhane rushatgabhane commented Jun 9, 2021 •
edited
Loading

Details

Provide feedback when declining/cancelling an IOU.
This is achieved by tracking the transaction ID that is being rejected (in Onyx).
Text vs ActivityIndicator is conditionally rendered based on if the transactionID is present in rejectTransaction array, which is maintained by Onyx.

State of rejectInProgress just after cancel of an IOU.
Screenshot from 2021-06-10 00-32-18

State of rejectInProgress after cancel of the IOU is done.
Screenshot from 2021-06-10 00-32-21

Cancelling multiple money requests.
Screenshot_20210610-020516_2

Fixed Issues

Fixes #3346

Tests

  1. Request money to user A.
  2. Click "View details" of the transaction.
  3. Cancel the request for money.
    You should be able to see a spinner. After rejection the component unmounts.

QA Steps

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Initial state.
Screenshot from 2021-06-10 00-42-01

Feedback for cancelling request.
Screenshot from 2021-06-10 00-42-03

Mobile Web

Initial state
Screenshot_20210610-005508

Feedback for cancelling request.
Screenshot_20210610-005511

Desktop

PXL_20210610_145650506 MP

iOS

121532193-6992bd80-c9ff-11eb-8fb9-0f58ae9cd0a8

Android

Initial state
Screenshot_20210610-015529

Feedback for canceling money request.
Screenshot_20210610-015522

@rushatgabhane rushatgabhane requested a review from a team as a code owner June 9, 2021 22:13
@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2021 •
edited
Loading

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@MelvinBot MelvinBot requested review from Beamanator and removed request for a team June 9, 2021 22:13
@rushatgabhane

This comment has been minimized.

Sign in to view
@rushatgabhane

This comment has been minimized.

Sign in to view
@rushatgabhane rushatgabhane mentioned this pull request Jun 9, 2021
Closed
5 tasks
@rushatgabhane
Copy link
Member Author

rushatgabhane commented Jun 10, 2021 •
edited
Loading

ActivityIndicator is now centered.

Screenshot_20210610-135623_2

Beamanator
Beamanator requested changes Jun 10, 2021
View reviewed changes
Copy link
Contributor

@Beamanator Beamanator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments I'd like to resolve, overall looks pretty solid! Nice job 👍

src/ONYXKEYS.js Outdated Show resolved Hide resolved
src/components/ReportTransaction.js Outdated Show resolved Hide resolved
src/components/ReportTransaction.js Outdated Show resolved Hide resolved
src/components/ReportTransaction.js Outdated Show resolved Hide resolved
@Beamanator
Copy link
Contributor

Also here's a screenshot I got from testing with iOS:

Screen Shot 2021-06-10 at 2 57 59 PM

Julesssss
Julesssss requested changes Jun 10, 2021
View reviewed changes
Copy link
Contributor

@Julesssss Julesssss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, requested a few changes

Beamanator
Beamanator requested changes Jun 11, 2021
View reviewed changes
Copy link
Contributor

@Beamanator Beamanator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great! Only a few more small comments 👍

src/components/ReportTransaction.js Outdated Show resolved Hide resolved
src/components/ReportTransaction.js Outdated Show resolved Hide resolved
src/components/ReportTransaction.js Outdated Show resolved Hide resolved
@rushatgabhane
Copy link
Member Author

rushatgabhane commented Jun 11, 2021 •
edited
Loading

Sorry, accidentally pushed without git signature.
Reverted and cleaned commit history.

@Julesssss
Copy link
Contributor

Hi @rushatgabhane, looks like there's a small conflict

Julesssss
Julesssss reviewed Jun 16, 2021
View reviewed changes
src/styles/styles.js Outdated Show resolved Hide resolved
Julesssss
Julesssss requested changes Jun 16, 2021
View reviewed changes
Copy link
Contributor

@Julesssss Julesssss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting some style and documentation changes

@rushatgabhane rushatgabhane requested a review from Julesssss June 16, 2021 12:15
Julesssss
Julesssss reviewed Jun 16, 2021
View reviewed changes
src/libs/actions/IOU.js Outdated Show resolved Hide resolved
@Julesssss
Copy link
Contributor

Tests well on all platforms, there's just the final comment to resolve

@rushatgabhane
Copy link
Member Author

Tests well on all platforms, there's just the final comment to resolve

Awesome! Thanks for your time, I'll make sure to be more proactive for future PRs.

@rushatgabhane rushatgabhane requested a review from Julesssss June 16, 2021 16:59
Julesssss
Julesssss previously approved these changes Jun 16, 2021
View reviewed changes
@Julesssss
Copy link
Contributor

Back to you @Beamanator

@Beamanator Beamanator self-requested a review June 17, 2021 10:50
Beamanator
Beamanator reviewed Jun 17, 2021
View reviewed changes
src/ONYXKEYS.js Outdated Show resolved Hide resolved
Beamanator
Beamanator approved these changes Jun 17, 2021
View reviewed changes
Copy link
Contributor

@Beamanator Beamanator left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests passed, looks great here 👍 Thanks @rushatgabhane ! We can merge once the last check passes

@Julesssss Julesssss merged commit fedf555 into Expensify:main Jun 17, 2021
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.70-1🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify OSBotify mentioned this pull request Jun 17, 2021
Closed
@rushatgabhane rushatgabhane deleted the decline-IOU-feedback branch June 22, 2021 19:41
@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.73-3🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Beamanator Beamanator Beamanator approved these changes

@Julesssss Julesssss Julesssss left review comments

Assignees

@rushatgabhane rushatgabhane

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Give users visual feedback when declining an IOU
4 participants
@rushatgabhane @Beamanator @Julesssss @OSBotify