Skip to content

Commit de10c46

Browse files
authored
feat: improve xchain swaps slippage settings with decimals and warnings (#29617)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> The slippage settings for xchain transactions was not allowing users to input a decimal (i.e. 0.5%), only whole numbers (0, 1, so on...). It also did not display a warning for low slippage settings. This addresses both by allowing the user to input decimal numbers, and displaying a warning when the custom slippage is set to <0.5% in the settings modal. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29617?quickstart=1) ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/MMS-1830 ## **Manual testing steps** 1. Go to the xchain swaps transaction preparation page. 2. Edit slippage settings. 3. Notice decimals are now allowed, and a warning is displayed with low slippage. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <img width="465" alt="Screenshot 2025-01-09 at 4 49 04 PM" src="https://github.com/user-attachments/assets/b7930dab-166d-4ad9-a622-efa118b6a196" /> <!-- [screenshots/recordings] --> ### **After** <img width="483" alt="Screenshot 2025-01-09 at 2 29 30 PM" src="https://github.com/user-attachments/assets/2918f0c3-98a6-4b28-ae88-42ea166c4921" /> <img width="489" alt="Screenshot 2025-01-09 at 2 29 22 PM" src="https://github.com/user-attachments/assets/0a7fdcdd-8057-4818-a22c-4ac7e1f3620e" /> <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] 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-extension/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). - [x] 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.
1 parent 840f214 commit de10c46

File tree

1 file changed

+54
-28
lines changed

1 file changed

+54
-28
lines changed

ui/pages/bridge/prepare/bridge-transaction-settings-modal.tsx

+54-28
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ import {
1616
Text,
1717
TextField,
1818
TextFieldType,
19+
BannerAlert,
20+
BannerAlertSeverity,
21+
Box,
1922
} from '../../../components/component-library';
2023
import { useI18nContext } from '../../../hooks/useI18nContext';
2124
import {
@@ -26,6 +29,7 @@ import {
2629
JustifyContent,
2730
TextColor,
2831
TextVariant,
32+
SEVERITIES,
2933
} from '../../../helpers/constants/design-system';
3034
import { getSlippage } from '../../../ducks/bridge/selectors';
3135
import { setSlippage } from '../../../ducks/bridge/actions';
@@ -50,13 +54,32 @@ export const BridgeTransactionSettingsModal = ({
5054
const [localSlippage, setLocalSlippage] = useState<number | undefined>(
5155
slippage,
5256
);
53-
const [customSlippage, setCustomSlippage] = useState<number | undefined>(
54-
slippage && HARDCODED_SLIPPAGE_OPTIONS.includes(slippage)
55-
? undefined
56-
: slippage,
57+
const [customSlippage, setCustomSlippage] = useState<string | undefined>(
58+
slippage && !HARDCODED_SLIPPAGE_OPTIONS.includes(slippage)
59+
? slippage.toString()
60+
: undefined,
5761
);
5862
const [showCustomButton, setShowCustomButton] = useState(true);
5963

64+
const getNotificationConfig = () => {
65+
if (!customSlippage) {
66+
return null;
67+
}
68+
69+
const slippageValue = Number(customSlippage.replace(',', '.'));
70+
if (slippageValue < 0.5) {
71+
return {
72+
severity: SEVERITIES.WARNING,
73+
text: t('swapSlippageLowDescription', [slippageValue]),
74+
title: t('swapSlippageLowTitle'),
75+
};
76+
}
77+
78+
return null;
79+
};
80+
81+
const notificationConfig = getNotificationConfig();
82+
6083
return (
6184
<Modal isOpen={isOpen} onClose={onClose} className="bridge-settings-modal">
6285
<ModalOverlay />
@@ -156,56 +179,59 @@ export const BridgeTransactionSettingsModal = ({
156179
type={TextFieldType.Text}
157180
value={customSlippage}
158181
onChange={(e) => {
159-
// Remove characters that are not numbers or decimal points if rendering a controlled or pasted value
160-
const cleanedValue = e.target.value.replace(/[^0-9.]+/gu, '');
161-
setLocalSlippage(undefined);
162-
setCustomSlippage(
163-
cleanedValue.length > 0 ? Number(cleanedValue) : undefined,
164-
);
182+
const { value } = e.target;
183+
if (value === '' || /^\d*[.,]?\d*$/u.test(value)) {
184+
setLocalSlippage(undefined);
185+
setCustomSlippage(value);
186+
}
165187
}}
166188
autoFocus={true}
167189
onBlur={() => {
168-
console.log('====blur');
169190
setShowCustomButton(true);
170191
}}
171192
onFocus={() => {
172-
console.log('====focus');
173193
setShowCustomButton(false);
174194
}}
175-
onKeyPress={(e?: React.KeyboardEvent<HTMLDivElement>) => {
176-
// Only allow numbers and at most one decimal point
177-
if (
178-
e &&
179-
!/^[0-9]*\.{0,1}[0-9]*$/u.test(
180-
`${customSlippage ?? ''}${e.key}`,
181-
)
182-
) {
183-
e.preventDefault();
184-
}
185-
}}
186195
endAccessory={<Text variant={TextVariant.bodyMd}>%</Text>}
187196
/>
188197
)}
189198
</Row>
199+
{notificationConfig && (
200+
<Box marginTop={5}>
201+
<BannerAlert
202+
severity={notificationConfig.severity as BannerAlertSeverity}
203+
title={notificationConfig.title}
204+
titleProps={{ 'data-testid': 'swaps-banner-title' }}
205+
>
206+
<Text>{notificationConfig.text}</Text>
207+
</BannerAlert>
208+
</Box>
209+
)}
190210
</Column>
191211
<ModalFooter>
192212
<ButtonPrimary
193213
width={BlockSize.Full}
194214
size={ButtonPrimarySize.Md}
195215
variant={TextVariant.bodyMd}
196-
disabled={(localSlippage || customSlippage) === slippage}
216+
disabled={
217+
(customSlippage !== undefined &&
218+
Number(customSlippage.replace(',', '.')) === slippage) ||
219+
(localSlippage !== undefined && localSlippage === slippage)
220+
}
197221
onClick={() => {
198-
const newSlippage = localSlippage || customSlippage;
199-
newSlippage &&
222+
const newSlippage =
223+
localSlippage ?? Number(customSlippage?.replace(',', '.'));
224+
if (newSlippage) {
200225
trackCrossChainSwapsEvent({
201226
event: MetaMetricsEventName.InputChanged,
202227
properties: {
203228
input: 'slippage',
204229
value: newSlippage.toString(),
205230
},
206231
});
207-
dispatch(setSlippage(newSlippage));
208-
onClose();
232+
dispatch(setSlippage(newSlippage));
233+
onClose();
234+
}
209235
}}
210236
>
211237
{t('submit')}

0 commit comments

Comments
 (0)