[pull] main from MetaMask:main#305
Merged
Merged
Conversation
…am) (#22271) <!-- 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** - Resolves an issue with the search wherein inactive markets were coming back from the `/public-search` endpoint due to a spurious querystring param being passed <!-- 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: null ## **Related issues** Fixes: [PRED-264](https://consensyssoftware.atlassian.net/browse/PRED-264) ## **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** <img width="420" alt="image" src="https://github.com/user-attachments/assets/255c6be7-c3d0-4d05-b913-627b16ee9d84" /> <!-- [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). - [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. [PRED-264]: https://consensyssoftware.atlassian.net/browse/PRED-264?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Removes the extra `presets=Events` from the Polymarket `public-search` request and updates tests to expect the new URL. > > - **Polymarket search**: > - Adjust `getParsedMarketsFromPolymarketApi` to build `public-search` URL without `presets=Events`, keeping only `presets=EventsTitle`. > - **Tests**: > - Update expected URL in `utils.test.ts` to match the revised `public-search` query (no `presets=Events`). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 1d9ea65. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Luis Taniça <matallui@gmail.com>
<!-- 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** This PR will update the ledger link to correct article. based on this Jira ticket. https://consensyssoftware.atlassian.net/browse/HWB-157?focusedCommentId=365665&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-365665 <!-- 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: Update Ledger support link to the correct article ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/HWB-157?focusedCommentId=365665&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-365665 ## **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** <!-- [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). - [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** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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] > Updates `LEDGER_SUPPORT_LINK` in `app/constants/urls.ts` to point to the correct Ledger support article. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 7c4fd14. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
<!-- 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** <!-- 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? --> Fixes an issue in which txs sometimes do not show on the details page for non-evm assets. ## **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: Fixed an issue in which txs sometimes do not show on the details page for non-evm assets. ## **Related issues** Fixes: #22251 ## **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] --> https://github.com/user-attachments/assets/d9a517d4-130b-42bc-9d5a-d216dff1d491 ### **After** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/d726164c-f231-402f-af58-f7ad67e810f6 ## **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] > Use a new selector to aggregate/filter non-EVM transactions by chain and asset so the Asset details page shows the correct txs. > > - **Asset view (`app/components/Views/Asset/index.js`)**: > - Replace `selectNonEvmTransactions` with `selectNonEvmTransactionsForSelectedAccountGroup`. > - For non-EVM assets, filter txs by `chainId` and asset (native vs token via `address`/`symbol`), add lightweight caching, and sort by time. > - **Selectors (`app/selectors/multichain/multichain.ts`)**: > - Mark `selectNonEvmTransactions` as deprecated. > - Add `selectNonEvmTransactionsForSelectedAccountGroup` to aggregate and flatten non-EVM txs across the selected account group, sorting by timestamp. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 037033a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
<!-- 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** This PR implements a complete navigation flow for the Trending feature, allowing users to browse web content from within the Trending tab without modifying the bottom navigation structure. you should set the two env vars: ``` export OVERRIDE_REMOTE_FEATURE_FLAGS="true" export ASSETS_TRENDING_TOKENS_ENABLED="true" ``` ### Key Changes: 1. **TrendingView Stack Navigator**: Converted TrendingView to use a Stack Navigator with two routes: - `TrendingFeed`: Pure React Native "coming soon" screen with an Explore button - `TrendingBrowser`: Full Browser component with all features (tabs, URL bar, search) 2. **Native Coming Soon Screen**: Replaced WebView-based HTML content with native React Native components using `@metamask/design-system-react-native` 3. **Navigation Flow**: Users can tap the Explore button (🔍) in TrendingFeed to open the Browser with the MetaMask portfolio, browse any website, and navigate back using: - ← Back arrow button (left side of URL bar) - ✕ Close button (right side of URL bar) 4. **Back Button Implementation**: Added a back button in `BrowserTab` that: - Appears on the left side of BrowserUrlBar - Only shows when `fromTrending && isAssetsTrendingTokensEnabled` is true - Uses `navigation.goBack()` to return to TrendingFeed 5. **No MainNavigator Changes**: All navigation is self-contained within TrendingView's Stack Navigator, maintaining the existing bottom tab structure ## **Changelog** CHANGELOG entry: Implemented Trending view with browser navigation support ## **Related issues** Fixes: N/A ## **Manual testing steps** ### **Before** - Trending tab showed WebView with HTML-based "coming soon" message - No way to access browser functionality from Trending tab ### **After** - Trending tab shows native React Native "coming soon" screen - Explore button opens full browser experience - Back button allows easy return to Trending feed - Complete browser functionality available within Trending context https://github.com/user-attachments/assets/4a37f02b-3c1b-4f85-a6fa-f319221e688f ## **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. - [x] I've converted TrendingView from WebView to native React Native components - [x] I've implemented a Stack Navigator for proper navigation flow - [x] I've added back button functionality that respects the feature flag ## **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. - [ ] I've verified the Explore button opens the browser with portfolio - [ ] I've verified the back button (←) and close button (✕) both navigate back to Trending - [ ] I've verified the back button only appears when navigating from Trending - [ ] I've verified all browser features work correctly (tabs, URL bar, search, history) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Introduces a stack navigator for Trending with a native feed and embedded browser, adds a back button in BrowserTab, and conditionally shows Trending tab via feature flag with last-screen persistence. > > - **Trending navigation (stack-based)**: > - Add `TrendingView` as a stack with `TrendingFeed` (native "coming soon") and `TrendingBrowser` (wrapped `Browser`). > - Intercept navigation in `BrowserWrapper` to `goBack` when targeting `Routes.TRENDING_VIEW`. > - Persist last viewed screen via `lastTrendingScreenRef` and `updateLastTrendingScreen`. > - Explore button navigates to `TrendingBrowser` with `newTabUrl`, `timestamp`, and `fromTrending`. > - **Browser tab UI/behavior**: > - Add left-side back button in `BrowserTab` header (Design System `ButtonIcon`) when assets trending flag is enabled; navigates to `TrendingFeed`. > - Refactor URL bar container to include back button; simplify cancel behavior; keep close button visibility for `fromTrending && isAssetsTrendingTokensEnabled`. > - **Main tabs logic**: > - Conditionally show `Trending` tab (or fallback to `Browser` tab) based on `assetsTrendingTokens` flag. > - **Tests & snapshots**: > - Update `TrendingView.test.tsx` for new stack flow and navigation params. > - Update BrowserTab snapshot to reflect new header layout. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8eeba12. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…22257) ## **Description** This PR fixes a bug in the Perps candle subscription system where the refresh interval was being continuously cleared and re-established, causing excessive logging and inefficient resource usage. **What is the reason for the change?** - `candleData` was in the dependency array of the candle refresh interval effect in `usePerpsPositionData.ts` - This created a self-triggering loop: the effect updates `candleData` → triggers re-run → clears interval → creates new interval → repeats - The result was continuous logging of "Cleared candle refresh interval" and "Setting up candle refresh every..." messages - This caused unnecessary interval teardown/recreation on every candle data update (potentially multiple times per second) **What is the improvement/solution?** - Removed `candleData` from the dependency array of the interval effect (line 230 in `usePerpsPositionData.ts`) - Removed duplicate `initializationState` dependency (already covered by `isControllerInitialized`) - Added explanatory comment and ESLint disable directive to document why `candleData` is intentionally excluded - The interval now only resets when configuration actually changes (interval period, duration, etc.), not on every data update ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: Infinite loop in candle subscription causing excessive logging ## **Manual testing steps** ```gherkin Feature: Candle Subscription Refresh Behavior Scenario: user observes stable candle refresh intervals Given user is on Perps position details screen And developer console is visible When user selects a time interval (e.g., "1h") Then user should see "Setting up candle refresh every 3600 seconds for 1h" ONCE And user should NOT see continuous "Cleared candle refresh interval" messages When user waits for 5 seconds Then user should NOT see any new interval setup messages When user changes the time interval to "3m" Then user should see "Cleared candle refresh interval" ONCE And user should see "Setting up candle refresh every 180 seconds for 3m" ONCE Scenario: user verifies candles still update correctly Given user is on Perps position details screen And user has selected any time interval When user observes the price chart Then the live candle should update in real-time as prices change And historical candles should remain stable When the interval period completes (e.g., after 3 minutes for 3m interval) Then a new candle should appear And the chart should refresh with updated historical data ``` ## **Screenshots/Recordings** N/A - Internal logging fix ## **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 (all existing tests pass) - [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. --- ## **Additional Context** ### Root Cause Analysis The bug was in the `useEffect` at lines 161-234 of `usePerpsPositionData.ts`: **Problem Chain:** 1. Effect depends on `candleData` (line 228 in original code) 2. Effect's interval callback calls `setCandleData()` (lines 207-216) 3. Separate effect merges live candles into `candleData` (lines 300-336) 4. Every `candleData` update triggers the interval effect to re-run 5. Re-run clears old interval and creates new one 6. New data from interval or live candle merge triggers step 4 again ### Technical Details **Changed File:** - `app/components/UI/Perps/hooks/usePerpsPositionData.ts` **Changes Made:** 1. Removed `candleData` from dependency array (line 230) 2. Removed duplicate `initializationState` from dependency array (line 232) 3. Added explanatory comment (lines 227-228) 4. Added `eslint-disable-next-line react-hooks/exhaustive-deps` (line 229) **Test Results:** - ✅ All 38 tests in `usePerpsPositionData.test.ts` pass - ✅ No functional changes to candle fetching or display - ✅ Interval still refreshes at correct intervals - ✅ Live candles still merge correctly ### Why ESLint Disable is Necessary The ESLint rule `react-hooks/exhaustive-deps` flags `candleData` as missing from dependencies. However: - Including `candleData` would recreate the bug - The effect doesn't depend on previous `candleData` values - It only needs to re-run when interval configuration changes - This is a legitimate case for disabling the rule with proper documentation ### Impact **Components Affected:** - `PerpsMarketDetailsView.tsx` (main consumer) - `usePerpsMarketStats.ts` (secondary consumer) **Performance Improvement:** - Before: Interval cleared/created multiple times per second - After: Interval cleared/created only on configuration changes - Reduction: ~99% fewer interval operations **User Impact:** - No visible changes to end users - Chart behavior remains identical - Reduced console noise for developers - Improved performance and memory usage <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Stops infinite re-creation of the candle refresh interval by removing `candleData` (and duplicate `initializationState`) from effect deps and documenting with comments/ESLint directive. > > - **Perps hook (`app/components/UI/Perps/hooks/usePerpsPositionData.ts`)** > - **Interval effect**: > - Remove `candleData` from dependency array to prevent infinite loop. > - Remove duplicate `initializationState` dependency (covered by `isControllerInitialized`). > - Add explanatory comments and `eslint-disable-next-line react-hooks/exhaustive-deps` to justify deps. > - Behavior: interval now re-initializes only when interval settings or controller init status change. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 22935dc. 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.
5 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 : )