Repository navigation
feat(stellar): use the shared Asset UI component - #428
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The migration preserves existing rendering behavior, removes obsolete code, and adds focused test coverage.
0 open findings
What changed in this PR
Migrates Stellar confirmation asset displays to the shared network utility components while preserving existing fiat behavior.
Changes:
- Replaces local asset UI components with shared
AssetandAssetIcon. - Keeps Stellar-specific fiat formatting at call sites and adds coverage.
- Updates SVG test transformation and removes obsolete assets/dependencies.
| File | Description |
|---|---|
yarn.lock |
Removes the unused SVG stub dependency. |
packages/stellar-wallet-snap/src/ui/images/question-mark.svg |
Deletes the local fallback icon. |
packages/stellar-wallet-snap/src/ui/images/index.ts |
Removes the deleted icon export. |
packages/stellar-wallet-snap/src/ui/confirmation/views/ConfirmSignChangeTrustOptOut/ConfirmSignChangeTrustOptOut.tsx |
Uses shared asset components. |
packages/stellar-wallet-snap/src/ui/confirmation/views/ConfirmSignChangeTrustOptIn/ConfirmSignChangeTrustOptIn.tsx |
Uses shared asset components. |
packages/stellar-wallet-snap/src/ui/confirmation/views/ConfirmSignChangeTrustOptIn/ConfirmSignChangeTrust.test.tsx |
Tests both change-trust views. |
packages/stellar-wallet-snap/src/ui/confirmation/components/ReadableParamsList.tsx |
Migrates readable asset rows. |
packages/stellar-wallet-snap/src/ui/confirmation/components/ReadableParamsList.test.tsx |
Covers asset-row rendering states. |
packages/stellar-wallet-snap/src/ui/confirmation/components/index.ts |
Removes obsolete exports. |
packages/stellar-wallet-snap/src/ui/confirmation/components/Fee.tsx |
Migrates fee asset rendering. |
packages/stellar-wallet-snap/src/ui/confirmation/components/Fee.test.tsx |
Covers fee and fiat rendering. |
packages/stellar-wallet-snap/src/ui/confirmation/components/AssetText.tsx |
Deletes the local text component. |
packages/stellar-wallet-snap/src/ui/confirmation/components/AssetIcon.tsx |
Deletes the local icon component. |
packages/stellar-wallet-snap/src/ui/confirmation/components/Asset.tsx |
Deletes the local asset component. |
packages/stellar-wallet-snap/snap.manifest.json |
Updates the built bundle checksum. |
packages/stellar-wallet-snap/package.json |
Removes the unused Jest transformer. |
packages/stellar-wallet-snap/jest.config.js |
Uses the shared SVG transformer. |
packages/stellar-wallet-snap/CHANGELOG.md |
Documents the component migration. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
stanleyyconsensys
left a comment
stanleyyconsensys
left a comment
Contributor
There was a problem hiding this comment.
some minor nit comment
| preferences={preferences} | ||
| priceLoading={priceLoading} | ||
| fiat={ | ||
| price |
Contributor
There was a problem hiding this comment.
can we place them before the render (after line 28)
| price={price ?? null} | ||
| priceLoading={priceLoading} | ||
| fiat={ | ||
| preferences && price && amount !== undefined |
Contributor
There was a problem hiding this comment.
looks likes it is keep repeat, please add a utils to handle
| import { FetchStatus } from '../../api'; | ||
| import { | ||
| Asset, | ||
| AssetIcon, |
Contributor
There was a problem hiding this comment.
question:
if no price, Asset doesnt need to pass any extra attribute , is that correct?
Comment on lines
+65
to
+72
| preferences | ||
| ? tokenPriceToFiat( | ||
| amount, | ||
| price, | ||
| preferences.currency, | ||
| preferences.locale, | ||
| ) | ||
| : null |
Contributor
There was a problem hiding this comment.
nit:
not a blocker, but we can do the same as Fee.tsx?
including the loading
stanleyyconsensys
approved these changes
Oct 9, 2026
|
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.
Explanation
Stellar integration of the shared
Assetcomponent (#418). The confirmation fee row, the readable operation asset rows and the change-trust confirmations now render the sharedAsset/AssetIcon, replacing Stellar's localAsset,AssetIconandAssetTextcomponents, which are deleted. Fiat formatting and gating stay at the call sites.Stellar tests now load SVGs with the shared
jest.svg-transformer.jsinstead of the empty stub, which the shared icon requires.References
Checklist