feat: disable wallet buttons for accounts that cannot sign transactions #11330

k-g-j · 2024-09-19T20:55:30Z

Description

This PR disables certain buttons in the WalletActions component when the selected account cannot sign transactions. This change improves the user experience by preventing attempts to perform actions that require transaction signing when the account cannot do so.

The reason for the change is to prevent users from attempting actions that their current account cannot perform.
The improvement is that Buy, Sell, Send, Swap, and Bridge buttons are now disabled when the selected account cannot sign transactions, providing clear visual feedback to the user about available actions.

Related issues

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

Manual testing steps

Go to the SSK and create an account
Update the account using this JSON, replacing the address and ID fields with the new account information (this update step removes eth_signTransaction from the account's methods)

{
    "id": "your new account ID",
    "address": "you new account address",
    "options": {},
    "methods": [
        "personal_sign",
        "eth_sign",
        "eth_signTypedData_v1",
        "eth_signTypedData_v3",
        "eth_signTypedData_v4"
    ],
    "type": "eip155:eoa"
}

Switch to the account that cannot sign transactions if not already there
Verify that the Buy, Sell, Send, Swap, and Bridge buttons are visually disabled with a message and not clickable
Click on the asset details view
Verify that the Buy, Sell, Send, Swap, and Bridge buttons are visually disabled and not clickable
Switch back to an account that can sign transactions
Verify that the buttons are now enabled and functional in wallet view and asset details view

Screenshots/Recordings

Before

Wallet view

Asset details view

After

Wallet view

Asset details view

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards.
I've completed the PR template to the best of my ability
I've included tests if applicable
I've documented my code using JSDoc format if applicable
I've applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

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 · 2024-09-19T20:55:41Z

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.

github-actions · 2024-09-23T16:37:47Z

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 0f8068e
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/5ef1ab5f-a183-4616-bbb1-1086411a722d

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

github-actions · 2024-09-23T17:17:02Z

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: ed95ed3
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/3efa2677-f1bb-4979-8a75-c2aaa4555cfe

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

gantunesr

looks good overall, left some minor comments

app/components/UI/WalletAction/WalletAction.types.ts

app/components/Views/WalletActions/WalletActions.test.tsx

app/components/UI/WalletAction/WalletAction.types.ts

app/components/Views/WalletActions/WalletActions.tsx

app/components/UI/WalletAction/WalletAction.types.ts

app/components/Views/WalletActions/WalletActions.test.tsx

owencraston · 2024-09-24T19:35:04Z

locales/languages/en.json

@@ -1237,6 +1238,14 @@
    "info": "Info",
    "swap": "Swap",
    "bridge": "Bridge",
+    "disabled_button": {


@AlexJupiter @eriknson what do you think of this copy?

github-actions · 2024-09-26T19:56:54Z

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: f5d01a6
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/4b8e980b-bc7a-4554-b6b7-b155a1c30f8d

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

app/components/UI/WalletAction/WalletAction.tsx

app/components/Views/AssetDetails/AssetDetailsActions/AssetDetailsActions.tsx

github-actions · 2024-09-27T16:48:46Z

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: abda0b1
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/56bd3f1b-5482-4883-a2e9-cfe49cb8cc4f

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

github-actions · 2024-10-24T19:54:02Z

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 153c801
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c590af70-6880-4c18-9f3a-f22061ae6ba2

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

owencraston

Looking very good and very nice 😁 ! The code looks good I just had a few comments/questions about all the memoization of props.

app/components/UI/WalletAction/WalletAction.tsx

owencraston · 2024-10-24T20:03:03Z

app/components/Views/AssetDetails/AssetDetailsActions/AssetDetailsActions.tsx

            onPress={onBuy}
-            iconStyle={styles.icon}
-            containerStyle={styles.containerStyle}
+            {...walletActionProps}


I THINK the spread operator here defeats the purpose of the memoization since spreading the object will create a new reference anyway. See this stackoverflow thread. I am not certain about this so please correct me if I am wrong.

the same goes for the rest of the file.

I THINK you are right

I updated the code in this commit 37077bf so we're not creating new object references unnecessarily, which was defeating the purpose of memoization. It also makes the code more explicit about which props are being passed to each WalletAction component.

owencraston · 2024-10-24T20:04:24Z

app/components/Views/WalletActions/WalletActions.tsx

            actionID={WalletActionsModalSelectorsIDs.BUY_BUTTON}
+            iconStyle={styles.icon}
+            {...walletActionBaseProps}


same question about spreading the memoized props.

Same logic as your above comment and my point about creating new object references unnecessarily
addressed here 5685ae6

owencraston · 2024-10-24T20:07:04Z

you will need to run yarn jest -u pp/components/UI/AssetOverview/AssetOverview.test.tsx to update the snapshot and get your unit tests passing.

github-actions · 2024-10-24T21:05:27Z

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 21342d7
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/749fe63d-1cb3-43e0-bc1f-9d2a9ebceb56

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

# Conflicts: # app/components/Views/AssetDetails/AssetDetailsActions/AssetDetailsActions.test.tsx # app/components/Views/AssetDetails/AssetDetailsActions/AssetDetailsActions.tsx

sonarqubecloud · 2024-10-25T19:31:09Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

plasmacorral · 2024-10-29T01:21:06Z

Tested on Pixel 5a with Android 14 with commit f714dd1d56a1b22e79b6b53cc8a5668294a22eca. Buttons were enabled in the wallet view bottom bar and asset view when signing was enabled on the snap account, and disabled when signing permission was removed. Re-enabling signing restored the buttons. Checked on Eth mainnet, Sepolia, Base, and Arbitrum. No impact on button display or function for HD, imported, QR, and Ledger accounts.

github-actions · 2024-10-29T14:42:27Z

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: f714dd1
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/b9492f57-5c3f-430e-9c1a-799a0ee86c53

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

brianacnguyen · 2024-10-29T16:34:22Z

app/components/UI/WalletAction/WalletAction.tsx

+  const avatarStyle = useMemo(
+    () => ({
+      ...iconStyle,
+      backgroundColor: disabled ? colors.primary.muted : colors.primary.default,


this disabled change is not needed since the opacity would take care of color change

brianacnguyen

1 small request regarding avatar disabled

owencraston · 2024-11-05T01:54:47Z

Do to difficulty rebasing this PR I am continuing the work over here.

k-g-j changed the base branch from main to feat/display-snap-accounts-list September 19, 2024 20:56

k-g-j added area-UI needs-qa Any New Features that needs a full manual QA prior to being added to a release. team-accounts labels Sep 19, 2024

k-g-j changed the base branch from feat/display-snap-accounts-list to main September 23, 2024 15:09

k-g-j marked this pull request as ready for review September 23, 2024 16:34

k-g-j requested a review from a team as a code owner September 23, 2024 16:34

k-g-j added the Run Smoke E2E Triggers smoke e2e on Bitrise label Sep 23, 2024

k-g-j added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Sep 23, 2024

gantunesr reviewed Sep 24, 2024

View reviewed changes

app/components/UI/WalletAction/WalletAction.types.ts Outdated Show resolved Hide resolved

app/components/UI/WalletAction/WalletAction.types.ts Outdated Show resolved Hide resolved

app/components/Views/WalletActions/WalletActions.test.tsx Outdated Show resolved Hide resolved

k-g-j removed the Run Smoke E2E Triggers smoke e2e on Bitrise label Sep 24, 2024