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

Only show fancy loading page when we are submitting verifications #5787

Merged
merged 5 commits into from
Oct 13, 2021
Merged

Only show fancy loading page when we are submitting verifications #5787

merged 5 commits into from
Oct 13, 2021

Conversation

marcaaron
Copy link
Contributor

@marcaaron marcaaron commented Oct 12, 2021 •
edited
Loading

Details

Fixed Issues

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

Tests / QA Steps

  1. Run through the VBA flow (use PENDING instructions here)
  2. Verify that when we have no bank account yet the loading spinner that displays is not fancy (just regular old spinner)
    2021-10-12_15-09-29
  3. Advance to the company step and submit
  4. Verify that the loading animation on the next page is fancy
    2021-10-12_15-10-41
  5. Continue advancing through the steps and verify that each step has a fancy indicator page except the validation step (this should have a regular spinner).

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@marcaaron marcaaron self-assigned this Oct 12, 2021
@marcaaron marcaaron marked this pull request as ready for review October 13, 2021 01:11
@marcaaron marcaaron requested a review from a team as a code owner October 13, 2021 01:11
@MelvinBot MelvinBot requested review from jasperhuangg and removed request for a team October 13, 2021 01:11
@marcaaron marcaaron requested a review from a team October 13, 2021 17:47
@MelvinBot MelvinBot requested review from MariaHCD and removed request for a team October 13, 2021 17:48
@marcaaron marcaaron removed the request for review from jasperhuangg October 13, 2021 17:49
@github-actions
Copy link
Contributor

⚠️ ⚠️ Heads up! This pull request has the CP Staging label. ⚠️ ⚠️
Merging it will cause it to be immediately deployed to staging, even if the open StagingDeployCash deploy checklist is locked.

@Jag96 Jag96 self-requested a review October 13, 2021 17:58
Jag96
Jag96 reviewed Oct 13, 2021
View reviewed changes
Copy link
Contributor

@Jag96 Jag96 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, just a couple of missing displayNames. Running though tests now

src/components/FullscreenLoadingIndicator.js Show resolved Hide resolved
src/components/ReimbursementAccountLoadingIndicator.js Show resolved Hide resolved
Jag96
Jag96 previously approved these changes Oct 13, 2021
View reviewed changes
Copy link
Contributor

@Jag96 Jag96 left a comment

Choose a reason for hiding this comment

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

Tests pass fine, once comments are address this is 👍

@marcaaron marcaaron merged commit 397510d into main Oct 13, 2021
@marcaaron marcaaron deleted the marcaaron-fixLoading branch October 13, 2021 19:48
github-actions bot pushed a commit that referenced this pull request Oct 13, 2021
@marcaaron
Merge pull request #5787 from Expensify/marcaaron-fixLoading
89e9d2e 
Only show fancy loading page when we are submitting verifications

(cherry picked from commit 397510d)
This was referenced Oct 13, 2021
Merged
Closed
@OSBotify
Copy link
Contributor

🚀 Cherry-picked to staging by @marcaaron in version: 1.1.7-11 🚀

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

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

@kevinksullivan
Copy link
Contributor

kevinksullivan commented Oct 14, 2021 •
edited
Loading

Tested this today on web and it worked like a charm, no interstitial when jumping into the flow, entering routing/account number, or entering validate step info 👍

@OSBotify OSBotify mentioned this pull request Oct 15, 2021
Closed
@OSBotify
Copy link
Contributor

🚀 Deployed to production by @AndrewGable in version: 1.1.7-24 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @marcaaron in version: 1.1.7-25 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.8-9 🚀

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

@OSBotify OSBotify mentioned this pull request Aug 16, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jag96 Jag96 Jag96 approved these changes

@MariaHCD MariaHCD Awaiting requested review from MariaHCD

Assignees

@marcaaron marcaaron

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@marcaaron @OSBotify @kevinksullivan @Jag96