Linearize Send Flow#2764
Conversation
leofelix077
added
wip
There was a problem hiding this comment.
Pull request overview
This PR introduces an initial “linearized” Send flow in the popup, aligning with the desired UX of selecting a token first (unless pre-selected via query params), then choosing destination, then entering amount, with step transition animations and updated tests.
Changes:
- Added a new initial Send step (
SELECT_SOURCE_ASSET) and step transition animations (enter from bottom/right/left + dismiss/exit overlay). - Updated Send screens to support the new step order, including “My Accounts” recipient picking and persisting/displaying a
recipientName. - Updated unit and e2e tests to reflect the new flow and added supporting selectors/testids.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| extension/src/popup/views/Send/styles.scss | Adds CSS keyframes/classes for step transition animations and dismiss overlay behavior. |
| extension/src/popup/views/Send/index.tsx | Implements the new step state machine (token picker first unless query param preselect) and animation wiring. |
| extension/src/popup/views/tests/SendPayment.test.tsx | Re-enables Send tests, adds initial-step assertions, and skips legacy tests pending rewrite. |
| extension/src/popup/locales/pt/translation.json | Adds new i18n keys used by the refreshed Send UI (currently with untranslated values). |
| extension/src/popup/locales/en/translation.json | Adds new i18n keys used by the refreshed Send UI (including a new “don’t” variant key). |
| extension/src/popup/ducks/transactionSubmission.ts | Persists recipientName in redux transaction data and exposes saveRecipientName. |
| extension/src/popup/constants/send-payment.ts | Adds the SELECT_SOURCE_ASSET step to the send flow enum. |
| extension/src/popup/components/send/styles.scss | Updates SendAmount styling for the new card layout, inline asset selector, and percentage buttons. |
| extension/src/popup/components/send/SendTo/index.tsx | Updates destination step header and adds “My Accounts” list that can set recipientName. |
| extension/src/popup/components/send/SendDestinationAsset/styles.scss | Adjusts destination-asset selector styling (collectibles heading/section). |
| extension/src/popup/components/send/SendDestinationAsset/index.tsx | Removes token/collectibles tabs and renders tokens + collectibles section in one view; adds returnToAmount. |
| extension/src/popup/components/send/SendAmount/index.tsx | Refactors amount screen layout (recipient at top, inline asset selector, pct buttons, settings button). |
| extension/src/popup/components/send/AddressTile/index.tsx | Allows displaying recipientName as primary text with address/federation as secondary. |
| extension/src/popup/components/InternalTransaction/TokenList/index.tsx | Adds data-testid="token-list" to support updated test assertions. |
| extension/src/popup/components/InternalTransaction/SubmitTransaction/index.tsx | Displays recipientName (if set) in the transaction summary instead of only truncated address. |
| extension/src/popup/components/InternalTransaction/SubmitTransaction/hooks/useSubmitTxData.tsx | Avoids adding self-owned destinations to recent addresses. |
| extension/e2e-tests/test-fixtures.ts | Adds optional viewport sizing to the Playwright context fixture. |
| extension/e2e-tests/sendPayment.test.ts | Updates existing e2e flows for the new step order and adds new flow-specific e2e cases. |
Comments suppressed due to low confidence (1)
extension/src/popup/views/tests/SendPayment.test.tsx:31
- The same module ("popup/helpers/blockaid") is imported twice under two different identifiers (
BlockaidHelpersandBlockAidHelpers). This is redundant and makes the test harder to follow. Consolidate to a single import name and update references accordingly.
import * as ApiInternal from "@shared/api/internal";
import * as UseNetworkFees from "popup/helpers/useNetworkFees";
import * as BlockaidHelpers from "popup/helpers/blockaid";
import * as UseGetCollectibles from "helpers/hooks/useGetCollectibles";
import * as ExtensionMessaging from "@shared/api/helpers/extensionMessaging";
import * as TokenList from "@shared/api/helpers/token-list";
import {
TESTNET_NETWORK_DETAILS,
DEFAULT_NETWORKS,
MAINNET_NETWORK_DETAILS,
} from "@shared/constants/stellar";
import { APPLICATION_STATE as ApplicationState } from "@shared/constants/applicationState";
import { ROUTES } from "popup/constants/routes";
import { Send } from "popup/views/Send";
import { initialState as transactionSubmissionInitialState } from "popup/ducks/transactionSubmission";
import * as CheckSuspiciousAsset from "popup/helpers/checkForSuspiciousAsset";
import * as tokenPaymentActions from "popup/ducks/token-payment";
import * as GetIconHelper from "@shared/api/helpers/getIconUrlFromIssuer";
import * as BlockAidHelpers from "popup/helpers/blockaid";
jest.mock("lodash/debounce", () => jest.fn((fn) => fn));
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (2)
extension/src/popup/views/Send/hooks/useSendQueryParams.ts:119
- When
?assetis missing, the hook still dispatchessaveIsToken(false)/saveIsCollectible(false)even iftransactionData.assetis already set to a token/collectible. That can desyncassetvsisTokenafter navigation. Consider only resetting these flags when you also set the default asset, or derive them from the existingtransactionData.asset.
} else {
// Set default asset to native if not already set
if (!transactionData.asset) {
dispatch(saveAsset("native"));
}
dispatch(saveIsCollectible(false));
dispatch(saveIsToken(false));
}
extension/src/popup/locales/pt/translation.json:728
- The pt locale value for the smart-apostrophe key "You don’t have enough {{asset}} in your account" is currently English. This string is still referenced elsewhere (e.g., SwapAmount), so users will see English error text in Portuguese UI. Translate this value (or consolidate keys) to keep locale coverage consistent.
"You can close this screen, your transaction should be complete in less than a minute.": "Você pode fechar esta tela, sua transação deve estar completa em menos de um minuto.",
"You can define your own assets lists in Settings.": "Você pode definir suas próprias listas de ativos nas Configurações.",
"You don't have enough {{asset}} in your account": "Você não tem {{asset}} suficiente em sua conta",
"You don’t have enough {{asset}} in your account": "You don’t have enough {{asset}} in your account",
"You have no assets added.": "Você não tem ativos adicionados.",
Code reviewFound 1 issue:
🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
| extensionId, | ||
| context, | ||
| }) => { | ||
| test.slow(); |
There was a problem hiding this comment.
I notice we're using a lot of test.slow(). Why is this necessary? This isn't necessarily wrong, but it does triple the time a test has to run (15s vs 45s). If a test fails, you'll have to wait 45s for it terminate.
It's best to avoid this unless it's absolutely necessary to make sure our tests run and fail quickly
There was a problem hiding this comment.
not needed. I re-ran all the tests without it and it works fine
| await expect(page.getByTestId("send-amount-fee-display")).toHaveText( | ||
| "0.00001 XLM", | ||
| "0.0093338 XLM", | ||
| { timeout: 10000 }, |
There was a problem hiding this comment.
10s is a pretty long timeout. Shouldn't this happen pretty quickly if the API calls are stubbed?
There was a problem hiding this comment.
same for the others. re-ran the tests adjusting the timeouts and the .slow()
|
Occasionally, I'm seeing a loader briefly under the destination address bar that pushes everything down. It's not clear what it's loading: Screen.Recording.2026-05-15.at.5.21.40.PM.mov |
|
The number input in the Swap amount input isn't quite displaying correctly Screen.Recording.2026-05-15.at.5.25.21.PM.mov |
|
If you click Swap from the Asset Detail, pressing back goes back to the Account screen But, if you click Send from the Asset Detail, pressing back goes back to the Asset Detail screen Screen.Recording.2026-05-15.at.5.27.28.PM.mov |
|
Found a few issues: 1 - Weird double AQUA balances when entering fiat amount 2 - When you start the Send flow from the Token Details screen it'll pre-select the token and it won't let you change the token using the picker. The picker opens, you tap to select a different token but nothing happens. |
|
PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-46359bb58ac541a5a985 (SDF collaborators only — install instructions in the release description) |
|
@piyalbasu -> The swap flow is untouched (except for the issue with the input. there were some overlapping classes on Swap and Send. both were using the SendAmount classes. Adjusted now @CassioMG adjusted the token change when coming from details and also the other minor parts added to the comments also merged the last changes from main on both mobile and extension Screen.Recording.2026-05-19.at.10.45.18.movScreen.Recording.2026-05-19.at.11.28.33.mov |
|
@leofelix077 I'm still seeing the double token balances issue when entering fiat amount, see video below. I'll cross-post here my comment from the mobile PR which has the same issue so you know how to repro it: What appears to happen here is that the "token available balance" value which usually appears aligned on the right side (right below the dropdown) is conflicting with the "token amount to send" value which usually appears aligned on the left side. This probably only happens for big string amounts. I'd suggest applying some UI constraints here to prevent this from happening, or keep things flexible, but ensure that:
double-amounts-issue.mov |
| @@ -1,4 +1,5 @@ | |||
| export enum STEPS { | |||
| SELECT_SOURCE_ASSET = "select-source-asset", | |||
There was a problem hiding this comment.
nit: should we name it as SET_SOURCE_ASSET for consistency with SET_DESTINATION_ASSET below or do they represent distinct things?
There was a problem hiding this comment.
it can be SET. it leads to the same action
|
@CassioMG not sure I'm catching what you mean by double token balances one of them is the conversion from USD to the token equivalent, and the second line is the total of the tokens UI-wise I had already discussed with @sdfcharles for breaking it into two lines. one for the current and one for total spendable, just missing the refresh icon staying on the first line, instead of dropping down to the second line. He also gave the green light after I showed the mocks, but I think he didn't see this small tweak that it should stay on the first line as it converts back and forth to the typed amount equivalent |
Sorry I think the expression I used was indeed ambiguous, by "double token balances" I meant the 2 amounts stacked on top of each other as it appear on the video, which is basically just the result of the line break that happens when the 2 amounts get too big and don't fit in a single line. I think now that the refresh icon position is fixed on the last commit it is looking better. My initial thought was that the available token balance should always stay right below the token dropdown for consistency as jumping to the opposite side of the screen could be confusing for users. But I'm not super strong on it, I'll defer the UI treatment to @minkyeongshin. What do you think? Screen.Recording.2026-05-26.at.17.43.41.mov |
|
@CassioMG Ahh I see. Now I get what you meant, hadn't fully understood it before. but not sure as well how we could handle it. will leave it like this for now until we get feedback if we keep it fixed below the token dropdown, then the first line needs to drop below it. might also get somewhat confusing 
Another option could be to display above For displaying below, long texts we probably would need some different UI I think, breaking into two lines on opposite sides doesn't look good imo. and not sure if we should ellipsize the amount values to make it fit into one line or if we keep as is, as it's kind of an edge-case even for small token to USD conversion it needs a very long value |
|
Leo's suggestion could work. we can move the available balance above the dropdown, which gives more room to display the balance. (screen 1) For cases where the typed amount gets long, we can reduce the text size up to a certain length to prevent it from being cut off. (screen 2) However, I think it would be better to set a maximum input length(screen 3). What do you think? @CassioMG @leofelix077 
|
|
Posting for both here so it's easier to keep track, but imo it looks good / doesn't look out of place. @minkyeongshin @CassioMG Implemented with dynamic font size and in case it's still too big it ellipsizes, to avoid breaking the UI into two lines what do you think? 
Also adjusted the logic to hide the fiat input when not supported (following extension logic) 
|
|
Looks good to me! 👍👍 @leofelix077 |
@leofelix077 looks good to me 👍 thanks for making those changes. I think we could do the same on mobile for consistency 📱 |
|
@CassioMG I applied for both. I meant to keep the conversation only in one thread, but screenshots 1,2,5,6 are for mobile ✅ . will crosspost the thread there |
| @@ -10,6 +10,9 @@ import { | |||
| import * as ApiInternal from "@shared/api/internal"; | |||
There was a problem hiding this comment.
just checking, is this test file in the expected folder?
| "At the end of this process, Freighter will only display accounts related to the new backup phrase.": "At the end of this process, Freighter will only display accounts related to the new backup phrase.", | ||
| "Authorizations": "Authorizations", | ||
| "Authorize": "Authorize", | ||
| "available": "available", |
There was a problem hiding this comment.
it seems this string is not being used
| export const normalizeNumericString = (value: string) => { | ||
| const cleaned = cleanAmount(value); | ||
| let hasDecimal = false; | ||
| let normalized = ""; | ||
|
|
||
| for (const char of cleaned) { | ||
| if (char === ".") { | ||
| if (hasDecimal) { | ||
| continue; | ||
| } | ||
| hasDecimal = true; | ||
| } | ||
| normalized += char; | ||
| } | ||
|
|
||
| return normalized; | ||
| }; | ||
|
|
||
| export const getValidBigNumber = (value: string): BigNumber | null => { | ||
| const cleanedValue = normalizeNumericString(value); | ||
|
|
||
| if (!cleanedValue || cleanedValue === ".") { | ||
| return null; | ||
| } | ||
|
|
||
| let numericValue: BigNumber; | ||
| try { | ||
| numericValue = new BigNumber(cleanedValue); | ||
| } catch { | ||
| return null; | ||
| } | ||
|
|
||
| return numericValue.isNaN() ? null : numericValue; | ||
| }; | ||
|
|
||
| export const isValidPositiveAmount = (value: string) => { | ||
| const numericValue = getValidBigNumber(value); | ||
|
|
||
| return Boolean(numericValue && numericValue.gt(0)); | ||
| }; | ||
|
|
There was a problem hiding this comment.
do we have tests for those 3 new helpers?
Closes #2686
This PR linearizes the extension Send flow so users move more consistently from recipient selection to amount entry to review, with cleaner state handoff between steps.
It improves recipient UX by keeping Recents and My Accounts visible while searching, making Suggestions pressable like other destination options, and debouncing validation/search feedback to avoid premature errors.
It also hardens amount handling by preserving the exact source value when switching between fiat and token views, improving numeric sanitization, and fixing edge cases like formatted thousands leading to invalid amount states.
Additionally, it includes reliability and polish updates across query-param handling, submit safeguards, collectible grouping/styling, locale cleanup, and Send flow test coverage.
initial.flow.and.navigation.mov
input-validation-and-preservation.mov
changing-address-and-token.mov
fullscreen-smoke-test.mov
sending-to-federated-p1.mov
sending-to-federated-p2-and-recents.mov
Adding back the memo and fee row that was missing from previous videos: