feat(transaction-panel): send assetIssuer and show a balance hint - #624
Open
16navigabraham wants to merge 1 commit into
Open
feat(transaction-panel): send assetIssuer and show a balance hint#62416navigabraham wants to merge 1 commit into
16navigabraham wants to merge 1 commit into
Conversation
TransactionPanel's asset selector already let users pick a non-native asset from their balances, but TransactionParams had no assetIssuer field and the payload never included one, so a payment in USDC (or any other issued asset) would submit with only the asset code and no way for the API to disambiguate which issuer's USDC to pay with. - lib/client.ts: add an optional assetIssuer field to TransactionParams. - TransactionPanel.tsx: pass the selected balance's assetIssuer through to transaction.submit (undefined for native XLM), and show the selected asset's balance as a hint under the Amount input via the existing Input hint prop. Closes Sorokit#565 Closes Sorokit#566
|
@16navigabraham Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Closes #565. Closes #566 (duplicate of #565 — same title/problem, filed separately).
TransactionPanel's asset selector already let a user pick a non-native asset from theirbalances(added in a prior PR), but the actual submission never sent enough information for the backend to know which issuer's asset to pay with, and the amount input gave no feedback on how much of the selected asset was actually available.Problem
TransactionParams(src/lib/client.ts) has noassetIssuerfield, andTransactionPanel'ssubmitTransaction()only ever sentasset: selectedAsset(an asset code like"USDC") — never the issuer account. Two different tokens can share the same code with different issuers, so this payload is ambiguous for anything other than native XLM.What changed
src/lib/client.ts— addedassetIssuer?: stringtoTransactionParams.src/components/TransactionPanel.tsxsubmitTransaction()now passesassetIssuer: selectedAssetBalance?.assetIssuerin thetransaction.submit(...)call —undefinedfor native XLM (which has no issuer), and the balance'sassetIssuerfor anything else.Balance: {balance} {asset}via the existingInputcomponent'shintprop, which is automatically suppressed whenever a validationerroris showing (no double messaging).src/components/TransactionPanel.test.tsx— 3 new tests:assetIssueris included for a non-native asset and correctly matches the selected balance's issuer,assetIssuerisundefinedfor XLM, and the balance hint updates when the asset selection changes.Acceptance criteria
Testing
npx vitest run src/components/TransactionPanel.test.tsx→ same 25/36 pass asmainplus the 3 new tests (28 total passing here); the 11 failures are pre-existing and unrelated — see note below.npm run typecheck→ clean.Note for maintainers
While working on this I found
mainalready has 11 pre-existing, unrelated failures inTransactionPanel.test.tsx(confirmed viagit stashagainst a clean checkout) — every test that completes a submission through the confirmation modal fails with"Wallet not connected", because the shareduseSorokit()mock at the top of the file was never updated to include aclientfield after the "context client" refactor (clientnow comes fromuseSorokit(), not a directgetClient()call). My 3 new tests work around it locally (client: getClient()in their own mock), but I left the shared, already-broken tests alone since fixing them isn't in scope for this issue. Flagging in case it's worth a follow-up.