From 789779f4d53068629a320e475ec991533d840833 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 7 Jun 2023 15:18:49 +0200 Subject: [PATCH] [FLASK] Rework Snaps headers and footers (#19442) * Add new snap header and footer to snap install * Add new snap header and footer to snap result and snap update * Fix loading state * Fix lint * Add required scrolling * Adjust avatar component * Apply new headers and footers to snaps confirmations * Rename previous SnapAuthorship component to SnapAuthorshipExpanded * Fix lint * Fix font weight * Fix fencing * Fix a test * Fix lint after rebase * Fix E2E * Fix locale lint * Fix another E2E * Fix test ID * Address PR comments * Better scroll button centering * Address design comments * Fix unit test * Fix E2Es --- app/_locales/en/messages.json | 7 +- test/data/mock-state.json | 8 +- test/e2e/snaps/test-snap-bip-32.spec.js | 2 + test/e2e/snaps/test-snap-cronjob.spec.js | 2 +- test/e2e/snaps/test-snap-dialog.spec.js | 2 +- .../e2e/snaps/test-snap-networkaccess.spec.js | 2 +- test/e2e/snaps/test-snap-rpc.spec.js | 2 + test/e2e/snaps/test-snap-update.spec.js | 4 + .../permissions-connect-header.component.js | 46 +----- .../snaps/snap-authorship-expanded/index.js | 1 + .../snap-authorship-expanded.js} | 151 +++++++++--------- .../app/snaps/snap-authorship-header/index.js | 1 + .../snap-authorship-header.js | 96 +++++++++++ .../snap-authorship-header.stories.js | 21 +++ .../app/snaps/snap-authorship/index.js | 1 - .../snap-authorship.stories.js | 21 --- .../app/snaps/snap-avatar/snap-avatar.js | 11 +- .../snaps/snap-delineator/snap-delineator.js | 9 +- ui/hooks/useScrollRequired.js | 44 +++++ .../confirmation-footer.js | 8 +- ui/pages/confirmation/confirmation.js | 26 ++- .../templates/snaps/snap-alert/snap-alert.js | 3 +- .../snap-confirmation/snap-confirmation.js | 1 + .../snaps/snap-prompt/snap-prompt.js | 1 + .../permissions-connect.component.js | 4 +- .../permissions-connect.container.js | 16 +- .../snaps/snap-install/index.scss | 13 +- .../snaps/snap-install/snap-install.js | 98 ++++++------ .../snap-install/snap-install.stories.js | 122 ++++++++++++++ .../snaps/snap-result/index.scss | 10 +- .../snaps/snap-result/snap-result.js | 14 +- .../snaps/snap-result/snap-result.stories.js | 64 ++++++++ .../snaps/snap-update/index.scss | 15 +- .../snaps/snap-update/snap-update.js | 77 +++++++-- .../snaps/snap-update/snap-update.stories.js | 124 ++++++++++++++ .../settings/snaps/view-snap/view-snap.js | 4 +- .../snaps/view-snap/view-snap.test.js | 8 +- 37 files changed, 766 insertions(+), 273 deletions(-) create mode 100644 ui/components/app/snaps/snap-authorship-expanded/index.js rename ui/components/app/snaps/{snap-authorship/snap-authorship.js => snap-authorship-expanded/snap-authorship-expanded.js} (57%) create mode 100644 ui/components/app/snaps/snap-authorship-header/index.js create mode 100644 ui/components/app/snaps/snap-authorship-header/snap-authorship-header.js create mode 100644 ui/components/app/snaps/snap-authorship-header/snap-authorship-header.stories.js delete mode 100644 ui/components/app/snaps/snap-authorship/index.js delete mode 100644 ui/components/app/snaps/snap-authorship/snap-authorship.stories.js create mode 100644 ui/hooks/useScrollRequired.js create mode 100644 ui/pages/permissions-connect/snaps/snap-install/snap-install.stories.js create mode 100644 ui/pages/permissions-connect/snaps/snap-result/snap-result.stories.js create mode 100644 ui/pages/permissions-connect/snaps/snap-update/snap-update.stories.js diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 639190bc6..e515c1b61 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1371,6 +1371,9 @@ "message": "enable $1", "description": "$1 is a token symbol, e.g. ETH" }, + "enabled": { + "message": "Enabled" + }, "encryptionPublicKeyNotice": { "message": "$1 would like your public encryption key. By consenting, this site will be able to compose encrypted messages to you.", "description": "$1 is the web3 site name" @@ -3777,10 +3780,6 @@ "message": "Install snap" }, "snapInstallRequest": { - "message": "$1 wants to install $2. Make sure you trust the authors before you proceed.", - "description": "$1 is the dApp origin requesting the snap and $2 is the snap name" - }, - "snapInstallRequestsPermission": { "message": "Installing $1 gives it the following permissions. Only continue if you trust $1.", "description": "$1 is the snap name." }, diff --git a/test/data/mock-state.json b/test/data/mock-state.json index 5bbb6386f..b4a9ab5b3 100644 --- a/test/data/mock-state.json +++ b/test/data/mock-state.json @@ -165,7 +165,13 @@ } }, "frequentRpcListDetail": [], - "subjectMetadata": {}, + "subjectMetadata": { + "npm:@metamask/test-snap-bip44": { + "name": "@metamask/test-snap-bip44", + "version": "1.2.3", + "subjectType": "snap" + } + }, "notifications": { "test": { "id": "test", diff --git a/test/e2e/snaps/test-snap-bip-32.spec.js b/test/e2e/snaps/test-snap-bip-32.spec.js index 22528373d..0cd744f2c 100644 --- a/test/e2e/snaps/test-snap-bip-32.spec.js +++ b/test/e2e/snaps/test-snap-bip-32.spec.js @@ -54,6 +54,8 @@ describe('Test Snap bip-32', function () { await driver.waitForSelector({ text: 'Install' }); + await driver.clickElement('[data-testid="snap-install-scroll"]'); + await driver.clickElement({ text: 'Install', tag: 'button', diff --git a/test/e2e/snaps/test-snap-cronjob.spec.js b/test/e2e/snaps/test-snap-cronjob.spec.js index e4e6e93fe..7fb31e361 100644 --- a/test/e2e/snaps/test-snap-cronjob.spec.js +++ b/test/e2e/snaps/test-snap-cronjob.spec.js @@ -90,7 +90,7 @@ describe('Test Snap Cronjob', function () { // try to click on the Ok button and pass test if it works await driver.clickElement({ - text: 'Ok', + text: 'OK', tag: 'button', }); }, diff --git a/test/e2e/snaps/test-snap-dialog.spec.js b/test/e2e/snaps/test-snap-dialog.spec.js index 0edb3b85b..28825c0e2 100644 --- a/test/e2e/snaps/test-snap-dialog.spec.js +++ b/test/e2e/snaps/test-snap-dialog.spec.js @@ -95,7 +95,7 @@ describe('Test Snap Dialog', function () { // click ok button await driver.clickElement({ - text: 'Ok', + text: 'OK', tag: 'button', }); diff --git a/test/e2e/snaps/test-snap-networkaccess.spec.js b/test/e2e/snaps/test-snap-networkaccess.spec.js index 6cb1b743b..e80012a55 100644 --- a/test/e2e/snaps/test-snap-networkaccess.spec.js +++ b/test/e2e/snaps/test-snap-networkaccess.spec.js @@ -97,7 +97,7 @@ describe('Test Snap networkAccess', function () { // click ok button await driver.clickElement({ - text: 'Ok', + text: 'OK', tag: 'button', }); }, diff --git a/test/e2e/snaps/test-snap-rpc.spec.js b/test/e2e/snaps/test-snap-rpc.spec.js index d329ce24c..5fe574490 100644 --- a/test/e2e/snaps/test-snap-rpc.spec.js +++ b/test/e2e/snaps/test-snap-rpc.spec.js @@ -55,6 +55,8 @@ describe('Test Snap RPC', function () { await driver.waitForSelector({ text: 'Install' }); + await driver.clickElement('[data-testid="snap-install-scroll"]'); + await driver.clickElement({ text: 'Install', tag: 'button', diff --git a/test/e2e/snaps/test-snap-update.spec.js b/test/e2e/snaps/test-snap-update.spec.js index 5898957ce..485c87b66 100644 --- a/test/e2e/snaps/test-snap-update.spec.js +++ b/test/e2e/snaps/test-snap-update.spec.js @@ -55,6 +55,8 @@ describe('Test Snap update', function () { await driver.waitForSelector({ text: 'Install' }); + await driver.clickElement('[data-testid="snap-install-scroll"]'); + await driver.clickElement({ text: 'Install', tag: 'button', @@ -105,6 +107,8 @@ describe('Test Snap update', function () { await driver.waitForSelector({ text: 'Update' }); + await driver.clickElement('[data-testid="snap-update-scroll"]'); + await driver.clickElement({ text: 'Update', tag: 'button', diff --git a/ui/components/app/permissions-connect-header/permissions-connect-header.component.js b/ui/components/app/permissions-connect-header/permissions-connect-header.component.js index 92aea43b5..5f3fc5366 100644 --- a/ui/components/app/permissions-connect-header/permissions-connect-header.component.js +++ b/ui/components/app/permissions-connect-header/permissions-connect-header.component.js @@ -7,17 +7,8 @@ import { FLEX_DIRECTION, JustifyContent, } from '../../../helpers/constants/design-system'; -///: BEGIN:ONLY_INCLUDE_IN(snaps) -import SnapAuthorship from '../snaps/snap-authorship'; -///: END:ONLY_INCLUDE_IN export default class PermissionsConnectHeader extends Component { - ///: BEGIN:ONLY_INCLUDE_IN(snaps) - static contextTypes = { - t: PropTypes.func, - }; - ///: END:ONLY_INCLUDE_IN - static propTypes = { className: PropTypes.string, iconUrl: PropTypes.string, @@ -28,10 +19,6 @@ export default class PermissionsConnectHeader extends Component { headerText: PropTypes.string, leftIcon: PropTypes.node, rightIcon: PropTypes.node, - ///: BEGIN:ONLY_INCLUDE_IN(snaps) - snapVersion: PropTypes.string, - isSnapInstallOrUpdate: PropTypes.bool, - ///: END:ONLY_INCLUDE_IN }; static defaultProps = { @@ -42,22 +29,7 @@ export default class PermissionsConnectHeader extends Component { }; renderHeaderIcon() { - const { - iconUrl, - iconName, - siteOrigin, - leftIcon, - rightIcon, - ///: BEGIN:ONLY_INCLUDE_IN(snaps) - isSnapInstallOrUpdate, - ///: END:ONLY_INCLUDE_IN - } = this.props; - - ///: BEGIN:ONLY_INCLUDE_IN(snaps) - if (isSnapInstallOrUpdate) { - return null; - } - ///: END:ONLY_INCLUDE_IN + const { iconUrl, iconName, siteOrigin, leftIcon, rightIcon } = this.props; return (
@@ -75,16 +47,7 @@ export default class PermissionsConnectHeader extends Component { } render() { - const { - boxProps, - className, - headerTitle, - headerText, - ///: BEGIN:ONLY_INCLUDE_IN(snaps) - siteOrigin, - isSnapInstallOrUpdate, - ///: END:ONLY_INCLUDE_IN - } = this.props; + const { boxProps, className, headerTitle, headerText } = this.props; return ( {this.renderHeaderIcon()}
{headerTitle}
- { - ///: BEGIN:ONLY_INCLUDE_IN(snaps) - isSnapInstallOrUpdate && - ///: END:ONLY_INCLUDE_IN - }
{headerText}
); diff --git a/ui/components/app/snaps/snap-authorship-expanded/index.js b/ui/components/app/snaps/snap-authorship-expanded/index.js new file mode 100644 index 000000000..9e3e60f81 --- /dev/null +++ b/ui/components/app/snaps/snap-authorship-expanded/index.js @@ -0,0 +1 @@ +export { default } from './snap-authorship-expanded'; diff --git a/ui/components/app/snaps/snap-authorship/snap-authorship.js b/ui/components/app/snaps/snap-authorship-expanded/snap-authorship-expanded.js similarity index 57% rename from ui/components/app/snaps/snap-authorship/snap-authorship.js rename to ui/components/app/snaps/snap-authorship-expanded/snap-authorship-expanded.js index 14842afaa..8c5e54a10 100644 --- a/ui/components/app/snaps/snap-authorship/snap-authorship.js +++ b/ui/components/app/snaps/snap-authorship-expanded/snap-authorship-expanded.js @@ -17,6 +17,7 @@ import { BorderStyle, Color, BorderRadius, + FontWeight, } from '../../../../helpers/constants/design-system'; import { formatDate, @@ -34,7 +35,7 @@ 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 SnapAuthorshipExpanded = ({ snapId, className, snap }) => { const t = useI18nContext(); const dispatch = useDispatch(); @@ -55,7 +56,6 @@ const SnapAuthorship = ({ snapId, className, expanded = false, snap }) => { const friendlyName = snapId && getSnapName(snapId, subjectMetadata); - // Expanded data const versionHistory = snap?.versionHistory ?? []; const installInfo = versionHistory.length ? versionHistory[versionHistory.length - 1] @@ -72,33 +72,35 @@ const SnapAuthorship = ({ snapId, className, expanded = false, snap }) => { return ( - {friendlyName} + + {friendlyName} + { {packageName} - {!expanded && ( - - - - )} - {expanded && ( - - - {t('enableSnap')} - - - - - + + + + {t('enabled')} + + + + + - - {installOrigin && installInfo && ( - - - {t('installOrigin')} - - - - {installOrigin.host} - - - {t('installedOn', [ - formatDate(installInfo.date, 'dd MMM yyyy'), - ])} - - - - )} + + + {installOrigin && installInfo && ( - {t('version')} - + + {t('installOrigin')} + + + + {installOrigin.host} + + + {t('installedOn', [ + formatDate(installInfo.date, 'dd MMM yyyy'), + ])} + + + )} + + + {t('version')} + + - )} + ); }; -SnapAuthorship.propTypes = { +SnapAuthorshipExpanded.propTypes = { /** * The id of the snap */ @@ -190,13 +189,9 @@ SnapAuthorship.propTypes = { */ 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 + * The snap object. */ snap: PropTypes.object, }; -export default SnapAuthorship; +export default SnapAuthorshipExpanded; diff --git a/ui/components/app/snaps/snap-authorship-header/index.js b/ui/components/app/snaps/snap-authorship-header/index.js new file mode 100644 index 000000000..4e34522a9 --- /dev/null +++ b/ui/components/app/snaps/snap-authorship-header/index.js @@ -0,0 +1 @@ +export { default } from './snap-authorship-header'; diff --git a/ui/components/app/snaps/snap-authorship-header/snap-authorship-header.js b/ui/components/app/snaps/snap-authorship-header/snap-authorship-header.js new file mode 100644 index 000000000..74bf2adab --- /dev/null +++ b/ui/components/app/snaps/snap-authorship-header/snap-authorship-header.js @@ -0,0 +1,96 @@ +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 Box from '../../../ui/box'; +import { + BackgroundColor, + TextColor, + FLEX_DIRECTION, + TextVariant, + AlignItems, + DISPLAY, + BLOCK_SIZES, + FontWeight, +} from '../../../../helpers/constants/design-system'; +import { + getSnapName, + removeSnapIdPrefix, +} from '../../../../helpers/utils/util'; + +import { Text } from '../../../component-library'; +import { getTargetSubjectMetadata } from '../../../../selectors'; +import SnapAvatar from '../snap-avatar'; +import SnapVersion from '../snap-version/snap-version'; + +const SnapAuthorshipHeader = ({ 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 + // like it is done with snap install + const snapPrefix = snapId && getSnapPrefix(snapId); + const packageName = snapId && removeSnapIdPrefix(snapId); + const isNPM = snapPrefix === 'npm:'; + const url = isNPM + ? `https://www.npmjs.com/package/${packageName}` + : packageName; + + const subjectMetadata = useSelector((state) => + getTargetSubjectMetadata(state, snapId), + ); + + const friendlyName = snapId && getSnapName(snapId, subjectMetadata); + + return ( + + + + + + + {friendlyName} + + + {packageName} + + + + + + + ); +}; + +SnapAuthorshipHeader.propTypes = { + /** + * The id of the snap + */ + snapId: PropTypes.string, + /** + * The className of the SnapAuthorship + */ + className: PropTypes.string, +}; + +export default SnapAuthorshipHeader; diff --git a/ui/components/app/snaps/snap-authorship-header/snap-authorship-header.stories.js b/ui/components/app/snaps/snap-authorship-header/snap-authorship-header.stories.js new file mode 100644 index 000000000..82b433b47 --- /dev/null +++ b/ui/components/app/snaps/snap-authorship-header/snap-authorship-header.stories.js @@ -0,0 +1,21 @@ +import React from 'react'; +import SnapAuthorshipHeader from './snap-authorship-header'; + +export default { + title: 'Components/App/Snaps/SnapAuthorshipHeader', + + component: SnapAuthorshipHeader, + argTypes: { + snapId: { + control: 'text', + }, + }, +}; + +export const DefaultStory = (args) => ; + +DefaultStory.storyName = 'Default'; + +DefaultStory.args = { + snapId: 'npm:@metamask/test-snap-bip44', +}; diff --git a/ui/components/app/snaps/snap-authorship/index.js b/ui/components/app/snaps/snap-authorship/index.js deleted file mode 100644 index 8dc331ebe..000000000 --- a/ui/components/app/snaps/snap-authorship/index.js +++ /dev/null @@ -1 +0,0 @@ -export { default } from './snap-authorship'; diff --git a/ui/components/app/snaps/snap-authorship/snap-authorship.stories.js b/ui/components/app/snaps/snap-authorship/snap-authorship.stories.js deleted file mode 100644 index f668b47f9..000000000 --- a/ui/components/app/snaps/snap-authorship/snap-authorship.stories.js +++ /dev/null @@ -1,21 +0,0 @@ -import React from 'react'; -import SnapAuthorship from '.'; - -export default { - title: 'Components/App/Snaps/SnapAuthorship', - - component: SnapAuthorship, - argTypes: { - snapId: { - control: 'text', - }, - }, -}; - -export const DefaultStory = (args) => ; - -DefaultStory.storyName = 'Default'; - -DefaultStory.args = { - snapId: 'npm:@metamask/test-snap-bip44', -}; diff --git a/ui/components/app/snaps/snap-avatar/snap-avatar.js b/ui/components/app/snaps/snap-avatar/snap-avatar.js index 2cd3d1ec6..f59d4bbcb 100644 --- a/ui/components/app/snaps/snap-avatar/snap-avatar.js +++ b/ui/components/app/snaps/snap-avatar/snap-avatar.js @@ -9,6 +9,7 @@ import { DISPLAY, JustifyContent, Size, + BackgroundColor, } from '../../../../helpers/constants/design-system'; import { getSnapName } from '../../../../helpers/utils/util'; import { @@ -39,10 +40,12 @@ const SnapAvatar = ({ snapId, className }) => { badge={ @@ -50,10 +53,10 @@ const SnapAvatar = ({ snapId, className }) => { position={BadgeWrapperPosition.bottomRight} > {iconUrl ? ( - + ) : ( { + const ref = useRef(); + const [isScrollableState, setIsScrollable] = useState(false); + const [isScrolledToBottomState, setIsScrolledToBottom] = useState(false); + + const update = () => { + const isScrollable = + ref.current && ref.current.scrollHeight > ref.current.clientHeight; + const isScrolledToBottom = isScrollable + ? Math.round(ref.current.scrollTop) + ref.current.offsetHeight >= + ref.current.scrollHeight + : true; + setIsScrollable(isScrollable); + setIsScrolledToBottom(isScrolledToBottom); + }; + + useEffect(update, [ref, ...dependencies]); + + const scrollToBottom = () => { + if (ref.current) { + ref.current.scrollTo(0, ref.current.scrollHeight); + } + }; + + return { + isScrollable: isScrollableState, + isScrolledToBottom: isScrolledToBottomState, + scrollToBottom, + ref, + onScroll: debounce(update, 25), + }; +}; diff --git a/ui/pages/confirmation/components/confirmation-footer/confirmation-footer.js b/ui/pages/confirmation/components/confirmation-footer/confirmation-footer.js index 30f116d3f..7da399a0d 100644 --- a/ui/pages/confirmation/components/confirmation-footer/confirmation-footer.js +++ b/ui/pages/confirmation/components/confirmation-footer/confirmation-footer.js @@ -12,12 +12,14 @@ export default function ConfirmationFooter({ alerts, loading, submitAlerts, + actionsStyle, + style, }) { return ( -
+
{alerts} {submitAlerts} -
+
{onCancel ? (