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

feat: display snap name #11511

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

feat: display snap name #11511

wants to merge 26 commits into from
+465 −450

Conversation

k-g-j
Copy link
Contributor

@k-g-j k-g-j commented Sep 30, 2024 •
edited
Loading

Description

This PR updates the display of snap accounts in the MetaMask mobile app. Previously, all snap accounts were shown with a generic “Snaps (Beta)” label. This change replaces the generic label with the specific name of the snap when available, providing users with clearer and more useful information about their snap accounts.

Key changes include:

  • Creation of the getInternalAccountByAddress utility function to retrieve account information of the InternalAccount type.
  • Modification of getAccountLabelTextByKeyring to return the snap name for snap accounts, or “Snaps (Beta)” if the name is unavailable.
  • Updates to affected components to display the correct snap name in the account list.
  • Unit tests added to verify the new functionality.

Related issues

Fixes: https://github.com/MetaMask/accounts-planning/issues/603

Manual testing steps

  1. Build the mobile client in Flask
  2. Navigate to the SSK dapp, install the snap, and create a snap account
  3. Open the account list
  4. You should see a tag under your new snap account that says "MetaMask Simple Snap Keyring"
  5. Note: we do not have any snaps yet that do not have names, so we will need to rely on the jest unit tests for the "Snaps (Beta)" rendering behavior

Screenshots/Recordings

Before

Simulator Screenshot - iPhone 15 Pro - 2024-10-16 at 11 43 53

After

Simulator Screenshot - iPhone 15 Pro - 2024-10-17 at 15 45 16

Simulator Screenshot - iPhone 15 Pro - 2024-10-17 at 15 48 22

Pre-merge author checklist

Pre-merge reviewer checklist

  • I’ve manually tested the PR by running the app and testing the modified behavior.
  • I confirm that this PR addresses all acceptance criteria described in the issue and includes the necessary testing evidence such as screenshots and recordings.

@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.

@k-g-j k-g-j marked this pull request as ready for review October 16, 2024 15:20
@k-g-j k-g-j requested review from a team as code owners October 16, 2024 15:20
owencraston
owencraston approved these changes Oct 17, 2024
View reviewed changes
Copy link
Contributor

@owencraston owencraston left a comment

Choose a reason for hiding this comment

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

Looks great!

Simulator Screenshot - iPhone 16 Pro - 2024-10-17 at 16 36 28

@k-g-j k-g-j added the Run Smoke E2E Triggers smoke e2e on Bitrise label Oct 18, 2024
@github-actions GitHub Actions
Copy link
Contributor

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

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 73b5448
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/72595d2b-4574-4631-9b48-72e0642ebba9

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

jpuri
jpuri approved these changes Oct 18, 2024
View reviewed changes
Copy link
Contributor

@jpuri jpuri left a comment

Choose a reason for hiding this comment

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

Changes in files owned by confirmations team look good.

@k-g-j k-g-j enabled auto-merge October 18, 2024 17:54
devin-ai-integration bot and others added 15 commits October 18, 2024 12:26
@devin-ai-integration @owencraston
chore(js-ts): Convert app/util/browserScripts.js to TypeScript (#11405)
0af0bc6 
Link to Devin run:
https://preview.devin.ai/devin/1b5085c4c16446edbf1768153a885382

If you have any feedback, you can leave comments in the PR and I'll
address them in the app!

Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>

parent 5b6a58802bcdb96763c475a1eba3603a9323a0a4
author Kate Johnson <johnson.kat.g@gmail.com> 1726169678 -0400
committer Kate Johnson <johnson.kat.g@gmail.com> 1727199130 -0400

parent 5b6a58802bcdb96763c475a1eba3603a9323a0a4
author Kate Johnson <johnson.kat.g@gmail.com> 1726169678 -0400
committer Kate Johnson <johnson.kat.g@gmail.com> 1727199127 -0400

parent 5b6a58802bcdb96763c475a1eba3603a9323a0a4
author Kate Johnson <johnson.kat.g@gmail.com> 1726169678 -0400
committer Kate Johnson <johnson.kat.g@gmail.com> 1727198968 -0400

parent 5b6a58802bcdb96763c475a1eba3603a9323a0a4
author Kate Johnson <johnson.kat.g@gmail.com> 1726169678 -0400
committer Kate Johnson <johnson.kat.g@gmail.com> 1727198956 -0400

parent 5b6a58802bcdb96763c475a1eba3603a9323a0a4
author Kate Johnson <johnson.kat.g@gmail.com> 1726169678 -0400
committer Kate Johnson <johnson.kat.g@gmail.com> 1727198918 -0400

parent 5b6a58802bcdb96763c475a1eba3603a9323a0a4
author Kate Johnson <johnson.kat.g@gmail.com> 1726169678 -0400
committer Kate Johnson <johnson.kat.g@gmail.com> 1727198912 -0400

parent 5b6a58802bcdb96763c475a1eba3603a9323a0a4
author Kate Johnson <johnson.kat.g@gmail.com> 1726169678 -0400
committer Kate Johnson <johnson.kat.g@gmail.com> 1727198902 -0400

parent 5b6a58802bcdb96763c475a1eba3603a9323a0a4
author Kate Johnson <johnson.kat.g@gmail.com> 1726169678 -0400
committer Kate Johnson <johnson.kat.g@gmail.com> 1727198892 -0400

revert: android and settings changes

chore: reverted unnecessary changes from main

feat: fixed test for new snaps tag
@k-g-j @owencraston
revert: changes in ios/MetaMask.xcodeproj/project.pbxproj
81ed973
@k-g-j @owencraston
feat: renamed 'getLabelTextByAddress' to 'getLabelTextByKeyring' and …
74ce63c 
…moved logic from AccountSelectorList
@k-g-j @owencraston
feat: added 'createMockAccountsControllerStateWithSnap' to accounts c…
96d9b21 
…ontroller test utils
@k-g-j @owencraston
revert: changes in xcodeproj
0606135
@k-g-j @owencraston
fix: code fence on KeyringTypes import
7da9cb1
@k-g-j @owencraston
feat: debugging mocked testing
7d7fa98
@k-g-j @owencraston
fix: unit test for 'getAccountLabelTextByKeyring'
1040291
@k-g-j @gantunesr @owencraston
feat: use KeyringTypes in accounts controller test utils
23988bd 
Co-authored-by: Gustavo Antunes <17601467+gantunesr@users.noreply.github.com>
@k-g-j @gantunesr @owencraston
feat: remove 'should' from test description
79aa332 
Co-authored-by: Gustavo Antunes <17601467+gantunesr@users.noreply.github.com>
@k-g-j @owencraston
fix: import KeyringTypes
e4564c1
@k-g-j @owencraston
feat: updated 'getAccountLabelTextByKeyring' JSDOC
99c5740
@k-g-j @owencraston
revert: changes in ios/MetaMask.xcodeproj/project.pbxproj
adc12d6
@k-g-j @owencraston
feat: added getInternalAccount by address util
40096a2
@k-g-j @owencraston
feat: JSDOC for 'getInternalAccountByAddress'
b19e128
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpuri jpuri jpuri approved these changes

@owencraston owencraston owencraston approved these changes

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

Assignees
No one assigned
Labels
area-UI mobile-snaps Run Smoke E2E Triggers smoke e2e on Bitrise team-accounts
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@k-g-j @jpuri @owencraston