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: improvement for how we display big and small numbers #25438

Merged
merged 48 commits into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
9e57e6e
fix: formatting of deadline in permit signature page
jpuri Jun 13, 2024
5ee78f2
fix
jpuri Jun 14, 2024
343b38a
fix
jpuri Jun 14, 2024
a15aaa1
fix
jpuri Jun 14, 2024
4e3dc68
fix
jpuri Jun 14, 2024
244eb3d
Merge branch 'develop' of github.com:MetaMask/metamask-extension into…
jpuri Jun 18, 2024
b6ddf18
Update
jpuri Jun 18, 2024
5960490
Merge branch 'develop' into permit_deadline
jpuri Jun 19, 2024
b0b36b8
merge
jpuri Jun 19, 2024
17b55f0
Merge branch 'permit_deadline' into permit_token_decimal_fix
jpuri Jun 19, 2024
448e558
update
jpuri Jun 19, 2024
a0b91ef
Merge branch 'permit_token_decimal_fix' of github.com:MetaMask/metama…
jpuri Jun 19, 2024
ac15aa4
update
jpuri Jun 19, 2024
8c99c92
Fix display of token value on permit signature pages
jpuri Jun 20, 2024
435bd22
merge
jpuri Jun 23, 2024
a4568ae
Merge branch 'permit_deadline' of github.com:MetaMask/metamask-extens…
jpuri Jun 23, 2024
5c745db
merge
jpuri Jun 23, 2024
4d49efb
merge
jpuri Jun 23, 2024
b750515
merge
jpuri Jun 24, 2024
7ca13a1
update
jpuri Jun 24, 2024
5ef63d3
update
jpuri Jun 24, 2024
673f415
Merge branch 'permit_deadline' into permit_token_decimal_fix
jpuri Jun 24, 2024
09682d1
merge
jpuri Jun 24, 2024
e277ff8
update
jpuri Jun 24, 2024
0e04701
update
jpuri Jun 24, 2024
e266251
update
jpuri Jun 24, 2024
5252c02
update
jpuri Jun 24, 2024
3af6950
update
jpuri Jun 24, 2024
6cff17f
update
jpuri Jun 24, 2024
8ae0cc1
update
jpuri Jun 24, 2024
b90c647
update
jpuri Jun 24, 2024
f915032
update
jpuri Jun 24, 2024
0e6ba95
update
jpuri Jun 24, 2024
677ddeb
update
jpuri Jun 24, 2024
71f6f54
update
jpuri Jun 24, 2024
cb7eb4c
merge
jpuri Jun 25, 2024
c5539fb
Merge branch 'develop' into permit_token_decimal_fix
jpuri Jun 25, 2024
5afd7fa
update
jpuri Jun 27, 2024
fabd406
Merge branch 'develop' into permit_token_decimal_fix
jpuri Jun 29, 2024
3c396ac
Merge branch 'permit_token_decimal_fix' into permit_value_display_fix
jpuri Jun 29, 2024
767c6b5
merge
jpuri Jul 2, 2024
8b43b18
Merge branch 'permit_value_display_fix' of github.com:MetaMask/metama…
jpuri Jul 2, 2024
ee06044
update
jpuri Jul 2, 2024
eea6d30
update
jpuri Jul 2, 2024
ea38343
update
jpuri Jul 2, 2024
e6d6fe1
update
jpuri Jul 2, 2024
8eb1284
update
jpuri Jul 2, 2024
7507aab
update
jpuri Jul 2, 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
1 change: 1 addition & 0 deletions ui/components/app/confirm/info/row/text.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const ConfirmInfoRowTextStory = {
export const DefaultStory = ({ text }) => <ConfirmInfoRowText text={text} />;
DefaultStory.args = {
text: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.',
tooltip: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.',
Copy link
Member

Choose a reason for hiding this comment

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

Minor, but is it worth a constant for this such as TEXT_MOCK?

};

export const EditableStory = (props) => <ConfirmInfoRowText {...props} />;
Expand Down
24 changes: 21 additions & 3 deletions ui/components/app/confirm/info/row/text.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,26 @@ import {
TextColor,
} from '../../../../../helpers/constants/design-system';
import { Box, ButtonIcon, IconName, Text } from '../../../../component-library';
import Tooltip from '../../../../ui/tooltip';

const InfoText = ({ text }: { text: string }) => (
<Text color={TextColor.inherit} style={{ whiteSpace: 'pre-wrap' }}>
{text}
</Text>
);

export type ConfirmInfoRowTextProps = {
text: string;
onEditClick?: () => void;
editIconClassName?: string;
tooltip?: string;
};

export const ConfirmInfoRowText: React.FC<ConfirmInfoRowTextProps> = ({
text,
onEditClick,
editIconClassName,
tooltip,
}) => {
const t = useContext(I18nContext);

Expand All @@ -31,9 +40,18 @@ export const ConfirmInfoRowText: React.FC<ConfirmInfoRowTextProps> = ({
flexWrap={FlexWrap.Wrap}
gap={2}
>
<Text color={TextColor.inherit} style={{ whiteSpace: 'pre-wrap' }}>
{text}
</Text>
{tooltip ? (
<Tooltip
position="bottom"
title={tooltip}
wrapperStyle={{ minWidth: 0 }}
interactive
>
<InfoText text={text} />
</Tooltip>
) : (
<InfoText text={text} />
)}
{isEditable ? (
<ButtonIcon
className={editIconClassName || undefined}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ exports[`TypedSignInfo correctly renders permit sign type 1`] = `
</p>
<div>
<div
aria-describedby="tippy-tooltip-6"
aria-describedby="tippy-tooltip-8"
class=""
data-original-title="This is the site asking for your signature."
data-tooltipped=""
Expand Down Expand Up @@ -397,12 +397,25 @@ exports[`TypedSignInfo correctly renders permit sign type 1`] = `
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
style="white-space: pre-wrap;"
<div
style="min-width: 0;"
>
3,000
</p>
<div
aria-describedby="tippy-tooltip-9"
class=""
data-original-title="3,000"
data-tooltipped=""
style="display: inline;"
tabindex="0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
style="white-space: pre-wrap;"
>
3,000
</p>
</div>
</div>
</div>
</div>
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,24 @@ exports[`PermitSimulation renders component correctly 1`] = `
<div
class="mm-box mm-box--margin-inline-end-1 mm-box--display-inline"
>
<p
class="mm-box mm-text mm-text--body-md mm-text--text-align-center mm-box--padding-inline-2 mm-box--color-text-default mm-box--background-color-background-alternative mm-box--rounded-xl"
<div
style="min-width: 0;"
>
30.00
</p>
<div
aria-describedby="tippy-tooltip-2"
class=""
data-original-title="30"
data-tooltipped=""
style="display: inline;"
tabindex="0"
>
<p
class="mm-box mm-text mm-text--body-md mm-text--text-align-center mm-box--padding-inline-2 mm-box--color-text-default mm-box--background-color-background-alternative mm-box--rounded-xl"
>
30
</p>
</div>
</div>
</div>
<div>
<div
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, { useMemo } from 'react';
import { useSelector } from 'react-redux';
import { NameType } from '@metamask/name-controller';
import { BigNumber } from 'bignumber.js';

import { parseTypedDataMessage } from '../../../../../../../../shared/modules/transaction.utils';
import { Numeric } from '../../../../../../../../shared/modules/Numeric';
Expand All @@ -12,6 +13,7 @@ import {
import { useI18nContext } from '../../../../../../../hooks/useI18nContext';
import { currentConfirmationSelector } from '../../../../../../../selectors';
import { Box, Text } from '../../../../../../../components/component-library';
import Tooltip from '../../../../../../../components/ui/tooltip';
import {
BackgroundColor,
BorderRadius,
Expand All @@ -21,7 +23,10 @@ import {
import { SignatureRequestType } from '../../../../../types/confirm';
import useTokenExchangeRate from '../../../../../../../components/app/currency-input/hooks/useTokenExchangeRate';
import { IndividualFiatDisplay } from '../../../../simulation-details/fiat-display';
import { formatNumber } from '../../../utils';
import {
formatAmount,
formatAmountMaxPrecision,
} from '../../../../simulation-details/formatAmount';
Copy link
Member

Choose a reason for hiding this comment

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

We could move this to a higher level util but I think this makes sense in this context since we're also a simulation component 👍

import { ConfirmInfoSection } from '../../../../../../../components/app/confirm/info/row/section';

const PermitSimulation: React.FC<{
Expand All @@ -46,6 +51,14 @@ const PermitSimulation: React.FC<{
return undefined;
}, [exchangeRate, value]);

const { tokenValue, tokenValueMaxPrecision } = useMemo(() => {
const valueBN = new BigNumber(value / Math.pow(10, tokenDecimals));
Copy link
Member

Choose a reason for hiding this comment

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

This is the same logic we do in the decimals PR, perhaps an applyDecimals 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.

A new util for just division does may not be very useful.

return {
tokenValue: formatAmount('en-US', valueBN),
tokenValueMaxPrecision: formatAmountMaxPrecision('en-US', valueBN),
Copy link
Contributor

Choose a reason for hiding this comment

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

similar to the existing simulation UI, I think we should support varying locales rather than only en-US

Copy link
Contributor

Choose a reason for hiding this comment

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

};
}, [tokenDecimals, value]);

return (
<ConfirmInfoSection>
<ConfirmInfoRow
Expand All @@ -58,17 +71,21 @@ const PermitSimulation: React.FC<{
<Box style={{ marginLeft: 'auto' }}>
<Box display={Display.Flex}>
<Box display={Display.Inline} marginInlineEnd={1}>
<Text
backgroundColor={BackgroundColor.backgroundAlternative}
borderRadius={BorderRadius.XL}
paddingInline={2}
textAlign={TextAlign.Center}
<Tooltip
position="bottom"
title={tokenValueMaxPrecision}
wrapperStyle={{ minWidth: 0 }}
interactive
>
{formatNumber(
value / Math.pow(10, tokenDecimals),
tokenDecimals,
)}
</Text>
<Text
backgroundColor={BackgroundColor.backgroundAlternative}
borderRadius={BorderRadius.XL}
paddingInline={2}
textAlign={TextAlign.Center}
>
{tokenValue}
</Text>
</Tooltip>
</Box>
<Name value={verifyingContract} type={NameType.ETHEREUM_ADDRESS} />
</Box>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -606,3 +606,102 @@ exports[`DataTree should match snapshot 1`] = `
</div>
</div>
`;

exports[`DataTree should match snapshot for permit signature type 1`] = `
<div>
<div
class="mm-box mm-box--width-full"
>
<div
class="mm-box confirm-info-row mm-box--margin-top-2 mm-box--margin-bottom-2 mm-box--padding-right-2 mm-box--padding-left-2 mm-box--display-flex mm-box--flex-direction-row mm-box--flex-wrap-wrap mm-box--justify-content-space-between mm-box--color-text-default mm-box--rounded-lg"
style="overflow-wrap: anywhere; min-height: 24px; padding-right: 0px;"
>
<div
class="mm-box mm-box--display-flex mm-box--flex-direction-row mm-box--justify-content-center mm-box--align-items-center"
>
<p
class="mm-box mm-text mm-text--body-md-medium mm-box--color-inherit"
>
Types:
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
style="white-space: pre-wrap;"
/>
</div>
</div>
<div
class="mm-box confirm-info-row mm-box--margin-top-2 mm-box--margin-bottom-2 mm-box--padding-right-2 mm-box--padding-left-2 mm-box--display-flex mm-box--flex-direction-row mm-box--flex-wrap-wrap mm-box--justify-content-space-between mm-box--color-text-default mm-box--rounded-lg"
style="overflow-wrap: anywhere; min-height: 24px; padding-right: 0px;"
>
<div
class="mm-box mm-box--display-flex mm-box--flex-direction-row mm-box--justify-content-center mm-box--align-items-center"
>
<p
class="mm-box mm-text mm-text--body-md-medium mm-box--color-inherit"
>
PrimaryType:
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
style="white-space: pre-wrap;"
/>
</div>
</div>
<div
class="mm-box confirm-info-row mm-box--margin-top-2 mm-box--margin-bottom-2 mm-box--padding-right-2 mm-box--padding-left-2 mm-box--display-flex mm-box--flex-direction-row mm-box--flex-wrap-wrap mm-box--justify-content-space-between mm-box--color-text-default mm-box--rounded-lg"
style="overflow-wrap: anywhere; min-height: 24px; padding-right: 0px;"
>
<div
class="mm-box mm-box--display-flex mm-box--flex-direction-row mm-box--justify-content-center mm-box--align-items-center"
>
<p
class="mm-box mm-text mm-text--body-md-medium mm-box--color-inherit"
>
Domain:
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
style="white-space: pre-wrap;"
/>
</div>
</div>
<div
class="mm-box confirm-info-row mm-box--margin-top-2 mm-box--margin-bottom-2 mm-box--padding-right-2 mm-box--padding-left-2 mm-box--display-flex mm-box--flex-direction-row mm-box--flex-wrap-wrap mm-box--justify-content-space-between mm-box--color-text-default mm-box--rounded-lg"
style="overflow-wrap: anywhere; min-height: 24px; padding-right: 0px;"
>
<div
class="mm-box mm-box--display-flex mm-box--flex-direction-row mm-box--justify-content-center mm-box--align-items-center"
>
<p
class="mm-box mm-text mm-text--body-md-medium mm-box--color-inherit"
>
Message:
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
style="white-space: pre-wrap;"
>
3000
</p>
</div>
</div>
</div>
</div>
`;
11 changes: 11 additions & 0 deletions ui/pages/confirmations/components/confirm/row/dataTree.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';

import { permitSignatureMsg } from '../../../../../../test/data/confirmations/typed_sign';
import mockState from '../../../../../../test/data/mock-state.json';
import { renderWithProvider } from '../../../../../../test/lib/render-helpers';
import configureStore from '../../../../../store/store';
Expand Down Expand Up @@ -69,6 +70,16 @@ describe('DataTree', () => {
expect(container).toMatchSnapshot();
});

it('should match snapshot for permit signature type', () => {
const { container } = renderWithProvider(
<DataTree
data={JSON.parse(permitSignatureMsg.msgParams?.data as string)}
/>,
store,
);
expect(container).toMatchSnapshot();
});

it('correctly renders reverse strings', () => {
const data = {
'Sign into \u202E EVIL': {
Expand Down
Loading