-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix: tempo disable confirm button until gas autoselect #28722
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
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 |
|---|---|---|
|
|
@@ -39,6 +39,7 @@ import { PredictClaimFooter } from '../predict-confirmations/predict-claim-foote | |
| import { useIsTransactionPayLoading } from '../../hooks/pay/useTransactionPayData'; | ||
| import { Skeleton } from '../../../../../component-library/components-temp/Skeleton'; | ||
| import { useQRHardwareContext } from '../../context/qr-hardware-context'; | ||
| import { useIsGaslessLoading } from '../../hooks/gas/useIsGaslessLoading'; | ||
|
|
||
| const HIDE_FOOTER_BY_DEFAULT_TYPES = [ | ||
| TransactionType.moneyAccountDeposit, | ||
|
|
@@ -71,6 +72,7 @@ export const Footer = () => { | |
| TRANSFER_TRANSACTION_TYPES.includes(transactionType) && | ||
| transactionMetadata?.origin === MMM_ORIGIN; | ||
| const isPayLoading = useIsTransactionPayLoading(); | ||
| const { isGaslessLoading } = useIsGaslessLoading(); | ||
|
|
||
| const { isFooterVisible: isFooterVisibleFlag, isTransactionValueUpdating } = | ||
| useConfirmationContext(); | ||
|
|
@@ -158,7 +160,8 @@ export const Footer = () => { | |
| needsCameraPermission || | ||
| hasBlockingAlerts || | ||
| isTransactionValueUpdating || | ||
| isPayLoading; | ||
| isPayLoading || | ||
| isGaslessLoading; | ||
|
Member
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. Does this have larger implications if mobile wasn't previously disabled while gas station was loading?
Contributor
Author
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. Yes it might actually feel less flaky on slower devices - the "Confirm" button blinks much more on Mobile than on Extension, maybe due to |
||
|
|
||
| const buttons = [ | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,209 @@ | ||
| import { GasFeeToken } from '@metamask/transaction-controller'; | ||
| import { Hex } from '@metamask/utils'; | ||
|
|
||
| import { useIsGaslessSupported } from './useIsGaslessSupported'; | ||
| import { useHasInsufficientBalance } from '../useHasInsufficientBalance'; | ||
| import { useIsGaslessLoading } from './useIsGaslessLoading'; | ||
| import { renderHookWithProvider } from '../../../../../util/test/renderWithProvider'; | ||
| import { useTransactionMetadataRequest } from '../transactions/useTransactionMetadataRequest'; | ||
| import { selectUseTransactionSimulations } from '../../../../../selectors/preferencesController'; | ||
|
|
||
| jest.mock('./useIsGaslessSupported'); | ||
| jest.mock('../useHasInsufficientBalance'); | ||
| jest.mock('../transactions/useTransactionMetadataRequest'); | ||
| jest.mock('../../../../../selectors/preferencesController'); | ||
|
|
||
| const mockedUseIsGaslessSupported = jest.mocked(useIsGaslessSupported); | ||
| const mockedUseHasInsufficientBalance = jest.mocked(useHasInsufficientBalance); | ||
| const mockedUseTransactionMetadataRequest = jest.mocked( | ||
| useTransactionMetadataRequest, | ||
| ); | ||
| const mockSelectUseTransactionSimulations = jest.mocked( | ||
| selectUseTransactionSimulations, | ||
| ); | ||
|
|
||
| async function runHook({ | ||
| simulationEnabled, | ||
| gaslessSupported, | ||
| insufficientBalance, | ||
| pending = false, | ||
| isSmartTransaction = true, | ||
| gasFeeTokens, | ||
| excludeNativeTokenForFee, | ||
| selectedGasFeeToken, | ||
| }: { | ||
| simulationEnabled: boolean; | ||
| gaslessSupported: boolean; | ||
| insufficientBalance: boolean; | ||
| pending?: boolean; | ||
| isSmartTransaction?: boolean; | ||
| gasFeeTokens?: GasFeeToken[]; | ||
| excludeNativeTokenForFee?: boolean; | ||
| selectedGasFeeToken?: Hex; | ||
| }) { | ||
| mockedUseIsGaslessSupported.mockReturnValue({ | ||
| isSupported: gaslessSupported, | ||
| isSmartTransaction, | ||
| pending, | ||
| }); | ||
| mockedUseHasInsufficientBalance.mockReturnValue({ | ||
| hasInsufficientBalance: insufficientBalance, | ||
| nativeCurrency: 'USD', | ||
| }); | ||
| mockedUseTransactionMetadataRequest.mockReturnValue({ | ||
| gasFeeTokens, | ||
| excludeNativeTokenForFee, | ||
| selectedGasFeeToken, | ||
| } as unknown as ReturnType<typeof useTransactionMetadataRequest>); | ||
| mockSelectUseTransactionSimulations.mockReturnValue(simulationEnabled); | ||
| const { result } = renderHookWithProvider(useIsGaslessLoading); | ||
| return result.current; | ||
| } | ||
|
|
||
| describe('useIsGaslessLoading', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('returns true if pending', async () => { | ||
| const result = await runHook({ | ||
| simulationEnabled: true, | ||
| gaslessSupported: true, | ||
| insufficientBalance: true, | ||
| pending: true, | ||
| }); | ||
|
|
||
| expect(result.isGaslessLoading).toBe(true); | ||
| }); | ||
|
|
||
| it('returns false if simulation is disabled', async () => { | ||
| const result = await runHook({ | ||
| simulationEnabled: false, | ||
| gaslessSupported: true, | ||
| insufficientBalance: true, | ||
| }); | ||
|
|
||
| expect(result.isGaslessLoading).toBe(false); | ||
| }); | ||
|
|
||
| it('returns false if gasless is not supported', async () => { | ||
| const result = await runHook({ | ||
| simulationEnabled: true, | ||
| gaslessSupported: false, | ||
| insufficientBalance: true, | ||
| }); | ||
|
|
||
| expect(result.isGaslessLoading).toBe(false); | ||
| }); | ||
|
|
||
| it('returns false if there is no insufficient balance', async () => { | ||
| const result = await runHook({ | ||
| simulationEnabled: true, | ||
| gaslessSupported: true, | ||
| insufficientBalance: false, | ||
| }); | ||
|
|
||
| expect(result.isGaslessLoading).toBe(false); | ||
| }); | ||
|
|
||
| it('returns true if gas fee tokens are undefined (still loading)', async () => { | ||
| const result = await runHook({ | ||
| simulationEnabled: true, | ||
| gaslessSupported: true, | ||
| insufficientBalance: true, | ||
| gasFeeTokens: undefined, // this triggers loading | ||
| }); | ||
|
|
||
| expect(result.isGaslessLoading).toBe(true); | ||
| }); | ||
|
|
||
| it('returns false if gas fee tokens are present', async () => { | ||
| const result = await runHook({ | ||
| simulationEnabled: true, | ||
| gaslessSupported: true, | ||
| insufficientBalance: true, | ||
| gasFeeTokens: [{ tokenAddress: '0x123' }] as unknown as GasFeeToken[], | ||
| }); | ||
|
|
||
| expect(result.isGaslessLoading).toBe(false); | ||
| }); | ||
|
|
||
| it('returns false if gas fee tokens is an empty array', async () => { | ||
| const result = await runHook({ | ||
| simulationEnabled: true, | ||
| gaslessSupported: true, | ||
| insufficientBalance: true, | ||
| gasFeeTokens: [], | ||
| }); | ||
|
|
||
| expect(result.isGaslessLoading).toBe(false); | ||
| }); | ||
|
|
||
| it('returns false if gas fee tokens are present and dont match selectedGasFeeToken (non-reg)', async () => { | ||
| const result = await runHook({ | ||
| simulationEnabled: true, | ||
| gaslessSupported: true, | ||
| insufficientBalance: true, | ||
| gasFeeTokens: [{ tokenAddress: '0x123' }] as unknown as GasFeeToken[], | ||
| selectedGasFeeToken: '0x456', | ||
| }); | ||
|
|
||
| expect(result.isGaslessLoading).toBe(false); | ||
| }); | ||
|
|
||
| it('returns true if gas fee tokens are present and dont match selectedGasFeeToken with excludeNativeTokenForFee being true (Tempo)', async () => { | ||
| const result = await runHook({ | ||
| simulationEnabled: true, | ||
| gaslessSupported: true, | ||
| insufficientBalance: true, | ||
| gasFeeTokens: [{ tokenAddress: '0x123' }] as unknown as GasFeeToken[], | ||
| selectedGasFeeToken: '0x456', | ||
| excludeNativeTokenForFee: true, | ||
| }); | ||
|
|
||
| expect(result.isGaslessLoading).toBe(true); | ||
| }); | ||
|
|
||
| it('returns false if gas fee tokens are present AND match selectedGasFeeToken with excludeNativeTokenForFee being true (Tempo)', async () => { | ||
| const result = await runHook({ | ||
| simulationEnabled: true, | ||
| gaslessSupported: true, | ||
| insufficientBalance: true, | ||
| gasFeeTokens: [ | ||
| { tokenAddress: '0x123' }, | ||
| { tokenAddress: '0x789' }, | ||
| ] as unknown as GasFeeToken[], | ||
| // Matches the first one | ||
| selectedGasFeeToken: '0x123', | ||
| excludeNativeTokenForFee: true, | ||
| }); | ||
|
|
||
| expect(result.isGaslessLoading).toBe(false); | ||
| }); | ||
|
|
||
| it('returns false if gas fee tokens are present AND selectedGasFeeToken is undefined with excludeNativeTokenForFee being true (Tempo)', async () => { | ||
| const result = await runHook({ | ||
| simulationEnabled: true, | ||
| gaslessSupported: true, | ||
| insufficientBalance: true, | ||
| gasFeeTokens: [{ tokenAddress: '0x123' }] as unknown as GasFeeToken[], | ||
| selectedGasFeeToken: undefined, | ||
| excludeNativeTokenForFee: true, | ||
| }); | ||
|
|
||
| expect(result.isGaslessLoading).toBe(false); | ||
| }); | ||
|
|
||
| it('returns false if gas fee tokens is empty array with excludeNativeTokenForFee being true (Tempo)', async () => { | ||
| const result = await runHook({ | ||
| simulationEnabled: true, | ||
| gaslessSupported: true, | ||
| insufficientBalance: true, | ||
| gasFeeTokens: [] as unknown as GasFeeToken[], | ||
| selectedGasFeeToken: '0x456', | ||
| excludeNativeTokenForFee: true, | ||
| }); | ||
|
|
||
| expect(result.isGaslessLoading).toBe(false); | ||
| }); | ||
| }); |
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.
We wouldn't need any changes in Predict code if we didn't change to
useQuery?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.
No but I couldn't find any clean alternative.
I modified those 2 test files after making the changes in renderWithProvider.tsx