-
Notifications
You must be signed in to change notification settings - Fork 5.4k
feat: Adding code to capture metrics for dapp swaps #36885
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
base: main
Are you sure you want to change the base?
Changes from all commits
c6e3040
ac4116b
6d9b5bd
4f5a040
df5310c
69bf689
a20fbc6
097da15
a5a9c8d
85b196f
ed0efc0
c7f4536
431e2c2
20be473
78488b9
c5177d1
bec49c0
dddb6bc
68908f3
761438f
cb5ecd7
2bc194b
e8b0252
54ce7ff
a7a0225
0deb589
d32985d
e8c1115
b07c21a
7333da1
24a64ca
94f2e93
35bd6ca
79b7ca9
1274e71
42c9ecd
366dda6
a6c074c
9bd2d91
f9ef3e0
8dba70b
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 |
---|---|---|
@@ -0,0 +1,31 @@ | ||
import React from 'react'; | ||
import configureMockStore from 'redux-mock-store'; | ||
|
||
import { getMockConfirmStateForTransaction } from '../../../../../../test/data/confirmations/helper'; | ||
import { renderWithConfirmContextProvider } from '../../../../../../test/lib/confirmations/render-helpers'; | ||
import { genUnapprovedContractInteractionConfirmation } from '../../../../../../test/data/confirmations/contract-interaction'; | ||
import { DappSwapComparisonBanner } from './dapp-swap-comparison-banner'; | ||
|
||
jest.mock('../../../hooks/transactions/useDappSwapComparisonInfo', () => ({ | ||
useDappSwapComparisonInfo: jest.fn(() => undefined), | ||
})); | ||
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. Bug: Mock Path Mismatch Causes Test IneffectivenessThe |
||
|
||
function render() { | ||
const state = getMockConfirmStateForTransaction( | ||
genUnapprovedContractInteractionConfirmation(), | ||
); | ||
|
||
const mockStore = configureMockStore()(state); | ||
|
||
return renderWithConfirmContextProvider( | ||
<DappSwapComparisonBanner />, | ||
mockStore, | ||
); | ||
} | ||
|
||
describe('<DappSwapComparisonBanner />', () => { | ||
it('renders component without errors', () => { | ||
const { container } = render(); | ||
expect(container).toBeEmptyDOMElement(); | ||
}); | ||
}); |
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. With the new folder structure we're using, should this just be in 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. I moved it from |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import 'react'; | ||
|
||
import { useDappSwapComparisonInfo } from '../../../hooks/transactions/useDappSwapComparisonInfo'; | ||
|
||
// The component is conditionally included for dapp swap origin | ||
// The only purpose of the component currently is to capture dapp swap comparison related metrics. | ||
|
||
export const DappSwapComparisonBanner = () => { | ||
useDappSwapComparisonInfo(); | ||
|
||
return null; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,15 @@ | ||
import { TransactionMeta } from '@metamask/transaction-controller'; | ||
import React from 'react'; | ||
import { useConfirmContext } from '../../../../context/confirm'; | ||
import { DappSwapComparisonBanner } from '../../dapp-swap-comparison-banner/dapp-swap-comparison-banner'; | ||
import { AdvancedDetails } from '../shared/advanced-details/advanced-details'; | ||
import { GasFeesSection } from '../shared/gas-fees-section/gas-fees-section'; | ||
import { TransactionDetails } from '../shared/transaction-details/transaction-details'; | ||
import { TransactionAccountDetails } from '../batch/transaction-account-details'; | ||
import { BatchSimulationDetails } from '../batch/batch-simulation-details/batch-simulation-details'; | ||
|
||
const DAPP_SWAP_COMPARISON_ORIGIN = 'https://app.uniswap.org'; | ||
|
||
const BaseTransactionInfo = () => { | ||
const { currentConfirmation: transactionMeta } = | ||
useConfirmContext<TransactionMeta>(); | ||
|
@@ -17,6 +20,9 @@ const BaseTransactionInfo = () => { | |
|
||
return ( | ||
<> | ||
{transactionMeta.origin === DAPP_SWAP_COMPARISON_ORIGIN && ( | ||
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. Is it cleaner to encapsulate this origin knowledge inside the 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. The reason I put this condition here is that even if I keep it inside By keeping this condition here all that code execution is by-passed. |
||
<DappSwapComparisonBanner /> | ||
)} | ||
<TransactionAccountDetails /> | ||
<BatchSimulationDetails /> | ||
<TransactionDetails /> | ||
|
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.
Ideally this would go in the
bridge-controller-init
file in theapi
property.