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: reduce the number of decimals shown for simulation detail amounts #24036

Merged
merged 8 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ describe('Simulation Details', () => {

await switchToNotificationWindow(driver);
await expectBalanceChange(driver, true, 0, '- 0.002', 'ETH');
await expectBalanceChange(driver, false, 0, '+ 6.756291', 'DAI');
await expectBalanceChange(driver, false, 0, '+ 6.756', 'DAI');
});
});

Expand Down
16 changes: 12 additions & 4 deletions ui/hooks/useCurrencyDisplay.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { useMemo } from 'react';
import { useSelector } from 'react-redux';
import BigNumber from 'bignumber.js';
import { formatCurrency } from '../helpers/utils/confirm-tx.util';
import { getCurrentCurrency } from '../selectors';
import {
Expand All @@ -12,9 +13,16 @@ import { TEST_NETWORK_TICKER_MAP } from '../../shared/constants/network';
import { Numeric } from '../../shared/modules/Numeric';
import { EtherDenomination } from '../../shared/constants/common';

export const MIN_DISPLAY_AMOUNT = '<0.000001';
// The smallest non-zero amount that can be displayed.
export const MIN_AMOUNT = 0.000001;

export const DEFAULT_PRECISION_DECIMALS = 6;
// The string to display when 0 < amount < MIN_AMOUNT.
// TODO(dbrans): Localize this string using Intl.NumberFormatter.
const MIN_AMOUNT_DISPLAY = `<${MIN_AMOUNT}`;

// The default precision for displaying currency values.
// It set to the number of decimal places in the minimum amount.
export const DEFAULT_PRECISION = new BigNumber(MIN_AMOUNT).decimalPlaces();

/**
* Defines the shape of the options parameter for useCurrencyDisplay
Expand Down Expand Up @@ -66,12 +74,12 @@ export function useCurrencyDisplay(
) {
const ethDisplayValue = new Numeric(inputValue, 16, EtherDenomination.WEI)
.toDenomination(denomination || EtherDenomination.ETH)
.round(numberOfDecimals || DEFAULT_PRECISION_DECIMALS)
.round(numberOfDecimals || DEFAULT_PRECISION)
.toBase(10)
.toString();

return ethDisplayValue === '0' && inputValue && Number(inputValue) !== 0
? MIN_DISPLAY_AMOUNT
? MIN_AMOUNT_DISPLAY
: ethDisplayValue;
} else if (isUserPreferredCurrency && conversionRate) {
return formatCurrency(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
import React from 'react';
import { render } from '@testing-library/react';
import { BigNumber } from 'bignumber.js';
import { TokenStandard } from '../../../../../shared/constants/transaction';
import { Numeric } from '../../../../../shared/modules/Numeric';
import Tooltip from '../../../../components/ui/tooltip';
import { AmountPill } from './amount-pill';
import {
Amount,
AssetIdentifier,
NATIVE_ASSET_IDENTIFIER,
TokenAssetIdentifier,
} from './types';

jest.mock('react-redux', () => ({
useSelector: jest.fn((selector) => selector()),
}));

jest.mock('../../../../ducks/locale/locale', () => ({
getIntlLocale: jest.fn(() => 'en-US'),
}));

jest.mock('../../../../components/ui/tooltip', () => ({
__esModule: true,
default: jest.fn(({ children }) => children),
Expand All @@ -35,7 +42,7 @@ const ERC1155_ASSET_MOCK: TokenAssetIdentifier = {

const renderAndExpect = (
asset: AssetIdentifier,
amount: Amount,
amount: BigNumber,
expected: { text: string; tooltip: string },
): void => {
const { getByText } = render(<AmountPill asset={asset} amount={amount} />);
Expand All @@ -47,50 +54,48 @@ const renderAndExpect = (
};

describe('AmountPill', () => {
beforeEach(() => {
jest.clearAllMocks();
});

const nativeAndErc20Cases = [
{
isNegative: true,
numeric: new Numeric(-123.1234567, 10),
amount: new BigNumber(-123.1234567),
expected: {
text: '- 123.123457',
text: '- 123.1',
tooltip: '123.1234567',
},
},
{
isNegative: false,
numeric: new Numeric(789.012, 10),
amount: new BigNumber(789.412),
expected: {
text: '+ 789.012',
tooltip: '789.012',
text: '+ 789.4',
tooltip: '789.412',
},
},
{
isNegative: true,
numeric: new Numeric(-0.000000001, 10),
amount: new BigNumber(-0.000000001),
expected: {
text: '- <0.000001',
tooltip: '0.000000001',
},
},
{
isNegative: false,
numeric: new Numeric(0.000000001, 10),
amount: new BigNumber(0.000000001),
expected: {
text: '+ <0.000001',
tooltip: '0.000000001',
},
},
{
isNegative: true,
numeric: new Numeric(0, 10),
amount: new BigNumber(-0),
expected: {
text: '- 0',
tooltip: '0',
},
},
{
isNegative: false,
numeric: new Numeric(0, 10),
amount: new BigNumber(0),
expected: {
text: '+ 0',
tooltip: '0',
Expand All @@ -100,43 +105,33 @@ describe('AmountPill', () => {

describe('Native', () => {
it.each(nativeAndErc20Cases)(
'renders the correct sign and amount for $numeric.value',
({ isNegative, numeric, expected }) => {
renderAndExpect(
NATIVE_ASSET_IDENTIFIER,
{ isNegative, numeric } as Amount,
expected,
);
'renders the correct sign and amount for $amount',
({ amount, expected }) => {
renderAndExpect(NATIVE_ASSET_IDENTIFIER, amount, expected);
},
);
});

describe('ERC20', () => {
it.each(nativeAndErc20Cases)(
'renders the correct sign and amount for $numeric.value',
({ isNegative, numeric, expected }) => {
renderAndExpect(
ERC20_ASSET_MOCK,
{ isNegative, numeric } as Amount,
expected,
);
'renders the correct sign and amount for $amount',
({ amount, expected }) => {
renderAndExpect(ERC20_ASSET_MOCK, amount, expected);
},
);
});

describe('ERC721', () => {
const cases = [
{
isNegative: true,
numeric: new Numeric(-1, 10),
amount: new BigNumber(-1),
expected: {
text: '- #2748',
tooltip: '#2748',
},
},
{
isNegative: false,
numeric: new Numeric(1, 10),
amount: new BigNumber(1),
expected: {
text: '+ #2748',
tooltip: '#2748',
Expand All @@ -146,37 +141,30 @@ describe('AmountPill', () => {

it.each(cases)(
'renders the token ID with just a plus or minus for $expected.text',
({ isNegative, numeric, expected }) => {
renderAndExpect(
ERC721_ASSET_MOCK,
{ isNegative, numeric } as Amount,
expected,
);
({ amount, expected }) => {
renderAndExpect(ERC721_ASSET_MOCK, amount, expected);
},
);
});

describe('ERC1155', () => {
const cases = [
{
isNegative: true,
numeric: new Numeric(-3, 10),
amount: new BigNumber(-3),
expected: {
text: '- 3 #2748',
tooltip: '3 #2748',
},
},
{
isNegative: false,
numeric: new Numeric(8, 10),
amount: new BigNumber(8),
expected: {
text: '+ 8 #2748',
tooltip: '8 #2748',
},
},
{
isNegative: true,
numeric: new Numeric(-12, 10),
amount: new BigNumber(-12),
expected: {
text: '- 12 #2748',
tooltip: '12 #2748',
Expand All @@ -186,12 +174,8 @@ describe('AmountPill', () => {

it.each(cases)(
'renders the correct sign, amount, and token ID for $expected.text',
({ isNegative, numeric, expected }) => {
renderAndExpect(
ERC1155_ASSET_MOCK,
{ isNegative, numeric } as Amount,
expected,
);
({ amount, expected }) => {
renderAndExpect(ERC1155_ASSET_MOCK, amount, expected);
},
);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import React from 'react';
import { BigNumber } from 'bignumber.js';
import { useSelector } from 'react-redux';
import { Box, Text } from '../../../../components/component-library';
import {
AlignItems,
Expand All @@ -11,21 +13,10 @@ import {
} from '../../../../helpers/constants/design-system';
import { hexToDecimal } from '../../../../../shared/modules/conversion.utils';
import { TokenStandard } from '../../../../../shared/constants/transaction';
import {
DEFAULT_PRECISION_DECIMALS,
MIN_DISPLAY_AMOUNT,
} from '../../../../hooks/useCurrencyDisplay';
import Tooltip from '../../../../components/ui/tooltip';
import { Amount, AssetIdentifier } from './types';

// Format an amount for display.
const formatAmount = (amount: Amount): string => {
const displayAmount = amount.numeric.abs().round(DEFAULT_PRECISION_DECIMALS);

return displayAmount.isZero() && !amount.numeric.isZero()
? MIN_DISPLAY_AMOUNT
: displayAmount.toString();
};
import { getIntlLocale } from '../../../../ducks/locale/locale';
import { AssetIdentifier } from './types';
import { formatAmount, formatAmountMaxPrecision } from './formatAmount';

/**
* Displays a pill with an amount and a background color indicating whether the amount
Expand All @@ -37,23 +28,25 @@ const formatAmount = (amount: Amount): string => {
*/
export const AmountPill: React.FC<{
asset: AssetIdentifier;
amount: Amount;
amount: BigNumber;
}> = ({ asset, amount }) => {
const backgroundColor = amount.isNegative
const locale = useSelector(getIntlLocale);

const backgroundColor = amount.isNegative()
? BackgroundColor.errorMuted
: BackgroundColor.successMuted;

const color = amount.isNegative
const color = amount.isNegative()
? TextColor.errorAlternative
: TextColor.successDefault;

const amountParts: string[] = [amount.isNegative ? '-' : '+'];
const amountParts: string[] = [amount.isNegative() ? '-' : '+'];
const tooltipParts: string[] = [];

// ERC721 amounts are always 1 are not displayed.
// ERC721 amounts are always 1 and are not displayed.
if (asset.standard !== TokenStandard.ERC721) {
const formattedAmount = formatAmount(amount);
const fullPrecisionAmount = amount.numeric.abs().toString();
const formattedAmount = formatAmount(locale, amount.abs());
const fullPrecisionAmount = formatAmountMaxPrecision(locale, amount.abs());

amountParts.push(formattedAmount);
tooltipParts.push(fullPrecisionAmount);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jest.mock('../../../../components/app/name', () => ({
}));

describe('AssetPill', () => {
afterEach(() => {
beforeEach(() => {
jest.clearAllMocks();
});

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { BigNumber } from 'bignumber.js';
import { formatAmount } from './formatAmount';

describe('formatAmount', () => {
const locale = 'en-US';

it('returns "0" for zero amount', () => {
expect(formatAmount(locale, new BigNumber(0))).toBe('0');
});

it('returns "<0.000001" for 0 < amount < MIN_AMOUNT', () => {
expect(formatAmount(locale, new BigNumber(0.0000009))).toBe('<0.000001');
});

it.each([
[0.0000456, '0.000046'],
[0.0004567, '0.000457'],
[0.003456, '0.00346'],
[0.023456, '0.0235'],
[0.125456, '0.125'],
])(
'formats amount less than 1 with maximum significant digits (%s => %s)',
(amount, expected) => {
expect(formatAmount(locale, new BigNumber(amount))).toBe(expected);
},
);

it.each([
[1.0034, '1.003'],
[1.034, '1.034'],
[1.3034, '1.303'],
[12.0345, '12.03'],
[121.456, '121.5'],
[1034.123, '1,034'],
[47361034.006, '47,361,034'],
['12130982923409.5', '12,130,982,923,410'],
['1213098292340944.5', '1,213,098,292,340,945'],
])(
'formats amount greater than or equal to 1 with appropriate decimal precision (%s => %s)',
(amount, expected) => {
expect(formatAmount(locale, new BigNumber(amount))).toBe(expected);
},
);
});
Loading
Loading