fix: resolve prop override in PerpsEmptyState component #20534

georgewrmarshall · 2025-09-29T16:09:20Z

Description

Updated the PerpsEmptyState component to use the onAction prop from TabEmptyStateProps instead of a custom onStartTrading prop. This fixes a prop spreading issue where any onAction prop passed to PerpsEmptyState would override the explicitly set onAction={onStartTrading} due to the {...props} spread coming after the explicit assignment.

What was the issue?
The component had both a custom onStartTrading prop and inherited onAction from TabEmptyStateProps, causing conflicts when props were spread.

What's the solution?
Removed the redundant onStartTrading prop and updated all usages to use the inherited onAction prop, maintaining consistency with the parent TabEmptyState component.

Changelog

CHANGELOG entry: null

Related issues

Fixes: Internal code improvement to resolve prop override issue

Manual testing steps

Feature: PerpsEmptyState component functionality

  Scenario: user interacts with empty state action button
    Given the app is running with no perp positions or orders
    And I navigate to the Perps tab
    When I see the empty state with "Start trading" button
    And I tap the "Start trading" button
    Then the onAction callback should be triggered correctly
    And I should navigate to the expected trading interface

Screenshots/Recordings

Before

No visual changes - this is an internal prop structure fix

After

No visual changes - this is an internal prop structure fix. Smoke test to show button still works

after720.mov

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards.
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 format if applicable
I've applied the right labels on the PR (see labeling guidelines). 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.

Note

Switches PerpsEmptyState to use onAction and introduces getPositionDirection, updating usages, mocks, and tests.

Perps UI:
- PerpsEmptyState: Remove custom onStartTrading prop; use inherited onAction. Update story, tests, and PerpsTabView to pass onAction.
Refactor:
- Add getPositionDirection(sizeString) in utils/positionCalculations and comprehensive unit tests.
- Replace inline direction parsing with getPositionDirection in PerpsPositionsView and PerpsTabView to build testIDs.

^{Written by Cursor Bugbot for commit 0628e62. This will update automatically on new commits. Configure here.}

github-actions · 2025-09-29T16:09:32Z

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.

georgewrmarshall · 2025-09-29T16:41:38Z

