-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[Awaiting checklist completion] [$500] Android-BA-Tapping connect online with plaid shows 2 spin circles loading #32298
Comments
Job added to Upwork: https://www.upwork.com/jobs/~0196db5598b3527ce7 |
Triggered auto assignment to @twisterdotcom ( |
Bug0 Triage Checklist (Main S/O)
|
Triggered auto assignment to Contributor-plus team member for initial proposal review - @akinwale ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Android-BA-Tapping connect online with plaid shows 2 spin circles loading What is the root cause of that problem?The problem is related to the display of components App/src/components/AddPlaidBankAccount.js Lines 188 to 227 in ee7d5dc
And that's why we see 2 loaders What changes do you think we should make in order to solve the problem?As for me instead of using My idea looks something like this
What alternative solutions did you explore? (Optional)We can always disable the loader by changing the values of |
ProposalPlease re-state the problem that we are trying to solve in this issue.Android-BA-Tapping connect online with plaid shows 2 spin circles loading What is the root cause of that problem?The first loader is ours and the second loader is Plaid Link's loader . We can not hide the Plaid link's loader and so that we should hide or custom loader when Plaid link's window displays What changes do you think we should make in order to solve the problem?We can change this line to following What alternative solutions did you explore? (Optional)We can update |
ProposalPlease re-state the problem that we are trying to solve in this issue.Android-BA-Tapping connect online with plaid shows 2 spin circles loading What is the root cause of that problem?The first loading is our custom loader and the second is coming from What changes do you think we should make in order to solve the problem?We need to initialize a new state called To determine the current state of Resultplaid_loader_demo.mp4 |
@akinwale would love your thoughts on the proposals. |
I suggest going with @ZhenjaHorbach's proposal since a refactor will result in clearer, more readable code making it easier to understand return paths. 🎀 👀 🎀 C+ reviewed. |
Triggered auto assignment to @Beamanator, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 @akinwale 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @ZhenjaHorbach 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
PR will be ready today or tomorrow |
@akinwale @Beamanator, refactoring wasn't the solution, the PR has a completely different approach. I believe my proposal is clearer and more trustworthy here. |
For all three components, we use plaidData So we don't have cases when we show PlaidLink but have error -isLoader (Loader) Lines 51 to 80 in 990815f
So my idea was to follow this logic and expected only one result And instead of arguing |
@akinwale @ZhenjaHorbach Based on the comment from the QA here the issue did not get resolved on Desktop. Would you be able to take a look into this one? |
That is apparently it so we are good |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.16-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-01-02. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
@Beamanator and @akinwale - is this correct? If so, I can pay out once #33890 is deployed at $250 instead. |
Hmm actually, @marcaaron removed the deployblocker. I'm going to pay this out as normal. Payment Summary: |
Just waiting on @akinwale to complete the checklist. |
Bump @akinwale |
Not a regression. This is a specific case that was not handled in the code.
Regression test steps
Do we agree 👍 or 👎? |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
**Version Number:**1.4.6-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
Action Performed:
Expected Result:
When user taps "connect online with plaid", single spinner loading in the page must be shown
Actual Result:
When user taps "connect online with plaid", double spinner can be seen in the page loading for few seconds
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6296385_1701369526058.plaid.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @akinwaleThe text was updated successfully, but these errors were encountered: