-
Notifications
You must be signed in to change notification settings - Fork 333
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
feat: Rework hiddenCollections to handle more easily displays #8457
base: develop
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git βοΈ
3 Skipped Deployments
|
8d3dadd
to
3cee042
Compare
collectionId: string, | ||
status: NftStatus, | ||
) => ({ | ||
type: "UPDATE_NFT_COLLECTION_STATUS", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion : you can extract it to actions/constants.ts
as
an event based action type UPDATE_NFT_COLLECTION_STATUS = settings/updateNftCollectionStatus
@themooneer For the moment no, Maybe we can see to improve that soon :) It's been a long time that this UI has been done, so maybe a refresh could be good ! |
3cee042
to
53ad8b3
Compare
const nftCollectionsStatusByNetwork = useSelector(nftCollectionsStatusByNetworkSelector); | ||
|
||
const shouldDisplaySpams = !nftsFromSimplehashFeature?.enabled; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both list
and whitelisted
have the same logic process except for the .filter(...)
business logic. In this case and to avoid code duplication i suggest a generic function with dependency inversion in the status filtering part.
The generic function can be easily extracted from the hook as a helper, but it's ok if you want to keep it her
// this is the generic function (that parses the nftCollection)
const neftCollectionParser = (nftCollection, applyFilterFn) => Object.values(nftCollection).flatMap(contracts =>
Object.entries(contracts)
.filter(appltFilterFn)
.map(([contract]) => contract),
);
const list = useMemo(() => {
neftCollectionParser(nftCollectionsStatusByNetwork,([_, status]) =>
!shouldDisplaySpams ? status !== NftStatus.whitelisted : status === NftStatus.blacklisted)
},[nftCollectionsStatusByNetwork, shouldDisplaySpams]);
const whitelisted = useMemo(() => {
neftCollectionParser(nftCollectionsStatusByNetwork,([_, status]) => status === NftStatus.whitelisted)
},[nftCollectionsStatusByNetwork, shouldDisplaySpams]);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the benefit from extracting neftCollectionParser
in a helper function , is that you can use it in other part of the app (for instance : in orderedVisibleNftsSelector
)
53ad8b3
to
f7b1c76
Compare
β Checklist
npx changeset
was attached.π Description
hiddenCollections
is now an object renamednftCollectionsStatusByNetwork
, containing the different contracts according to their network3 types of status :
FF-based display (nftFromSimpleHash) :
Screen.Recording.2024-11-25.at.11.16.10.1.mov
β Context
π§ Checklist for the PR Reviewers