Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: fix balance display in account info signature request #13740

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

fix: fix balance display in account info signature request #13740

wants to merge 12 commits into from

Conversation

sahar-fehri
Copy link
Contributor

@sahar-fehri sahar-fehri commented Feb 26, 2025 •
edited
Loading

Description

PR to fix balance display in signature confirmation component.
It also fixes the balance display when the network is being switched within a dapp.

Related issues

Fixes: #13730
Fixes: #13727

Manual testing steps

Description of manual testing steps are included in reported issues

  1. Go to this page...

Screenshots/Recordings

Before

After

Screen.Recording.2025-02-26.at.19.09.10.mov
Screen.Recording.2025-02-27.at.10.49.26.mov

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@github-actions GitHub Actions
Copy link
Contributor

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.

@sahar-fehri sahar-fehri added needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. Run Smoke E2E Triggers smoke e2e on Bitrise labels Feb 26, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 26, 2025 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: d575046
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/37320427-7103-4055-80aa-743634ecbcb8

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

Prithpal-Sooriya
Prithpal-Sooriya reviewed Feb 26, 2025
View reviewed changes
...ts/Confirm/AccountNetworkInfo/AccountNetworkInfoExpanded/AccountNetworkInfoExpanded.test.tsx
Comment on lines +12 to +17
useMultichainBalances: () => ({
multichainBalances: {
isPortfolioVieEnabled: true,
displayBalance: '$100',
},
}),
Copy link
Contributor

Choose a reason for hiding this comment

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

It would've been nice to test using pure state (and let the hooks correctly fire), but this hook is really complex!

@sahar-fehri sahar-fehri added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Feb 26, 2025
@sahar-fehri sahar-fehri marked this pull request as ready for review February 26, 2025 22:41
@sahar-fehri sahar-fehri requested a review from a team as a code owner February 26, 2025 22:41
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 26, 2025 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: ab5960a
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/bb4bc0df-76ce-4481-bdb0-585eb88cd879

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@sahar-fehri sahar-fehri requested a review from a team as a code owner February 26, 2025 23:51
@sahar-fehri sahar-fehri added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Feb 26, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 26, 2025 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: bb03cd0
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/62a945c4-b0d7-46ef-88ae-ede53e0c9e0a

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@sleepytanya sleepytanya mentioned this pull request Feb 27, 2025
Open
sahar-fehri
sahar-fehri commented Feb 27, 2025
View reviewed changes
app/components/Nav/Main/index.js
@@ -249,6 +257,14 @@ const Main = (props) => {
if (
hasNetworkChanged(chainId, previousProviderConfig.current, isEvmSelected)
) {
//set here token network filter if portfolio view is enabled
if (isPortfolioViewEnabled()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When a network is being switched within a dapp we need to set the correct tokenNetworkFilter so that the portfolio components returns the right aggregated balance.

@sleepytanya
Copy link
Contributor

@sahar-fehri @salimtb

Fixed!

Android prod build: https://app.bitrise.io/build/2823e9a0-0af9-4606-a2b4-3b3988dcd15c?tab=artifacts

fix_balance_dapp.mov
Screenshot 2025-02-27 at 12 28 30

@sleepytanya sleepytanya added the QA Passed A successful QA run through has been done label Feb 27, 2025
@salimtb salimtb added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Feb 27, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 27, 2025 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 507eb2d
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/6b8c609d-0c65-4dda-9c82-29cf62fa6f57

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
20.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@salimtb salimtb added Run Smoke E2E Triggers smoke e2e on Bitrise skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Feb 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Prithpal-Sooriya Prithpal-Sooriya Prithpal-Sooriya left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. QA Passed A successful QA run through has been done Run Smoke E2E Triggers smoke e2e on Bitrise skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. team-assets
Projects
None yet
Milestone
No milestone
5 participants
@sahar-fehri @sleepytanya @Prithpal-Sooriya @salimtb @metamaskbot