feat: new chain logos #7438
feat: new chain logos #7438
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. |
6 flaky tests on run #14951 ↗︎
Details:
|
|||||||||||||||||||||||||||
| Test | Artifacts | |
|---|---|---|
| Swap errors > slippage failure |
Output
Screenshots
|
|
swapFlowLogging.test.ts • 1 flaky test • e2e
| Test | Artifacts | |
|---|---|---|
| swap flow logging > completes two swaps and verifies the TTS logging for the first, plus all intermediate steps along the way |
Output
Screenshots
|
|
Review all test suite changes for PR #7438 ↗︎
There was a problem hiding this comment.
See a couple more logos left in the files. Can these be deleted?
- src/assets/images/celoCircle.png
- src/assets/svg/avax_logo.svg
- src/assets/svg/bnb-logo.svg
- src/assets/svg/celo_logo.svg
- src/assets/svg/ethereum_square_logo.svg
There's also a function getNetworkLogoUrl that uses old images, it seems to be only used in one place. Can that be replaced as well?
| {...props} | ||
| /> | ||
| ) | ||
| return <PortfolioLogo currencies={[currency]} chainId={chainId} {...props} /> |
There was a problem hiding this comment.
ooh this [currency] should be memoized to prevent rerendering
There was a problem hiding this comment.
gr8 point, i think this is an issue elsewhere, I made a ticket to fix this across all PortfolioLogo callsites
| @@ -212,7 +212,7 @@ export default function TokenDetails({ | |||
| </BreadcrumbNavLink> | |||
| <TokenInfoContainer data-testid="token-info-container"> | |||
| <TokenNameCell> | |||
| <CurrencyLogo currency={detailedToken} size="32px" hideL2Icon={false} /> | |||
| <PortfolioLogo currencies={[detailedToken]} chainId={detailedToken.chainId} size="32px" /> | |||
There was a problem hiding this comment.
typechecking gets ugly trying to memo here so I'm gonna fix this one in the ticket i mentioned above
|
|
||
| type NetworkAlertChains = keyof typeof SHOULD_SHOW_ALERT | ||
|
|
||
| const BG_COLORS_BY_DARK_MODE_AND_CHAIN_ID: { |
| padding: 6px 8px; | ||
| text-decoration: none !important; | ||
| width: 100%; | ||
| const TitleText = styled(ThemedText.BodyPrimary).attrs({ fontWeight: 535 })<{ color: string }>` |
There was a problem hiding this comment.
why did you use the attrs pattern here over adding the fontWeight 535 as a style below?
There was a problem hiding this comment.
nit: $color for var name of a css style
There was a problem hiding this comment.
setting it in the style didn't work?! I've noticed I've had to manually set it in attr's lately. I can investigate why separately
eth square and celo circle can be removed! ty. the others are still used as native currency logos. getNetworkLogoUrl is used in a cloudflare function so I don't wanna couple any changes to that in this PR |
| {...props} | ||
| /> | ||
| ) | ||
| return <PortfolioLogo currencies={useMemo(() => [currency], [currency])} chainId={chainId} {...props} /> |
There was a problem hiding this comment.
another idea is instead of declaring currency as the constant above, make it
const currencies = useMemo(() => {
return props.token ? [gqlToCurrency(props.token)] : undefined
}, [props.token])
return <PortfolioLogo currencies={currencies} chainId={chainId} {...props} />
Description
Updates interface to use new network icons. Removes logoUrl, circleLogoUrl, and squareLogoUrl from ChainInfo.
New l2 icons are react components that build and return an based on props rather than static svg files, in order to enable lightmode vs darkmode and dynamic border radius based on size. The only new static svg files are the symbols inside of the chain logos themselves.
Screen capture
Test plan
QA (ie manual testing)