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

Improve token transfer listing for tokens #738

Merged
merged 8 commits into from
Jul 16, 2023

Conversation

csillag
Copy link
Contributor

@csillag csillag commented Jul 14, 2023

This depends on #743.

Improve pagination experience by proving rudimentary client-side pagination (limitation: looking only at the first 1000 records)

This impacts two pages:

  • Tokens's token transfer listing (for example /mainnet/sapphire/token/0x998633BDF6eE32A9CcA6c9A247F428596e8e65d8 )
  • Account's token transfer listing (for example /mainnet/sapphire/address/0x998633BDF6eE32A9CcA6c9A247F428596e8e65d8/token-transfers#transfers

I think there is some problem with the pagination widget sometimes displaying the wrong number of total pages, but that is totally unrelated. Update: workaround added for anomalous behavior.

Technical note: great emphasis has been placed in developing this in separate commits so that it's easier to follow what is going on, so I suggest reviewing commit by commit.

@github-actions
Copy link

github-actions bot commented Jul 14, 2023

Deployed to Cloudflare Pages

Latest commit: 7de4f79dec7afd5b7336e8d07634713c3132c02c
Status:✅ Deploy successful!
Preview URL: https://1cd8c4a3.oasis-explorer.pages.dev

@csillag csillag force-pushed the csillag/improve-token-transfer-listings branch from 4ff0aad to 284c539 Compare July 14, 2023 14:08
@csillag csillag marked this pull request as ready for review July 15, 2023 16:01
@csillag csillag requested review from lukaw3d and buberdds July 15, 2023 16:07
@csillag csillag force-pushed the csillag/improve-token-transfer-listings branch from 7cfedae to 7507ce1 Compare July 15, 2023 16:25
@csillag
Copy link
Contributor Author

csillag commented Jul 15, 2023

First commit should be a separate PR.

OK, see #743

@csillag csillag force-pushed the csillag/improve-token-transfer-listings branch from ad7c964 to 3e7f427 Compare July 15, 2023 23:48
@csillag
Copy link
Contributor Author

csillag commented Jul 15, 2023

can we combine useTokenTransfers and useAccountTokenTransfers now?

Done.

Abstract away more things and centralize them in the pagination engine,
so that the pagination engine is in a position where it could do
some more sophisticated stuff, like decoupling client-side and
server-side behavior.
... of ComprehensiveSearchParamsPagination

No actual behavior is changed yet, just copied and renamed.
@csillag csillag force-pushed the csillag/improve-token-transfer-listings branch 2 times, most recently from 293902e to 262a7e2 Compare July 16, 2023 00:14
Copy link
Member

@lukaw3d lukaw3d left a comment

Choose a reason for hiding this comment

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

Looks easily reversible when we stop needing it

switch (candidates.length) {
case 0:
throw new Error('Data not found!')
case 1:
Copy link
Member

Choose a reason for hiding this comment

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

With strict types this could be ensured at compile time, but not worth inventing it for this temporary workaround until backend adds filtering by signature

src/app/components/Table/useClientSidePagination.ts Outdated Show resolved Hide resolved
@csillag csillag force-pushed the csillag/improve-token-transfer-listings branch from 262a7e2 to 7de4f79 Compare July 16, 2023 00:20
@csillag csillag enabled auto-merge July 16, 2023 00:21
@csillag
Copy link
Contributor Author

csillag commented Jul 16, 2023

Looks easily reversible when we stop needing it

Yes, there is useComprehensiveSearchParamsPagination(), which implements the exact same interface as the client-side pagination engine, so it's a drop-in replacement.

@csillag csillag merged commit 1573138 into master Jul 16, 2023
7 checks passed
@csillag csillag deleted the csillag/improve-token-transfer-listings branch July 16, 2023 00:23
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.

None yet

2 participants