Skip to content

Commit a472bd4

Browse files
bfullamGeorgeGkas
bfullam
and
GeorgeGkas
authored
feat: improve price impact row and warnings (#26390)
## **Description** Previously, the price impact value color was driven by a warning boolean and effectively rendered as default or red. This change updates the value color by threshold so the UI communicates risk levels more clearly: - Price impact `>= 5%` shows warning color (yellow) - Price impact `>= 25%` shows error color (red) - Otherwise it remains the default alternative text color ## **Changelog** CHANGELOG entry: Updated swap price impact text coloring. ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/SWAPS-4020 https://consensyssoftware.atlassian.net/browse/SWAPS-4024 ## **Manual testing steps** ```gherkin Ensure acceptance criteria pass. ``` ## **Screenshots/Recordings** ### **Before** Price impact value color only switched between default and red. ### **After** Price impact value color now maps to thresholds: - default: `< 5%` - yellow: `>= 5%` - red: `>= 25%` ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes the Bridge confirmation flow to conditionally gate execution behind a new price-impact modal and adds navigation resets on quote expiry, which can affect swap completion and modal routing. Risk is mitigated by extensive new unit tests but touches user-critical transaction submission UX. > > **Overview** > Adds a new `PriceImpactModal` (with header/description/footer) and wires it into Bridge modal routes, including new i18n copy for *info*, *warning*, and *high price impact* states. > > Updates `QuoteDetailsCard` to always show a **Price impact** row with an info button that opens the modal, and switches price-impact coloring/iconography to threshold-based view data (warning at `>=5%`, error at `>=25%`) plus safer formatting for missing/invalid/negative values. > > Refactors swap submission by introducing `useBridgeConfirm` and updating `SwapsConfirmButton` to accept/forward an explicit analytics `location`; if price impact meets the error threshold it now navigates to the modal (`Execution` type) instead of submitting immediately. Adds `useModalCloseOnQuoteExpiry` and applies it across Bridge modals to reset the modal stack to `QuoteExpiredModal` when quotes expire. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 3941641. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: GeorgeGkas <georgegkas@gmail.com>
1 parent c3d9fe3 commit a472bd4

34 files changed

Lines changed: 2726 additions & 271 deletions

‎app/components/UI/Bridge/Views/BridgeView/BridgeView.test.tsx‎

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@ import { Hex } from '@metamask/utils';
1414
import BridgeView from '.';
1515
import type { BridgeRouteParams } from '../../hooks/useSwapBridgeNavigation';
1616
import { createBridgeTestState } from '../../testUtils';
17-
import { RequestStatus, type QuoteResponse } from '@metamask/bridge-controller';
17+
import {
18+
MetaMetricsSwapsEventSource,
19+
RequestStatus,
20+
type QuoteResponse,
21+
} from '@metamask/bridge-controller';
1822
import { SolScope } from '@metamask/keyring-api';
1923
import { mockUseBridgeQuoteData } from '../../_mocks_/useBridgeQuoteData.mock';
2024
import { useBridgeQuoteData } from '../../hooks/useBridgeQuoteData';
@@ -1577,6 +1581,65 @@ describe('BridgeView', () => {
15771581
});
15781582
});
15791583

