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.

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

Add IOU Menu Options #2612

Merged
merged 4 commits into from
Apr 29, 2021
Merged

Add IOU Menu Options #2612

merged 4 commits into from
Apr 29, 2021

Conversation

marcaaron
Copy link
Contributor

@marcaaron marcaaron commented Apr 28, 2021

Details

cc @Julesssss @iwiznia adds the menu options behind beta

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/161984

Tests

QA Steps

  1. Login and tap the FAB menu +
  2. Verify that the two new options appear

Screen Shot 2021-04-28 at 7 43 35 AM

  1. Verify that clicking each one will bring you to the associated view
  2. Select a chat with a single participant
  3. Tap the + menu next to the comment compose area
  4. Verify there is an option to Request Money
  5. Tap and verify the request money screen appears and the participant is already selected
  6. Select a chat with multiple participants
  7. Tap the + menu next to the comment compose area
  8. Verify there is an option to Split Bill
  9. Tap and verify the split bill screen appears and all participants are already selected on the next screen
  10. Log in with an account that is not under this beta and verify that the menus look as they did before and the new options are not present.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Screen Shot 2021-04-28 at 7 43 35 AM

Mobile Web

2021-04-28_10-40-24

Desktop

2021-04-28_11-20-27

iOS

2021-04-28_11-07-55

Android

2021-04-28_11-14-27

@marcaaron marcaaron requested a review from a team as a code owner April 28, 2021 17:46
@marcaaron marcaaron self-assigned this Apr 28, 2021
@MelvinBot MelvinBot requested review from francoisl and removed request for a team April 28, 2021 17:47
@marcaaron
Copy link
Contributor Author

Will add screenshots and test on all platforms in a bit but wanted to get this into review sooner than later.

@marcaaron marcaaron requested a review from Julesssss April 28, 2021 17:47
@francoisl
Copy link
Contributor

Code looks good but when I create a split bill from a convo with multiple people and click the Split Bill option, the panel doesn't populate with anything other than the currency, and the Next button is disabled. Nothing in the JS console or logs, any idea?

@marcaaron
Copy link
Contributor Author

Code looks good but when I create a split bill from a convo with multiple people and click the Split Bill option, the panel doesn't populate with anything other than the currency, and the Next button is disabled. Nothing in the JS console or logs, any idea?

This PR shouldn't really have any effects on the way things are working there. But I think if you click to the right of the USD then you can enter a number and should be able to "Next" after that. I found this a little confusing myself as the view doesn't really look like an input of any kind so it's unclear what we can do on desktop.

@francoisl
Copy link
Contributor

Oh derp, got it... I was expecting the view would directly show the bill participants.

francoisl
francoisl previously approved these changes Apr 28, 2021
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.

The issue tests well.

One request, would you mind removing the other 'Hiding menu options' comment here, as this PR makes it irrelevant.

Also, could you add a test case for confirming that the IOU beta flag is working as expected in the description? (I was able to test this locally by removing the all beta flag, but for QA we should probably use accounts with/without the flag to verify)

@marcaaron
Copy link
Contributor Author

Thanks for spotting that @Julesssss! I have made that requested change and added a test step to verify the new options do not appear for non beta users.

@trjExpensify
Copy link
Contributor

Are we good to go here without @Julesssss re-review so we can get testing, as I think he's out for the evening now?

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.

Yeah, this change will not invalidate the tests I ran earlier. Merging

@Julesssss
Copy link
Contributor

I can't find the merge button on the GitHub app, but we're good to go. Feel free to merge

@francoisl francoisl merged commit d68061b into main Apr 29, 2021
@francoisl francoisl deleted the marcaaron-addIOUMenuOptions branch April 29, 2021 18:45
@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.34-1🚀

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants