feat: add search functionality to NFTs #1039
Conversation
Allure Report
smokeTests: ✅ test report for 8294dd25
|
apps/browser-extension-wallet/src/views/browser-view/features/nfts/components/NftsLayout.tsx
Outdated
Show resolved
Hide resolved
apps/browser-extension-wallet/src/features/nfts/components/Nfts.tsx
Outdated
Show resolved
Hide resolved
| @@ -219,7 +239,31 @@ export const NftsLayout = withNftsFoldersContext((): React.ReactElement => { | |||
| </div> | |||
| <div className={styles.content} data-testid="nft-list-container"> | |||
| {items.length > 0 ? ( | |||
| <NftList items={items} rows={4} /> | |||
| <> | |||
There was a problem hiding this comment.
The search-related code is repeated twice. Consider creating a component handling the search and use in both places.
This component could render the search box, handle the case of empty results and have a children prop of a function type that accepts nft items for rendering so you could do the folowing:
<NftFilter nfts={nfts} {...otherRequiredData}>
{(filteredNfts) => <NftList items={filteredNfts} rows={4} />}
</NftFilter>
There was a problem hiding this comment.
We are already grouping the duplicated logic in a hook. The right approach here would be to unify the Nft screens into a responsive one but it falls out of scope. I don't think filters should be components.
There was a problem hiding this comment.
The handleNftSearch functions are identical and it is the same for the markup here. It increases the duplication rate which was here before your changes. IMO the correct approach is to at least not increase the duplication.
apps/browser-extension-wallet/src/views/browser-view/features/nfts/selectors.ts
Show resolved
Hide resolved
packages/common/src/ui/components/InlineInfoList/InlineInfoList.tsx
Outdated
Show resolved
Hide resolved
packages/common/src/ui/components/InlineInfoList/InlineInfoList.tsx
Outdated
Show resolved
Hide resolved
|
Checklist
Proposed solution
Added search functionality to NFTs (when user has 10+).
Logging events to PostHog when user is searching.
Breadcrumbs added for NFT directory path.
Testing
Note: Enable the Post Hog environment variable when testing events unless you are using an account that is opted in. Use an account with 10 or more NFTs.
Screenshots