Skip to content

Commit 4f4f756

Browse files
authored
fix(perps): limit price percentage buttons now compound on current price (#29373)
## **Description** Pressing `-1%` or `-2%` in the "Set limit price" modal only changed the displayed price on the first press. Subsequent presses had no effect. Root cause: `calculatePriceForPercentage` always used the live market price as the base. Every press returned the same calculated value, so React skipped the state update entirely. Fix: use the currently displayed `limitPrice` as the base for each calculation, falling back to market price only when the field is empty. Each press now compounds correctly ($78,181 → $77,399 → $76,625 → …). Regression introduced in #24069 (TAT-2114). ## **Changelog** CHANGELOG entry: Fixed a bug where the -1%/-2% limit price preset buttons only applied on the first press. ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TAT-3097 ## **Manual testing steps** \`\`\`gherkin Feature: Limit price percentage presets Scenario: User presses -1% multiple times Given user is on the order form with direction set to Long When user taps "Limit" order type and opens the limit price modal And user taps the "-1%" button once Then limit price decreases by 1% from market price When user taps the "-1%" button again Then limit price decreases by another 1% from the previous value When user taps the "-1%" button a third time Then limit price decreases by another 1% again (compounds correctly) \`\`\` ## **Screenshots/Recordings** ### **Before** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/e3a9de99-a0ba-4f65-a0fb-f163021acef4 ### **After** [<!-- [screenshots/recordings] --> ](https://github.com/user-attachments/assets/4fb3e130-b802-477c-af0d-5e664dc93556) ## **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. #### 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 ## **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] > **Low Risk** > Small, localized change to UI price calculation logic with added test coverage; minimal risk beyond preset price behavior. > > **Overview** > Fixes the limit-price percentage preset calculation in `PerpsLimitPriceBottomSheet` so `%` buttons compound off the *currently entered* `limitPrice` (sanitized/parsed), falling back to `currentPrice` only when no limit is set. > > Adds a regression test (TAT-3097) asserting the percentage calculation uses `limitPrice` as the BigNumber base when it’s already populated, preventing the “only first press works” bug from returning. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 699decd. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 8cdc767 commit 4f4f756

2 files changed

Lines changed: 38 additions & 6 deletions

File tree

app/components/UI/Perps/components/PerpsLimitPriceBottomSheet/PerpsLimitPriceBottomSheet.test.tsx

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,37 @@ describe('PerpsLimitPriceBottomSheet', () => {
511511
});
512512
});
513513

514+
describe('Compound percentage regression (TAT-3097)', () => {
515+
it('uses limitPrice as base (not currentPrice) when limitPrice is already set', () => {
516+
// Regression: old code always used currentPrice, so repeated presses had no effect
517+
// because the calculated value never changed.
518+
const { BigNumber: MockBigNumber } = jest.requireMock('bignumber.js');
519+
520+
render(
521+
<PerpsLimitPriceBottomSheet
522+
{...defaultProps}
523+
direction="long"
524+
limitPrice="3100"
525+
currentPrice={3000}
526+
/>,
527+
);
528+
529+
// Clear constructor calls that may have occurred during rendering
530+
MockBigNumber.mockClear();
531+
532+
fireEvent.press(screen.getByText('-1%'));
533+
534+
// BigNumber must be called with the limitPrice (3100), not currentPrice (3000).
535+
// The mock's toString() returns its constructor argument, so this assertion
536+
// would fail on the pre-fix code that always passed currentPrice.
537+
const constructorArgs = MockBigNumber.mock.calls.map(
538+
([arg]: [number]) => arg,
539+
);
540+
expect(constructorArgs).toContain(3100);
541+
expect(constructorArgs).not.toContain(3000);
542+
});
543+
});
544+
514545
describe('Preset decimal precision (TAT-2399)', () => {
515546
const xrpProps = {
516547
...defaultProps,

app/components/UI/Perps/components/PerpsLimitPriceBottomSheet/PerpsLimitPriceBottomSheet.tsx

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,8 @@ const PerpsLimitPriceBottomSheet: React.FC<PerpsLimitPriceBottomSheetProps> = ({
284284
}, [limitPrice, currentPrice, direction, isClosingPosition]);
285285

286286
/**
287-
* Calculate limit price based on percentage from current market price
287+
* Calculate limit price based on percentage from the current limit price (or
288+
* market price when no limit price is set yet).
288289
* @param percentage - Percentage to add/subtract from current price
289290
* @returns Calculated price as string (e.g., "45123.50")
290291
*
@@ -293,8 +294,10 @@ const PerpsLimitPriceBottomSheet: React.FC<PerpsLimitPriceBottomSheetProps> = ({
293294
*/
294295
const calculatePriceForPercentage = useCallback(
295296
(percentage: number) => {
296-
// Use the current market price (not limit price) for percentage calculations
297-
const basePrice = currentPrice;
297+
const parsedLimitPrice = limitPrice
298+
? parseFloat(limitPrice.replace(/[$,]/g, ''))
299+
: 0;
300+
const basePrice = parsedLimitPrice > 0 ? parsedLimitPrice : currentPrice;
298301

299302
if (!basePrice || basePrice === 0) {
300303
return '';
@@ -304,11 +307,9 @@ const PerpsLimitPriceBottomSheet: React.FC<PerpsLimitPriceBottomSheetProps> = ({
304307
const calculatedPrice = BigNumber(basePrice)
305308
.multipliedBy(multiplier)
306309
.toString();
307-
// Return the raw numeric value as a string (without formatting)
308-
// The display will format it when needed
309310
return calculatedPrice;
310311
},
311-
[currentPrice],
312+
[currentPrice, limitPrice],
312313
);
313314

314315
const footerButtonProps = [

0 commit comments

Comments
 (0)