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(NFT): add nft_id to search bar #317
Conversation
src/containers/Header/Search.jsx
Outdated
await getTransaction(rippledContext, id) | ||
return 'transactions' | ||
} catch (e) { | ||
return 'token' |
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 key for NFTs should really be something like nft
or nftoken
, instead of using token
. It's a different page type.
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.
Agreed
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.
do you mean we should change the url to something like /nft/:id
?
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.
yeah i agree
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.
also i don't like the name of this function with relation to its return type. this name should return a boolean and be named something like isTransaction
. Given what this does, it should be named something like determineTypeOfHash
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.
@ledhed2222 soon we will have AMM Instances added to the list of 64 character hexs that we have to determine the type of.
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.
see comments
High Level Overview of Change
nft_info
to check if its NFT (sincetransactions
andnft_id
share the same 64-digit hex syntax)async
withinSearch
component to support for async API calltoken/:id
tonft/:id
Context of Change
Type of Change
TypeScript/Hooks Update
Before / After
After clicking Enter
Test Plan