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: duplicate fee display for l2 networks #24264

Merged
merged 4 commits into from
May 1, 2024
Merged

fix: duplicate fee display for l2 networks #24264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a52b4b8
Fix duplicate fee display for l2 networks
jpuri Apr 26, 2024
7173065
Merge branch 'develop' into l2_layer_display_fix
jpuri Apr 26, 2024
b63e5d0
update
jpuri Apr 27, 2024
db06164
Merge branch 'l2_layer_display_fix' of github.com:MetaMask/metamask-e…
jpuri Apr 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 0 additions & 40 deletions ...s/components/confirm-gas-display/confirm-legacy-gas-display/confirm-legacy-gas-display.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ import {
TextColor,
} from '../../../../../helpers/constants/design-system';
import { useDraftTransactionWithTxParams } from '../../../hooks/useDraftTransactionWithTxParams';
import { getNativeCurrency } from '../../../../../ducks/metamask/metamask';
import MultilayerFeeMessage from '../../multilayer-fee-message/multi-layer-fee-message';
import {
Icon,
IconName,
Expand All @@ -41,7 +39,6 @@ const ConfirmLegacyGasDisplay = ({ 'data-testid': dataTestId } = {}) => {
const isMainnet = useSelector(getIsMainnet);
const useCurrencyRateCheck = useSelector(getUseCurrencyRateCheck);
const { useNativeCurrencyAsPrimaryCurrency } = useSelector(getPreferences);
const nativeCurrency = useSelector(getNativeCurrency);
const unapprovedTxs = useSelector(getUnapprovedTransactions);
const transactionData = useDraftTransactionWithTxParams();
const txData = useSelector((state) => txDataSelector(state));
Expand All @@ -52,43 +49,6 @@ const ConfirmLegacyGasDisplay = ({ 'data-testid': dataTestId } = {}) => {
const { hexMinimumTransactionFee, hexMaximumTransactionFee } = useSelector(
(state) => transactionFeeSelector(state, transaction),
);
const { layer1GasFee } = txData;
const hasLayer1GasFee = layer1GasFee !== undefined;

if (hasLayer1GasFee) {
return [
<TransactionDetailItem
key="legacy-total-item"
data-testid={dataTestId}
detailTitle={t('transactionDetailLayer2GasHeading')}
detailTotal={
<UserPreferencedCurrencyDisplay
type={PRIMARY}
value={hexMinimumTransactionFee}
hideLabel={!useNativeCurrencyAsPrimaryCurrency}
numberOfDecimals={18}
/>
}
detailText={
useCurrencyRateCheck && (
<UserPreferencedCurrencyDisplay
type={SECONDARY}
value={hexMinimumTransactionFee}
hideLabel={Boolean(useNativeCurrencyAsPrimaryCurrency)}
/>
)
}
noBold
flexWidthValues
/>,
<MultilayerFeeMessage
key="confirm-layer-1"
transaction={txData}
layer2fee={hexMinimumTransactionFee}
nativeCurrency={nativeCurrency}
/>,
];
}

return (
<TransactionDetailItem
Expand Down
6 changes: 3 additions & 3 deletions ...ponents/confirm-gas-display/confirm-legacy-gas-display/confirm-legacy-gas-display.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ describe('ConfirmLegacyGasDisplay', () => {
});
});

it('should contain L1 L2 fee details', async () => {
it('should display Estimated gas fee for L2 networks', async () => {
render({
...mmState,
confirmTransaction: {
Expand All @@ -124,8 +124,8 @@ describe('ConfirmLegacyGasDisplay', () => {
});

await waitFor(() => {
expect(screen.queryByText('Layer 1 fees')).toBeInTheDocument();
expect(screen.queryByText('Layer 2 gas fee')).toBeInTheDocument();
expect(screen.queryByText('Estimated gas fee')).toBeInTheDocument();
expect(screen.queryByText('Max fee:')).toBeInTheDocument();
});
});
});
18 changes: 10 additions & 8 deletions ui/pages/confirmations/components/fee-details-component/fee-details-component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,14 +140,16 @@ export default function FeeDetailsComponent({
boldHeadings={false}
/>
)}
<TransactionDetailItem
detailTitle={t('total')}
detailText={
useCurrencyRateCheck &&
renderTotalDetailText(getTransactionFeeTotal)
}
detailTotal={renderTotalDetailValue(getTransactionFeeTotal)}
/>
{!hasLayer1GasFee && (
<TransactionDetailItem
detailTitle={t('total')}
detailText={
useCurrencyRateCheck &&
renderTotalDetailText(getTransactionFeeTotal)
}
detailTotal={renderTotalDetailValue(getTransactionFeeTotal)}
/>
)}
</Box>
)}
</>
Expand Down
31 changes: 30 additions & 1 deletion ui/pages/confirmations/components/fee-details-component/fee-details-component.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ describe('FeeDetailsComponent', () => {
await act(async () => {
screen.getByRole('button').click();
});
expect(screen.getAllByTitle('0 ETH')).toHaveLength(3);
expect(screen.getAllByTitle('0 ETH')).toHaveLength(2);
expect(screen.getAllByTitle('0 ETH')[0]).toBeInTheDocument();
});

Expand Down Expand Up @@ -89,4 +89,33 @@ describe('FeeDetailsComponent', () => {
});
expect(screen.queryByText('Fee details')).toBeInTheDocument();
});

it('should not display total in details section for layer 2 network', async () => {
await render({
...mockState,
metamask: {
...mockState.metamask,
networkDetails: {
EIPS: {
1559: false,
},
},
networksMetadata: {
goerli: {
EIPS: {
1559: false,
},
status: 'available',
},
},
providerConfig: {
chainId: CHAIN_IDS.OPTIMISM,
},
},
});
await act(async () => {
screen.getByRole('button').click();
});
expect(screen.queryByText('Totals')).not.toBeInTheDocument();
});
});
5 changes: 2 additions & 3 deletions ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ describe('Confirm Transaction Base', () => {
expect(securityProviderBanner).toBeInTheDocument();
});

it('should contain L1 L2 fee details for optimism', async () => {
it('should estimated fee details for optimism', async () => {
const state = {
metamask: {
...baseStore.metamask,
Expand All @@ -353,8 +353,7 @@ describe('Confirm Transaction Base', () => {

const { queryByText } = await render({ state });

expect(queryByText('Layer 1 fees')).toBeInTheDocument();
expect(queryByText('Layer 2 gas fee')).toBeInTheDocument();
expect(queryByText('Estimated fee')).not.toBeInTheDocument();
});

it('should render NoteToTrader when isNoteToTraderSupported is true', async () => {
Expand Down