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

fix: Infinite loading on admin approved list #2674

Merged

Conversation

victorgcramos
Copy link
Member

Closes #2672.

  • Fixes the admin billing changes infinite loading after login with a
    previously loaded approved proposals list.

  • Adds loading indicator for billing status changes

Ui Changes

2672-loading-billing

Copy link
Member

@lukebp lukebp left a comment

Choose a reason for hiding this comment

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

  1. Don't add a separate loading animation for the button. Having multiple loading animations all over the place is distracting and bad UX. The main loading animation should be displayed until all of the data has been retrieved to display the list.

  2. This is caught in an infinte loop on refresh, so something is off anyway.

Fixes the admin billing changes infinite loading after login with a previously loaded approved proposals list.

I wasn't able to reproduce this on master. What are the reproduction steps?

tmp-2021-11-24_10.44.35.mp4

@lukebp
Copy link
Member

lukebp commented Nov 24, 2021

The infinite loop is actually a loop that fetches all existing approved proposals. I wasn't able to reproduce it initially becuase I only had a single page of approved proposals in my test env.

@lukebp
Copy link
Member

lukebp commented Nov 24, 2021

Reproduction steps for button infinite loading on firefox:

  1. Load Approved tab without being logged in
  2. Log in as an admin
  3. Navigate to the Approved tab
  4. Refresh the page while on the Approved tab

Copy link
Member

@lukebp lukebp left a comment

Choose a reason for hiding this comment

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

@victorgcramos I'm not seeing the infinite loading anymore, but the billing status buttons still disappear on refresh. The infinite loading was caused by the billing statuses not being fetched. Looks like this is the same root cause.

tmp-2021-11-29_13.59.58.mp4

@victorgcramos
Copy link
Member Author

@lukebp this is how it looks like now. Thanks for the review and the helpful thoughts.

2672-5-approved

Copy link
Member

@lukebp lukebp left a comment

Choose a reason for hiding this comment

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

tACK

Needs code approval from @tiagoalvesdulce.

Copy link
Member

@tiagoalvesdulce tiagoalvesdulce left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +91 to +94
const needsBillingStatus =
!isLegacy && !isRfp && isApproved && currentUser?.isadmin;
const isSetBillingStatusAllowed =
!isLegacy &&
!isRfp &&
isApproved &&
numbillingstatuschanges < billingstatuschangesmax;
needsBillingStatus && numbillingstatuschanges < billingstatuschangesmax;
Copy link
Member

Choose a reason for hiding this comment

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

👍

@tiagoalvesdulce tiagoalvesdulce merged commit 1ccf8f7 into decred:master Dec 1, 2021
@lukebp lukebp added 91cfcc8 bug A bug that made it into a production environment. labels Dec 1, 2021
@tiagoalvesdulce tiagoalvesdulce added the pi-not-deployed This item has been merged into master, but has not been deployed to production yet. label Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug that made it into a production environment. pi-not-deployed This item has been merged into master, but has not been deployed to production yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Infinite loading on test-proposals.
3 participants