From 43ce5f3f9476deb89a0b21b63a8154eb886af2a4 Mon Sep 17 00:00:00 2001 From: david0xd Date: Mon, 10 Jun 2024 15:57:40 +0200 Subject: [PATCH 1/5] Add abstraction for Snaps permissions Refactor and handle edge cases Turn off abstraction for Snap settings Fix broken unit test and refactor some stuff Fix Snap View unit test Fix lint in mock-state.json Try fixing scrolling fix hasScrolledToBottom Fix scrolling on snap update Apply some non-functional refactoring Update and refactor Permission Weights Remove unused function for permission icon Fix another edge case with required scrolling Update permission weights again Fix permission fade-overlay on Snap update Try fixing failing e2e tests Add additional e2e test fix Minor refactoring Do some requested refactoring Fix edge case when showing only less important permissions Add some comments Fix permission weight spec Add different implementation for rendering permission list Update permission weight for accounts Revert e2e test changes for accounts Fix unit test for permission list Revert some other e2e test changes as well Refactor initial show all logic Remove useMemo on Snap update Refactor usage of permission adapter Update show all initial state logic for update --- app/_locales/en/messages.json | 4 + shared/constants/permissions.ts | 38 +++++ test/data/mock-state.json | 48 ++++++ .../snaps/snap-permission-adapter/index.js | 1 + .../snap-permission-adapter.js | 36 +++++ .../snap-permission-adapter.test.js | 88 ++++++++++ .../snap-permissions-list.js | 96 ++++++++--- .../update-snap-permission-list.js | 151 +++++++++++------- ui/helpers/utils/permission.js | 99 +++--------- ui/helpers/utils/util.js | 27 ++++ ui/helpers/utils/util.test.js | 86 ++++++++++ ui/hooks/useScrollRequired.js | 15 +- .../snaps/snap-install/snap-install.js | 23 ++- .../snaps/snap-update/snap-update.js | 23 ++- ui/pages/snaps/snap-view/snap-settings.js | 1 + ui/pages/snaps/snap-view/snap-view.test.js | 2 +- 16 files changed, 567 insertions(+), 171 deletions(-) create mode 100644 ui/components/app/snaps/snap-permission-adapter/index.js create mode 100644 ui/components/app/snaps/snap-permission-adapter/snap-permission-adapter.js create mode 100644 ui/components/app/snaps/snap-permission-adapter/snap-permission-adapter.test.js diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 13406d331c36..14146236e935 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -4602,6 +4602,10 @@ "message": "Powered by $1", "description": "The security provider that is providing data" }, + "seeAllPermissions": { + "message": "See all permissions", + "description": "Used for revealing more content (e.g. permission list, etc.)" + }, "seeDetails": { "message": "See details" }, diff --git a/shared/constants/permissions.ts b/shared/constants/permissions.ts index 57cccfc81efb..a9bdd1e1e281 100644 --- a/shared/constants/permissions.ts +++ b/shared/constants/permissions.ts @@ -29,4 +29,42 @@ export const ConnectionPermission = Object.freeze({ connection_permission: 'connection_permission', }); +// This configuration specifies permission weight thresholds used to determine which +// permissions to show or hide on certain Snap-related flows (Install, Update, etc.) +export const PermissionWeightThreshold = Object.freeze({ + snapInstall: 3 as const, + snapUpdateApprovedPermissions: 2 as const, +}); + +// Specify number of permissions used as threshold for permission abstraction logic to be applied +export const PermissionsAbstractionThreshold = 3; + +export const PermissionWeight = Object.freeze({ + eth_accounts: 3, + permittedChains: 3, + snap_dialog: 4, + snap_notify: 4, + snap_getBip32PublicKey: 3, + snap_getBip32Entropy: 1, + snap_getBip44Entropy: 1, + snap_getEntropy: 3, + snap_manageState: 4, + snap_getLocale: 4, + wallet_snap: 4, + endowment_networkAccess: 3, + endowment_webassembly: 4, + endowment_transactionInsight: 4, + endowment_cronjob: 4, + endowment_ethereumProvider: 4, + endowment_rpc: 4, + endowment_lifecycleHooks: 4, + endowment_pageHome: 4, + snap_manageAccounts: 3, + endowment_keyring: 4, + endowment_nameLookup: 3, + endowment_signatureInsight: 4, + connection_permission: 3, + unknown_permission: 3, +}); + export * from './snaps/permissions'; diff --git a/test/data/mock-state.json b/test/data/mock-state.json index cbd53b0fe68d..2cc738853fa1 100644 --- a/test/data/mock-state.json +++ b/test/data/mock-state.json @@ -1937,6 +1937,54 @@ } } } + }, + "subjects": { + "npm:@metamask/test-snap-bip44": { + "origin": "npm:@metamask/test-snap-bip44", + "permissions": { + "endowment:rpc": { + "caveats": [ + { + "type": "rpcOrigin", + "value": { + "allowedOrigins": ["npm:@metamask/json-rpc-example-snap"], + "dapps": true + } + } + ], + "date": 1718117256761, + "id": "MhjpHKQFfGpMzI6YzkPGU", + "invoker": "npm:@metamask/test-snap-bip44", + "parentCapability": "endowment:rpc" + }, + "snap_dialog": { + "caveats": null, + "date": 1718117256761, + "id": "sBxmdvnow7QiN9aS4uSdn", + "invoker": "npm:@metamask/test-snap-bip44", + "parentCapability": "snap_dialog" + }, + "snap_getBip44Entropy": { + "caveats": [ + { + "type": "permittedCoinTypes", + "value": [ + { + "coinType": 1 + }, + { + "coinType": 3 + } + ] + } + ], + "date": 1718117256762, + "id": "R9tggB2pDzyCcbt6dIebN", + "invoker": "npm:@metamask/test-snap-bip44", + "parentCapability": "snap_getBip44Entropy" + } + } + } } }, "ramps": { diff --git a/ui/components/app/snaps/snap-permission-adapter/index.js b/ui/components/app/snaps/snap-permission-adapter/index.js new file mode 100644 index 000000000000..56f2ed001541 --- /dev/null +++ b/ui/components/app/snaps/snap-permission-adapter/index.js @@ -0,0 +1 @@ +export { default } from './snap-permission-adapter'; diff --git a/ui/components/app/snaps/snap-permission-adapter/snap-permission-adapter.js b/ui/components/app/snaps/snap-permission-adapter/snap-permission-adapter.js new file mode 100644 index 000000000000..548a68ccf3ed --- /dev/null +++ b/ui/components/app/snaps/snap-permission-adapter/snap-permission-adapter.js @@ -0,0 +1,36 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import SnapPermissionCell from '../snap-permission-cell'; + +export default function SnapPermissionAdapter({ + snapId, + permissions, + showOptions, + targetSubjectsMetadata, + revoked, + approved, +}) { + return permissions.map((permission, index) => ( + + )); +} + +SnapPermissionAdapter.propTypes = { + snapId: PropTypes.string.isRequired, + snapName: PropTypes.string.isRequired, + permissions: PropTypes.array.isRequired, + showOptions: PropTypes.bool, + targetSubjectsMetadata: PropTypes.object, + weightThreshold: PropTypes.number, + revoked: PropTypes.bool, + approved: PropTypes.bool, +}; diff --git a/ui/components/app/snaps/snap-permission-adapter/snap-permission-adapter.test.js b/ui/components/app/snaps/snap-permission-adapter/snap-permission-adapter.test.js new file mode 100644 index 000000000000..47703de424d7 --- /dev/null +++ b/ui/components/app/snaps/snap-permission-adapter/snap-permission-adapter.test.js @@ -0,0 +1,88 @@ +import React from 'react'; +import { screen } from '@testing-library/react'; +import { renderWithProvider } from '../../../../../test/jest'; +import configureStore from '../../../../store/store'; +import SnapPermissionAdapter from './snap-permission-adapter'; + +describe('Snap Permission Adapter', () => { + const mockSnapId = 'mock-snap-id'; + const mockSnapName = 'Snap Name'; + const mockTargetSubjectMetadata = { + extensionId: null, + iconUrl: null, + name: 'TypeScript Example Snap', + origin: 'local:http://localhost:8080', + subjectType: 'snap', + version: '0.2.2', + }; + const mockState = { + metamask: { + subjectMetadata: { + 'npm:@metamask/notifications-example-snap': { + name: 'Notifications Example Snap', + version: '1.2.3', + subjectType: 'snap', + }, + }, + snaps: { + 'npm:@metamask/notifications-example-snap': { + id: 'npm:@metamask/notifications-example-snap', + version: '1.2.3', + manifest: { + proposedName: 'Notifications Example Snap', + description: 'A snap', + }, + }, + }, + }, + }; + const mockWeightedPermissions = [ + { + leftIcon: 'hierarchy', + weight: 3, + label: 'Allow websites to communicate directly with Dialog Example Snap.', + description: {}, + permissionName: 'endowment:rpc', + permissionValue: { + caveats: [ + { + type: 'rpcOrigin', + value: { + dapps: true, + }, + }, + ], + }, + }, + { + label: 'Display dialog windows in MetaMask.', + description: {}, + leftIcon: 'messages', + weight: 4, + permissionName: 'snap_dialog', + permissionValue: {}, + }, + ]; + + const store = configureStore(mockState); + + it('renders set of provided permissions', () => { + renderWithProvider( + , + store, + ); + expect( + screen.queryByText('Display dialog windows in MetaMask.'), + ).toBeInTheDocument(); + expect( + screen.queryByText( + 'Allow websites to communicate directly with Dialog Example Snap.', + ), + ).toBeInTheDocument(); + }); +}); diff --git a/ui/components/app/snaps/snap-permissions-list/snap-permissions-list.js b/ui/components/app/snaps/snap-permissions-list/snap-permissions-list.js index 99b4a8b79995..1faac6805494 100644 --- a/ui/components/app/snaps/snap-permissions-list/snap-permissions-list.js +++ b/ui/components/app/snaps/snap-permissions-list/snap-permissions-list.js @@ -1,15 +1,27 @@ -import React from 'react'; +import React, { useState, useMemo } from 'react'; import PropTypes from 'prop-types'; import { useSelector } from 'react-redux'; -import { getWeightedPermissions } from '../../../../helpers/utils/permission'; import { useI18nContext } from '../../../../hooks/useI18nContext'; -import { Box } from '../../../component-library'; +import { Box, ButtonLink } from '../../../component-library'; import { getMultipleTargetsSubjectMetadata, getSnapsMetadata, } from '../../../../selectors'; -import { getSnapName } from '../../../../helpers/utils/util'; -import SnapPermissionCell from '../snap-permission-cell'; +import { + Display, + FlexDirection, + JustifyContent, +} from '../../../../helpers/constants/design-system'; +import { + PermissionsAbstractionThreshold, + PermissionWeightThreshold, +} from '../../../../../shared/constants/permissions'; +import { + getFilteredSnapPermissions, + getSnapName, +} from '../../../../helpers/utils/util'; +import { getWeightedPermissions } from '../../../../helpers/utils/permission'; +import SnapPermissionAdapter from '../snap-permission-adapter'; export default function SnapPermissionsList({ snapId, @@ -17,36 +29,68 @@ export default function SnapPermissionsList({ permissions, connections, showOptions, + showAllPermissions, + onShowAllPermissions, }) { const t = useI18nContext(); - const snapsMetadata = useSelector(getSnapsMetadata); - const permissionsToShow = { - ...permissions, - connection_permission: connections ?? {}, - }; + + const combinedPermissions = useMemo(() => { + return { ...permissions, connection_permission: connections ?? {} }; + }, [permissions, connections]); + const targetSubjectsMetadata = useSelector((state) => getMultipleTargetsSubjectMetadata(state, connections), ); + const snapsMetadata = useSelector(getSnapsMetadata); + + const weightedPermissions = getWeightedPermissions({ + t, + permissions: combinedPermissions, + subjectName: snapName, + getSubjectName: getSnapName(snapsMetadata), + }); + + const [showAll, setShowAll] = useState( + showAllPermissions || + Object.keys(weightedPermissions).length <= + PermissionsAbstractionThreshold, + ); + + const filteredPermissions = getFilteredSnapPermissions( + weightedPermissions, + PermissionWeightThreshold.snapInstall, + true, + ); + + const onShowAllPermissionsHandler = () => { + onShowAllPermissions(); + setShowAll(true); + }; + return ( - - {getWeightedPermissions({ - t, - permissions: permissionsToShow, - subjectName: snapName, - getSubjectName: getSnapName(snapsMetadata), - }).map((permission, index) => ( - + + - ))} + + {showAll ? null : ( + + onShowAllPermissionsHandler()}> + {t('seeAllPermissions')} + + + )} ); } @@ -57,4 +101,6 @@ SnapPermissionsList.propTypes = { permissions: PropTypes.object.isRequired, connections: PropTypes.object, showOptions: PropTypes.bool, + showAllPermissions: PropTypes.bool, + onShowAllPermissions: PropTypes.func, }; diff --git a/ui/components/app/snaps/update-snap-permission-list/update-snap-permission-list.js b/ui/components/app/snaps/update-snap-permission-list/update-snap-permission-list.js index 4f6546e76851..98d7721f1d14 100644 --- a/ui/components/app/snaps/update-snap-permission-list/update-snap-permission-list.js +++ b/ui/components/app/snaps/update-snap-permission-list/update-snap-permission-list.js @@ -1,16 +1,24 @@ -import React from 'react'; +import React, { useState } from 'react'; import { useSelector } from 'react-redux'; import PropTypes from 'prop-types'; -import { getWeightedPermissions } from '../../../../helpers/utils/permission'; import { useI18nContext } from '../../../../hooks/useI18nContext'; -import { Box } from '../../../component-library'; +import { Box, ButtonLink } from '../../../component-library'; import { getMultipleTargetsSubjectMetadata, getSnapMetadata, getSnapsMetadata, } from '../../../../selectors'; -import { getSnapName } from '../../../../helpers/utils/util'; -import SnapPermissionCell from '../snap-permission-cell'; +import SnapPermissionAdapter from '../snap-permission-adapter'; +import { + Display, + JustifyContent, +} from '../../../../helpers/constants/design-system'; +import { PermissionWeightThreshold } from '../../../../../shared/constants/permissions'; +import { + getFilteredSnapPermissions, + getSnapName, +} from '../../../../helpers/utils/util'; +import { getWeightedPermissions } from '../../../../helpers/utils/permission'; export default function UpdateSnapPermissionList({ approvedPermissions, @@ -20,6 +28,7 @@ export default function UpdateSnapPermissionList({ revokedConnections, newConnections, targetSubjectMetadata, + showAllPermissions, }) { const t = useI18nContext(); const snapId = targetSubjectMetadata.origin; @@ -37,75 +46,95 @@ export default function UpdateSnapPermissionList({ ); const snapsMetadata = useSelector(getSnapsMetadata); - const snapsNameGetter = getSnapName(snapsMetadata); - const approvedPermissionsToShow = { + const approvedCombinedPermissions = { ...approvedPermissions, connection_permission: approvedConnections ?? {}, }; - const revokedPermissionsToShow = { + const revokedCombinedPermissions = { ...revokedPermissions, connection_permission: revokedConnections ?? {}, }; - const newPermissionsToShow = { + const newCombinedPermissions = { ...newPermissions, connection_permission: newConnections ?? {}, }; + const newWeightedPermissions = getWeightedPermissions({ + t, + permissions: newCombinedPermissions, + subjectName: snapName, + getSubjectName: getSnapName(snapsMetadata), + }); + + const revokedWeightedPermissions = getWeightedPermissions({ + t, + permissions: revokedCombinedPermissions, + subjectName: snapName, + getSubjectName: getSnapName(snapsMetadata), + }); + + const approvedWeightedPermissions = getWeightedPermissions({ + t, + permissions: approvedCombinedPermissions, + subjectName: snapName, + getSubjectName: getSnapName(snapsMetadata), + }); + + const [showAll, setShowAll] = useState( + Object.keys(approvedWeightedPermissions).length < 1, + ); + + const filteredApprovedWeightedPermissions = getFilteredSnapPermissions( + approvedWeightedPermissions, + PermissionWeightThreshold.snapUpdateApprovedPermissions, + ); + + const onShowAllPermissions = () => { + showAllPermissions(); + setShowAll(true); + }; + return ( - {getWeightedPermissions({ - t, - permissions: newPermissionsToShow, - subjectName: snapName, - getSubjectName: snapsNameGetter, - }).map((permission, index) => ( - - ))} - {getWeightedPermissions({ - t, - permissions: revokedPermissionsToShow, - subjectName: snapName, - getSubjectName: snapsNameGetter, - }).map((permission, index) => ( - - ))} - {getWeightedPermissions({ - t, - permissions: approvedPermissionsToShow, - subjectName: snapName, - getSubjectName: snapsNameGetter, - }).map((permission, index) => ( - - ))} + + + + {showAll ? null : ( + + onShowAllPermissions()}> + {t('seeAllPermissions')} + + + )} ); } @@ -135,5 +164,9 @@ UpdateSnapPermissionList.propTypes = { * New pre-approved connections that are being requested */ newConnections: PropTypes.object.isRequired, + /** + * Callback function used to handle revealing all permissions action in UI. + */ + showAllPermissions: PropTypes.func.isRequired, targetSubjectMetadata: PropTypes.object.isRequired, }; diff --git a/ui/helpers/utils/permission.js b/ui/helpers/utils/permission.js index 420edc2098e1..740a9de49de0 100644 --- a/ui/helpers/utils/permission.js +++ b/ui/helpers/utils/permission.js @@ -8,13 +8,12 @@ import { getSnapDerivationPathName, } from '@metamask/snaps-utils'; import { isNonEmptyArray } from '@metamask/controller-utils'; -import classnames from 'classnames'; import { RestrictedMethods, EndowmentPermissions, ConnectionPermission, + PermissionWeight, } from '../../../shared/constants/permissions'; -import Tooltip from '../../components/ui/tooltip'; import { Icon, Text, @@ -52,12 +51,12 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ [RestrictedMethods.eth_accounts]: ({ t }) => ({ label: t('permission_ethereumAccounts'), leftIcon: IconName.Eye, - weight: 3, + weight: PermissionWeight.eth_accounts, }), [PermissionNames.permittedChains]: ({ t }) => ({ label: t('permission_walletSwitchEthereumChain'), leftIcon: IconName.Wifi, - weight: 3, + weight: PermissionWeight.permittedChains, }), [RestrictedMethods.snap_dialog]: ({ t, subjectName }) => ({ label: t('permission_dialog'), @@ -65,7 +64,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ getSnapNameComponent(subjectName), ]), leftIcon: IconName.Messages, - weight: 4, + weight: PermissionWeight.snap_dialog, }), [RestrictedMethods.snap_notify]: ({ t, subjectName }) => ({ label: t('permission_notifications'), @@ -73,7 +72,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ getSnapNameComponent(subjectName), ]), leftIcon: IconName.Notification, - weight: 4, + weight: PermissionWeight.snap_notify, }), [RestrictedMethods.snap_getBip32PublicKey]: ({ t, @@ -83,7 +82,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ permissionValue.caveats[0].value.map(({ path, curve }, i) => { const baseDescription = { leftIcon: IconName.SecuritySearch, - weight: 2, + weight: PermissionWeight.snap_getBip32PublicKey, id: `public-key-access-bip32-${path .join('-') ?.replace(/'/gu, 'h')}-${curve}-${i}`, @@ -154,7 +153,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ permissionValue.caveats[0].value.map(({ path, curve }, i) => { const baseDescription = { leftIcon: IconName.Key, - weight: 1, + weight: PermissionWeight.snap_getBip32Entropy, id: `key-access-bip32-${path .join('-') ?.replace(/'/gu, 'h')}-${curve}-${i}`, @@ -221,7 +220,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ getSnapNameComponent(subjectName), ]), leftIcon: IconName.Key, - weight: 1, + weight: PermissionWeight.snap_getBip44Entropy, id: `key-access-bip44-${coinType}-${i}`, warningMessageSubject: getSlip44ProtocolName(coinType) ?? @@ -233,7 +232,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ getSnapNameComponent(subjectName), ]), leftIcon: IconName.SecurityKey, - weight: 4, + weight: PermissionWeight.snap_getEntropy, }), [RestrictedMethods.snap_manageState]: ({ t, subjectName }) => ({ @@ -242,7 +241,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ getSnapNameComponent(subjectName), ]), leftIcon: IconName.AddSquare, - weight: 4, + weight: PermissionWeight.snap_manageState, }), [RestrictedMethods.snap_getLocale]: ({ t, subjectName }) => ({ label: t('permission_getLocale'), @@ -250,13 +249,14 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ getSnapNameComponent(subjectName), ]), leftIcon: IconName.Global, - weight: 4, + weight: PermissionWeight.snap_getLocale, }), [RestrictedMethods.wallet_snap]: ({ t, permissionValue, getSubjectName }) => { const snaps = permissionValue.caveats[0].value; const baseDescription = { leftIcon: IconName.Flash, rightIcon: RIGHT_INFO_ICON, + weight: PermissionWeight.wallet_snap, }; return Object.keys(snaps).map((snapId) => { @@ -291,7 +291,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ getSnapNameComponent(subjectName), ]), leftIcon: IconName.Wifi, - weight: 3, + weight: PermissionWeight.endowment_networkAccess, }), [EndowmentPermissions['endowment:webassembly']]: ({ t, subjectName }) => ({ label: t('permission_webAssembly'), @@ -300,7 +300,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ ]), leftIcon: IconName.DocumentCode, rightIcon: null, - weight: 3, + weight: PermissionWeight.endowment_webassembly, }), [EndowmentPermissions['endowment:transaction-insight']]: ({ t, @@ -309,7 +309,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ }) => { const baseDescription = { leftIcon: IconName.Speedometer, - weight: 4, + weight: PermissionWeight.endowment_transactionInsight, }; const result = [ @@ -345,7 +345,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ getSnapNameComponent(subjectName), ]), leftIcon: IconName.Clock, - weight: 3, + weight: PermissionWeight.endowment_cronjob, }), [EndowmentPermissions['endowment:ethereum-provider']]: ({ t, @@ -356,7 +356,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ getSnapNameComponent(subjectName), ]), leftIcon: IconName.Ethereum, - weight: 3, + weight: PermissionWeight.endowment_ethereumProvider, id: 'ethereum-provider-access', message: t('ethereumProviderAccess', [getSnapNameComponent(subjectName)]), }), @@ -367,7 +367,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ }) => { const baseDescription = { leftIcon: IconName.Hierarchy, - weight: 3, + weight: PermissionWeight.endowment_rpc, }; const { snaps, dapps, allowedOrigins } = @@ -466,7 +466,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ getSnapNameComponent(subjectName), ]), leftIcon: IconName.Hierarchy, - weight: 4, + weight: PermissionWeight.endowment_lifecycleHooks, }), [EndowmentPermissions['endowment:page-home']]: ({ t, subjectName }) => ({ label: t('permission_homePage'), @@ -474,7 +474,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ getSnapNameComponent(subjectName), ]), leftIcon: IconName.Home, - weight: 4, + weight: PermissionWeight.endowment_pageHome, }), ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) [RestrictedMethods.snap_manageAccounts]: ({ t, subjectName }) => ({ @@ -484,7 +484,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ ]), leftIcon: IconName.UserCircleAdd, rightIcon: null, - weight: 3, + weight: PermissionWeight.snap_manageAccounts, }), [EndowmentPermissions['endowment:keyring']]: ({ t, subjectName }) => ({ label: t('permission_keyring'), @@ -493,7 +493,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ ]), leftIcon: IconName.UserCircleAdd, rightIcon: null, - weight: 3, + weight: PermissionWeight.endowment_keyring, }), ///: END:ONLY_INCLUDE_IF ///: BEGIN:ONLY_INCLUDE_IF(build-flask) @@ -501,7 +501,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ label: t('permission_nameLookup'), description: t('permission_nameLookupDescription'), leftIcon: IconName.Search, - weight: 4, + weight: PermissionWeight.endowment_nameLookup, }), ///: END:ONLY_INCLUDE_IF [EndowmentPermissions['endowment:signature-insight']]: ({ @@ -511,7 +511,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ }) => { const baseDescription = { leftIcon: IconName.Warning, - weight: 3, + weight: PermissionWeight.endowment_signatureInsight, }; const result = [ @@ -582,7 +582,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ connection, connectionName, subjectName, - weight: 3, + weight: PermissionWeight.connection_permission, }; }); }, @@ -590,7 +590,7 @@ export const PERMISSION_DESCRIPTIONS = deepFreeze({ label: t('permission_unknown', [permissionName ?? 'undefined']), leftIcon: IconName.Question, rightIcon: null, - weight: 5, + weight: PermissionWeight.unknown_permission, }), }); @@ -687,50 +687,3 @@ export function getWeightedPermissions({ ) .sort((left, right) => left.weight - right.weight); } - -/** - * Get the right icon for a permission. If a description is provided, the icon - * will be wrapped in a tooltip. Otherwise, the icon will be rendered as-is. If - * there's no right icon, this function will return null. - * - * If the weight is 1, the icon will be rendered with a warning color. - * - * @param {PermissionLabelObject} permission - The permission object. - * @param {JSX.Element | string} permission.rightIcon - The right icon. - * @param {string} permission.description - The description. - * @param {number} permission.weight - The weight. - * @returns {JSX.Element | null} The right icon, or null if there's no - * right icon. - */ -export function getRightIcon({ rightIcon, description, weight }) { - if (rightIcon && description) { - return ( - {description}} - position="bottom" - > - {typeof rightIcon === 'string' ? ( - - ) : ( - rightIcon - )} - - ); - } - - if (rightIcon) { - if (typeof rightIcon === 'string') { - return ( - - ); - } - - return rightIcon; - } - - return null; -} diff --git a/ui/helpers/utils/util.js b/ui/helpers/utils/util.js index 2c55f520ad6d..1fa1d9abd784 100644 --- a/ui/helpers/utils/util.js +++ b/ui/helpers/utils/util.js @@ -781,3 +781,30 @@ export const hexToText = (hex) => { export const getAvatarFallbackLetter = (subjectName) => { return subjectName?.match(/[a-z0-9]/iu)?.[0] ?? '?'; }; + +/** + * Get Snap permissions filtered by weight. + * + * @param weightedPermissions - Set of Snap permissions that have 'weight' property assigned. + * @param weightThreshold - Number that represents weight threshold for filtering. + * @param showFirstThreeOnEmpty - Boolean flag to control if first three permissions should be shown + * when no permissions meet the weight criteria. + * @returns Subset of permissions passing weight threshold. + */ +export const getFilteredSnapPermissions = ( + weightedPermissions, + weightThreshold, + showFirstThreeOnEmpty = false, +) => { + let filteredPermissions = weightedPermissions.filter( + (permission) => permission.weight <= (weightThreshold ?? Infinity), + ); + + // If there are no permissions that fall into desired set filtered by weight, + // then show only the first three, no matter what the weight is + if (showFirstThreeOnEmpty && filteredPermissions.length === 0) { + filteredPermissions = weightedPermissions.slice(0, 3); + } + + return filteredPermissions; +}; diff --git a/ui/helpers/utils/util.test.js b/ui/helpers/utils/util.test.js index 2ad0db6e50a0..5b59b0a15502 100644 --- a/ui/helpers/utils/util.test.js +++ b/ui/helpers/utils/util.test.js @@ -1093,4 +1093,90 @@ describe('util', () => { ).toStrictEqual('0x12...'); }); }); + + describe('getFilteredSnapPermissions', () => { + it('should return permission filtered by weight', () => { + const WEIGHT_THRESHOLD = 3; + const mockPermissions = [ + { + label: 'Permission A', + weight: 4, + }, + { + label: 'Permission B', + weight: 4, + }, + { + label: 'Permission C', + weight: 1, + }, + { + label: 'Permission D', + weight: 5, + }, + { + label: 'Permission E', + weight: 2, + }, + ]; + expect( + util.getFilteredSnapPermissions(mockPermissions, WEIGHT_THRESHOLD), + ).toStrictEqual([ + { + label: 'Permission C', + weight: 1, + }, + { + label: 'Permission E', + weight: 2, + }, + ]); + }); + + it('should return the first three permissions because none matches the filter criteria', () => { + const WEIGHT_THRESHOLD = 3; + const mockPermissions = [ + { + label: 'Permission A', + weight: 4, + }, + { + label: 'Permission B', + weight: 4, + }, + { + label: 'Permission C', + weight: 5, + }, + { + label: 'Permission D', + weight: 5, + }, + { + label: 'Permission E', + weight: 6, + }, + ]; + expect( + util.getFilteredSnapPermissions( + mockPermissions, + WEIGHT_THRESHOLD, + true, + ), + ).toStrictEqual([ + { + label: 'Permission A', + weight: 4, + }, + { + label: 'Permission B', + weight: 4, + }, + { + label: 'Permission C', + weight: 5, + }, + ]); + }); + }); }); diff --git a/ui/hooks/useScrollRequired.js b/ui/hooks/useScrollRequired.js index 196a91e3f3db..20ea25121227 100644 --- a/ui/hooks/useScrollRequired.js +++ b/ui/hooks/useScrollRequired.js @@ -1,5 +1,6 @@ import { useEffect, useRef, useState } from 'react'; import { debounce } from 'lodash'; +import { usePrevious } from './usePrevious'; /** * Utility hook for requiring users to scroll through content. @@ -17,6 +18,7 @@ export const useScrollRequired = ( { offsetPxFromBottom = 16 } = {}, ) => { const ref = useRef(null); + const prevOffsetHeight = usePrevious(ref.current?.offsetHeight); const [hasScrolledToBottomState, setHasScrolledToBottom] = useState(false); const [isScrollableState, setIsScrollable] = useState(false); @@ -26,6 +28,7 @@ export const useScrollRequired = ( if (!ref.current) { return; } + const isScrollable = ref.current && ref.current.scrollHeight > ref.current.clientHeight; @@ -38,7 +41,11 @@ export const useScrollRequired = ( offsetPxFromBottom >= ref.current.scrollHeight; - setIsScrollable(isScrollable); + if (isScrollable !== isScrollableState) { + setHasScrolledToBottom(false); + setIsScrollable(isScrollable); + } + setIsScrolledToBottom(!isScrollable || isScrolledToBottom); if (!isScrollable || isScrolledToBottom) { @@ -48,6 +55,12 @@ export const useScrollRequired = ( useEffect(update, [ref, ...dependencies]); + useEffect(() => { + if (prevOffsetHeight !== ref.current?.offsetHeight) { + update(); + } + }, [ref.current?.offsetHeight]); + const scrollToBottom = () => { setIsScrolledToBottom(true); setHasScrolledToBottom(true); diff --git a/ui/pages/permissions-connect/snaps/snap-install/snap-install.js b/ui/pages/permissions-connect/snaps/snap-install/snap-install.js index 9cda972b8ef5..3b636bdddb45 100644 --- a/ui/pages/permissions-connect/snaps/snap-install/snap-install.js +++ b/ui/pages/permissions-connect/snaps/snap-install/snap-install.js @@ -47,8 +47,9 @@ export default function SnapInstall({ const { origin, iconUrl } = siteMetadata; const [isShowingWarning, setIsShowingWarning] = useState(false); const snapsMetadata = useSelector(getSnapsMetadata); + const [showAllPermissions, setShowAllPermissions] = useState(false); - const { isScrollable, isScrolledToBottom, scrollToBottom, ref, onScroll } = + const { isScrollable, hasScrolledToBottom, scrollToBottom, ref, onScroll } = useScrollRequired([requestState]); const onCancel = useCallback( @@ -96,6 +97,10 @@ export default function SnapInstall({ return 'confirm'; }; + const onShowAllPermissionsHandler = () => { + setShowAllPermissions(true); + }; + return ( - {isScrollable && !isScrolledToBottom ? ( - + + + {isScrollable && !hasScrolledToBottom && !showAllPermissions ? ( - - ) : null} + ) : null} + )} @@ -219,7 +226,11 @@ export default function SnapInstall({ cancelButtonType="default" hideCancel={hasError} disabled={ - isLoading || (!hasError && isScrollable && !isScrolledToBottom) + isLoading || + (!hasError && + isScrollable && + !hasScrolledToBottom && + !showAllPermissions) } onCancel={onCancel} cancelText={t('cancel')} diff --git a/ui/pages/permissions-connect/snaps/snap-update/snap-update.js b/ui/pages/permissions-connect/snaps/snap-update/snap-update.js index dd2ff4a96ce6..1bde0f37198f 100644 --- a/ui/pages/permissions-connect/snaps/snap-update/snap-update.js +++ b/ui/pages/permissions-connect/snaps/snap-update/snap-update.js @@ -45,7 +45,9 @@ export default function SnapUpdate({ const [isShowingWarning, setIsShowingWarning] = useState(false); - const { isScrollable, isScrolledToBottom, scrollToBottom, ref, onScroll } = + const [showAllPermissions, setShowAllPermissions] = useState(false); + + const { isScrollable, hasScrolledToBottom, scrollToBottom, ref, onScroll } = useScrollRequired([requestState]); const snapsMetadata = useSelector(getSnapsMetadata); @@ -93,6 +95,10 @@ export default function SnapUpdate({ } }; + const onShowAllPermissions = () => { + setShowAllPermissions(true); + }; + return ( - {isScrollable && !isScrolledToBottom ? ( - + + {isScrollable && !hasScrolledToBottom && !showAllPermissions ? ( - - ) : null} + ) : null} + )} @@ -225,7 +232,11 @@ export default function SnapUpdate({ cancelButtonType="default" hideCancel={hasError} disabled={ - isLoading || (!hasError && isScrollable && !isScrolledToBottom) + isLoading || + (!hasError && + isScrollable && + !hasScrolledToBottom && + !showAllPermissions) } onCancel={onCancel} cancelText={t('cancel')} diff --git a/ui/pages/snaps/snap-view/snap-settings.js b/ui/pages/snaps/snap-view/snap-settings.js index b7bd8990af49..ff16dd6d7158 100644 --- a/ui/pages/snaps/snap-view/snap-settings.js +++ b/ui/pages/snaps/snap-view/snap-settings.js @@ -162,6 +162,7 @@ function SnapSettings({ snapId, initRemove, resetInitRemove }) { snapName={snapName} permissions={permissions ?? {}} showOptions + showAllPermissions /> diff --git a/ui/pages/snaps/snap-view/snap-view.test.js b/ui/pages/snaps/snap-view/snap-view.test.js index ac1746c9dea6..3bc65835f8d9 100644 --- a/ui/pages/snaps/snap-view/snap-view.test.js +++ b/ui/pages/snaps/snap-view/snap-view.test.js @@ -40,7 +40,7 @@ describe('SnapView', () => { renderWithProvider(, mockStore); // Snap name & Snap authorship component - expect(getAllByText('BIP-44 Test Snap')).toHaveLength(1); + expect(getAllByText('BIP-44 Test Snap')).toHaveLength(3); expect( container.getElementsByClassName('snaps-authorship-expanded')?.length, ).toBe(1); From 1ea21d9fc86d140b1eceedd976464e07cd895578 Mon Sep 17 00:00:00 2001 From: david0xd Date: Tue, 13 Aug 2024 17:37:03 +0200 Subject: [PATCH 2/5] Refactor and update logic around permission abstraction for edge cases --- shared/constants/permissions.ts | 3 ++ .../snap-permissions-list.js | 3 +- .../update-snap-permission-list.js | 22 ++++++++ ui/helpers/utils/util.js | 36 +++++++++---- ui/helpers/utils/util.test.js | 51 ++++++++++++++++++- 5 files changed, 102 insertions(+), 13 deletions(-) diff --git a/shared/constants/permissions.ts b/shared/constants/permissions.ts index a9bdd1e1e281..d55083b0b84a 100644 --- a/shared/constants/permissions.ts +++ b/shared/constants/permissions.ts @@ -36,6 +36,9 @@ export const PermissionWeightThreshold = Object.freeze({ snapUpdateApprovedPermissions: 2 as const, }); +// Specify minimum number of permissions to be shown, when abstraction is applied +export const MinPermissionAbstractionDisplayCount = 3; + // Specify number of permissions used as threshold for permission abstraction logic to be applied export const PermissionsAbstractionThreshold = 3; diff --git a/ui/components/app/snaps/snap-permissions-list/snap-permissions-list.js b/ui/components/app/snaps/snap-permissions-list/snap-permissions-list.js index 1faac6805494..498bab4d9e7f 100644 --- a/ui/components/app/snaps/snap-permissions-list/snap-permissions-list.js +++ b/ui/components/app/snaps/snap-permissions-list/snap-permissions-list.js @@ -13,6 +13,7 @@ import { JustifyContent, } from '../../../../helpers/constants/design-system'; import { + MinPermissionAbstractionDisplayCount, PermissionsAbstractionThreshold, PermissionWeightThreshold, } from '../../../../../shared/constants/permissions'; @@ -60,7 +61,7 @@ export default function SnapPermissionsList({ const filteredPermissions = getFilteredSnapPermissions( weightedPermissions, PermissionWeightThreshold.snapInstall, - true, + MinPermissionAbstractionDisplayCount, ); const onShowAllPermissionsHandler = () => { diff --git a/ui/components/app/snaps/update-snap-permission-list/update-snap-permission-list.js b/ui/components/app/snaps/update-snap-permission-list/update-snap-permission-list.js index 98d7721f1d14..4e39fb6f88b3 100644 --- a/ui/components/app/snaps/update-snap-permission-list/update-snap-permission-list.js +++ b/ui/components/app/snaps/update-snap-permission-list/update-snap-permission-list.js @@ -87,9 +87,31 @@ export default function UpdateSnapPermissionList({ Object.keys(approvedWeightedPermissions).length < 1, ); + // Because approved permissions are sometimes hidden following the abstraction logic, + // it is needed sometimes to fill the gap in permission display, in certain edge cases + // when there is not enough new and revoked permissions to be shown. + let minApprovedPermissionsToShow; + const totalNewAndRevokedPermissions = + newWeightedPermissions.length + revokedWeightedPermissions.length; + + switch (totalNewAndRevokedPermissions) { + case 0: + minApprovedPermissionsToShow = 3; + break; + case 1: + minApprovedPermissionsToShow = 2; + break; + case 2: + minApprovedPermissionsToShow = 1; + break; + default: + minApprovedPermissionsToShow = 0; + } + const filteredApprovedWeightedPermissions = getFilteredSnapPermissions( approvedWeightedPermissions, PermissionWeightThreshold.snapUpdateApprovedPermissions, + minApprovedPermissionsToShow, ); const onShowAllPermissions = () => { diff --git a/ui/helpers/utils/util.js b/ui/helpers/utils/util.js index 1fa1d9abd784..fa19dc8a4a2b 100644 --- a/ui/helpers/utils/util.js +++ b/ui/helpers/utils/util.js @@ -783,27 +783,43 @@ export const getAvatarFallbackLetter = (subjectName) => { }; /** - * Get Snap permissions filtered by weight. + * Get abstracted Snap permissions filtered by weight. * * @param weightedPermissions - Set of Snap permissions that have 'weight' property assigned. * @param weightThreshold - Number that represents weight threshold for filtering. - * @param showFirstThreeOnEmpty - Boolean flag to control if first three permissions should be shown - * when no permissions meet the weight criteria. - * @returns Subset of permissions passing weight threshold. + * @param minPermissionCount - Minimum number of permissions to show, + * if filtered permissions count are less than the value specified. + * @returns Subset of permissions passing weight criteria. */ export const getFilteredSnapPermissions = ( weightedPermissions, weightThreshold, - showFirstThreeOnEmpty = false, + minPermissionCount = 3, ) => { - let filteredPermissions = weightedPermissions.filter( + const filteredPermissions = weightedPermissions.filter( (permission) => permission.weight <= (weightThreshold ?? Infinity), ); - // If there are no permissions that fall into desired set filtered by weight, - // then show only the first three, no matter what the weight is - if (showFirstThreeOnEmpty && filteredPermissions.length === 0) { - filteredPermissions = weightedPermissions.slice(0, 3); + // If there are not enough permissions that fall into desired set filtered by weight, + // then fill the gap, no matter what the weight is + if (minPermissionCount && filteredPermissions.length < minPermissionCount) { + const remainingPermissions = [...weightedPermissions]; + + // Remove already filtered permissions to avoid duplicates + for (const permission of filteredPermissions) { + const index = remainingPermissions.indexOf(permission); + if (index > -1) { + remainingPermissions.splice(index, 1); + } + } + + // Add permissions until desired count is reached + while ( + filteredPermissions.length < minPermissionCount && + remainingPermissions.length > 0 + ) { + filteredPermissions.push(remainingPermissions.shift()); + } } return filteredPermissions; diff --git a/ui/helpers/utils/util.test.js b/ui/helpers/utils/util.test.js index 5b59b0a15502..e9485a262197 100644 --- a/ui/helpers/utils/util.test.js +++ b/ui/helpers/utils/util.test.js @@ -3,6 +3,7 @@ import { BN, toChecksumAddress } from 'ethereumjs-util'; import { CHAIN_IDS } from '../../../shared/constants/network'; import { addHexPrefixToObjectValues } from '../../../shared/lib/swaps-utils'; import { toPrecisionWithoutTrailingZeros } from '../../../shared/lib/transactions-controller-utils'; +import { MinPermissionAbstractionDisplayCount } from '../../../shared/constants/permissions'; import * as util from './util'; describe('util', () => { @@ -1120,7 +1121,7 @@ describe('util', () => { }, ]; expect( - util.getFilteredSnapPermissions(mockPermissions, WEIGHT_THRESHOLD), + util.getFilteredSnapPermissions(mockPermissions, WEIGHT_THRESHOLD, 2), ).toStrictEqual([ { label: 'Permission C', @@ -1161,7 +1162,7 @@ describe('util', () => { util.getFilteredSnapPermissions( mockPermissions, WEIGHT_THRESHOLD, - true, + MinPermissionAbstractionDisplayCount, ), ).toStrictEqual([ { @@ -1178,5 +1179,51 @@ describe('util', () => { }, ]); }); + + it('should return permissions filtered by weight and gap filled with other permissions', () => { + const WEIGHT_THRESHOLD = 3; + const mockPermissions = [ + { + label: 'Permission A', + weight: 4, + }, + { + label: 'Permission B', + weight: 4, + }, + { + label: 'Permission C', + weight: 1, + }, + { + label: 'Permission D', + weight: 5, + }, + { + label: 'Permission E', + weight: 6, + }, + ]; + expect( + util.getFilteredSnapPermissions( + mockPermissions, + WEIGHT_THRESHOLD, + MinPermissionAbstractionDisplayCount, + ), + ).toStrictEqual([ + { + label: 'Permission C', + weight: 1, + }, + { + label: 'Permission A', + weight: 4, + }, + { + label: 'Permission B', + weight: 4, + }, + ]); + }); }); }); From 810dedfbd83a8b739ef5f9a6c118a681bee73eb8 Mon Sep 17 00:00:00 2001 From: david0xd Date: Wed, 14 Aug 2024 12:23:49 +0200 Subject: [PATCH 3/5] Add some refactoring changes requested in review comments --- .../snap-permissions-list.js | 2 +- .../update-snap-permission-list.js | 24 +++++--------- ui/helpers/utils/util.js | 31 +++++++------------ 3 files changed, 21 insertions(+), 36 deletions(-) diff --git a/ui/components/app/snaps/snap-permissions-list/snap-permissions-list.js b/ui/components/app/snaps/snap-permissions-list/snap-permissions-list.js index 498bab4d9e7f..23d101f1179c 100644 --- a/ui/components/app/snaps/snap-permissions-list/snap-permissions-list.js +++ b/ui/components/app/snaps/snap-permissions-list/snap-permissions-list.js @@ -87,7 +87,7 @@ export default function SnapPermissionsList({ paddingTop={2} paddingBottom={2} > - onShowAllPermissionsHandler()}> + {t('seeAllPermissions')} diff --git a/ui/components/app/snaps/update-snap-permission-list/update-snap-permission-list.js b/ui/components/app/snaps/update-snap-permission-list/update-snap-permission-list.js index 4e39fb6f88b3..dc6ff15ded32 100644 --- a/ui/components/app/snaps/update-snap-permission-list/update-snap-permission-list.js +++ b/ui/components/app/snaps/update-snap-permission-list/update-snap-permission-list.js @@ -13,7 +13,10 @@ import { Display, JustifyContent, } from '../../../../helpers/constants/design-system'; -import { PermissionWeightThreshold } from '../../../../../shared/constants/permissions'; +import { + MinPermissionAbstractionDisplayCount, + PermissionWeightThreshold, +} from '../../../../../shared/constants/permissions'; import { getFilteredSnapPermissions, getSnapName, @@ -90,23 +93,12 @@ export default function UpdateSnapPermissionList({ // Because approved permissions are sometimes hidden following the abstraction logic, // it is needed sometimes to fill the gap in permission display, in certain edge cases // when there is not enough new and revoked permissions to be shown. - let minApprovedPermissionsToShow; const totalNewAndRevokedPermissions = newWeightedPermissions.length + revokedWeightedPermissions.length; - - switch (totalNewAndRevokedPermissions) { - case 0: - minApprovedPermissionsToShow = 3; - break; - case 1: - minApprovedPermissionsToShow = 2; - break; - case 2: - minApprovedPermissionsToShow = 1; - break; - default: - minApprovedPermissionsToShow = 0; - } + const minApprovedPermissionsToShow = Math.max( + MinPermissionAbstractionDisplayCount - totalNewAndRevokedPermissions, + 0, + ); const filteredApprovedWeightedPermissions = getFilteredSnapPermissions( approvedWeightedPermissions, diff --git a/ui/helpers/utils/util.js b/ui/helpers/utils/util.js index fa19dc8a4a2b..0ab2aa40504b 100644 --- a/ui/helpers/utils/util.js +++ b/ui/helpers/utils/util.js @@ -793,33 +793,26 @@ export const getAvatarFallbackLetter = (subjectName) => { */ export const getFilteredSnapPermissions = ( weightedPermissions, - weightThreshold, + weightThreshold = Infinity, minPermissionCount = 3, ) => { - const filteredPermissions = weightedPermissions.filter( - (permission) => permission.weight <= (weightThreshold ?? Infinity), + let filteredPermissions = weightedPermissions.filter( + (permission) => permission.weight <= weightThreshold, ); // If there are not enough permissions that fall into desired set filtered by weight, // then fill the gap, no matter what the weight is if (minPermissionCount && filteredPermissions.length < minPermissionCount) { - const remainingPermissions = [...weightedPermissions]; - - // Remove already filtered permissions to avoid duplicates - for (const permission of filteredPermissions) { - const index = remainingPermissions.indexOf(permission); - if (index > -1) { - remainingPermissions.splice(index, 1); - } - } - + const remainingPermissions = weightedPermissions.filter( + (permission) => permission.weight > weightThreshold, + ); // Add permissions until desired count is reached - while ( - filteredPermissions.length < minPermissionCount && - remainingPermissions.length > 0 - ) { - filteredPermissions.push(remainingPermissions.shift()); - } + filteredPermissions = filteredPermissions.concat( + remainingPermissions.slice( + 0, + minPermissionCount - filteredPermissions.length, + ), + ); } return filteredPermissions; From 4af485142af94207877f47aae57976c08a94742c Mon Sep 17 00:00:00 2001 From: david0xd Date: Wed, 14 Aug 2024 12:49:39 +0200 Subject: [PATCH 4/5] Remove redundant condition for disabling the confirm button --- .../permissions-connect/snaps/snap-install/snap-install.js | 6 +----- .../permissions-connect/snaps/snap-update/snap-update.js | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/ui/pages/permissions-connect/snaps/snap-install/snap-install.js b/ui/pages/permissions-connect/snaps/snap-install/snap-install.js index 3b636bdddb45..f26fbb8fe54a 100644 --- a/ui/pages/permissions-connect/snaps/snap-install/snap-install.js +++ b/ui/pages/permissions-connect/snaps/snap-install/snap-install.js @@ -226,11 +226,7 @@ export default function SnapInstall({ cancelButtonType="default" hideCancel={hasError} disabled={ - isLoading || - (!hasError && - isScrollable && - !hasScrolledToBottom && - !showAllPermissions) + isLoading || (!hasError && isScrollable && !hasScrolledToBottom) } onCancel={onCancel} cancelText={t('cancel')} diff --git a/ui/pages/permissions-connect/snaps/snap-update/snap-update.js b/ui/pages/permissions-connect/snaps/snap-update/snap-update.js index 1bde0f37198f..6fc328330a55 100644 --- a/ui/pages/permissions-connect/snaps/snap-update/snap-update.js +++ b/ui/pages/permissions-connect/snaps/snap-update/snap-update.js @@ -232,11 +232,7 @@ export default function SnapUpdate({ cancelButtonType="default" hideCancel={hasError} disabled={ - isLoading || - (!hasError && - isScrollable && - !hasScrolledToBottom && - !showAllPermissions) + isLoading || (!hasError && isScrollable && !hasScrolledToBottom) } onCancel={onCancel} cancelText={t('cancel')} From 1861d6e9203df0e9c6f3ba2f412a654d0e5cceea Mon Sep 17 00:00:00 2001 From: david0xd Date: Wed, 14 Aug 2024 14:35:35 +0200 Subject: [PATCH 5/5] Add few more minor refactoring changes --- shared/constants/permissions.ts | 2 +- ui/helpers/utils/util.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/shared/constants/permissions.ts b/shared/constants/permissions.ts index d55083b0b84a..a34065b2ee31 100644 --- a/shared/constants/permissions.ts +++ b/shared/constants/permissions.ts @@ -33,7 +33,7 @@ export const ConnectionPermission = Object.freeze({ // permissions to show or hide on certain Snap-related flows (Install, Update, etc.) export const PermissionWeightThreshold = Object.freeze({ snapInstall: 3 as const, - snapUpdateApprovedPermissions: 2 as const, + snapUpdateApprovedPermissions: 3 as const, }); // Specify minimum number of permissions to be shown, when abstraction is applied diff --git a/ui/helpers/utils/util.js b/ui/helpers/utils/util.js index 0ab2aa40504b..7eeab828a750 100644 --- a/ui/helpers/utils/util.js +++ b/ui/helpers/utils/util.js @@ -796,7 +796,7 @@ export const getFilteredSnapPermissions = ( weightThreshold = Infinity, minPermissionCount = 3, ) => { - let filteredPermissions = weightedPermissions.filter( + const filteredPermissions = weightedPermissions.filter( (permission) => permission.weight <= weightThreshold, ); @@ -807,7 +807,7 @@ export const getFilteredSnapPermissions = ( (permission) => permission.weight > weightThreshold, ); // Add permissions until desired count is reached - filteredPermissions = filteredPermissions.concat( + return filteredPermissions.concat( remainingPermissions.slice( 0, minPermissionCount - filteredPermissions.length,