Skip to content

feat(solana-wallet-snap): use the shared EstimatedChanges UI component - #384

Open
Julink-eth wants to merge 2 commits into
mainfrom
feat/solana-shared-estimated-changes
Open

Julink-eth wants to merge 2 commits into
mainfrom
feat/solana-shared-estimated-changes

Conversation

@Julink-eth

Copy link
Copy Markdown
Contributor

Explanation

This PR replaces Solana’s local EstimatedChanges implementation with the shared component from @metamask/snap-networks-utils.

Behavior change
Before: Solana showed its local loading skeleton again during a background re-scan, even when estimated changes were already available.
After: Previously estimated changes stay visible during re-scans, and scan errors only show the not-available state when there are no rows to display.
Notes
Keeps Solana-specific formatting and localization in the wrapper.

Removes the now-unused Solana EstimatedChangesHeader, EstimatedChangesSkeleton, and AssetChange components.
Adds/updates tests and changelog entries.

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Nullable scan amounts are incorrectly displayed as zero instead of the shared component’s unknown-value state.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Replaces Solana’s local estimated-changes UI with the shared network utility component while retaining Solana-specific formatting and localization.

Changes:

  • Integrates the shared EstimatedChanges component.
  • Removes obsolete local UI components.
  • Adds wrapper tests, changelog details, and generated configuration updates.
File Description
EstimatedChanges.tsx Adapts Solana scan data for the shared component.
EstimatedChanges.test.tsx Tests formatting and result states.
EstimatedChangesSkeleton.tsx Removes the local skeleton.
EstimatedChangesHeader.tsx Removes the local header.
AssetChange.tsx Removes local asset-row rendering.
snap.manifest.json Updates the bundle checksum.
jest.config.js Raises coverage thresholds.
CHANGELOG.md Documents the UI migration.
eslint-suppressions.json Removes suppressions for deleted code.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@Julink-eth
Julink-eth requested a balanced review from Copilot September 29, 2026 15:11
@sonarqubecloud

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The shared component integration preserves formatting and implements the documented loading and error behavior with suitable tests.

Review effort: Balanced
Findings: None

Resolved since last review (1)

This branch was successfully deployed

1 active (outdated) deployment
default-branch — 835b7608 Deployed Sep 29, 2026 by Julink-eth via Determine whether this PR is a release PR #1363
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants