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: ecocredit table with ghost credits on first page #2176

Merged
merged 2 commits into from
Oct 17, 2023

Conversation

flagrede
Copy link
Collaborator

@flagrede flagrede commented Oct 10, 2023

Description

Closes: #2097

  • display ecocredits table when the first page is full of ghost credits.

The problem here was, for this specific account on main net, about ghost credits (empty balances on credits that used to have one returned by the ledger). We took the decision to hide those ghost credits before so we ended up with a first page without credit and the empty state was displayed for that reason.

In this PR we display the table if there are no rows on the first page but the table has more pages.

empty table - page 1
empty table - page 2


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • provided a link to the relevant issue or specification
  • provided instructions on how to test
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

How to test

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items
.

I have...

  • confirmed all author checklist items have been addressed
  • reviewed code correctness and readability
  • verified React components follow DRY principles
  • reviewed documentation is accurate
  • reviewed tests
  • manually tested (if applicable)

@netlify
Copy link

netlify bot commented Oct 10, 2023

Deploy Preview for regen-website ready!

Name Link
🔨 Latest commit ce881bb
🔍 Latest deploy log https://app.netlify.com/sites/regen-website/deploys/652e4346c6a2b50007e8a674
😎 Deploy Preview https://deploy-preview-2176--regen-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@blushi
Copy link
Member

blushi commented Oct 10, 2023

In this PR we display the table if there are no rows on the first page but the table has more pages.

Would it be tricky to just display the page(s) with some data in this case (dropping the first empty one)?

@flagrede
Copy link
Collaborator Author

In this PR we display the table if there are no rows on the first page but the table has more pages.

Would it be tricky to just display the page(s) with some data in this case (dropping the first empty one)?

We have no way of knowing on which page we will have data beforehand, unfortunately (there could be several empty pages).

@blushi
Copy link
Member

blushi commented Oct 10, 2023

In this PR we display the table if there are no rows on the first page but the table has more pages.

Would it be tricky to just display the page(s) with some data in this case (dropping the first empty one)?

We have no way of knowing on which page we will have data beforehand, unfortunately (there could be several empty pages).

I see, I guess we just have to live with it then until this is fixed on regen ledger side eventually: regen-network/regen-ledger#1889 cc/ @erikalogie @ryanchristo

@blushi
Copy link
Member

blushi commented Oct 10, 2023

is there an address that we can use to test this on redwood?

@flagrede
Copy link
Collaborator Author

@blushi not to my knowledge, unfortunately.

@blushi
Copy link
Member

blushi commented Oct 16, 2023

@flagrede can we update this to display 10 items per page by default as discussed last week?

@flagrede flagrede force-pushed the fix-2097-profolio-balance branch from 50bee00 to ce881bb Compare October 17, 2023 08:18
@flagrede flagrede merged commit ae97495 into dev Oct 17, 2023
11 checks passed
@flagrede flagrede deleted the fix-2097-profolio-balance branch October 17, 2023 08:53
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.

Portfolio doesn't show KSH01 ecocredit balance on mainnet / production
2 participants