chore: consolidate navigation style - swaps team#25879
Conversation
|
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. |
af91719 to
77d89e3
Compare
Update Minus and Add icon sizes from 24x24 to 32x32 to reflect design system changes.
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection: These changes affect:
SmokeTrade is selected because it covers bridge and swap functionality, including transaction details viewing. SmokeConfirmations is selected as a dependent tag per the tag description guidance (when selecting SmokeTrade for swap/bridge flows, also select SmokeConfirmations). The risk is medium because:
Performance Test Selection: |
|
| navigation.navigate(Routes.BROWSER.HOME, { | ||
| screen: Routes.BROWSER.VIEW, | ||
| params: { | ||
| newTabUrl: swapSrcExplorerData.explorerTxUrl, |
There was a problem hiding this comment.
Direct browser navigation bypasses required nested navigator pattern
High Severity
Navigating directly to Routes.BROWSER.VIEW instead of through Routes.BROWSER.HOME with a nested screen/params object breaks navigation. BrowserView is a screen inside the BrowserFlow stack navigator, which is the component for the BrowserTabHome tab — and that tab uses UnmountOnBlur, so when the user is on a different tab (e.g., viewing transaction details), the BrowserFlow navigator is unmounted and BrowserView is not in the active navigation tree. Every other call site in the codebase correctly uses the nested pattern navigate(Routes.BROWSER.HOME, { screen: Routes.BROWSER.VIEW, params: {...} }).
Additional Locations (2)
| multiChainTx: props.route.params.multiChainTx, | ||
| }, | ||
| }); | ||
| ); |
There was a problem hiding this comment.
Direct modal navigation bypasses nested navigator mounting
High Severity
Navigating directly to Routes.BRIDGE.MODALS.TRANSACTION_DETAILS_BLOCK_EXPLORER instead of through Routes.BRIDGE.MODALS.ROOT with a nested screen/params object breaks the bridge block explorer modal. TransactionDetailsBlockExplorer is a screen inside BridgeModalStack, which is the component for Routes.BRIDGE.MODALS.ROOT in the main stack. If the modal stack hasn't been visited, its child screens aren't in the navigation tree. Every other bridge modal navigation in the codebase correctly uses navigate(Routes.BRIDGE.MODALS.ROOT, { screen: ..., params: {...} }).
Description
This PR consolidates navigation styles across the codebase by replacing inline
navigationOptionsstyle definitions with the sharedgetNavigationOptionsTitleutility function.Reason for change:
navigationOptionspatternSolution:
getNavigationOptionsTitlefunction fromapp/components/UI/NavbarChangelog
CHANGELOG entry: null
Related issues
Fixes: #23765 Part of React Navigation v6 upgrade preparation (split from original PR for easier code review by team)
Manual testing steps
Feature: Navigation header consistency
Scenario: User navigates to screens affected by this PR
Given the app is running
Screenshots/Recordings
Before
N/A - No visual changes
After
N/A - No visual changes
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Low Risk
Low risk UI navigation change limited to route targets/params for opening the in-app browser and the bridge block-explorer modal; main risk is misrouting if route signatures differ across navigators.
Overview
Bridge transaction details and the block-explorer modal now navigate directly to
Routes.BROWSER.VIEWwithnewTabUrl/timestamp, instead of routing throughRoutes.BROWSER.HOMEwith nestedscreen/params.For bridge transactions, opening the block explorer modal now navigates straight to
Routes.BRIDGE.MODALS.TRANSACTION_DETAILS_BLOCK_EXPLORERrather than via the modal stack root. Associated tests were updated to assert the new navigation call shape.Written by Cursor Bugbot for commit 79a9f8d. This will update automatically on new commits. Configure here.