-
Notifications
You must be signed in to change notification settings - Fork 9
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
Display type in tokens table #685
Conversation
Deployed to Cloudflare Pages
|
ec39378
to
b5842b4
Compare
@donouwens what do you think? |
b5842b4
to
61597e7
Compare
We could also consider using some kind of icons instead of written text, if there were good icons for this. (Are there?) |
I'm very happy with how it looks already @csillag I agree that icons would be nice, but I've struggled to come up with anything that's clearly identifiable at a glance (erc20 vs erc721), so I'd opt for having tags like this (not set on the colors yet but obviously each type would have its own color). Would that be okay for you? |
@donouwens To be honest, I am not sure about our nomenclature here. Looking at Etherscan, they simply call the ERC-20 stuff "Tokens" and the ERC-721 and similar stuff "NFTs". (But then for the second category, they do show "type" in the table. Maybe what they are doing is not a bad idea? But then this brings up the question, do we even want to have these two categories (tokens and NFTs) in the same table? If yes, let's just say "Tokens" and "NFTs" as the type? Or have two different tables? Does this also mean new routes and new pages? So many possibilities. In any case, I'm ok with colored tags, but I also would also consider adding "NFT" to the tags like this: |
My suggestion would be: same table for both, no type column, name = "Wrapped Rose" | "AI ROSE (NFT)". But then vertical view becomes "AI ROSE (AI ROSE) (NFT)" |
@csillag True, but that leaves us in a pickle further down the line when we add ERC-1155 to the list as these can be NFTs or fungible tokens. I think your final suggestion could work in which we combine the type and "NFT" or "Token", just not only "NFT" or "Token". I've updated the visual now. We should keep everything combined in one table as we will add filtering options in the next iteration after public launch which should address that point. @lukaw3d - but again if we add ERC-1155 to that mix we'd need to show more than just NFT or Token, right? Still working through the colors btw.. |
f923562
to
0d579e3
Compare
Tweaked the look according to the visuals Updated screenshots above. |
|
0d579e3
to
a64cc77
Compare
Updated styling. I replaced the first 2 screenshot, but not the others. |
5cfba24
to
624764c
Compare
624764c
to
3985b5b
Compare
@@ -15,12 +25,40 @@ type TokensProps = { | |||
pagination: false | TablePaginationProps | |||
} | |||
|
|||
export const TokenTypeTag: FC<{ tokenType: EvmTokenType; sx?: SxProps }> = ({ tokenType, sx = {} }) => { |
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.
This is similar to TokenPills
that wraps around mui's Chip
.
We should choose more consistent names for "the background rounded label thing"
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.
How about DogTag
?
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.
(I mean for a common ancestor, with common sizing and fond settings, and then create or descendants from that, with consistent names, either Tag or Pill)
Now that we have two types of tokens, ERC-20 and ERC-721, it helps to show which is which in the tables.