-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: add percent change for 24h on asset list #14839
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
Changes from all commits
766c563
a52f4c7
4597d04
16ab115
b4ac7d5
8e819ce
8996f86
2844d0f
2ed94ce
35cb919
e6fc961
bc27757
9575438
d079304
73e8397
bc51d06
1725579
a9a34a7
5102a40
3df6ffe
cdb9327
9969265
283eb7b
5427bd4
fffb8e7
37466be
b0b6244
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,173 @@ | ||
import React from 'react'; | ||
import { render } from '@testing-library/react-native'; | ||
import { mockTheme } from '../../../../util/theme'; | ||
import { useSelector } from 'react-redux'; | ||
import { selectCurrentCurrency } from '../../../../selectors/currencyRateController'; | ||
import { | ||
FORMATTED_VALUE_PRICE_TEST_ID, | ||
FORMATTED_PERCENTAGE_TEST_ID, | ||
} from './AggregatedPercentage.constants'; | ||
import NonEvmAggregatedPercentage from './NonEvmAggregatedPercentage'; | ||
// eslint-disable-next-line import/no-namespace | ||
import * as multichain from '../../../../selectors/multichain/multichain'; | ||
import { selectMultichainAssetsRates } from '../../../../selectors/multichain/multichain'; | ||
|
||
const mockGetMultichainNetworkAggregatedBalance = { | ||
balances: { | ||
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { | ||
amount: '1', | ||
unit: 'SOL', | ||
}, | ||
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:2zMMhcVQEXDtdE6vsFS7S7D5oUodfJHE8vd1gnBouauv': | ||
{ | ||
amount: '5', | ||
unit: 'PENGU', | ||
}, | ||
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v': | ||
{ | ||
amount: '10', | ||
unit: 'USDC', | ||
}, | ||
}, | ||
fiatBalances: { | ||
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': '1', | ||
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:2zMMhcVQEXDtdE6vsFS7S7D5oUodfJHE8vd1gnBouauv': | ||
'5.4', | ||
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v': | ||
'10.', | ||
}, | ||
totalBalanceFiat: 16.4, | ||
totalNativeTokenBalance: { | ||
amount: '1', | ||
unit: 'SOL', | ||
}, | ||
}; | ||
|
||
const mockMultichainAssetsRatesPositive = { | ||
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { | ||
marketData: { | ||
pricePercentChange: { | ||
P1D: +1.1, | ||
}, | ||
}, | ||
rate: '147.98', | ||
}, | ||
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:2zMMhcVQEXDtdE6vsFS7S7D5oUodfJHE8vd1gnBouauv': | ||
{ | ||
marketData: { | ||
pricePercentChange: { | ||
P1D: +2.7, | ||
}, | ||
}, | ||
rate: '0.00624788', | ||
}, | ||
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v': | ||
{ | ||
marketData: { | ||
pricePercentChange: { | ||
P1D: +0.5, | ||
}, | ||
}, | ||
rate: '0.999998', | ||
}, | ||
}; | ||
|
||
const mockMultichainAssetsRatesNegative = { | ||
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { | ||
marketData: { | ||
pricePercentChange: { | ||
P1D: -1.1, | ||
}, | ||
}, | ||
rate: '147.98', | ||
}, | ||
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:2zMMhcVQEXDtdE6vsFS7S7D5oUodfJHE8vd1gnBouauv': | ||
{ | ||
marketData: { | ||
pricePercentChange: { | ||
P1D: -2.7, | ||
}, | ||
}, | ||
rate: '0.00624788', | ||
}, | ||
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v': | ||
{ | ||
marketData: { | ||
pricePercentChange: { | ||
P1D: -0.5, | ||
}, | ||
}, | ||
rate: '0.999998', | ||
}, | ||
}; | ||
|
||
jest.mock('react-redux', () => ({ | ||
...jest.requireActual('react-redux'), | ||
useSelector: jest.fn(), | ||
})); | ||
describe('NonEvmAggregatedPercentage', () => { | ||
beforeEach(() => { | ||
(useSelector as jest.Mock).mockImplementation((selector) => { | ||
if (selector === selectCurrentCurrency) return 'USD'; | ||
}); | ||
}); | ||
afterEach(() => { | ||
(useSelector as jest.Mock).mockClear(); | ||
}); | ||
|
||
it('renders positive percentage change correctly', () => { | ||
jest | ||
.spyOn(multichain, 'getMultichainNetworkAggregatedBalance') | ||
.mockReturnValue(mockGetMultichainNetworkAggregatedBalance); | ||
(useSelector as jest.Mock).mockImplementation((selector) => { | ||
if (selector === selectCurrentCurrency) return 'USD'; | ||
if (selector === selectMultichainAssetsRates) | ||
return mockMultichainAssetsRatesPositive; | ||
}); | ||
const { getByText } = render(<NonEvmAggregatedPercentage />); | ||
|
||
expect(getByText('(+1.25%)')).toBeTruthy(); | ||
expect(getByText('+0.2 USD')).toBeTruthy(); | ||
|
||
expect(getByText('(+1.25%)').props.style).toMatchObject({ | ||
color: mockTheme.colors.success.default, | ||
}); | ||
}); | ||
|
||
it('renders negative percentage change correctly', () => { | ||
jest | ||
.spyOn(multichain, 'getMultichainNetworkAggregatedBalance') | ||
.mockReturnValue(mockGetMultichainNetworkAggregatedBalance); | ||
(useSelector as jest.Mock).mockImplementation((selector) => { | ||
if (selector === selectCurrentCurrency) return 'USD'; | ||
if (selector === selectMultichainAssetsRates) | ||
return mockMultichainAssetsRatesNegative; | ||
}); | ||
const { getByText } = render(<NonEvmAggregatedPercentage />); | ||
|
||
expect(getByText('(-1.27%)')).toBeTruthy(); | ||
expect(getByText('-0.21 USD')).toBeTruthy(); | ||
|
||
expect(getByText('(-1.27%)').props.style).toMatchObject({ | ||
color: mockTheme.colors.error.default, | ||
}); | ||
}); | ||
|
||
it('renders correctly with privacy mode on', () => { | ||
jest | ||
.spyOn(multichain, 'getMultichainNetworkAggregatedBalance') | ||
.mockReturnValue(mockGetMultichainNetworkAggregatedBalance); | ||
(useSelector as jest.Mock).mockImplementation((selector) => { | ||
if (selector === selectCurrentCurrency) return 'USD'; | ||
if (selector === selectMultichainAssetsRates) | ||
return mockMultichainAssetsRatesNegative; | ||
}); | ||
const { getByTestId } = render(<NonEvmAggregatedPercentage privacyMode />); | ||
|
||
const formattedPercentage = getByTestId(FORMATTED_PERCENTAGE_TEST_ID); | ||
const formattedValuePrice = getByTestId(FORMATTED_VALUE_PRICE_TEST_ID); | ||
|
||
expect(formattedPercentage.props.children).toBe('••••••••••'); | ||
expect(formattedValuePrice.props.children).toBe('••••••••••'); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,146 @@ | ||
import React from 'react'; | ||
import { | ||
TextColor, | ||
TextVariant, | ||
} from '../../../../component-library/components/Texts/Text'; | ||
import SensitiveText from '../../../../component-library/components/Texts/SensitiveText'; | ||
import { View } from 'react-native'; | ||
import { renderFiat } from '../../../../util/number'; | ||
import { useSelector } from 'react-redux'; | ||
import { selectCurrentCurrency } from '../../../../selectors/currencyRateController'; | ||
import styleSheet from './AggregatedPercentage.styles'; | ||
import { useStyles } from '../../../hooks'; | ||
import { | ||
FORMATTED_VALUE_PRICE_TEST_ID, | ||
FORMATTED_PERCENTAGE_TEST_ID, | ||
} from './AggregatedPercentage.constants'; | ||
import { DECIMALS_TO_SHOW } from '../../../../components/UI/Tokens/constants'; | ||
import { | ||
getMultichainNetworkAggregatedBalance, | ||
selectMultichainAssets, | ||
selectMultichainAssetsRates, | ||
selectMultichainBalances, | ||
} from '../../../../selectors/multichain/multichain'; | ||
import { selectSelectedInternalAccount } from '../../../../selectors/accountsController'; | ||
import { selectSelectedNonEvmNetworkChainId } from '../../../../selectors/multichainNetworkController'; | ||
import { InternalAccount } from '@metamask/keyring-internal-api'; | ||
import { CaipAssetType } from '@metamask/keyring-api'; | ||
import { getCalculatedTokenAmount1dAgo } from './AggregatedPercentageCrossChains'; | ||
|
||
const isValidAmount = (amount: number | null | undefined): boolean => | ||
amount !== null && amount !== undefined && !Number.isNaN(amount); | ||
|
||
const NonEvmAggregatedPercentage = ({ | ||
privacyMode = false, | ||
}: { | ||
privacyMode?: boolean; | ||
}) => { | ||
const { styles } = useStyles(styleSheet, {}); | ||
|
||
const currentCurrency = useSelector(selectCurrentCurrency); | ||
|
||
const account = useSelector(selectSelectedInternalAccount); | ||
const multichainBalances = useSelector(selectMultichainBalances); | ||
const multichainAssets = useSelector(selectMultichainAssets); | ||
const multichainAssetsRates = useSelector(selectMultichainAssetsRates); | ||
const nonEvmChainId = useSelector(selectSelectedNonEvmNetworkChainId); | ||
|
||
const nonEvmAccountBalance = getMultichainNetworkAggregatedBalance( | ||
account as unknown as InternalAccount, | ||
multichainBalances, | ||
multichainAssets, | ||
multichainAssetsRates, | ||
nonEvmChainId, | ||
); | ||
|
||
const nonEvmFiatBalancesArray = Object.entries( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both this and nonEvmAccountBalance should be memoized. The |
||
nonEvmAccountBalance?.fiatBalances ?? {}, | ||
).map(([id, amount]) => ({ | ||
id: id as CaipAssetType, | ||
fiatAmount: Number(amount), | ||
})); | ||
|
||
const getNonEvmTotalFiat1dAgo = () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we also memoize this? |
||
const totalNonEvm1dAgo = nonEvmFiatBalancesArray.reduce( | ||
(total1dAgo: number, item: { id: CaipAssetType; fiatAmount: number }) => { | ||
const pricePercentChange1d = | ||
multichainAssetsRates?.[item.id]?.marketData?.pricePercentChange | ||
.P1D ?? 0; | ||
const splTokenFiat1dAgo = getCalculatedTokenAmount1dAgo( | ||
Number(item.fiatAmount), | ||
pricePercentChange1d, | ||
); | ||
return total1dAgo + Number(splTokenFiat1dAgo); | ||
}, | ||
0, | ||
); | ||
|
||
return totalNonEvm1dAgo; | ||
}; | ||
|
||
const totalNonEvmBalance = nonEvmAccountBalance?.totalBalanceFiat; | ||
|
||
if (!totalNonEvmBalance) { | ||
return null; | ||
} | ||
|
||
const totalNonEvmBalance1dAgo = getNonEvmTotalFiat1dAgo(); | ||
const amountChange = totalNonEvmBalance - totalNonEvmBalance1dAgo; | ||
|
||
const percentageChange = | ||
((totalNonEvmBalance - totalNonEvmBalance1dAgo) / totalNonEvmBalance1dAgo) * | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we make sure that the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also can we just reuse the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point; i can add a check but i think that to be in that case everything would have to be zero
|
||
100 || 0; | ||
|
||
let percentageTextColor = TextColor.Default; | ||
|
||
if (!privacyMode) { | ||
if (percentageChange === 0) { | ||
percentageTextColor = TextColor.Default; | ||
} else if (percentageChange > 0) { | ||
percentageTextColor = TextColor.Success; | ||
} else { | ||
percentageTextColor = TextColor.Error; | ||
} | ||
} else { | ||
percentageTextColor = TextColor.Alternative; | ||
} | ||
|
||
const formattedPercentage = isValidAmount(percentageChange) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should move these into utils. |
||
? `(${(percentageChange as number) >= 0 ? '+' : ''}${( | ||
percentageChange as number | ||
).toFixed(2)}%)` | ||
: ''; | ||
|
||
const formattedValuePrice = isValidAmount(amountChange) | ||
? `${(amountChange as number) >= 0 ? '+' : ''}${renderFiat( | ||
amountChange, | ||
currentCurrency, | ||
DECIMALS_TO_SHOW, | ||
)} ` | ||
: ''; | ||
|
||
return ( | ||
<View style={styles.wrapper}> | ||
<SensitiveText | ||
isHidden={privacyMode} | ||
length="10" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the length always going to be 10? |
||
color={percentageTextColor} | ||
variant={TextVariant.BodyMDMedium} | ||
testID={FORMATTED_VALUE_PRICE_TEST_ID} | ||
> | ||
{formattedValuePrice} | ||
</SensitiveText> | ||
<SensitiveText | ||
isHidden={privacyMode} | ||
length="10" | ||
color={percentageTextColor} | ||
variant={TextVariant.BodyMDMedium} | ||
testID={FORMATTED_PERCENTAGE_TEST_ID} | ||
> | ||
{formattedPercentage} | ||
</SensitiveText> | ||
</View> | ||
); | ||
}; | ||
|
||
export default NonEvmAggregatedPercentage; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets wrap this whole thing in a React.memo |
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 think the casting is unnecessary here. If not we should explicit mark the return type of this selector to be InternalAccount.
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 can make the return type explicit in the selector;
Although i think i might need to still cast because the return type is InternalAccount | undefined