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

Don't show loading state in workspace setup button #4401

Merged
merged 1 commit into from
Aug 4, 2021

Conversation

stitesExpensify
Copy link
Contributor

Details

We were getting double spinners when setting up workspaces. We only want the sidebar loading icon, not the button.
image

Fixed Issues

$ #4384

Tests

  1. Open a workspace
  2. Set up a bank account
  3. Between steps make sure the button does not have a loading state

QA Steps

n/a

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

2021-08-03_17-02-07

@stitesExpensify stitesExpensify requested a review from a team as a code owner August 3, 2021 23:02
@stitesExpensify stitesExpensify self-assigned this Aug 3, 2021
@MelvinBot MelvinBot requested review from alex-mechler and removed request for a team August 3, 2021 23:02
@@ -179,7 +179,6 @@ const WorkspaceCardPage = ({
success
large
text={buttonText}
isLoading={reimbursementAccount.loading}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any case where this can be true, but the modal is not open?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, not that I know of / could figure out

@alex-mechler alex-mechler merged commit c2d21c0 into main Aug 4, 2021
@alex-mechler alex-mechler deleted the stites-dontShowLoadingOnButton branch August 4, 2021 22:37
@OSBotify
Copy link
Contributor

OSBotify commented Aug 4, 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 Aug 6, 2021

🚀 Deployed to staging in version: 1.0.82-8🚀

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

@isagoico
Copy link

isagoico commented Aug 6, 2021

@stitesExpensify Hello! Our testing domains already have a Expensify card set and we're not able to access this flow. Can you provide a testing account for this or test this internally?
image

@stitesExpensify
Copy link
Contributor Author

This is no QA and can be checked off. I mentioned it in the PR body but forgot to put it in the title, my bad

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @francoisl in version: 1.0.83-1 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 failure ❌
🍎 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