Fix(wallet): paginate portfolio NFTs page #23816

josheleonard · 2024-05-23T22:07:53Z

Resolves brave/brave-browser#38547
Resolves brave/brave-browser#26782
Resolves brave/brave-browser#38549

Submitter Checklist:

I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally:
- npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
- npm run presubmit wiki, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

The associated issue milestone is set to the smallest version that the
changes has landed on
All relevant documentation has been updated, for instance:

Test Plan:

Hide unowned NFTs

Create a wallet with many NFTs
import an NFT that you do not own
view the NFT tab of the portfolio page
open the filters modal
toggle the "hide unowned NFTs" filter
The unowned NFT should appear only if the option is set to not hide unowned NFTs

NFT list pagination

Create a wallet with many NFTs
import an NFT that you do not own
view the NFT tab of the portfolio page
scroll to the bottom of the page
click the navigation arrows or a page number to change page
page number should reset to the first page when searching NFTs or adjusting filters

brave-builds · 2024-05-23T23:20:28Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2024-05-24T15:17:08Z

A Storybook has been deployed to preview UI for the latest push

mkarolin

strings++

brave-builds · 2024-05-29T19:41:42Z

A Storybook has been deployed to preview UI for the latest push

supermassive · 2024-05-30T02:22:13Z

components/brave_wallet_ui/common/async/base-query-cache.ts

+
+      const chainIds = networksRegistry.ids.map(
+        (network) => networksRegistry.entities[network]!.chainId
+      )
+
+      let currentCursor: string | null = null
+      const accountSpamNfts = []
+
+      do {
+        const {
+          tokens,
+          cursor
+        }: {
+          tokens: BraveWallet.BlockchainToken[]
+          cursor: string | null
+        } = await braveWalletService.getSimpleHashSpamNFTs(
+          address,
+          chainIds,
+          coin,
+          currentCursor
+        )
+
+        accountSpamNfts.push(...tokens)
+        currentCursor = cursor
+      } while (currentCursor)
+
+      this.spamNftsForAccountRegistry[accountId.uniqueKey] = accountSpamNfts
+    }


This just fetches everything with cursor. Should we do that in backend instead?

We could if you would like to take a look are porting this to core

@supermassive Do we want to handle this in this PR or in a follow-up?

Some another PR would be ok

Logged: brave/brave-browser#38823

supermassive · 2024-05-30T02:25:27Z

components/brave_wallet_ui/common/slices/endpoints/token_balances.endpoints.ts

+  /** if true, only spam NFT balances will be fetched, if falsey, only user
+   * token balances will be fetched */
+  isSpamRegistry?: boolean


It makes sense to be consistent and comment with slashes //

This was purposely commented using doc-comments to allow the comment to be visible when hovering over the variable within a code editor

Douglashdaniel

++

github-actions · 2024-06-05T15:38:38Z

[puLL-Merge] - brave/brave-core@23816

Description

This PR adds several new features and improvements to the Brave Wallet UI:

Adds pagination for the NFTs list view to improve performance with large collections
Adds a new filter toggle to hide NFTs that are not owned by any of the user's accounts
Fetches spam NFT balances separately from user token balances to improve performance
Updates routing logic for the NFTs list to handle pagination and tab selection

The motivation for these changes is to improve the user experience and performance of the NFTs list view, especially for users with large collections of NFTs.

Changes

Changes

brave_wallet_constants.h: Adds a new localized string for the "Hide not owned NFTs" toggle.
base-query-cache.ts:
- Adds a new spamNftsForAccountRegistry to cache spam NFTs by account.
- Adds a new getSpamNftsForAccountId method to fetch spam NFTs for a given account from the Simple Hash API.
local-storage-keys.ts: Adds new local storage keys for persisting spam token balances and the "Hide not owned NFTs" setting.
use-balances-fetcher.tsx: Adds an isSpamRegistry option to the useBalancesFetcher hook to fetch spam NFT balances separately.
nfts.endpoints.ts: Updates the getSimpleHashSpamNfts endpoint to optionally fetch spam NFTs for specific accounts.
token_balances.endpoints.ts:
- Adds isSpamRegistry option to GetTokenBalancesRegistryArg to fetch spam or user token balances.
- Updates persisting of token balances to handle spam registry separately.
portfolio-filters-modal.tsx: Adds a new toggle for the "Hide not owned NFTs" setting.
nfts.tsx:
- Adds pagination to the NFTs list view.
- Fetches spam NFT balances separately.
- Filters NFTs list by owned/not owned based on setting.
- Updates routing to handle pagination and tab selection.
pagination.tsx: Adds a new Pagination component for handling pagination UI.
routes-utils.ts: Adds a new makePortfolioNftsRoute function to generate URLs for the NFTs list view with pagination and tab selection.

