Repository navigation
refactor(bridge): migrate the slippage settings modal to MMDS - #47683
Open
georgewrmarshall wants to merge 5 commits into
Open
georgewrmarshall wants to merge 5 commits into
georgewrmarshall wants to merge 5 commits into
Conversation
georgewrmarshall
temporarily deployed
to
pr-comment
November 5, 2026 22:06 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
✨ Files requiring CODEOWNER review ✨🔄 @MetaMask/swaps-engineers (3 files, +195 -170)
|
metamask-ci
Bot
added
the
team-design-system
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
|
Bugbot Autofix prepared a fix for the issue found in the latest run.
Or push these changes by commenting: Preview (1f747521cc)diff --git a/ui/pages/bridge/prepare/__snapshots__/bridge-transaction-settings-modal.test.tsx.snap b/ui/pages/bridge/prepare/__snapshots__/bridge-transaction-settings-modal.test.tsx.snap
--- a/ui/pages/bridge/prepare/__snapshots__/bridge-transaction-settings-modal.test.tsx.snap
+++ b/ui/pages/bridge/prepare/__snapshots__/bridge-transaction-settings-modal.test.tsx.snap
@@ -109,7 +109,7 @@
%
</button>
<div
- class="inline-flex items-center rounded-xl border transition-colors h-10 pl-0 pr-4 border-default w-24 bg-default"
+ class="inline-flex items-center rounded-xl border transition-colors h-10 pl-0 pr-4 w-24 border-muted bg-default"
>
<input
class="w-full rounded text-default outline-none transition-colors border-transparent placeholder:text-alternative text-s-body-md leading-s-body-md tracking-s-body-md md:text-l-body-md md:leading-l-body-md md:tracking-l-body-md font-regular m-0 h-full min-w-0 flex-1 border-0 bg-transparent p-0 pl-4 pr-2 truncate"
diff --git a/ui/pages/bridge/prepare/bridge-transaction-settings-modal.tsx b/ui/pages/bridge/prepare/bridge-transaction-settings-modal.tsx
--- a/ui/pages/bridge/prepare/bridge-transaction-settings-modal.tsx
+++ b/ui/pages/bridge/prepare/bridge-transaction-settings-modal.tsx
@@ -217,7 +217,7 @@
{showCustomInput && (
<TextField
size={TextFieldSize.Md}
- className="w-24 bg-default"
+ className="w-24 border-muted bg-default"
type={TextFieldType.Text}
value={inputValue}
autoFocus={true}You can send follow-ups to the cloud agent here. |
georgewrmarshall
temporarily deployed
to
pr-comment
November 5, 2026 22:19 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Builds ready [ffb986d]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 2 warn · 🔴 4 fail)
Bundle Size Diffs
|
Accessibility (draft)I know this is still a draft, so no rush. Just flagging for when you wrap up. The custom slippage field in An AI accessibility agent posted this. If this is wrong, reply here. |
Replace the legacy Modal, Button, Text and TextField in the bridge transaction settings modal with @metamask/design-system-react. Layout moves to MMDS Box and Tailwind. The slippage tooltip stays the bridge Tooltip, which still needs z-index 1051 above the modal. Co-authored-by: George Marshall <georgewrmarshall@users.noreply.github.com>
georgewrmarshall
force-pushed
the
cursor/bridge-settings-modal-mmds-9438
branch
from
November 6, 2026 19:48
ffb986d to
a3167b7
Compare
Contributor
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
The custom slippage input had no accessible name once opened. Set aria-label to the existing "Slippage tolerance" string on the native input and cover it in the modal test and snapshot. Also drop the stale bg-default class from the snapshot left over from the background color removal. Add a TODO pointing to metamask-design-system#1667 for removing the data-testid cast once the design system accepts data-* in inputProps. Co-authored-by: Cursor <cursoragent@cursor.com>
georgewrmarshall
temporarily deployed
to
pr-comment
November 6, 2026 20:47 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
SonarCloud flags the global isNaN calls in the slippage settings modal. Both values are already numbers, so Number.isNaN keeps the same result. Co-authored-by: Cursor <cursoragent@cursor.com>
georgewrmarshall
temporarily deployed
to
pr-comment
November 6, 2026 20:49 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
georgewrmarshall
marked this pull request as ready for review
November 6, 2026 20:58
georgewrmarshall
temporarily deployed
to
pr-comment
November 6, 2026 20:58 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
georgewrmarshall
temporarily deployed
to
pr-comment
November 6, 2026 21:04 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Builds ready [3e4d897]
⚡ Performance Benchmarks (Total: 🟢 6 pass · 🟡 5 warn · 🔴 2 fail)
Bundle Size Diffs
|
|
3 of 7 tasks
…ridge-settings-modal-mmds-9438 Co-authored-by: George Marshall <georgewrmarshall@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Option labels skip Button text styles
- Changed hardcoded slippage labels to template strings so MMDS Button wraps them in body-md Text like Auto and Custom.
Or push these changes by commenting:
@cursor push 0c2d4c1cf6
Preview (0c2d4c1cf6)
diff --git a/ui/pages/bridge/prepare/__snapshots__/bridge-transaction-settings-modal.test.tsx.snap b/ui/pages/bridge/prepare/__snapshots__/bridge-transaction-settings-modal.test.tsx.snap
--- a/ui/pages/bridge/prepare/__snapshots__/bridge-transaction-settings-modal.test.tsx.snap
+++ b/ui/pages/bridge/prepare/__snapshots__/bridge-transaction-settings-modal.test.tsx.snap
@@ -98,15 +98,21 @@
class="inline-flex items-center justify-center rounded-full px-4 font-medium overflow-hidden relative h-10 transition-all duration-100 ease-linear active:scale-[0.97] active:ease-[cubic-bezier(0.3,0.8,0.3,1)] bg-muted text-default hover:bg-muted-hover active:bg-muted-pressed focus-visible:ring-0 focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary-default hover:!bg-default-hover"
role="button"
>
- 0.5
- %
+ <span
+ class="text-inherit text-s-body-md leading-s-body-md tracking-s-body-md md:text-l-body-md md:leading-l-body-md md:tracking-l-body-md font-medium font-default"
+ >
+ 0.5%
+ </span>
</button>
<button
class="inline-flex items-center justify-center rounded-full px-4 font-medium overflow-hidden relative h-10 transition-all duration-100 ease-linear active:scale-[0.97] active:ease-[cubic-bezier(0.3,0.8,0.3,1)] bg-muted text-default hover:bg-muted-hover active:bg-muted-pressed focus-visible:ring-0 focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary-default hover:!bg-default-hover"
role="button"
>
- 2
- %
+ <span
+ class="text-inherit text-s-body-md leading-s-body-md tracking-s-body-md md:text-l-body-md md:leading-l-body-md md:tracking-l-body-md font-medium font-default"
+ >
+ 2%
+ </span>
</button>
<div
class="inline-flex items-center rounded-xl border bg-muted transition-colors h-10 pl-0 pr-4 border-default w-24"
@@ -248,15 +254,21 @@
class="inline-flex items-center justify-center rounded-full px-4 font-medium overflow-hidden relative h-10 transition-all duration-100 ease-linear active:scale-[0.97] active:ease-[cubic-bezier(0.3,0.8,0.3,1)] bg-muted text-default hover:bg-muted-hover active:bg-muted-pressed focus-visible:ring-0 focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary-default hover:!bg-default-hover"
role="button"
>
- 0.5
- %
+ <span
+ class="text-inherit text-s-body-md leading-s-body-md tracking-s-body-md md:text-l-body-md md:leading-l-body-md md:tracking-l-body-md font-medium font-default"
+ >
+ 0.5%
+ </span>
</button>
<button
class="inline-flex items-center justify-center rounded-full px-4 font-medium overflow-hidden relative h-10 transition-all duration-100 ease-linear active:scale-[0.97] active:ease-[cubic-bezier(0.3,0.8,0.3,1)] bg-muted text-default hover:bg-muted-hover active:bg-muted-pressed focus-visible:ring-0 focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary-default hover:!bg-default-hover"
role="button"
>
- 2
- %
+ <span
+ class="text-inherit text-s-body-md leading-s-body-md tracking-s-body-md md:text-l-body-md md:leading-l-body-md md:tracking-l-body-md font-medium font-default"
+ >
+ 2%
+ </span>
</button>
<button
class="inline-flex items-center justify-center rounded-full px-4 font-medium overflow-hidden relative h-10 transition-all duration-100 ease-linear active:scale-[0.97] active:ease-[cubic-bezier(0.3,0.8,0.3,1)] bg-muted text-default hover:bg-muted-hover active:bg-muted-pressed focus-visible:ring-0 focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary-default hover:!bg-default-hover"
diff --git a/ui/pages/bridge/prepare/bridge-transaction-settings-modal.tsx b/ui/pages/bridge/prepare/bridge-transaction-settings-modal.tsx
--- a/ui/pages/bridge/prepare/bridge-transaction-settings-modal.tsx
+++ b/ui/pages/bridge/prepare/bridge-transaction-settings-modal.tsx
@@ -197,7 +197,7 @@
selectSlippageOption(hardcodedSlippage);
}}
>
- {hardcodedSlippage}%
+ {`${hardcodedSlippage}%`}
</Button>
);
})}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 592a453. Configure here.
| onClick={(e: React.MouseEvent<HTMLAnchorElement>) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| onClick={() => { |
Contributor
There was a problem hiding this comment.
Option labels skip Button text styles
Medium Severity
The 0.5% and 2% option labels are a number child plus a % text node, so MMDS Button does not wrap them in its Text span. Auto and Custom stay single strings and keep text-s-body-md / md:text-l-body-md. Those preset labels therefore lose design-system size, leading, and tracking, so they can render differently from the other options.
Reviewed by Cursor Bugbot for commit 592a453. Configure here.
Contributor
Builds ready [592a453]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 2 warn · 🔴 4 fail)
Bundle Size Diffs
|
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Part of the MMDS migration epic: https://consensyssoftware.atlassian.net/browse/TMCU-1741
Stack: #47558 is merged. This PR is stacked on #47597, which is stacked on #47596. It does not belong in #47596: the slippage modal is a separate surface (legacy
Modal,Button,Text, andTextField), not the input-group migration.ui/pages/bridge/prepare/bridge-transaction-settings-modal.tsxnow uses@metamask/design-system-reactfor the dialog, option buttons, label, custom field, and submit button. Layout that was bridgeRow/Columnplus design-system spacing enums is an MMDSBoxwith Tailwind (flex,gap-3,px-4,pb-4).ButtonVariant.Primary(bg-icon-default). Unselected isButtonVariant.Secondarywithhover:!bg-default-hover, so hover stays the previous default-hover fill instead of MMDShover:bg-muted-hover.TextField(rounded-xl, muted border, 40pxMdheight) withw-24 bg-default.bg-defaultkeeps the legacy field fill; MMDS otherwise usesbg-muted. The%suffix is an MMDSTextend accessory. Paste and decimal-key filtering stay oninputProps.ModalFooterprimaryButtonProps(ButtonSize.Lg, full width of the footer row).isDisabledreplacesdisabled.data-testid="bridge__tx-settings-modal-close-button"and passesariaLabel={t('close')}, which MMDSModalHeaderrequires.Left on purpose: the slippage info icon is still the bridge
Tooltip(../layout), which renders a legacyPopover. It keepsstyle={{ zIndex: 1051 }}so the popover sits above the modal. There is no MMDS tooltip, and Tailwind in this repo has noz-1051step.Changelog
CHANGELOG entry: null
Related issues
https://consensyssoftware.atlassian.net/browse/TMCU-1741
Manual testing steps
1.5, blur, and submit. Reopen and confirm Custom shows1.5%.Screenshots/Recordings
Bridge transaction settings modal, DAI → mUSD (
getBridgeFixtures+enableFiatToggle), captured at 2x in the side panel (360×768) and the popup (400×600). Modal opened viadata-testid="bridge__header-settings-button".Before
Legacy
Modal/Button/Text/TextFieldon #47597 head138b2aecf6.After
MMDS modal chrome, option buttons, custom slippage field, and submit on #47683 head
ffb986d045.Pre-merge author checklist
Pre-merge reviewer checklist
Note
Low Risk
UI refactor on a swap settings surface with existing test coverage; slippage dispatch logic is unchanged aside from presentation and a11y attributes.
Reviewed by Cursor Bugbot for commit 592a453. Bugbot is set up for automated code reviews on this repo. Configure here.