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

…d label from account overview" This reverts commit 54ac028.

github-actions · 2024-10-18T21:31:57Z

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: ab171e4
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/ec23b2a7-71bd-4803-82ad-722de18d238b

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

plasmacorral · 2024-10-18T22:06:07Z

Having an issue with local builds of flask on this branch in commit ab171e4

ERROR  Error: createSelector expects all input-selectors to be functions, but received the following types: [undefined, function memoized(), function memoized()], js engine: hermes
 ERROR  TypeError: Cannot read property 'store' of undefined, js engine: hermes
 ERROR  Error: createSelector expects all input-selectors to be functions, but received the following types: [undefined, function memoized(), function memoized()], js engine: hermes
 ERROR  TypeError: Cannot read property 'store' of undefined, js engine: hermes

Not observed with a flask build of main commit 076fa31

app/components/UI/WalletAction/WalletAction.tsx

brianacnguyen

1 small request

k-g-j · 2024-10-24T19:47:17Z

1 small request

I adjusted the styles to only use opacity: 0.5 in this commit 153c801 :)

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.

sonarcloud · 2024-10-24T21:03:38Z

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

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

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