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

refactor(1702-3): batch account balance component #11368

Merged
merged 19 commits into from
Oct 4, 2024
Merged

refactor(1702-3): batch account balance component #11368

merged 19 commits into from
Oct 4, 2024
+316 −60

Conversation

vinnyhoward
Copy link
Contributor

@vinnyhoward vinnyhoward commented Sep 21, 2024 •
edited
Loading

Description

This PR is part of a series that breaks down the original large PR #11127 into smaller, more manageable chunks.

The BatchAccountBalance functionality is valuable beyond its original implementation. By converting it into a reusable component, we can:

  1. Improve code maintainability
  2. Reduce duplication
  3. Enhance consistency across the application and ease of use

Changes

  • Extracted BatchAccountBalance logic from its original location SecuritySettings and into a separate component file
  • Included new tests
  • Updated the SecuritySettings to use the new component
  • Removed related tests/constants within SecuritySettings and moved them into the new component folder

Impact

This refactored component will be used in:

  • Its current original screen which is SecuritySettings
  • New screens coming from the enhanced onboarding settings #1702 that is currently under progress

Related issues

Feature: #1702

Manual testing steps

  1. Make sure to have at least 2 accounts with a balance
  2. On the wallet home screen click on the accounts drop down and you will see a bottom sheet modal appear with your account names as well as a balance for all of your accounts
  3. Goto the security settings screen, find the "Batch account balance requests" section, and tap to toggle off
  4. Go back to the wallet home screen and click on the account picker once again
  5. Your active account should show your balance in the bottom sheet however inactive account balances will not be shown
  6. Repeat steps to toggle on/off to see your inactive balance appear and disappear

Screenshots/Recordings

batch_account

Before

NA

After

NA

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.

@vinnyhoward vinnyhoward added No QA Needed Apply this label when your PR does not need any QA effort. team-wallet-ux labels Sep 21, 2024
@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.

@vinnyhoward vinnyhoward marked this pull request as ready for review September 23, 2024 16:33
@vinnyhoward vinnyhoward requested a review from a team as a code owner September 23, 2024 16:33
@vinnyhoward vinnyhoward added Run Smoke E2E Triggers smoke e2e on Bitrise needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) and removed No QA Needed Apply this label when your PR does not need any QA effort. labels Sep 23, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 23, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: effd626
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/f4de151c-8710-42a4-b29c-bc0324284b3e

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

@vinnyhoward
Merge branch 'main' of github.com:MetaMask/metamask-mobile into feat-…
dc58be5 
…1702-3-refactor-batchaccountbalance-to-reusable-component
@vinnyhoward
Merge branch 'main' of github.com:MetaMask/metamask-mobile into feat-…
4b6b30b 
…1702-3-refactor-batchaccountbalance-to-reusable-component
@vinnyhoward vinnyhoward added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Sep 24, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 24, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 4b6b30b
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/8dea0d45-7592-4f81-97b6-0ecc38f887f0

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

@vinnyhoward
Merge branch 'main' of github.com:MetaMask/metamask-mobile into feat-…
3c789ae 
…1702-3-refactor-batchaccountbalance-to-reusable-component
@vinnyhoward vinnyhoward added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Sep 24, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 24, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 3c789ae
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c5e4ec4e-30cb-4e0e-b95a-041ecf3ef74d

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

@cortisiko
Copy link
Member

Smoke and Regression runs are passing on this PR.

@vinnyhoward vinnyhoward added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Sep 26, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 26, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 3a2756d
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/4fd0b92c-7083-45e4-9598-a22c4c6ff8ff

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

brianacnguyen
brianacnguyen previously requested changes Sep 27, 2024
View reviewed changes
Copy link
Contributor

@brianacnguyen brianacnguyen left a comment

Choose a reason for hiding this comment

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

Can you add a story for easy viewing?

app/components/Views/Settings/BatchAccountBalanceSettings/index.tsx Outdated Show resolved Hide resolved
@vinnyhoward vinnyhoward removed the Run Smoke E2E Triggers smoke e2e on Bitrise label Sep 27, 2024
gambinish
gambinish previously approved these changes Oct 3, 2024
View reviewed changes
Copy link
Contributor

@gambinish gambinish left a comment

Choose a reason for hiding this comment

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

Couple nits but LGTM ✅

@vinnyhoward vinnyhoward dismissed brianacnguyen’s stale review October 3, 2024 18:26

Stories are not needed here because its too specific and isn't in the component library. I can follow up with another PR to append stories to new components if needed

@EtherWizard33 EtherWizard33 self-requested a review October 3, 2024 18:32
EtherWizard33
EtherWizard33 previously approved these changes Oct 3, 2024
View reviewed changes
@gambinish gambinish mentioned this pull request Oct 3, 2024
Merged
7 tasks
@vinnyhoward
fix: removed white space and comments due to potential confilcts and …
80710fb 
…atropy from auto importing which might cause confusion down the line
@vinnyhoward vinnyhoward added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 3, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 3, 2024

https://bitrise.io/ Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: 91b206e
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/35e704cf-143e-41bd-8f2c-a3761b03f99e

Note

  • This comment will auto-update when build completes
  • 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

@vinnyhoward
fix: removed white space
17df8e6
@vinnyhoward
Merge branch 'feat-1702-3-refactor-batchaccountbalance-to-reusable-co…
eb503ed 
…mponent' of github.com:MetaMask/metamask-mobile into feat-1702-3-refactor-batchaccountbalance-to-reusable-component
andreahaku
andreahaku approved these changes Oct 4, 2024
View reviewed changes
Copy link
Member

@andreahaku andreahaku left a comment

Choose a reason for hiding this comment

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

LGTM

@vinnyhoward vinnyhoward added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 4, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 4, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: e494a19
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/110ca4da-dd02-442c-ab8e-2d222279869a

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

@EtherWizard33 EtherWizard33 self-requested a review October 4, 2024 13:48
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Oct 4, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@vinnyhoward vinnyhoward added this pull request to the merge queue Oct 4, 2024
Merged via the queue into main with commit bdc9043 Oct 4, 2024
43 checks passed
@vinnyhoward vinnyhoward deleted the feat-1702-3-refactor-batchaccountbalance-to-reusable-component branch October 4, 2024 14:32
@github-actions github-actions bot locked and limited conversation to collaborators Oct 4, 2024
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Oct 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@EtherWizard33 EtherWizard33 EtherWizard33 approved these changes

@andreahaku andreahaku andreahaku approved these changes

@gambinish gambinish gambinish left review comments

@brianacnguyen brianacnguyen brianacnguyen left review comments

Assignees
No one assigned
Labels
release-7.34.0 Run Smoke E2E Triggers smoke e2e on Bitrise team-assets team-wallet-ux
Projects
PR review queue
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@vinnyhoward @cortisiko @codecov-commenter @andreahaku @brianacnguyen @gambinish @EtherWizard33 @gauthierpetetin