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.

Sign up for GitHub

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

Jump to bottom

fix: Reduce usage of scientific notation #27992

Merged
merged 2 commits into from
Oct 28, 2024
Merged

fix: Reduce usage of scientific notation #27992

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ce9f337
fix: Reduce usage of scientific notation
pedronfigueiredo Oct 21, 2024
c805139
wip
pedronfigueiredo Oct 24, 2024
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
26 changes: 23 additions & 3 deletions ui/pages/confirmations/components/confirm/info/hooks/use-token-values.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import mockState from '../../../../../../../test/data/mock-state.json';
import { renderHookWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers';
import useTokenExchangeRate from '../../../../../../components/app/currency-input/hooks/useTokenExchangeRate';
import { useAssetDetails } from '../../../../hooks/useAssetDetails';
import { useTokenValues } from './use-token-values';
import { toNonScientificString, useTokenValues } from './use-token-values';
import { useDecodedTransactionData } from './useDecodedTransactionData';

jest.mock('../../../../hooks/useAssetDetails', () => ({
Expand Down Expand Up @@ -73,7 +73,8 @@ describe('useTokenValues', () => {
await waitForNextUpdate();

expect(result.current).toEqual({
decodedTransferValue: 7,
decodedTransferValue: '7',
displayTransferValue: '7',
fiatDisplayValue: '$6.37',
pending: false,
});
Expand Down Expand Up @@ -118,9 +119,28 @@ describe('useTokenValues', () => {
await waitForNextUpdate();

expect(result.current).toEqual({
decodedTransferValue: 7,
decodedTransferValue: '7',
displayTransferValue: '7',
fiatDisplayValue: null,
pending: false,
});
});
});

describe('toNonScientificString', () => {
const TEST_CASES = [
{ scientific: 1.23e-5, expanded: '0.0000123' },
{ scientific: 1e-10, expanded: '0.0000000001' },
{ scientific: 1.23e-21, expanded: '1.23e-21' },
];

// @ts-expect-error This is missing from the Mocha type definitions
it.each(TEST_CASES)(
'Expand $scientific to "$expanded"',
({ scientific, expanded }: { scientific: number; expanded: string }) => {
const actual = toNonScientificString(scientific);

expect(actual).toEqual(expanded);
},
);
});
21 changes: 20 additions & 1 deletion ui/pages/confirmations/components/confirm/info/hooks/use-token-values.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ import { isHexString } from '@metamask/utils';
import { BigNumber } from 'bignumber.js';
import { isBoolean } from 'lodash';
import { useMemo, useState } from 'react';
import { useSelector } from 'react-redux';
import { Numeric } from '../../../../../../../shared/modules/Numeric';
import useTokenExchangeRate from '../../../../../../components/app/currency-input/hooks/useTokenExchangeRate';
import { getIntlLocale } from '../../../../../../ducks/locale/locale';
import { useFiatFormatter } from '../../../../../../hooks/useFiatFormatter';
import { useAssetDetails } from '../../../../hooks/useAssetDetails';
import { formatAmount } from '../../../simulation-details/formatAmount';
import { useDecodedTransactionData } from './useDecodedTransactionData';

export const useTokenValues = (transactionMeta: TransactionMeta) => {
Expand Down Expand Up @@ -56,9 +59,25 @@ export const useTokenValues = (transactionMeta: TransactionMeta) => {
const fiatDisplayValue =
fiatValue && fiatFormatter(fiatValue, { shorten: true });

const locale = useSelector(getIntlLocale);
const displayTransferValue = formatAmount(
locale,
new BigNumber(decodedTransferValue),
);

return {
decodedTransferValue,
decodedTransferValue: toNonScientificString(decodedTransferValue),
displayTransferValue,
fiatDisplayValue,
pending,
};
};

export function toNonScientificString(num: number): string {
if (num >= 10e-18) {
return num.toFixed(18).replace(/\.?0+$/u, '');
}

// keep in scientific notation
return num.toString();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Will be nice to extract this to a util.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I extracted the logic into a contained function but since this function is only used on this hook I think it makes sense for it to live on the same file for now.

43 changes: 31 additions & 12 deletions ui/pages/confirmations/components/confirm/info/shared/send-heading/send-heading.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {
Box,
Text,
} from '../../../../../../../components/component-library';
import Tooltip from '../../../../../../../components/ui/tooltip';
import { getIntlLocale } from '../../../../../../../ducks/locale/locale';
import {
AlignItems,
BackgroundColor,
Expand All @@ -16,25 +18,32 @@ import {
TextColor,
TextVariant,
} from '../../../../../../../helpers/constants/design-system';
import { MIN_AMOUNT } from '../../../../../../../hooks/useCurrencyDisplay';
import { getWatchedToken } from '../../../../../../../selectors';
import { MultichainState } from '../../../../../../../selectors/multichain';
import { useConfirmContext } from '../../../../../context/confirm';
import { useTokenDetails } from '../../hooks/useTokenDetails';
import { formatAmountMaxPrecision } from '../../../../simulation-details/formatAmount';
import { useTokenValues } from '../../hooks/use-token-values';
import { useTokenDetails } from '../../hooks/useTokenDetails';
import { ConfirmLoader } from '../confirm-loader/confirm-loader';

const SendHeading = () => {
const { currentConfirmation: transactionMeta } =
useConfirmContext<TransactionMeta>();
const locale = useSelector(getIntlLocale);
const selectedToken = useSelector((state: MultichainState) =>
getWatchedToken(transactionMeta)(state),
);
const { tokenImage, tokenSymbol } = useTokenDetails(
transactionMeta,
selectedToken,
);
const { decodedTransferValue, fiatDisplayValue, pending } =
useTokenValues(transactionMeta);
const {
decodedTransferValue,
displayTransferValue,
fiatDisplayValue,
pending,
} = useTokenValues(transactionMeta);

const TokenImage = (
<AvatarToken
Expand All @@ -52,19 +61,28 @@ const SendHeading = () => {
/>
);

const TokenValue = (
<>
const TokenValue =
displayTransferValue ===
`<${formatAmountMaxPrecision(locale, MIN_AMOUNT)}` ? (
<Tooltip title={decodedTransferValue.toString()} position="right">
<Text
variant={TextVariant.headingLg}
color={TextColor.inherit}
marginTop={3}
>{`${displayTransferValue} ${tokenSymbol}`}</Text>
</Tooltip>
) : (
<Text
variant={TextVariant.headingLg}
color={TextColor.inherit}
marginTop={3}
>{`${decodedTransferValue || ''} ${tokenSymbol}`}</Text>
{fiatDisplayValue && (
<Text variant={TextVariant.bodyMd} color={TextColor.textAlternative}>
{fiatDisplayValue}
</Text>
)}
</>
>{`${displayTransferValue} ${tokenSymbol}`}</Text>
);

const TokenFiatValue = fiatDisplayValue && (
<Text variant={TextVariant.bodyMd} color={TextColor.textAlternative}>
{fiatDisplayValue}
</Text>
);

if (pending) {
Expand All @@ -81,6 +99,7 @@ const SendHeading = () => {
>
{TokenImage}
{TokenValue}
{TokenFiatValue}
</Box>
);
};
Expand Down
2 changes: 1 addition & 1 deletion ...mations/components/confirm/info/token-transfer/__snapshots__/token-transfer.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ exports[`TokenTransferInfo renders correctly 1`] = `
<h2
class="mm-box mm-text mm-text--heading-lg mm-box--margin-top-3 mm-box--color-inherit"
>
Unknown
0 Unknown
</h2>
</div>
<div
Expand Down