Possible Issues

The separate fetching of spam NFT balances could lead to some UI flickering or inconsistency if the requests are not properly synchronized. The UI should handle the loading states gracefully.
The pagination implementation currently loads all NFTs and then paginates them in the UI. For very large collections, it may be more efficient to paginate the data fetching as well.

brave-builds · 2024-06-05T17:37:42Z

A Storybook has been deployed to preview UI for the latest push

github-actions bot assigned josheleonard May 23, 2024

github-actions bot added CI/storybook-url Deploy storybook and provide a unique URL for each build feature/web3/wallet labels May 23, 2024

josheleonard force-pushed the fix--paginate-portfolio-NFTs-page branch from c354693 to ff57232 Compare May 23, 2024 22:23

github-actions bot added the feature/web3/wallet/core label May 23, 2024

josheleonard force-pushed the fix--paginate-portfolio-NFTs-page branch from ff57232 to 604753c Compare May 24, 2024 14:18

josheleonard marked this pull request as ready for review May 24, 2024 14:23

josheleonard requested review from a team as code owners May 24, 2024 14:23

josheleonard changed the title ~~Fix paginate portfolio nf ts page~~ Fix(wallet): paginate portfolio NFTs page May 24, 2024

mkarolin approved these changes May 24, 2024

View reviewed changes

josheleonard force-pushed the fix--paginate-portfolio-NFTs-page branch from 604753c to 69eeb0b Compare May 29, 2024 18:41

github-actions bot added the puLL-Merge label May 29, 2024

supermassive reviewed May 30, 2024

View reviewed changes

Douglashdaniel approved these changes Jun 3, 2024

View reviewed changes

josheleonard requested a review from supermassive June 4, 2024 14:30

supermassive approved these changes Jun 5, 2024

View reviewed changes

josheleonard mentioned this pull request Jun 5, 2024

[Wallet] Move braveWalletService.getSimpleHashSpamNFTs cursor logic to core brave/brave-browser#38823

Open

josheleonard added 2 commits June 5, 2024 09:19

fix: paginate portfolio NFTs page

ef6b943

feat(wallet): allow hiding unowned NFTs

e529466

josheleonard force-pushed the fix--paginate-portfolio-NFTs-page branch from 69eeb0b to e529466 Compare June 5, 2024 15:37

josheleonard added 3 commits June 5, 2024 10:00

fix(wallet): remove group-by filter from NFTs portfolio view

3ce020f

fix(wallet): reset nfts portfolio page number when saving new filters

d800264

fix(wallet): update found portfolio NFTs counting and loading logic

fa435d8

josheleonard merged commit 407295f into master Jun 5, 2024
19 checks passed

josheleonard deleted the fix--paginate-portfolio-NFTs-page branch June 5, 2024 19:07

github-actions bot added this to the 1.68.x - Nightly milestone Jun 5, 2024

srirambv mentioned this pull request Jun 12, 2024

[Wallet] Paginate portfolio NFT screen brave/brave-browser#38547

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix(wallet): paginate portfolio NFTs page #23816

Fix(wallet): paginate portfolio NFTs page #23816

josheleonard commented May 23, 2024 •

edited

Loading

brave-builds commented May 23, 2024

brave-builds commented May 24, 2024

mkarolin left a comment

brave-builds commented May 29, 2024

supermassive May 30, 2024

josheleonard May 30, 2024

josheleonard May 31, 2024

supermassive Jun 5, 2024

josheleonard Jun 5, 2024

supermassive May 30, 2024

josheleonard May 30, 2024

Douglashdaniel left a comment

github-actions bot commented Jun 5, 2024

Changes

brave-builds commented Jun 5, 2024

Fix(wallet): paginate portfolio NFTs page #23816

Fix(wallet): paginate portfolio NFTs page #23816

Conversation

josheleonard commented May 23, 2024 • edited Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

Hide unowned NFTs

NFT list pagination

brave-builds commented May 23, 2024

brave-builds commented May 24, 2024

mkarolin left a comment

Choose a reason for hiding this comment

brave-builds commented May 29, 2024

supermassive May 30, 2024

Choose a reason for hiding this comment

josheleonard May 30, 2024

Choose a reason for hiding this comment

josheleonard May 31, 2024

Choose a reason for hiding this comment

supermassive Jun 5, 2024

Choose a reason for hiding this comment

josheleonard Jun 5, 2024

Choose a reason for hiding this comment

supermassive May 30, 2024

Choose a reason for hiding this comment

josheleonard May 30, 2024

Choose a reason for hiding this comment

Douglashdaniel left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 5, 2024

Description

Changes

Possible Issues

brave-builds commented Jun 5, 2024

josheleonard commented May 23, 2024 •

edited

Loading