Skip to content

Commit b0db4ba

Browse files
fix: dismiss Batch Sell final review bottom sheet after submit (MetaMask#31842)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until this PR meets the canonical Definition of Ready For Review in `docs/readme/ready-for-review.md`. In short: the template must be materially complete (not just section titles present), all status checks must be currently passing, and the only expected follow-up commits must be reviewer-driven. --> <!-- mms-check directive vocabulary — read by .github/scripts/shared/pr-template-checks.ts at module load to build the validation plan. Directives are invisible in rendered markdown and must NOT be removed or edited without updating the validator registry. type=text Section must contain non-placeholder prose. type=changelog Section must have a valid CHANGELOG entry: line. type=issue-link Section must have a Fixes:/Closes:/Refs: line with a value. type=manual-testing Section must have real testing steps or an explicit N/A. type=screenshot Section must have evidence (image/URL) or an explicit N/A. type=checklist Section must have all checkboxes consciously checked. required=true|false Whether a missing/invalid section runs the validator at all. blocking=true|false Whether a failure of this check fails the CI workflow. Default: false — failures are shown as warnings in the sticky comment but do not block the PR. Sections without a directive are checked for structural presence only. --> ## **Description** <!-- mms-check: type=text required=true --> After submitting a Batch Sell transaction, `BatchSellFinalReviewModal` navigated to the Activity tab without dismissing the final review bottom sheet. The modal stack stayed on screen as a transparent overlay, so users saw the activity list with the review sheet still visible. This change closes the sheet via `onCloseBottomSheet` before navigating to `Routes.TRANSACTIONS_VIEW`, matching the pattern used in `PostTradeBottomSheet` when viewing activity. The close button also routes through the same sheet ref so dismiss behavior is consistent. ## **Changelog** <!-- mms-check: type=changelog required=true blocking=true --> CHANGELOG entry: Fixed Batch Sell final review bottom sheet persisting after submitting a transaction. ## **Related issues** <!-- mms-check: type=issue-link required=true --> Fixes: MetaMask#31839 ## **Manual testing steps** <!-- mms-check: type=manual-testing required=true --> ```gherkin Feature: Batch Sell final review bottom sheet dismiss Scenario: user submits a Batch Sell transaction Given the user has selected multiple tokens for Batch Sell And quotes have loaded on the final review bottom sheet And the user has sufficient funds for gas When the user taps Sell all and the transaction submits successfully Then the final review bottom sheet dismisses And the app navigates to the Activity list And no Batch Sell review sheet remains visible over Activity Scenario: user closes the final review sheet without submitting Given the user is on the Batch Sell final review bottom sheet When the user taps the close button Then the bottom sheet dismisses And the user returns to the Batch Sell review screen ``` ## **Screenshots/Recordings** <!-- mms-check: type=screenshot required=true --> ### **Before** See issue screenshot: MetaMask#31839 — Activity list visible with the final review bottom sheet still overlaying the screen after submit. ### **After** https://github.com/user-attachments/assets/94a53e32-eded-46fb-bc33-c3a1c9f188c7 ## **Pre-merge author checklist** <!-- mms-check: type=checklist required=true --> - [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. #### Performance checks (if applicable) - [x] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [x] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [x] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **Pre-merge reviewer checklist** <!-- Reviewer checklist items follow the same semantics as the author checklist: an unchecked box is ambiguous, a checked box means the reviewer consciously assessed that responsibility. See `docs/readme/ready-for-review.md`. --> - [ ] 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. Made with [Cursor](https://cursor.com)
1 parent 2b5571b commit b0db4ba

2 files changed

Lines changed: 67 additions & 5 deletions

File tree

app/components/UI/Bridge/components/BatchSellFinalReviewModal/BatchSellFinalReviewModal.test.tsx

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import { BatchSellFinalReviewModalSelectorsIDs } from './BatchSellFinalReviewMod
1111
const mockGoBack = jest.fn();
1212
const mockNavigate = jest.fn();
1313
const mockReplace = jest.fn();
14+
const mockOnCloseBottomSheet = jest.fn((callback?: () => void) => {
15+
callback?.();
16+
});
1417
const mockDispatch = jest.fn();
1518
const mockUpdateBatchSellQuoteParams = jest.fn();
1619
const mockGetNewQuote = jest.fn();
@@ -143,6 +146,34 @@ const defaultQuoteData: MockBatchSellQuoteData = {
143146
let mockSelectedTokens = defaultSelectedTokens;
144147
let mockBatchSellQuoteData = defaultQuoteData;
145148

149+
jest.mock('@metamask/design-system-react-native', () => {
150+
const ReactActual = jest.requireActual('react');
151+
const actual = jest.requireActual('@metamask/design-system-react-native');
152+
const { View } = jest.requireActual('react-native');
153+
154+
return {
155+
...actual,
156+
BottomSheet: ReactActual.forwardRef(
157+
(
158+
{
159+
children,
160+
testID,
161+
}: {
162+
children?: React.ReactNode;
163+
testID?: string;
164+
},
165+
ref: React.Ref<{ onCloseBottomSheet: (callback?: () => void) => void }>,
166+
) => {
167+
ReactActual.useImperativeHandle(ref, () => ({
168+
onCloseBottomSheet: mockOnCloseBottomSheet,
169+
}));
170+
171+
return ReactActual.createElement(View, { testID }, children);
172+
},
173+
),
174+
};
175+
});
176+
146177
jest.mock('@react-navigation/native', () => ({
147178
useNavigation: () => ({
148179
goBack: mockGoBack,
@@ -274,6 +305,27 @@ describe('BatchSellFinalReviewModal', () => {
274305
expect(mockNavigate).toHaveBeenCalledWith(Routes.TRANSACTIONS_VIEW);
275306
});
276307

308+
it('closes the sheet before navigating to activity after submit', async () => {
309+
mockOnCloseBottomSheet.mockImplementation((callback?: () => void) => {
310+
callback?.();
311+
});
312+
313+
const { getByTestId } = renderModal();
314+
315+
fireEvent.press(
316+
getByTestId(BatchSellFinalReviewModalSelectorsIDs.SELL_ALL_BUTTON),
317+
);
318+
319+
await waitFor(() => {
320+
expect(mockOnCloseBottomSheet).toHaveBeenCalledTimes(1);
321+
});
322+
expect(mockOnCloseBottomSheet).toHaveBeenCalledWith(expect.any(Function));
323+
expect(mockNavigate).toHaveBeenCalledWith(Routes.TRANSACTIONS_VIEW);
324+
expect(mockOnCloseBottomSheet.mock.invocationCallOrder[0]).toBeLessThan(
325+
mockNavigate.mock.invocationCallOrder[0],
326+
);
327+
});
328+
277329
it('blocks Sell all while submitting', () => {
278330
mockIsSubmittingTx = true;
279331

@@ -290,14 +342,15 @@ describe('BatchSellFinalReviewModal', () => {
290342
).toBe(true);
291343
});
292344

293-
it('closes with navigation when the close button is pressed', () => {
345+
it('closes the sheet when the close button is pressed', () => {
294346
const { getByTestId } = renderModal();
295347

296348
fireEvent.press(
297349
getByTestId(BatchSellFinalReviewModalSelectorsIDs.CLOSE_BUTTON),
298350
);
299351

300-
expect(mockGoBack).toHaveBeenCalledTimes(1);
352+
expect(mockOnCloseBottomSheet).toHaveBeenCalledTimes(1);
353+
expect(mockGoBack).not.toHaveBeenCalled();
301354
});
302355

303356
it('starts collapsed and hides token rows while keeping received summary rows visible', () => {

app/components/UI/Bridge/components/BatchSellFinalReviewModal/index.tsx

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { useNavigation } from '@react-navigation/native';
22
import { StackNavigationProp } from '@react-navigation/stack';
3-
import React, { useCallback, useMemo, useState } from 'react';
3+
import React, { useCallback, useMemo, useRef, useState } from 'react';
44
import { Pressable } from 'react-native';
55
import { useDispatch, useSelector } from 'react-redux';
66
import { useTailwind } from '@metamask/design-system-twrnc-preset';
@@ -10,6 +10,7 @@ import {
1010
AvatarTokenSize,
1111
BottomSheet,
1212
BottomSheetHeader,
13+
BottomSheetRef,
1314
Box,
1415
BoxAlignItems,
1516
BoxFlexDirection,
@@ -303,6 +304,7 @@ export function BatchSellFinalReviewModal() {
303304
networkFee: batchSellQuoteData.networkFee,
304305
});
305306
const surfaceClass = useElevatedSurface();
307+
const sheetRef = useRef<BottomSheetRef>(null);
306308
const [isTokenDetailsExpanded, setIsTokenDetailsExpanded] = useState(false);
307309
const finalReviewQuoteData = useMemo(
308310
() =>
@@ -366,6 +368,10 @@ export function BatchSellFinalReviewModal() {
366368
return strings('bridge.batch_sell_sell_all');
367369
})();
368370

371+
const handleClose = useCallback(() => {
372+
sheetRef.current?.onCloseBottomSheet();
373+
}, []);
374+
369375
const handleToggleTokenDetails = () => {
370376
setIsTokenDetailsExpanded((isExpanded) => !isExpanded);
371377
};
@@ -400,7 +406,9 @@ export function BatchSellFinalReviewModal() {
400406
console.error('Error submitting Batch Sell tx', error);
401407
} finally {
402408
dispatch(setIsSubmittingTx(false));
403-
navigation.navigate(Routes.TRANSACTIONS_VIEW);
409+
sheetRef.current?.onCloseBottomSheet(() => {
410+
navigation.navigate(Routes.TRANSACTIONS_VIEW);
411+
});
404412
}
405413
}, [
406414
batchSellQuoteData.recommendedQuotes,
@@ -411,12 +419,13 @@ export function BatchSellFinalReviewModal() {
411419

412420
return (
413421
<BottomSheet
422+
ref={sheetRef}
414423
testID={BatchSellFinalReviewModalSelectorsIDs.SHEET}
415424
goBack={navigation.goBack}
416425
twClassName={surfaceClass}
417426
>
418427
<BottomSheetHeader
419-
onClose={navigation.goBack}
428+
onClose={handleClose}
420429
closeButtonProps={{
421430
size: ButtonIconSize.Md,
422431
testID: BatchSellFinalReviewModalSelectorsIDs.CLOSE_BUTTON,

0 commit comments

Comments
 (0)