Skip to content

Commit faff79c

Browse files
Matt561Matt561
authored
feat: TAT-1671 remove order type button when closing position (MetaMask#19695)
<!-- 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** `PerpsClosePositionView` Changes: - Removed order type button - Added tooltip to "you'll receive" row - Updated tooltip icon sizes to match designs <!-- 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? --> ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: removed order type button from PerpsClosePositionView CHANGELOG entry: added tooltip to "you'll receive" row on PerpsClosePositionView ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** https://github.com/user-attachments/assets/5e4f2586-f268-4f0d-9d68-b1864212ff34 <!-- [screenshots/recordings] --> ## **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). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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.
1 parent 9a8c06a commit faff79c

7 files changed

Lines changed: 56 additions & 193 deletions

File tree

‎app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.test.tsx‎

Lines changed: 0 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import { strings } from '../../../../../../locales/i18n';
1818
import {
1919
PerpsClosePositionViewSelectorsIDs,
2020
PerpsAmountDisplaySelectorsIDs,
21-
PerpsOrderHeaderSelectorsIDs,
2221
} from '../../../../../../e2e/selectors/Perps/Perps.selectors';
2322

2423
// Mock navigation
@@ -65,10 +64,6 @@ jest.mock('../../components/PerpsSlider/PerpsSlider', () => ({
6564
// Mock PerpsAmountDisplay to allow triggering onPress but keep it simple
6665
jest.mock('../../components/PerpsAmountDisplay');
6766

68-
jest.mock('../../components/PerpsOrderTypeBottomSheet', () => ({
69-
__esModule: true,
70-
default: 'PerpsOrderTypeBottomSheet',
71-
}));
7267
jest.mock('../../components/PerpsLimitPriceBottomSheet', () => ({
7368
__esModule: true,
7469
default: 'PerpsLimitPriceBottomSheet',
@@ -204,20 +199,6 @@ describe('PerpsClosePositionView', () => {
204199
).toBeDefined();
205200
});
206201

207-
it('shows default market order type on initial render', () => {
208-
// Arrange & Act
209-
const { getByText } = renderWithProvider(
210-
<PerpsClosePositionView />,
211-
{
212-
state: STATE_MOCK,
213-
},
214-
true,
215-
);
216-
217-
// Assert
218-
expect(getByText(strings('perps.order.market'))).toBeDefined();
219-
});
220-
221202
it('displays order details section', () => {
222203
// Arrange & Act
223204
const { getByText } = renderWithProvider(
@@ -378,26 +359,6 @@ describe('PerpsClosePositionView', () => {
378359
});
379360
});
380361

381-
describe('Order Type Management', () => {
382-
it('handles order type change from market to limit', () => {
383-
// Given the default market order is displayed
384-
const { getByText } = renderWithProvider(
385-
<PerpsClosePositionView />,
386-
{
387-
state: STATE_MOCK,
388-
},
389-
true,
390-
);
391-
392-
// When order type changes to limit
393-
// Simulate the component re-rendering with limit order type
394-
// Note: In real implementation, this would be triggered by bottom sheet selection
395-
396-
// Then the limit order text should be visible
397-
expect(getByText(strings('perps.order.market'))).toBeDefined();
398-
});
399-
});
400-
401362
describe('Fee Calculations', () => {
402363
it('calculates and displays correct fees', () => {
403364
// Arrange
@@ -997,23 +958,6 @@ describe('PerpsClosePositionView', () => {
997958
});
998959

999960
describe('Limit Order Features', () => {
1000-
it('switches between market and limit order types', () => {
1001-
// Given a close position view
1002-
const { getByTestId, getByText } = renderWithProvider(
1003-
<PerpsClosePositionView />,
1004-
{
1005-
state: STATE_MOCK,
1006-
},
1007-
true,
1008-
);
1009-
1010-
// Then it should show market order by default
1011-
expect(
1012-
getByTestId(PerpsOrderHeaderSelectorsIDs.ORDER_TYPE_BUTTON),
1013-
).toBeDefined();
1014-
expect(getByText(strings('perps.order.market'))).toBeDefined();
1015-
});
1016-
1017961
it('validates limit order requires price', () => {
1018962
// Given validation returns error for missing limit price
1019963
usePerpsClosePositionValidationMock.mockReturnValue({
@@ -1415,29 +1359,6 @@ describe('PerpsClosePositionView', () => {
14151359
});
14161360
});
14171361

1418-
describe('Order Type Selection', () => {
1419-
it('opens order type bottom sheet when order type button is pressed', () => {
1420-
// Arrange
1421-
const { getByTestId } = renderWithProvider(
1422-
<PerpsClosePositionView />,
1423-
{
1424-
state: STATE_MOCK,
1425-
},
1426-
true,
1427-
);
1428-
1429-
// Act - Press the order type button
1430-
const orderTypeButton = getByTestId(
1431-
PerpsOrderHeaderSelectorsIDs.ORDER_TYPE_BUTTON,
1432-
);
1433-
fireEvent.press(orderTypeButton);
1434-
1435-
// Assert - Order type selection should trigger state change
1436-
// The actual bottom sheet rendering is handled by the PerpsOrderTypeBottomSheet component
1437-
expect(orderTypeButton).toBeDefined();
1438-
});
1439-
});
1440-
14411362
describe('Tooltip Management', () => {
14421363
it('handles tooltip interactions for closing fees', () => {
14431364
// Arrange
@@ -1494,28 +1415,6 @@ describe('PerpsClosePositionView', () => {
14941415
});
14951416
});
14961417

1497-
describe('Limit Order Auto-Open', () => {
1498-
it('opens limit price bottom sheet when switching to limit order without price', () => {
1499-
// Arrange
1500-
const { getByTestId } = renderWithProvider(
1501-
<PerpsClosePositionView />,
1502-
{
1503-
state: STATE_MOCK,
1504-
},
1505-
true,
1506-
);
1507-
1508-
// Act - Press order type button to trigger limit order selection
1509-
const orderTypeButton = getByTestId(
1510-
PerpsOrderHeaderSelectorsIDs.ORDER_TYPE_BUTTON,
1511-
);
1512-
fireEvent.press(orderTypeButton);
1513-
1514-
// Assert - Bottom sheet should be triggered
1515-
expect(orderTypeButton).toBeDefined();
1516-
});
1517-
});
1518-
15191418
describe('Navigation Callbacks', () => {
15201419
it('navigates back on successful position close', async () => {
15211420
// Arrange
@@ -1844,46 +1743,6 @@ describe('PerpsClosePositionView', () => {
18441743
});
18451744
});
18461745

1847-
describe('Limit Order Auto-Open Logic', () => {
1848-
it('auto-opens limit price sheet when switching to limit without price', () => {
1849-
// Arrange
1850-
const TestComponent = () => {
1851-
const [orderType, setOrderType] = React.useState<'market' | 'limit'>(
1852-
'market',
1853-
);
1854-
const [limitPrice] = React.useState('');
1855-
const [isLimitPriceVisible, setIsLimitPriceVisible] =
1856-
React.useState(false);
1857-
1858-
// Simulate the useEffect logic from the component
1859-
React.useEffect(() => {
1860-
if (orderType === 'limit' && !limitPrice) {
1861-
setIsLimitPriceVisible(true);
1862-
}
1863-
}, [orderType, limitPrice]);
1864-
1865-
return (
1866-
<View>
1867-
<TouchableOpacity onPress={() => setOrderType('limit')}>
1868-
<Text>Switch to Limit</Text>
1869-
</TouchableOpacity>
1870-
<Text testID="limit-sheet-visible">
1871-
{isLimitPriceVisible.toString()}
1872-
</Text>
1873-
</View>
1874-
);
1875-
};
1876-
1877-
// Act
1878-
const { getByTestId, getByText } = render(<TestComponent />);
1879-
const switchButton = getByText('Switch to Limit');
1880-
fireEvent.press(switchButton);
1881-
1882-
// Assert - Should auto-open limit price sheet
1883-
expect(getByTestId('limit-sheet-visible').props.children).toBe('true');
1884-
});
1885-
});
1886-
18871746
describe('Input Focus State Management', () => {
18881747
it('shows keypad and hides validation when input is focused', () => {
18891748
// Arrange

‎app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.tsx‎

Lines changed: 23 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ import {
4747
} from '../../utils/positionCalculations';
4848
import PerpsSlider from '../../components/PerpsSlider/PerpsSlider';
4949
import PerpsAmountDisplay from '../../components/PerpsAmountDisplay';
50-
import PerpsOrderTypeBottomSheet from '../../components/PerpsOrderTypeBottomSheet';
5150
import PerpsLimitPriceBottomSheet from '../../components/PerpsLimitPriceBottomSheet';
5251
import PerpsBottomSheetTooltip from '../../components/PerpsBottomSheetTooltip';
5352
import { PerpsTooltipContentKey } from '../../components/PerpsBottomSheetTooltip/PerpsBottomSheetTooltip.types';
@@ -91,7 +90,6 @@ const PerpsClosePositionView: React.FC = () => {
9190

9291
// State for order type and bottom sheets
9392
const [orderType, setOrderType] = useState<OrderType>('market');
94-
const [isOrderTypeVisible, setIsOrderTypeVisible] = useState(false);
9593
const [isLimitPriceVisible, setIsLimitPriceVisible] = useState(false);
9694
const [isInputFocused, setIsInputFocused] = useState(false);
9795
const [isUserInputActive, setIsUserInputActive] = useState(false);
@@ -458,13 +456,14 @@ const PerpsClosePositionView: React.FC = () => {
458456
<TouchableOpacity
459457
onPress={() => handleTooltipPress('closing_fees')}
460458
style={styles.labelWithTooltip}
459+
testID={PerpsClosePositionViewSelectorsIDs.FEES_TOOLTIP_BUTTON}
461460
>
462461
<Text variant={TextVariant.BodyMD} color={TextColor.Alternative}>
463462
{strings('perps.close_position.fees')}
464463
</Text>
465464
<Icon
466465
name={IconName.Info}
467-
size={IconSize.Xs}
466+
size={IconSize.Sm}
468467
color={IconColor.Muted}
469468
/>
470469
</TouchableOpacity>
@@ -478,15 +477,29 @@ const PerpsClosePositionView: React.FC = () => {
478477

479478
<View style={[styles.summaryRow, styles.summaryTotalRow]}>
480479
<View style={styles.summaryLabel}>
481-
<Text variant={TextVariant.BodyMD}>
482-
{strings('perps.close_position.you_receive')}
483-
</Text>
480+
<TouchableOpacity
481+
onPress={() => handleTooltipPress('close_position_you_receive')}
482+
style={styles.labelWithTooltip}
483+
testID={
484+
PerpsClosePositionViewSelectorsIDs.YOU_RECEIVE_TOOLTIP_BUTTON
485+
}
486+
>
487+
<Text variant={TextVariant.BodyMD}>
488+
{strings('perps.close_position.you_receive')}
489+
</Text>
490+
<Icon
491+
name={IconName.Info}
492+
size={IconSize.Sm}
493+
color={IconColor.Muted}
494+
/>
495+
</TouchableOpacity>
484496
</View>
485497
<View style={styles.summaryValue}>
486-
<Text variant={TextVariant.BodyMD}>
487-
{formatPrice(Math.max(0, receiveAmount), {
488-
maximumDecimals: 2,
489-
})}
498+
<Text
499+
variant={TextVariant.BodyMD}
500+
color={receiveAmount > 0 ? TextColor.Success : TextColor.Default}
501+
>
502+
{formatPrice(receiveAmount, { maximumDecimals: 2 })}
490503
</Text>
491504
</View>
492505
</View>
@@ -501,8 +514,6 @@ const PerpsClosePositionView: React.FC = () => {
501514
priceChange={parseFloat(
502515
priceData[position.coin]?.percentChange24h ?? '0',
503516
)}
504-
orderType={orderType}
505-
onOrderTypePress={() => setIsOrderTypeVisible(true)}
506517
title={strings('perps.close_position.title')}
507518
isLoading={isClosing}
508519
/>
@@ -695,23 +706,6 @@ const PerpsClosePositionView: React.FC = () => {
695706
)}
696707
</View>
697708

698-
{/* Order Type Bottom Sheet */}
699-
<PerpsOrderTypeBottomSheet
700-
isVisible={isOrderTypeVisible}
701-
onClose={() => setIsOrderTypeVisible(false)}
702-
onSelect={(type) => {
703-
setOrderType(type);
704-
// Clear limit price when switching to market order
705-
if (type === 'market') {
706-
setLimitPrice('');
707-
}
708-
setIsOrderTypeVisible(false);
709-
}}
710-
currentOrderType={orderType}
711-
asset={position.coin}
712-
direction={isLong ? 'long' : 'short'}
713-
/>
714-
715709
{/* Limit Price Bottom Sheet */}
716710
<PerpsLimitPriceBottomSheet
717711
isVisible={isLimitPriceVisible}

‎app/components/UI/Perps/components/PerpsBottomSheetTooltip/PerpsBottomSheetTooltip.types.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,5 @@ export type PerpsTooltipContentKey =
3939
| 'geo_block'
4040
| 'estimated_pnl'
4141
| 'limit_price'
42-
| 'tp_sl';
42+
| 'tp_sl'
43+
| 'close_position_you_receive';

‎app/components/UI/Perps/components/PerpsBottomSheetTooltip/content/contentRegistry.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ export const tooltipContentRegistry: ContentRegistry = {
2323
open_interest: undefined,
2424
funding_rate: undefined,
2525
geo_block: undefined,
26-
estimated_pnl: undefined, // Uses default string content
27-
limit_price: undefined, // Uses default string content
26+
estimated_pnl: undefined,
27+
limit_price: undefined,
2828
tp_sl: undefined,
29+
close_position_you_receive: undefined,
2930
};

‎app/components/UI/Perps/components/PerpsOrderHeader/PerpsOrderHeader.tsx‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ interface PerpsOrderHeaderProps {
2525
asset: string;
2626
price: number;
2727
priceChange: number;
28-
orderType: OrderType;
28+
orderType?: OrderType;
2929
direction?: 'long' | 'short';
3030
onBack?: () => void;
3131
title?: string;
@@ -108,25 +108,27 @@ const PerpsOrderHeader: React.FC<PerpsOrderHeaderProps> = ({
108108
)}
109109
</View>
110110
</View>
111-
<TouchableOpacity
112-
onPress={handleOrderTypePress}
113-
testID={PerpsOrderHeaderSelectorsIDs.ORDER_TYPE_BUTTON}
114-
disabled={isLoading}
115-
>
116-
<View style={styles.marketButton}>
117-
<Text variant={TextVariant.BodyMD} color={TextColor.Default}>
118-
{orderType === 'market'
119-
? strings('perps.order.market')
120-
: strings('perps.order.limit')}
121-
</Text>
122-
<Icon
123-
name={IconName.ArrowDown}
124-
size={IconSize.Xs}
125-
color={IconColor.Default}
126-
style={styles.marketButtonIcon}
127-
/>
128-
</View>
129-
</TouchableOpacity>
111+
{Boolean(orderType) && (
112+
<TouchableOpacity
113+
onPress={handleOrderTypePress}
114+
testID={PerpsOrderHeaderSelectorsIDs.ORDER_TYPE_BUTTON}
115+
disabled={isLoading}
116+
>
117+
<View style={styles.marketButton}>
118+
<Text variant={TextVariant.BodyMD} color={TextColor.Default}>
119+
{orderType === 'market'
120+
? strings('perps.order.market')
121+
: strings('perps.order.limit')}
122+
</Text>
123+
<Icon
124+
name={IconName.ArrowDown}
125+
size={IconSize.Xs}
126+
color={IconColor.Default}
127+
style={styles.marketButtonIcon}
128+
/>
129+
</View>
130+
</TouchableOpacity>
131+
)}
130132
</View>
131133
);
132134
};

‎e2e/selectors/Perps/Perps.selectors.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,8 @@ export const PerpsClosePositionViewSelectorsIDs = {
425425
DISPLAY_TOGGLE_BUTTON: 'display-toggle-button',
426426
CLOSE_POSITION_CONFIRM_BUTTON: 'close-position-confirm-button',
427427
CLOSE_POSITION_CANCEL_BUTTON: 'close-position-cancel-button',
428+
FEES_TOOLTIP_BUTTON: 'close-position-fees-tooltip-button',
429+
YOU_RECEIVE_TOOLTIP_BUTTON: 'close-position-you-receive-tooltip-button',
428430
};
429431

430432
// ========================================

‎locales/languages/en.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1372,6 +1372,10 @@
13721372
"title": "Take Profit & Stop Loss",
13731373
"content": "Take Profit (TP) automatically closes your position when you reach your target profit. Stop Loss (SL) limits your losses by closing your position if the price moves against you."
13741374
},
1375+
"close_position_you_receive": {
1376+
"title": "Receive amount",
1377+
"content": "Your receive amount is estimated and may change slightly due to slippage."
1378+
},
13751379
"notifications": {
13761380
"title": "Turn on notifications",
13771381
"description": "Get notified about important perps trading events like order fills, liquidations, and market updates.",

0 commit comments

Comments
 (0)