Pool overview page redesign #2402
Conversation
|
PR deployed in Google Cloud |
|
PR deployed in Google Cloud |
kattylucy
force-pushed
the
pool_overview_page_redesign
branch
5 times, most recently
from
b813e6e
to
ddb4160
Compare
There was a problem hiding this comment.
Nice work @kattylucy 🔥
The Banner that's sometimes displayed in the bottom right corner could most definitely benefit from a style upgrade. What do you think about a black background with white text?
In general do you think it would make sense to replace the primary color in the theme for gold to ease the transition?
| const lastYearNAV = aggregatedNetAssetValue.toNumber() | ||
| const currentYearNAV = aggregatedListedPoolsNav.toNumber() | ||
|
|
||
| // YoY growth | ||
| const totalValueLockedGrowth = | ||
| lastYearNAV && currentYearNAV ? ((currentYearNAV - lastYearNAV) / lastYearNAV) * 100 : 0 |
There was a problem hiding this comment.
while precision is definitely not vital for this feature, I would still recommend doing all of the math in the decimal library and converting to a number just before its displayed
| const totalValueLockedGrowth = | ||
| lastYearNAV && currentYearNAV ? ((currentYearNAV - lastYearNAV) / lastYearNAV) * 100 : 0 | ||
|
|
||
| return { totalValueLockedGrowth, isLoading } |
There was a problem hiding this comment.
I think it would make sense to rename this to something like totalYearOverYEarGrowth or totalYoyGrowth
fabric/src/theme/tokens/colors.ts
Outdated
| } | ||
|
|
||
| export const centrifugeBlue = blueScale[500] | ||
| export const altairYellow = yellowScale[500] | ||
| export const black = '#252B34' | ||
| export const gold = '#FFC012' |
There was a problem hiding this comment.
seeing a slightly different hex code for gold in fabric/src/theme/tokens/theme.ts. They should probably be the same? Think we can just store them in one place?
fabric/src/theme/tokens/theme.ts
Outdated
| @@ -29,6 +34,7 @@ const lightColors = { | |||
| backgroundInput: 'white', | |||
| backgroundThumbnail: grayScale[500], | |||
| backgroundInverted: grayScale[900], | |||
| backgroundBlack: black, | |||
There was a problem hiding this comment.
I would change the backgroundInverted to be black and use that for the sidebar
There was a problem hiding this comment.
or better yet, change grayScale[900] to be #252B34.
fabric/src/theme/tokens/theme.ts
Outdated
| textPrimary: grayScale[900], | ||
| textSecondary: grayScale[800], | ||
| textDisabled: grayScale[500], | ||
| textInverted: 'white', | ||
| textGold: gold, | ||
| textBlack: black, |
There was a problem hiding this comment.
I would just use textPrimary. If the primary text color has changed, we should update that instead
kattylucy
force-pushed
the
pool_overview_page_redesign
branch
2 times, most recently
from
cfc6af0
to
794fca6
Compare
| ) || [] | ||
|
|
||
| // Aggregate NAV from last year | ||
| const aggregatedNetAssetValue = flattenedData.reduce((accumulator: any, item: FlattenedDataItem) => { |
There was a problem hiding this comment.
| const aggregatedNetAssetValue = flattenedData.reduce((accumulator: any, item: FlattenedDataItem) => { | |
| const aggregatedNetAssetValue = flattenedData.reduce((accumulator: Decimal, item: FlattenedDataItem) => { |
There was a problem hiding this comment.
you'll have to import it
import Decimal from 'decimal.js-light'There was a problem hiding this comment.
Oh i missed this change. THANK YOU!
kattylucy
force-pushed
the
pool_overview_page_redesign
branch
2 times, most recently
from
9e28af5
to
a79a77a
Compare
kattylucy
force-pushed
the
pool_overview_page_redesign
branch
from
a79a77a
to
c7d9d5e
Compare
Description
#2390
Approvals