feat: [info] add new tdp nav #7531
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
Passing run #15401 ↗︎
Details:
Review all test suite changes for PR #7531 ↗︎ |
|||||||||||||||
| <Trans>Explore</Trans> <ChevronRight data-testid="token-details-return-button" size={14} />{' '} | ||
| <Trans>Tokens</Trans> <ChevronRight data-testid="token-details-return-button" size={14} /> |
There was a problem hiding this comment.
should these two be individually clickable? Explore to /explore and Tokens to /explore/tokens/chain?
There was a problem hiding this comment.
hm, design says clicking on Explore should open back up to the chain-specific page. And since /explore defaults to the same tokens tab anyway, I think it makes sense to have it be one link.
There was a problem hiding this comment.
but should be two separate links in PDP nav, i've noted it in Linear
| display: flex; | ||
| align-items: center; | ||
| color: ${({ theme }) => theme.neutral2}; | ||
| transition-duration: ${({ theme }) => theme.transition.duration.fast}; |
There was a problem hiding this comment.
Which transition is this controlling the duration of? If the animation / transition has been removed, this is no longer needed.
There was a problem hiding this comment.
The hover - makes the hoverover feel just a touch less sudden
| <ArrowLeft size={14} /> Tokens | ||
| </BreadcrumbNavLink> | ||
| {isInfoTDPEnabled ? ( | ||
| <NavBubble /> |
There was a problem hiding this comment.
I think we should (at least partially) render the nav on the skeleton. It doesn't depend on remote data, so it's already available when the page starts rendering. Also, this would allow the user to navigate back to the main explore page if the TDP never actually finishes loading. This appears to be what the existing breadcrumb nav does by rendering the back-link in the skeleton.
| <CopyContractAddress | ||
| address={address} | ||
| showTruncatedOnly | ||
| truncatedAddress={shortenAddress(address)} |
There was a problem hiding this comment.
Should the address be uppercase or lowercase? It's currently lowercase in the URL params but contains uppercase letters in the AddressSection.
There was a problem hiding this comment.
on the blockchain, it's the same bc ethereum addresses are not case-sensitive. but in practice, we use the case-sensitive version in the ui because its checksummed
New breadcrumb nav for info project TDP
Linear ticket: https://linear.app/uniswap/issue/WEB-2948/refreshed-header-and-navigation
Slack thread:
Relevant docs: https://www.figma.com/file/2yKuZXmQ2UvgBbXR3txz4O/Dot-Info?type=design&node-id=474-690576&mode=design&t=WOe5mdnJC3jq5auJ-0
Screen capture
Before
After