1584+
describe('location forwarding', () => {
1585+
it('forwards route.params.location to SwapsConfirmButton via price impact modal navigation', async () => {
1586+
mockRoute.params = {
1587+
sourcePage: 'test',
1588+
location: MetaMetricsSwapsEventSource.MainView,
1589+
} as BridgeRouteParams;
1590+
1591+
// A priceImpact above the error threshold (25) causes handleContinue to
1592+
// navigate to the PriceImpactModal — the location value is embedded in
1593+
// the navigation params, making this the easiest observable side-effect
1594+
// to assert for location forwarding.
1595+
jest
1596+
.mocked(useBridgeQuoteData as unknown as jest.Mock)
1597+
.mockImplementation(() => ({
1598+
...mockUseBridgeQuoteData,
1599+
activeQuote: mockQuoteWithMetadata,
1600+
formattedQuoteData: {
1601+
...mockUseBridgeQuoteData.formattedQuoteData,
1602+
priceImpact: '30%',
1603+
},
1604+
}));
1605+
1606+
const testState = createBridgeTestState(
1607+
{
1608+
bridgeControllerOverrides: {
1609+
quotesLoadingStatus: RequestStatus.FETCHED,
1610+
quotes: [mockQuoteWithMetadata as unknown as QuoteResponse],
1611+
quotesLastFetched: Date.now(),
1612+
},
1613+
bridgeReducerOverrides: {
1614+
sourceAmount: '1.0',
1615+
},
1616+
},
1617+
mockState,
1618+
);
1619+
1620+
const { getByTestId } = renderScreen(
1621+
BridgeView,
1622+
{ name: Routes.BRIDGE.ROOT },
1623+
{ state: testState },
1624+
);
1625+
1626+
await act(async () => {
1627+
fireEvent.press(getByTestId(BridgeViewSelectorsIDs.CONFIRM_BUTTON));
1628+
});
1629+
1630+
await waitFor(() => {
1631+
expect(mockNavigate).toHaveBeenCalledWith(
1632+
Routes.BRIDGE.MODALS.ROOT,
1633+
expect.objectContaining({
1634+
params: expect.objectContaining({
1635+
location: MetaMetricsSwapsEventSource.MainView,
1636+
}),
1637+
}),
1638+
);
1639+
});
1640+
});
1641+
});
1642+
15801643
describe('gas included support hooks', () => {
15811644
beforeEach(() => {
15821645
jest.clearAllMocks();

‎app/components/UI/Bridge/Views/BridgeView/index.tsx‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,9 @@ const BridgeView = () => {
147147
const isSolanaSourced = useSelector(selectIsSolanaSourced);
148148
const isDestNetworkEnabled = useIsNetworkEnabled(destToken?.chainId);
149149

150+
/** The entry point location for analytics (e.g. Main View, Token View, Trending Explore) */
151+
const location = route.params?.location;
152+
150153
// inputRef is used to programmatically blur the input field after a delay
151154
// This gives users time to type before the keyboard disappears
152155
// The ref is typed to only expose the blur method we need
@@ -419,7 +422,10 @@ const BridgeView = () => {
419422
/>
420423
)}
421424

422-
<SwapsConfirmButton latestSourceBalance={latestSourceBalance} />
425+
<SwapsConfirmButton
426+
location={location}
427+
latestSourceBalance={latestSourceBalance}
428+
/>
423429
<Box flexDirection={FlexDirection.Row} alignItems={AlignItems.center}>
424430
<Text variant={TextVariant.BodySM} color={TextColor.Alternative}>
425431
{hasFee
@@ -536,6 +542,7 @@ const BridgeView = () => {
536542
{contentMode === 'quote' ? (
537543
<Box style={styles.quoteContainer}>
538544
<QuoteDetailsCard
545+
location={location}
539546
hasInsufficientBalance={hasInsufficientBalance}
540547
/>
541548
</Box>
@@ -557,6 +564,7 @@ const BridgeView = () => {
557564
>
558565
{sourceAmount && sourceAmount !== '0' ? (
559566
<SwapsConfirmButton
567+
location={location}
560568
latestSourceBalance={latestSourceBalance}
561569
testID={BridgeViewSelectorsIDs.CONFIRM_BUTTON_KEYPAD}
562570
/>

‎app/components/UI/Bridge/components/PriceImpactModal/PriceImpactDescription.test.tsx‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
import React from 'react';
2+
import { render } from '@testing-library/react-native';
3+
import { PriceImpactDescription } from './PriceImpactDescription';
4+
import { PriceImpactModalType } from './constants';
5+
import { strings } from '../../../../../../locales/i18n';
6+
7+
describe('PriceImpactDescription', () => {
8+
describe('Execution type', () => {
9+
it('renders the execution description with the given priceImpact', () => {
10+
const { getByText } = render(
11+
<PriceImpactDescription
12+
type={PriceImpactModalType.Execution}
13+
priceImpact="-30%"
14+
/>,
15+
);
16+
17+
expect(
18+
getByText(
19+
strings('bridge.price_impact_execution_description', {
20+
priceImpact: '-30%',
21+
}),
22+
),
23+
).toBeTruthy();
24+
});
25+
26+
it('renders the execution description with "0" when priceImpact is undefined', () => {
27+
const { getByText } = render(
28+
<PriceImpactDescription
29+
type={PriceImpactModalType.Execution}
30+
priceImpact={undefined}
31+
/>,
32+
);
33+
34+
expect(
35+
getByText(
36+
strings('bridge.price_impact_execution_description', {
37+
priceImpact: '0',
38+
}),
39+
),
40+
).toBeTruthy();
41+
});
42+
43+
it('does not render the info description', () => {
44+
const { queryByText } = render(
45+
<PriceImpactDescription
46+
type={PriceImpactModalType.Execution}
47+
priceImpact="-30%"
48+
/>,
49+
);
50+
51+
expect(
52+
queryByText(strings('bridge.price_impact_info_description')),
53+
).toBeNull();
54+
});
55+
});
56+
57+
describe('Info type — with priceImpact (warning state)', () => {
58+
it('renders the warning description with the given priceImpact', () => {
59+
const { getByText } = render(
60+
<PriceImpactDescription
61+
type={PriceImpactModalType.Info}
62+
priceImpact="-10%"
63+
/>,
64+
);
65+
66+
expect(
67+
getByText(
68+
strings('bridge.price_impact_warning_description', {
69+
priceImpact: '-10%',
70+
}),
71+
),
72+
).toBeTruthy();
73+
});
74+
75+
it('does not render the info description when priceImpact is provided', () => {
76+
const { queryByText } = render(
77+
<PriceImpactDescription
78+
type={PriceImpactModalType.Info}
79+
priceImpact="-10%"
80+
/>,
81+
);
82+
83+
expect(
84+
queryByText(strings('bridge.price_impact_info_description')),
85+
).toBeNull();
86+
});
87+
88+
it('treats the string "0" as a truthy priceImpact and renders the warning description', () => {
89+
const { getByText } = render(
90+
<PriceImpactDescription
91+
type={PriceImpactModalType.Info}
92+
priceImpact="0"
93+
/>,
94+
);
95+
96+
expect(
97+
getByText(
98+
strings('bridge.price_impact_warning_description', {
99+
priceImpact: '0',
100+
}),
101+
),
102+
).toBeTruthy();
103+
});
104+
});
105+
106+
describe('Info type — without priceImpact (info state)', () => {
107+
it('renders the info description when priceImpact is undefined', () => {
108+
const { getByText } = render(
109+
<PriceImpactDescription
110+
type={PriceImpactModalType.Info}
111+
priceImpact={undefined}
112+
/>,
113+
);
114+
115+
expect(
116+
getByText(strings('bridge.price_impact_info_description')),
117+
).toBeTruthy();
118+
});
119+
120+
it('renders the info description when priceImpact is an empty string', () => {
121+
const { getByText } = render(
122+
<PriceImpactDescription
123+
type={PriceImpactModalType.Info}
124+
priceImpact=""
125+
/>,
126+
);
127+
128+
expect(
129+
getByText(strings('bridge.price_impact_info_description')),
130+
).toBeTruthy();
131+
});
132+
133+
it('does not render the warning description when priceImpact is absent', () => {
134+
const { queryByText } = render(
135+
<PriceImpactDescription
136+
type={PriceImpactModalType.Info}
137+
priceImpact={undefined}
138+
/>,
139+
);
140+
141+
expect(
142+
queryByText(
143+
strings('bridge.price_impact_warning_description', {
144+
priceImpact: undefined,
145+
}),
146+
),
147+
).toBeNull();
148+
});
149+
});
150+
151+
describe('priority — Execution type takes precedence over warning state', () => {
152+
it('renders the execution description rather than the warning description when type is Execution and priceImpact is provided', () => {
153+
const { getByText, queryByText } = render(
154+
<PriceImpactDescription
155+
type={PriceImpactModalType.Execution}
156+
priceImpact="-10%"
157+
/>,
158+
);
159+
160+
expect(
161+
getByText(
162+
strings('bridge.price_impact_execution_description', {
163+
priceImpact: '-10%',
164+
}),
165+
),
166+
).toBeTruthy();
167+
168+
expect(
169+
queryByText(
170+
strings('bridge.price_impact_warning_description', {
171+
priceImpact: '-10%',
172+
}),
173+
),
174+
).toBeNull();
175+
});
176+
});
177+
});

‎app/components/UI/Bridge/components/PriceImpactModal/PriceImpactDescription.tsx‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import React, { useMemo } from 'react';
2+
import { strings } from '../../../../../../locales/i18n';
3+
import { Box, Text, TextColor } from '@metamask/design-system-react-native';
4+
import { PriceImpactModalType } from './constants';
5+
6+
interface PriceImpactDescriptionProps {
7+
type: PriceImpactModalType;
8+
priceImpact?: string;
9+
}
10+
11+
export function PriceImpactDescription({
12+
type,
13+
priceImpact,
14+
}: PriceImpactDescriptionProps) {
15+
const isWarning = Boolean(priceImpact);
16+
17+
const body = useMemo(() => {
18+
if (type === PriceImpactModalType.Execution) {
19+
return strings('bridge.price_impact_execution_description', {
20+
priceImpact: priceImpact ?? '0',
21+
});
22+
}
23+
if (isWarning) {
24+
return strings('bridge.price_impact_warning_description', {
25+
priceImpact: priceImpact ?? '0',
26+
});
27+
}
28+
return strings('bridge.price_impact_info_description');
29+
}, [type, priceImpact, isWarning]);
30+
31+
return (
32+
<Box paddingHorizontal={4} paddingVertical={2}>
33+
<Text color={TextColor.TextAlternative}>{body}</Text>
34+
</Box>
35+
);
36+
}

0 commit comments

Comments
 (0)