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

Update Terms and Fees step to match latest requirements #6573

Merged
merged 11 commits into from
Dec 8, 2021

Conversation

Jag96
Copy link
Contributor

@Jag96 Jag96 commented Dec 3, 2021

cc @shawnborton for design 👀

Details

This PR updates the styles for the Terms and Fees step of the VBA flow based on the mockups in https://github.com/Expensify/Expensify/issues/167278#issuecomment-982972716.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/167278

Tests

The easiest way to get to the Terms Page without going through the entire OnFido flow is to update the following lines locally:

  1. Undo this change which makes the default step the terms page
  2. Undo this change which will let you navigate to the terms page by tapping on the + button and selecting New Chat
  3. Confirm the design matches the updated layout in https://github.com/Expensify/Expensify/issues/167278#issuecomment-982972716

QA Steps

No QA

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Mobile Web
image image
image image

@Jag96 Jag96 self-assigned this Dec 3, 2021
@Jag96 Jag96 marked this pull request as ready for review December 3, 2021 19:26
@Jag96 Jag96 requested a review from a team as a code owner December 3, 2021 19:26
@MelvinBot MelvinBot requested review from MonilBhavsar and removed request for a team December 3, 2021 19:26
Copy link
Contributor

@MonilBhavsar MonilBhavsar 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.
Not affected by this PR, but I think we should update the error message

Screenshot 2021-12-06 at 5 36 10 PM

@shawnborton
Copy link
Contributor

Looking pretty good! I forget where we landed with this general UX discussion, but should we actually move the green button to be at the very bottom of the disclosure text instead of being fixed to the bottom of the viewport? I think that makes sense given that you basically need to scroll all the way to the bottom and tap on the checkboxes before moving forward. Thoughts on that?

@Jag96
Copy link
Contributor Author

Jag96 commented Dec 6, 2021

Not affected by this PR, but I think we should update the error message

Good find, I'll have a look

I forget where we landed with this general UX discussion, but should we actually move the green button to be at the very bottom of the disclosure text instead of being fixed to the bottom of the viewport?

Yes, I'll update!

@Jag96
Copy link
Contributor Author

Jag96 commented Dec 6, 2021

Fixed the error message and button and added some spacing under the button as well, screenshots have been updated!

Copy link
Contributor

@MonilBhavsar MonilBhavsar left a comment

Choose a reason for hiding this comment

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

Thanks!

@MonilBhavsar MonilBhavsar merged commit f0a1cc4 into main Dec 8, 2021
@MonilBhavsar MonilBhavsar deleted the joe-update-terms-page branch December 8, 2021 05:21
@OSBotify
Copy link
Contributor

OSBotify commented Dec 8, 2021

✋ 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

OSBotify commented Dec 8, 2021

🚀 Deployed to staging by @MonilBhavsar in version: 1.1.18-4 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @Julesssss in version: 1.1.21-1 🚀

platform result
🤖 android 🤖 failure ❌
🖥 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.

4 participants