app/components/UI/Perps/Views/PerpsEmptyState/PerpsEmptyState.test.tsx

 import { PerpsEmptyState } from './PerpsEmptyState';

 describe('PerpsEmptyState', () => {
-  const mockOnStartTrading = jest.fn();


Updating tests

georgewrmarshall · 2025-09-29T16:42:36Z

app/components/UI/Perps/Views/PerpsEmptyState/PerpsEmptyState.tsx

 import emptyStatePerpsDark from '../../../../../images/empty-state-perps-dark.png';

 export interface PerpsEmptyStateProps extends TabEmptyStateProps {
-  onStartTrading: () => void;


Removing onStartTrading and using the provided onAction prop from TabEmptyState component. Also changing it from a required prop.

georgewrmarshall · 2025-09-29T16:42:48Z

app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.test.tsx

      <View testID={testID}>
        <Text>Bet on price movements with up to 40x leverage.</Text>
-        <TouchableOpacity onPress={onStartTrading}>
+        <TouchableOpacity onPress={onAction}>


Updating mock

georgewrmarshall · 2025-09-29T16:43:07Z

app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.tsx

            {!isInitialLoading && hasNoPositionsOrOrders ? (
              <PerpsEmptyState
-                onStartTrading={handleNewTrade}
+                onAction={handleNewTrade}


Updating instance in PerpsTabView

georgewrmarshall · 2025-09-29T18:22:14Z

Quality Gate failed

Failed conditions D Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

These issues already existed and are not related to the changes in this PR https://sonarcloud.io/project/issues?sinceLeakPeriod=true&issueStatuses=OPEN%2CCONFIRMED&pullRequest=20534&id=metamask-mobile&open=AZmWdBouHRaSraV4LiXl

Matt561

Tests well on iOS 👍

georgewrmarshall · 2025-09-29T22:40:04Z

app/components/UI/Perps/utils/positionCalculations.test.ts

+      expect(getPositionDirection(' -10.5 ')).toBe('short');
+      expect(getPositionDirection('  0  ')).toBe('unknown');
+    });
+  });


Adding tests

georgewrmarshall · 2025-09-29T22:41:57Z

app/components/UI/Perps/Views/PerpsPositionsView/PerpsPositionsView.tsx

-          const sizeValue = parseFloat(position.size);
-          const directionSegment = Number.isFinite(sizeValue)
-            ? sizeValue > 0
-              ? 'long'
-              : sizeValue < 0
-              ? 'short'
-              : 'unknown'
-            : 'unknown';
+          const directionSegment = getPositionDirection(position.size);


I was getting SonarCloud code quality errors in this PR due to these ternary operators, which were unrelated to my changes. I moved the logic into a getPositionDirection util. This could likely be reused in other places where we get the position, but for now I only updated it here to reduce unrelated changes.

https://sonarcloud.io/project/issues?sinceLeakPeriod=true&issueStatuses=OPEN%2CCONFIRMED&pullRequest=20534&id=metamask-mobile&open=AZmWdBouHRaSraV4LiXl

Copilot

Pull Request Overview

This PR fixes a prop override issue in the PerpsEmptyState component by removing the custom onStartTrading prop and using the inherited onAction prop from TabEmptyStateProps. Additionally, it introduces a new utility function getPositionDirection to standardize how position directions are calculated across components.

Removed redundant onStartTrading prop from PerpsEmptyState to prevent prop spreading conflicts
Added getPositionDirection utility function to centralize position direction logic
Updated all usages of PerpsEmptyState to use onAction instead of onStartTrading

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
app/components/UI/Perps/utils/positionCalculations.ts	Added new `getPositionDirection` utility function
app/components/UI/Perps/utils/positionCalculations.test.ts	Added comprehensive tests for the new utility function
app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.tsx	Updated to use `getPositionDirection` utility and `onAction` prop
app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.test.tsx	Updated mock to use `onAction` instead of `onStartTrading`
app/components/UI/Perps/Views/PerpsPositionsView/PerpsPositionsView.tsx	Refactored to use `getPositionDirection` utility
app/components/UI/Perps/Views/PerpsEmptyState/PerpsEmptyState.tsx	Removed `onStartTrading` prop, now uses inherited `onAction`
app/components/UI/Perps/Views/PerpsEmptyState/PerpsEmptyState.test.tsx	Updated test to use `onAction` prop
app/components/UI/Perps/Views/PerpsEmptyState/PerpsEmptyState.stories.tsx	Updated Storybook story to use `onAction`

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

app/components/UI/Perps/utils/positionCalculations.ts

app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.tsx

sonarqubecloud · 2025-10-01T16:40:37Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

gambinish

lgtm

georgewrmarshall self-assigned this Sep 29, 2025

metamaskbot added the team-design-system All issues relating to design system in Mobile label Sep 29, 2025

github-actions bot added the size-S label Sep 29, 2025

georgewrmarshall added No QA Needed Apply this label when your PR does not need any QA effort. No E2E Smoke Needed If the PR does not need E2E smoke test run labels Sep 29, 2025

georgewrmarshall commented Sep 29, 2025

View reviewed changes

georgewrmarshall added the no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed label Sep 29, 2025

georgewrmarshall mentioned this pull request Sep 29, 2025

fix: tab empty state design polish #20483

Merged

8 tasks

georgewrmarshall marked this pull request as ready for review September 29, 2025 18:22

georgewrmarshall requested a review from a team as a code owner September 29, 2025 18:22

This comment was marked as outdated.

Sign in to view

georgewrmarshall enabled auto-merge September 29, 2025 18:32

Matt561 previously approved these changes Sep 29, 2025

View reviewed changes

georgewrmarshall dismissed Matt561’s stale review via 8d426e5 September 29, 2025 22:39

github-actions bot added size-M and removed size-S labels Sep 29, 2025

georgewrmarshall commented Sep 29, 2025

View reviewed changes

This comment was marked as outdated.

Sign in to view

georgewrmarshall requested a review from Copilot September 29, 2025 23:01

Copilot AI reviewed Sep 29, 2025

View reviewed changes

app/components/UI/Perps/utils/positionCalculations.ts Show resolved Hide resolved

app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.tsx Outdated Show resolved Hide resolved

georgewrmarshall added 3 commits October 1, 2025 08:50

chore: startTrading prop rename to prevent overrides

39dbfb1

chore: fixing story

9acdf15

chore: sonar cloud code improvements

cd7f377

georgewrmarshall force-pushed the perp-empty-tab-prop-name-fix branch from 8d426e5 to be02486 Compare October 1, 2025 16:20

chore: reverting metametrics logic

0628e62

georgewrmarshall force-pushed the perp-empty-tab-prop-name-fix branch from be02486 to 0628e62 Compare October 1, 2025 16:22

georgewrmarshall requested a review from Matt561 October 1, 2025 18:18

gambinish approved these changes Oct 1, 2025

View reviewed changes

georgewrmarshall added this pull request to the merge queue Oct 1, 2025

Merged via the queue into main with commit 4f32864 Oct 1, 2025
208 of 213 checks passed

georgewrmarshall deleted the perp-empty-tab-prop-name-fix branch October 1, 2025 20:10

github-actions bot locked and limited conversation to collaborators Oct 1, 2025

metamaskbot added the release-7.57.0 Issue or pull request that will be included in release 7.57.0 label Oct 1, 2025

Uh oh!

fix: resolve prop override in PerpsEmptyState component #20534

fix: resolve prop override in PerpsEmptyState component #20534

Conversation

georgewrmarshall commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changelog

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Sep 29, 2025

Uh oh!

georgewrmarshall Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

georgewrmarshall Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

georgewrmarshall Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

georgewrmarshall Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

georgewrmarshall commented Sep 29, 2025

Quality Gate failed

Uh oh!

This comment was marked as outdated.

Uh oh!

Matt561 left a comment

Choose a reason for hiding this comment

Uh oh!

georgewrmarshall Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

georgewrmarshall Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Oct 1, 2025

Quality Gate passed

Uh oh!

gambinish left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

georgewrmarshall commented Sep 29, 2025 •

edited

Loading

georgewrmarshall Sep 29, 2025 •

edited

Loading