[pull] main from MetaMask:main#580
Merged
Merged
Conversation
## **Description** Addresses all actionable review comments from the Core perps-controller PR ([MetaMask/core#7941](MetaMask/core#7941)). **Bug fixes:** - Fix cancellation detection inconsistency — add `'User '` prefix to match the pattern used elsewhere - Fix memoization regression — pass `timestamp` as selector input so re-renders fire correctly - Resolve circular dependency in `perps-types.ts` - Extract hardcoded withdrawal fee and fee estimate to named constants **Refactors (per core review feedback):** - Replace 5 barrel `export *` wildcards with explicit named exports in `index.ts` - Extract 34 method action types to `PerpsController-method-action-types.ts` - Extract inline provider instantiation into private `#createProviders()` factory - Move `registerMethodActionHandlers` from constructor to `initialize()` with an idempotency guard - Extract magic numbers to config constants (`DEPOSIT_GAS_LIMIT`, `SPOT_ASSET_ID_OFFSET`, `MYX_TRADING_DEFAULTS`, `MYX_FEE_CONFIG`) - Add `fetchHistoricalCandles` to `PerpsProvider` interface (removes a type cast in `PerpsController`) ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: Perps controller cleanup Scenario: user opens Perps and trades normally Given the app is running with Perps enabled When user navigates to Perps, connects, and places an order Then all perps features work as before — no regressions ``` ## **Screenshots/Recordings** ### **Before** <!-- n/a — refactor only --> ### **After** <!-- n/a — refactor only --> ## **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. ## **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] > **Medium Risk** > Touches Perps controller initialization/messenger wiring and public exports, which could cause runtime or build-time breakages if any consumers rely on previous timing or wildcard exports. Behavior changes are mostly bounded (constants, cancellation detection, candle fetching interface), but span multiple core perps modules. > > **Overview** > **Perps controller refactor and API cleanup.** Extracts `PerpsController` method action type definitions into `PerpsController-method-action-types.ts`, moves `registerMethodActionHandlers` from the constructor into `init()` with an idempotency guard, and factors provider instantiation/active-provider selection into a private `#createProviders()`. > > **Provider/API surface updates.** Replaces `export *` barrels in `controllers/perps/index.ts` with explicit named exports, adds optional `fetchHistoricalCandles` to the `PerpsProvider` interface and implements/tests it in `HyperLiquidProvider`, and removes a circular dependency by relocating `OrderType` into `types/perps-types.ts`. > > **Bug fixes and config/constants.** Updates deposit cancellation detection strings to match `User cancelled/canceled`, fixes a selector memoization ordering regression around `timestamp`, drops unsupported hex-wei handling in `convertPerpsAmountToUSD` (now returns `'$0'`), and centralizes fee/gas/ID constants (e.g., `DEPOSIT_CONFIG.EstimatedGasLimit`, `WITHDRAWAL_CONSTANTS.DefaultFeeAmount`, `ESTIMATED_FEE_RATE`, `SPOT_ASSET_ID_OFFSET`). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 29b3e69. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
) ## **Description** Removes `offramp-token-amount.spec.ts` e2e test. All behaviors tested in this e2e test are already covered by existing unit tests in `BuildQuote.test.tsx`. Coverage mapping: | E2E Behavior | Unit Test in BuildQuote.test.tsx | |---|---| | Enter amount via keypad | `updates the amount input` | | Quick amount buttons (25%, MAX) | `updates the amount input with quick amount buttons` | | MAX capped by gas for native token | `updates the amount input up to the max considering gas for native asset` | | Percentage capped by gas (75%, 50%) | `updates the amount input up to the percentage considering gas` | | Freshly opened keyboard clears amount | `clears the amount when the keyboard is freshly opened` | This is part of the effort to convert ramps e2e tests to unit/component tests (MMQA-1492). ### Why not a separate component-view test? `BuildQuote.test.tsx` already **is** a component-view test — it uses `renderScreen()` which renders the full `BuildQuote` component inside a real navigation stack with Redux state. It simulates user interactions via `fireEvent.press` on keypad buttons, asserts displayed text, and tests quick amount buttons (25%, 50%, 75%, MAX) with gas capping. Creating a separate component-view test would duplicate existing coverage. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: [MMQA-1520](https://consensyssoftware.atlassian.net/browse/MMQA-1520) ## **Manual testing steps** ```gherkin Feature: Offramp token amount test removal Scenario: Existing unit tests cover all e2e behaviors Given the BuildQuote.test.tsx unit tests exist And they cover keypad entry, quick amounts, gas capping, and validation When the offramp-token-amount.spec.ts e2e test is removed Then no test coverage is lost And the unit test suite continues to pass ``` ## **Screenshots/Recordings** ### **Before** N/A — test removal, not a UI change. ### **After** N/A — test removal, not a UI change. ## **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 - [ ] I've included tests if applicable - [ ] 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. ## **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. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
## **Description** Updates the shared "View more" card in the homepage carousels to match the updated design, and changes the Perps "View more" entrypoint to route to the market list page instead of the main Perps home. Changes: - Added `rounded-xl bg-background-muted` background to the ViewMoreCard outer Box - Removed the circular bubble wrapper around the ArrowRight icon - Made Predictions ViewMoreCard use `flex-1` and default `BodyMd` text size to match Perps - Split Perps navigation: section title still goes to Perps home, View more card now goes to the market list page ## **Changelog** CHANGELOG entry: Updated View more card styling with background color and updated Perps View more to navigate to market list ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TMCU-513 ## **Manual testing steps** ```gherkin Feature: View more card styling and routing Scenario: View more cards display with background Given user is on the homepage And Perps or Predictions sections show a carousel with a View more card When the user views the View more card Then it has a rounded grey background (bg-background-muted) And the arrow icon has no circular bubble around it Scenario: Perps View more navigates to market list Given user is on the homepage with Perps section visible When user taps the View more card in the Perps carousel Then the app navigates to the Perps market list page Scenario: Perps section title navigates to Perps home Given user is on the homepage with Perps section visible When user taps the Perpetuals section title Then the app navigates to the Perps home page ``` ## **Screenshots/Recordings** Verified on device -- both Perps and Predictions View more cards now show consistent styling. ### **Before** View more card had no background and a circular bubble around the arrow icon. ### **After** <img width="300" src="https://github.com/user-attachments/assets/23c07566-2996-4f26-9717-481a07ba783e" /> ## **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. ## **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** > Low risk UI and navigation tweak limited to homepage carousels; main risk is an incorrect route/params causing the Perps "View more" CTA to land on the wrong screen. > > **Overview** > Updates the shared homepage `ViewMoreCard` to match new design by moving the muted background and rounded corners to the outer card and removing the circular icon bubble. > > Changes the Perps homepage carousel "View more" CTA to navigate to `Routes.PERPS.MARKET_LIST` (while the section title still routes to `Routes.PERPS.PERPS_HOME`) and updates the Perps unit test accordingly. Aligns the Predictions carousel `ViewMoreCard` sizing/typography usage with Perps by using `flex-1` and default text variant. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit a2cb634. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…27077) ## **Description** The NFT grid skeleton loading state had minimal padding (`p-1`) regardless of context, while the actual NFT grid `FlatList` uses `px-4` horizontal padding in full view mode. This caused an inconsistent layout jump when loading finished and the skeleton was replaced by real content. The fix threads `isFullView` from `NftGrid` through `NftGridContent` to `NftGridSkeleton`, so the skeleton conditionally applies `px-4` in full view and `px-1` in tab/homepage view -- matching the padding of the content it replaces in each context. ## **Changelog** CHANGELOG entry: Fixed missing horizontal padding on NFT skeleton loading state in full view ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TMCU-539 ## **Manual testing steps** ```gherkin Feature: NFT skeleton padding in full view Scenario: user sees skeleton with correct padding in NFTs full view Given user navigates to the NFTs full view And NFTs are being fetched When the skeleton loading state is displayed Then the skeleton has the same horizontal padding as the NFT grid content Scenario: skeleton padding is unchanged in tab/homepage view Given user is on the homepage with the NFTs section visible And NFTs are being fetched When the skeleton loading state is displayed Then the skeleton retains minimal horizontal padding (matching tab layout) ``` ## **Screenshots/Recordings** N/A -- padding-only fix, verified via tests. ### **Before** <img width="300" src="https://github.com/user-attachments/assets/1fb110de-ed76-4a41-8c8f-be1202db4e12" /> ### **After** <img width="300" src="https://github.com/user-attachments/assets/3c7730bd-76ca-4973-a11a-d8eedf53d1c7" /> ## **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. ## **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** > Low risk UI-only change that adjusts loading-state layout; no business logic, data handling, or navigation behavior is modified. > > **Overview** > Fixes a layout jump in the NFT grid loading state by **matching `NftGridSkeleton` horizontal padding to the rendered grid**. > > Threads `isFullView` from `NftGrid` through `NftGridContent` into `NftGridSkeleton`, where the container now applies `px-4` in full view and `px-1` otherwise (replacing the previous fixed `p-1`). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit afdbe36. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…ion (#27070) ## **Description** Replaces the flat `IconName.WifiOff` design system icon in the shared `ErrorState` component with themed PNG illustrations matching Vinay's new "No connection" design. Uses `useAssetFromTheme` to switch between light and dark variants, following the same pattern as `CollectiblesEmptyState`. This change affects all 4 homepage sections that render error states: Tokens, Predictions, Perpetuals, and DeFi. ## **Changelog** CHANGELOG entry: Updated the error state icon on the homepage to a new no-connection illustration ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TMCU-538 ## **Manual testing steps** ```gherkin Feature: Updated error state icon on homepage sections Scenario: user sees error state in light mode Given user is on the homepage in light mode And a section fails to load (e.g., Tokens, Predictions) When the error state is displayed Then the new no-connection illustration is shown (light variant) And the Retry button is visible below the illustration Scenario: user sees error state in dark mode Given user is on the homepage in dark mode And a section fails to load When the error state is displayed Then the new no-connection illustration is shown (dark variant) ``` ## **Screenshots/Recordings** Verified on device in both light and dark modes. ### **Before** Flat `WifiOff` icon from the design system. <img width="300" src="https://github.com/user-attachments/assets/49379087-1929-43a2-9dee-3a7566df73a7" /> ### **After** New themed no-connection illustration (72x72) with light/dark variants. <img width="300" src="https://github.com/user-attachments/assets/e5b51582-2e93-4f44-a45f-49f36451571c" />w ## **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. ## **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** > Low risk UI-only change that swaps a design-system icon for themed PNG assets in the shared homepage `ErrorState` component. Main risk is limited to missing/incorrect asset bundling or sizing regressions across sections that reuse this component. > > **Overview** > Updates the shared homepage `ErrorState` UI to render a themed no-connection PNG illustration (light/dark via `useAssetFromTheme`) instead of the design-system `WifiOff` icon. > > Adds `react-native` `Image` rendering with Tailwind-based sizing (72x72) and removes the unused icon imports, affecting all homepage sections that reuse `ErrorState`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit d58c424. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )