Skip to content
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: [info] update token links #7505

Merged
merged 12 commits into from Nov 2, 2023
Merged

Conversation

cbachmeier
Copy link
Contributor

@cbachmeier cbachmeier commented Oct 20, 2023

Description

  • Token Descriptions have been updated to just be a Links section. This updates the UX accordingly.
  • The Link's styles changes depending on if it's for a Pool or a Token.
  • Users can copy the pool/token's address and be taken to its explorer page
  • Clicking on a Token's name will take you to it's TDP. Tapping on the pool's name does nothing since you're already on the PDP.

Linear ticket: https://linear.app/uniswap/issue/WEB-2985/update-token-links
Slack thread: https://uniswapteam.slack.com/archives/C05NXMASXNZ/p1697748986768909
Relevant docs: https://www.figma.com/file/2yKuZXmQ2UvgBbXR3txz4O/Dot-Info?type=design&node-id=1547%3A229624&mode=dev

Screen capture

Mobile
Screenshot 2023-10-20 at 11 59 11 AM
Tablet
Screenshot 2023-10-20 at 11 59 18 AM
Desktop
Screenshot 2023-10-20 at 12 04 22 PM
Non-Mainnet
Screenshot 2023-10-20 at 11 48 21 AM

Light mode
Screenshot 2023-10-20 at 11 48 55 AM

Test plan

QA (ie manual testing)

  • Go to a PDP with flags on and make sure buttons copy address
  • links to TDP pages and to explorer urls work correctly
  • test on alt networks

Automated testing

  • Unit tests and snapshots added

@linear
Copy link

linear bot commented Oct 20, 2023

WEB-2985 Update Token Links

Design updated on 10/11

@vercel
Copy link

vercel bot commented Oct 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
interface ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 2, 2023 3:39pm

@codecov
Copy link

codecov bot commented Oct 20, 2023

Codecov Report

Merging #7505 (fb411b0) into main (f9a9469) will increase coverage by 0.11%.
The diff coverage is 93.24%.

Flag Coverage Δ
cloud-tests 83.60% <ø> (ø)
unit-tests 43.11% <93.24%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@cypress
Copy link

cypress bot commented Oct 20, 2023

Passing run #15311 ↗︎

0 125 0 0 Flakiness 0

Details:

feat: [info] update token links
Project: Uniswap Interface Commit: fb411b0da1
Status: Passed Duration: 04:50 💡
Started: Nov 2, 2023 3:42 PM Ended: Nov 2, 2023 3:47 PM

Review all test suite changes for PR #7505 ↗︎

const TokenName = styled(ThemedText.BodyPrimary)`
display: none;

@media (max-width: ${BREAKPOINTS.lg - 1}px) and (min-width: ${BREAKPOINTS.xs - 1}px) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question for my own understanding: why don't we use @media for all our other responsive css as well? why do we use @media only screen or @media screen when virtually all browsers support media queries + do we need to specify screen for responsive purposes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we currently don't enforce one particular standard over the other so I think it just comes to implementers style. This might be something we want to codify via linter however if we see a compelling reason to use one over the other

<TokenDetailsWrapper>
<TokenDetailsHeader>
<Trans>Links</Trans>
</TokenDetailsHeader>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: rename to PoolDetailsHeader/Wrapper?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section is specifically referring to the Token Details of the Tokens in the pool not like in TDP.

Copy link
Contributor

@tinaszheng tinaszheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Comment on lines +148 to +151
<PoolDetailsLink address={poolAddress} chainId={chainId} tokens={[token0, token1]} loading={loading} />
<PoolDetailsLink address={token0?.id} chainId={chainId} tokens={[token0]} loading={loading} />
<PoolDetailsLink address={token1?.id} chainId={chainId} tokens={[token1]} loading={loading} />
</LinksContainer>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should probably memo the tokens prop array before passing them in, or change the prop to take in token0 and token1 to prevent rerenders

@cbachmeier cbachmeier merged commit c2ca9ab into main Nov 2, 2023
27 checks passed
@cbachmeier cbachmeier deleted the cab/web-2985-update-token-links branch November 2, 2023 20:53
natew pushed a commit that referenced this pull request Nov 2, 2023
* update pdp link styles

* dynamic link text

* move links to their own file

* border width case

* todo comments

* add explorer icon

* hide chain logos on other chain

* remove quotes

* clean up

* unused style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants