From 6126c156ea25cddd2d3ef116f02cb36c62d3c632 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Tue, 25 Apr 2023 19:20:37 +0200 Subject: [PATCH] [FLASK] Expanded Snap authorship (#18775) --- app/_locales/en/messages.json | 31 ++- ui/components/app/app-components.scss | 1 + .../snaps/snap-authorship/snap-authorship.js | 167 ++++++++++++---- .../app/snaps/snap-avatar/snap-avatar.js | 4 +- ui/components/app/snaps/snap-version/index.js | 1 + .../app/snaps/snap-version/index.scss | 15 ++ .../app/snaps/snap-version/snap-version.js | 72 +++++++ .../snap-version/snap-version.stories.js | 20 ++ .../snaps/snap-version/snap-version.test.js | 27 +++ ui/helpers/constants/flask/delineator.ts | 3 + .../snaps/snap-install/index.scss | 2 +- .../snaps/snap-install/snap-install.js | 12 +- .../snaps/snap-update/index.scss | 2 +- .../snaps/snap-update/snap-update.js | 8 +- ui/pages/settings/snaps/view-snap/index.scss | 33 ++- .../settings/snaps/view-snap/view-snap.js | 188 ++++++------------ .../snaps/view-snap/view-snap.test.js | 7 +- 17 files changed, 396 insertions(+), 197 deletions(-) create mode 100644 ui/components/app/snaps/snap-version/index.js create mode 100644 ui/components/app/snaps/snap-version/index.scss create mode 100644 ui/components/app/snaps/snap-version/snap-version.js create mode 100644 ui/components/app/snaps/snap-version/snap-version.stories.js create mode 100644 ui/components/app/snaps/snap-version/snap-version.test.js diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 8ad86eebb..545acc4f4 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1066,6 +1066,10 @@ "description": { "message": "Description" }, + "descriptionFromSnap": { + "message": "Description from $1", + "description": "$1 represents the name of the snap" + }, "desktopConnectionCriticalErrorDescription": { "message": "This error could be intermittent, so try restarting the extension or disable MetaMask Desktop." }, @@ -1331,10 +1335,7 @@ "message": "Enable smart transactions" }, "enableSnap": { - "message": "Enable snap" - }, - "enableSnapDescription": { - "message": "Your installed snap will only have access to its permissions and run if it’s enabled." + "message": "Enable" }, "enableToken": { "message": "enable $1", @@ -1851,6 +1852,13 @@ "install": { "message": "Install" }, + "installOrigin": { + "message": "Install origin" + }, + "installedOn": { + "message": "Installed on $1", + "description": "$1 is the date when the snap has been installed" + }, "institutionalFeatures": { "message": "Institutional Features" }, @@ -2192,6 +2200,9 @@ "mmiAuthenticate": { "message": "The page at $1 would like to authorise the following project’s compliance settings in MetaMask Institutional" }, + "more": { + "message": "more" + }, "moreComingSoon": { "message": "More coming soon..." }, @@ -3576,6 +3587,10 @@ "settingsSearchMatchingNotFound": { "message": "No matching results found." }, + "shortVersion": { + "message": "v$1", + "description": "$1 is the version number to show" + }, "show": { "message": "Show" }, @@ -4510,7 +4525,6 @@ "transactionFailed": { "message": "Transaction Failed" }, - "transactionFee": { "message": "Transaction fee" }, @@ -4737,6 +4751,9 @@ "message": "Verify this token on $1 and make sure this is the token you want to trade.", "description": "Points the user to etherscan as a place they can verify information about a token. $1 is replaced with the translation for \"etherscan\"" }, + "version": { + "message": "Version" + }, "view": { "message": "View" }, @@ -4870,10 +4887,6 @@ "message": "You've added all the popular networks. You can discover more networks $1 Or you can $2", "description": "$1 is a link with the text 'here' and $2 is a button with the text 'add more networks manually'" }, - "youInstalled": { - "message": "You installed", - "description": "Part of version description for installed snap" - }, "youNeedToAllowCameraAccess": { "message": "You need to allow camera access to use this feature." }, diff --git a/ui/components/app/app-components.scss b/ui/components/app/app-components.scss index b6422b632..81ab85809 100644 --- a/ui/components/app/app-components.scss +++ b/ui/components/app/app-components.scss @@ -43,6 +43,7 @@ @import 'snaps/snap-settings-card/index'; @import 'snaps/update-snap-permission-list/index'; @import 'snaps/copyable/index'; +@import 'snaps/snap-version/index'; @import 'gas-details-item/index'; @import 'gas-details-item/gas-details-item-title/index'; @import 'gas-timing/index'; diff --git a/ui/components/app/snaps/snap-authorship/snap-authorship.js b/ui/components/app/snaps/snap-authorship/snap-authorship.js index 66f3fa59b..14842afaa 100644 --- a/ui/components/app/snaps/snap-authorship/snap-authorship.js +++ b/ui/components/app/snaps/snap-authorship/snap-authorship.js @@ -2,30 +2,42 @@ import React from 'react'; import PropTypes from 'prop-types'; import classnames from 'classnames'; import { getSnapPrefix } from '@metamask/snaps-utils'; -import { useSelector } from 'react-redux'; +import { useDispatch, useSelector } from 'react-redux'; import Box from '../../../ui/box'; import { BackgroundColor, TextColor, - IconColor, FLEX_DIRECTION, TextVariant, BorderColor, AlignItems, DISPLAY, - BorderRadius, BLOCK_SIZES, + JustifyContent, + BorderStyle, + Color, + BorderRadius, } from '../../../../helpers/constants/design-system'; import { + formatDate, getSnapName, removeSnapIdPrefix, } from '../../../../helpers/utils/util'; -import { ButtonIcon, IconName, Text } from '../../../component-library'; +import { Text, ButtonLink } from '../../../component-library'; import { getTargetSubjectMetadata } from '../../../../selectors'; import SnapAvatar from '../snap-avatar'; +import { useI18nContext } from '../../../../hooks/useI18nContext'; +import Tooltip from '../../../ui/tooltip/tooltip'; +import ToggleButton from '../../../ui/toggle-button'; +import { disableSnap, enableSnap } from '../../../../store/actions'; +import { useOriginMetadata } from '../../../../hooks/useOriginMetadata'; +import SnapVersion from '../snap-version/snap-version'; + +const SnapAuthorship = ({ snapId, className, expanded = false, snap }) => { + const t = useI18nContext(); + const dispatch = useDispatch(); -const SnapAuthorship = ({ snapId, className }) => { // We're using optional chaining with snapId, because with the current implementation // of snap update in the snap controller, we do not have reference to snapId when an // update request is rejected because the reference comes from the request itself and not subject metadata @@ -43,48 +55,127 @@ const SnapAuthorship = ({ snapId, className }) => { const friendlyName = snapId && getSnapName(snapId, subjectMetadata); + // Expanded data + const versionHistory = snap?.versionHistory ?? []; + const installInfo = versionHistory.length + ? versionHistory[versionHistory.length - 1] + : undefined; + const installOrigin = useOriginMetadata(installInfo?.origin); + + const onToggle = () => { + if (snap?.enabled) { + dispatch(disableSnap(snap?.id)); + } else { + dispatch(enableSnap(snap?.id)); + } + }; + return ( - - - - {friendlyName} - + + + - {packageName} - + {friendlyName} + + {packageName} + + + {!expanded && ( + + + + )} - + {expanded && ( + + + {t('enableSnap')} + + + + + + + + {installOrigin && installInfo && ( + + + {t('installOrigin')} + + + + {installOrigin.host} + + + {t('installedOn', [ + formatDate(installInfo.date, 'dd MMM yyyy'), + ])} + + + + )} + + {t('version')} + + + + + )} ); }; @@ -98,6 +189,14 @@ SnapAuthorship.propTypes = { * The className of the SnapAuthorship */ className: PropTypes.string, + /** + * If the authorship component should be expanded + */ + expanded: PropTypes.bool, + /** + * The snap object. Can be undefined if the component is not expanded + */ + snap: PropTypes.object, }; export default SnapAuthorship; diff --git a/ui/components/app/snaps/snap-avatar/snap-avatar.js b/ui/components/app/snaps/snap-avatar/snap-avatar.js index cdf85e117..2cd3d1ec6 100644 --- a/ui/components/app/snaps/snap-avatar/snap-avatar.js +++ b/ui/components/app/snaps/snap-avatar/snap-avatar.js @@ -50,10 +50,10 @@ const SnapAvatar = ({ snapId, className }) => { position={BadgeWrapperPosition.bottomRight} > {iconUrl ? ( - + ) : ( { + const t = useI18nContext(); + return ( + + ); +}; + +SnapVersion.propTypes = { + /** + * The version of the snap + */ + version: PropTypes.string, + /** + * The url to the snap package + */ + url: PropTypes.string, +}; + +export default SnapVersion; diff --git a/ui/components/app/snaps/snap-version/snap-version.stories.js b/ui/components/app/snaps/snap-version/snap-version.stories.js new file mode 100644 index 000000000..90f179d87 --- /dev/null +++ b/ui/components/app/snaps/snap-version/snap-version.stories.js @@ -0,0 +1,20 @@ +import React from 'react'; +import SnapVersion from '.'; + +export default { + title: 'Components/App/Snaps/SnapVersion', + component: SnapVersion, +}; +export const DefaultStory = (args) => ; + +DefaultStory.args = { + version: '1.4.2', + url: 'https://www.npmjs.com/package/@metamask/test-snap-error', +}; + +export const LoadingStory = (args) => ; + +LoadingStory.args = { + version: undefined, + url: 'https://www.npmjs.com/package/@metamask/test-snap-error', +}; diff --git a/ui/components/app/snaps/snap-version/snap-version.test.js b/ui/components/app/snaps/snap-version/snap-version.test.js new file mode 100644 index 000000000..447a95931 --- /dev/null +++ b/ui/components/app/snaps/snap-version/snap-version.test.js @@ -0,0 +1,27 @@ +import * as React from 'react'; +import { renderWithLocalization } from '../../../../../test/lib/render-helpers'; +import SnapVersion from './snap-version'; + +describe('SnapVersion', () => { + const args = { + version: '1.4.2', + url: 'https://www.npmjs.com/package/@metamask/test-snap-error', + }; + + it('should render the SnapVersion without crashing and display a version', () => { + const { getByText, container } = renderWithLocalization( + , + ); + expect(getByText(`v${args.version}`)).toBeDefined(); + expect(container.firstChild).toHaveAttribute('href', args.url); + }); + + it('should have a loading state if no version is passed', () => { + args.version = undefined; + + const { container } = renderWithLocalization(); + + expect(container.getElementsByClassName('preloader__icon')).toHaveLength(1); + expect(container.firstChild).toHaveAttribute('href', args.url); + }); +}); diff --git a/ui/helpers/constants/flask/delineator.ts b/ui/helpers/constants/flask/delineator.ts index ce5d5002b..7e3fd9f32 100644 --- a/ui/helpers/constants/flask/delineator.ts +++ b/ui/helpers/constants/flask/delineator.ts @@ -3,6 +3,7 @@ export enum DelineatorType { // eslint-disable-next-line @typescript-eslint/no-shadow Error = 'error', Insights = 'insights', + Description = 'description', } export const getDelineatorTitle = (type: DelineatorType) => { @@ -11,6 +12,8 @@ export const getDelineatorTitle = (type: DelineatorType) => { return 'errorWithSnap'; case DelineatorType.Insights: return 'insightsFromSnap'; + case DelineatorType.Description: + return 'descriptionFromSnap'; default: return 'contentFromSnap'; } diff --git a/ui/pages/permissions-connect/snaps/snap-install/index.scss b/ui/pages/permissions-connect/snaps/snap-install/index.scss index 654653ab9..4b52d3037 100644 --- a/ui/pages/permissions-connect/snaps/snap-install/index.scss +++ b/ui/pages/permissions-connect/snaps/snap-install/index.scss @@ -1,7 +1,7 @@ .snap-install { box-shadow: none; - .headers { + .content { flex: 1; .loader-container { 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 8fc4fe2b4..077971e57 100644 --- a/ui/pages/permissions-connect/snaps/snap-install/snap-install.js +++ b/ui/pages/permissions-connect/snaps/snap-install/snap-install.js @@ -84,13 +84,13 @@ export default function SnapInstall({ flexDirection={FLEX_DIRECTION.COLUMN} > - - - + {!hasError && ( )} + + {isLoading && ( )} + + {isLoading && ( {t('snapUpdateRequestsPermission', [ diff --git a/ui/pages/settings/snaps/view-snap/index.scss b/ui/pages/settings/snaps/view-snap/index.scss index 6d6395cf6..745bda669 100644 --- a/ui/pages/settings/snaps/view-snap/index.scss +++ b/ui/pages/settings/snaps/view-snap/index.scss @@ -1,19 +1,36 @@ .view-snap { max-width: 475px; - &__version_info { - &__version-number { - font-weight: bold; + &__description { + &__wrapper { + position: relative; + overflow: hidden; + max-height: 5.5rem; + + @include screen-md-min { + max-height: 6rem; + } + + &.open { + max-height: none; + } } - &__link { - vertical-align: top; + &__more-button { + position: absolute; + bottom: 0; + right: 0; + width: unset; + padding: 0; + padding-left: 32px; + background: linear-gradient(90deg, rgba(255, 255, 255, 0) 0%, var(--color-background-default) 33%); } } - &__enable { - &__tooltip_wrapper { - max-width: 52px; + + &__permissions { + .permission-cell { + margin: 0; } } diff --git a/ui/pages/settings/snaps/view-snap/view-snap.js b/ui/pages/settings/snaps/view-snap/view-snap.js index 3d4bbf8a5..b7ea31df5 100644 --- a/ui/pages/settings/snaps/view-snap/view-snap.js +++ b/ui/pages/settings/snaps/view-snap/view-snap.js @@ -1,28 +1,26 @@ -import React, { useEffect, useState } from 'react'; +import React, { useEffect, useRef, useState } from 'react'; import { useHistory, useLocation } from 'react-router-dom'; import { useDispatch, useSelector } from 'react-redux'; import { SnapCaveatType, WALLET_SNAP_PERMISSION_KEY, } from '@metamask/rpc-methods'; -import { getSnapPrefix } from '@metamask/snaps-utils'; +import classnames from 'classnames'; import Button from '../../../../components/ui/button'; import { useI18nContext } from '../../../../hooks/useI18nContext'; import { - Size, + Color, + FLEX_WRAP, TextColor, TextVariant, } from '../../../../helpers/constants/design-system'; import SnapAuthorship from '../../../../components/app/snaps/snap-authorship'; import Box from '../../../../components/ui/box'; import SnapRemoveWarning from '../../../../components/app/snaps/snap-remove-warning'; -import ToggleButton from '../../../../components/ui/toggle-button'; import ConnectedSitesList from '../../../../components/app/connected-sites-list'; -import Tooltip from '../../../../components/ui/tooltip'; + import { SNAPS_LIST_ROUTE } from '../../../../helpers/constants/routes'; import { - disableSnap, - enableSnap, removeSnap, removePermissionsFor, updateCaveat, @@ -34,18 +32,17 @@ import { getPermissionSubjects, getTargetSubjectMetadata, } from '../../../../selectors'; -import { - formatDate, - getSnapName, - removeSnapIdPrefix, -} from '../../../../helpers/utils/util'; -import { ButtonLink, Text } from '../../../../components/component-library'; +import { getSnapName } from '../../../../helpers/utils/util'; +import { Text, BUTTON_TYPES } from '../../../../components/component-library'; import SnapPermissionsList from '../../../../components/app/snaps/snap-permissions-list'; +import { SnapDelineator } from '../../../../components/app/snaps/snap-delineator'; +import { DelineatorType } from '../../../../helpers/constants/flask'; function ViewSnap() { const t = useI18nContext(); const history = useHistory(); const location = useLocation(); + const descriptionRef = useRef(null); const { pathname } = location; // The snap ID is in URI-encoded form in the last path segment of the URL. const decodedSnapId = decodeURIComponent(pathname.match(/[^/]+$/u)[0]); @@ -55,6 +52,8 @@ function ViewSnap() { .find((snapState) => snapState.id === decodedSnapId); const [isShowingRemoveWarning, setIsShowingRemoveWarning] = useState(false); + const [isDescriptionOpen, setIsDescriptionOpen] = useState(false); + const [isOverflowing, setIsOverflowing] = useState(false); useEffect(() => { if (!snap) { @@ -62,6 +61,14 @@ function ViewSnap() { } }, [history, snap]); + useEffect(() => { + setIsOverflowing( + descriptionRef.current && + descriptionRef.current.offsetHeight < + descriptionRef.current.scrollHeight, + ); + }, [descriptionRef]); + const connectedSubjects = useSelector((state) => getSubjectsWithSnapPermission(state, snap?.id), ); @@ -74,14 +81,6 @@ function ViewSnap() { ); const dispatch = useDispatch(); - const onToggle = () => { - if (snap.enabled) { - dispatch(disableSnap(snap.id)); - } else { - dispatch(enableSnap(snap.id)); - } - }; - const onDisconnect = (connectedOrigin, snapId) => { const caveatValue = subjects[connectedOrigin].permissions[WALLET_SNAP_PERMISSION_KEY] @@ -110,121 +109,51 @@ function ViewSnap() { return null; } - const versionHistory = snap.versionHistory ?? []; - const installInfo = versionHistory.length - ? versionHistory[versionHistory.length - 1] - : undefined; - const packageName = snap.id && removeSnapIdPrefix(snap.id); - const snapPrefix = snap.id && getSnapPrefix(snap.id); - const isNPM = snapPrefix === 'npm:'; - const url = isNPM - ? `https://www.npmjs.com/package/${packageName}` - : packageName; const snapName = getSnapName(snap.id, targetSubjectMetadata); + const shouldDisplayMoreButton = isOverflowing && !isDescriptionOpen; + const handleMoreClick = () => { + setIsDescriptionOpen(true); + }; + return ( - - - - - - {snap.manifest.description} - - - - - {`${t('youInstalled')} `} - - v{snap.version} - - {` ${t('ofTextNofM')} `} - + + + - {packageName} - - {installInfo && ` ${t('from').toLowerCase()} `} - {installInfo && ( - - {installInfo.origin} - - )} - {installInfo && - ` ${t('on').toLowerCase()} ${formatDate( - installInfo.date, - 'dd MMM yyyy', - )}`} - . - - - - {t('enableSnap')} - - {t('enableSnapDescription')} - - - - - - + {snap?.manifest.description} + {shouldDisplayMoreButton && ( + + )} + + - - {t('permissions')} - + {t('permissions')} - + {t('connectedSites')} @@ -235,12 +164,7 @@ function ViewSnap() { }} /> - + {t('removeSnap')} @@ -253,7 +177,13 @@ function ViewSnap() { type="danger" onClick={() => setIsShowingRemoveWarning(true)} > - + {`${t('remove')} ${snapName}`} diff --git a/ui/pages/settings/snaps/view-snap/view-snap.test.js b/ui/pages/settings/snaps/view-snap/view-snap.test.js index 320077cf6..3ce293094 100644 --- a/ui/pages/settings/snaps/view-snap/view-snap.test.js +++ b/ui/pages/settings/snaps/view-snap/view-snap.test.js @@ -48,12 +48,7 @@ describe('ViewSnap', () => { // Snap version info expect(getByText('v5.1.2')).toBeDefined(); // Enable Snap - expect(getByText('Enable snap')).toBeDefined(); - expect( - getByText( - 'Your installed snap will only have access to its permissions and run if it’s enabled.', - ), - ).toBeDefined(); + expect(getByText('Enable')).toBeDefined(); expect(container.getElementsByClassName('toggle-button')?.length).toBe(1); // Permissions expect(getByText('Permissions')).toBeDefined();