From 9d5be5d29fcdab1273e30810f87de4624b8622a1 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Fri, 2 Aug 2019 18:01:26 -0230 Subject: [PATCH] New notification fixes (#6955) * Replace use of backup-notification with use of home notification * Pin notifications relative to window * Remove unneeded isRequired condition on some home.component properties * Refactor rendering of home notifications * UX for multiple notifications * Adds dismissal to provider request notification. * Fix test failures The e2e tests have been updated to reference `home-notification` classnames instead of the removed `background-notification`. The active tab proptypes and default values were updated as well. --- app/_locales/en/messages.json | 9 +++ app/scripts/controllers/provider-approval.js | 31 ++++++- test/e2e/incremental-security.spec.js | 4 +- .../backup-notification.component.js | 50 ------------ .../backup-notification.container.js | 16 ---- .../app/backup-notification/index.js | 1 - .../app/backup-notification/index.scss | 75 ----------------- .../home-notification.component.js | 6 +- .../app/home-notification/index.scss | 16 +++- ui/app/components/app/index.scss | 2 +- .../app/multiple-notifications/index.js | 1 + .../app/multiple-notifications/index.scss | 80 +++++++++++++++++++ .../multiple-notifications.component.js | 39 +++++++++ ui/app/css/itcss/generic/index.scss | 11 +++ ui/app/pages/home/home.component.js | 67 ++++++++++------ ui/app/pages/home/home.container.js | 9 ++- 16 files changed, 238 insertions(+), 179 deletions(-) delete mode 100644 ui/app/components/app/backup-notification/backup-notification.component.js delete mode 100644 ui/app/components/app/backup-notification/backup-notification.container.js delete mode 100644 ui/app/components/app/backup-notification/index.js delete mode 100644 ui/app/components/app/backup-notification/index.scss create mode 100644 ui/app/components/app/multiple-notifications/index.js create mode 100644 ui/app/components/app/multiple-notifications/index.scss create mode 100644 ui/app/components/app/multiple-notifications/multiple-notifications.component.js diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 4f9dff0c8..430d1b50c 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -208,6 +208,12 @@ "backToAll": { "message": "Back to All" }, + "backupApprovalNotice": { + "message": "Backup your Secret Recovery code to keep your wallet and funds secure." + }, + "backupApprovalInfo": { + "message": "This secret code is required to recover your wallet in case you lose your device, forget your password, have to re-install MetaMask, or want to access your wallet on another device." + }, "backupNow": { "message": "Backup now" }, @@ -550,6 +556,9 @@ "directDepositEtherExplainer": { "message": "If you already have some Ether, the quickest way to get Ether in your new wallet by direct deposit." }, + "dismiss": { + "message": "Dismiss" + }, "done": { "message": "Done" }, diff --git a/app/scripts/controllers/provider-approval.js b/app/scripts/controllers/provider-approval.js index 00ec0aea1..5d565c385 100644 --- a/app/scripts/controllers/provider-approval.js +++ b/app/scripts/controllers/provider-approval.js @@ -24,6 +24,7 @@ class ProviderApprovalController extends SafeEventEmitter { this.preferencesController = preferencesController this.store = new ObservableStore({ approvedOrigins: {}, + dismissedOrigins: {}, providerRequests: [], }) } @@ -66,7 +67,9 @@ class ProviderApprovalController extends SafeEventEmitter { _handleProviderRequest (origin, siteTitle, siteImage) { this.store.updateState({ providerRequests: [{ origin, siteTitle, siteImage }] }) const isUnlocked = this.keyringController.memStore.getState().isUnlocked - if (this.store.getState().approvedOrigins[origin] && this.caching && isUnlocked) { + const { approvedOrigins, dismissedOrigins } = this.store.getState() + const originAlreadyHandled = approvedOrigins[origin] || dismissedOrigins[origin] + if (originAlreadyHandled && this.caching && isUnlocked) { return } this.openPopup && this.openPopup() @@ -82,13 +85,21 @@ class ProviderApprovalController extends SafeEventEmitter { this.closePopup() } - const { approvedOrigins, providerRequests } = this.store.getState() + const { approvedOrigins, dismissedOrigins, providerRequests } = this.store.getState() + + let _dismissedOrigins = dismissedOrigins + if (dismissedOrigins[origin]) { + _dismissedOrigins = Object.assign({}, dismissedOrigins) + delete _dismissedOrigins[origin] + } + const remainingProviderRequests = providerRequests.filter(request => request.origin !== origin) this.store.updateState({ approvedOrigins: { ...approvedOrigins, [origin]: true, }, + dismissedOrigins: _dismissedOrigins, providerRequests: remainingProviderRequests, }) this.emit(`resolvedRequest:${origin}`, { approved: true }) @@ -104,7 +115,7 @@ class ProviderApprovalController extends SafeEventEmitter { this.closePopup() } - const { approvedOrigins, providerRequests } = this.store.getState() + const { approvedOrigins, providerRequests, dismissedOrigins } = this.store.getState() const remainingProviderRequests = providerRequests.filter(request => request.origin !== origin) // We're cloning and deleting keys here because we don't want to keep unneeded keys @@ -114,6 +125,10 @@ class ProviderApprovalController extends SafeEventEmitter { this.store.putState({ approvedOrigins: _approvedOrigins, providerRequests: remainingProviderRequests, + dismissedOrigins: { + ...dismissedOrigins, + [origin]: true, + }, }) this.emit(`resolvedRequest:${origin}`, { approved: false }) } @@ -124,13 +139,21 @@ class ProviderApprovalController extends SafeEventEmitter { * @param {string} origin - origin of the domain that had provider access approved */ forceApproveProviderRequestByOrigin (origin) { - const { approvedOrigins, providerRequests } = this.store.getState() + const { approvedOrigins, dismissedOrigins, providerRequests } = this.store.getState() const remainingProviderRequests = providerRequests.filter(request => request.origin !== origin) + + let _dismissedOrigins = dismissedOrigins + if (dismissedOrigins[origin]) { + _dismissedOrigins = Object.assign({}, dismissedOrigins) + delete _dismissedOrigins[origin] + } + this.store.updateState({ approvedOrigins: { ...approvedOrigins, [origin]: true, }, + dismissedOrigins: _dismissedOrigins, providerRequests: remainingProviderRequests, }) diff --git a/test/e2e/incremental-security.spec.js b/test/e2e/incremental-security.spec.js index c2b231c86..5b0990581 100644 --- a/test/e2e/incremental-security.spec.js +++ b/test/e2e/incremental-security.spec.js @@ -207,12 +207,12 @@ describe('MetaMask', function () { describe('backs up the seed phrase', () => { it('should show a backup reminder', async () => { - const backupReminder = await findElements(driver, By.css('.backup-notification')) + const backupReminder = await findElements(driver, By.xpath("//div[contains(@class, 'home-notification__text') and contains(text(), 'Backup your Secret Recovery code to keep your wallet and funds secure')]")) assert.equal(backupReminder.length, 1) }) it('should take the user to the seedphrase backup screen', async () => { - const backupButton = await findElement(driver, By.css('.backup-notification__submit-button')) + const backupButton = await findElement(driver, By.css('.home-notification__accept-button')) await backupButton.click() await delay(regularDelayMs) }) diff --git a/ui/app/components/app/backup-notification/backup-notification.component.js b/ui/app/components/app/backup-notification/backup-notification.component.js deleted file mode 100644 index dba79186c..000000000 --- a/ui/app/components/app/backup-notification/backup-notification.component.js +++ /dev/null @@ -1,50 +0,0 @@ -import React, { PureComponent } from 'react' -import PropTypes from 'prop-types' -import Button from '../../ui/button' -import { - INITIALIZE_SEED_PHRASE_ROUTE, -} from '../../../helpers/constants/routes' - -export default class BackupNotification extends PureComponent { - static propTypes = { - history: PropTypes.object, - showSeedPhraseBackupAfterOnboarding: PropTypes.func, - } - - static contextTypes = { - t: PropTypes.func, - metricsEvent: PropTypes.func, - } - - handleSubmit = () => { - const { history, showSeedPhraseBackupAfterOnboarding } = this.props - showSeedPhraseBackupAfterOnboarding() - history.push(INITIALIZE_SEED_PHRASE_ROUTE) - } - - render () { - const { t } = this.context - - return ( -
-
- -
Backup your Secret Recovery code to keep your wallet and funds secure.
- -
-
- -
-
- ) - } -} diff --git a/ui/app/components/app/backup-notification/backup-notification.container.js b/ui/app/components/app/backup-notification/backup-notification.container.js deleted file mode 100644 index 6996770bc..000000000 --- a/ui/app/components/app/backup-notification/backup-notification.container.js +++ /dev/null @@ -1,16 +0,0 @@ -import { connect } from 'react-redux' -import { withRouter } from 'react-router-dom' -import { compose } from 'recompose' -import BackupNotification from './backup-notification.component' -import { showSeedPhraseBackupAfterOnboarding } from '../../../store/actions' - -const mapDispatchToProps = dispatch => { - return { - showSeedPhraseBackupAfterOnboarding: () => dispatch(showSeedPhraseBackupAfterOnboarding()), - } -} - -export default compose( - withRouter, - connect(null, mapDispatchToProps) -)(BackupNotification) diff --git a/ui/app/components/app/backup-notification/index.js b/ui/app/components/app/backup-notification/index.js deleted file mode 100644 index a1cbfe75a..000000000 --- a/ui/app/components/app/backup-notification/index.js +++ /dev/null @@ -1 +0,0 @@ -export { default } from './backup-notification.container' diff --git a/ui/app/components/app/backup-notification/index.scss b/ui/app/components/app/backup-notification/index.scss deleted file mode 100644 index 2d76c6ce4..000000000 --- a/ui/app/components/app/backup-notification/index.scss +++ /dev/null @@ -1,75 +0,0 @@ -.backup-notification { - background: rgba(36, 41, 46, 0.9); - box-shadow: 0px 2px 10px rgba(0, 0, 0, 0.12); - border-radius: 8px; - height: 116px; - padding: 16px; - margin: 8px; - - display: flex; - flex-flow: column; - justify-content: space-between; - - position: absolute; - right: 0; - bottom: 0; - - &__header { - display: flex; - } - - &__text { - font-family: Roboto; - font-style: normal; - font-weight: normal; - font-size: 12px; - color: #FFFFFF; - margin-left: 10px; - margin-right: 8px; - } - - .fa-info-circle { - color: #6A737D; - } - - &__ignore-button { - border: 2px solid #6A737D; - box-sizing: border-box; - border-radius: 6px; - color: $white; - background-color: rgba(36, 41, 46, 0.9); - height: 34px; - width: 155px; - padding: 0; - - &:hover { - border-color: #6A737D; - background-color: #6A737D; - } - } - - &__submit-button { - border: 2px solid #6A737D; - box-sizing: border-box; - border-radius: 6px; - color: $white; - background-color: rgba(36, 41, 46, 0.9); - height: 34px; - width: 155px; - padding: 0; - - &:hover { - background-color: #3b4046; - } - - &:active { - background-color:#141618; - } - } - - &__buttons { - display: flex; - width: 130px; - align-self: flex-end; - } -} \ No newline at end of file diff --git a/ui/app/components/app/home-notification/home-notification.component.js b/ui/app/components/app/home-notification/home-notification.component.js index cc46eb53a..cc86ef6d8 100644 --- a/ui/app/components/app/home-notification/home-notification.component.js +++ b/ui/app/components/app/home-notification/home-notification.component.js @@ -1,4 +1,5 @@ import React, { PureComponent } from 'react' +import classnames from 'classnames' import {Tooltip as ReactTippy} from 'react-tippy' import PropTypes from 'prop-types' import Button from '../../ui/button' @@ -22,6 +23,7 @@ export default class HomeNotification extends PureComponent { onIgnore: PropTypes.func, descriptionText: PropTypes.string.isRequired, infoText: PropTypes.string, + classNames: PropTypes.array, } handleAccept = () => { @@ -33,10 +35,10 @@ export default class HomeNotification extends PureComponent { } render () { - const { descriptionText, acceptText, onAccept, ignoreText, onIgnore, infoText } = this.props + const { descriptionText, acceptText, onAccept, ignoreText, onIgnore, infoText, classNames = [] } = this.props return ( -
+
div { + position: relative; + margin-top: 8px; + } + + .fa-sm { + margin-bottom: 8px; + } + +} + +.home-notification-wrapper--show-first { + > div { + position: absolute; + bottom: 0; + right: 0; + visibility: hidden; + } + + > div:first-of-type { + visibility: visible; + + } + + .fa-sm { + position: relative; + display: initial; + } +} + +.flipped { + transform: rotate(180deg); +} diff --git a/ui/app/components/app/multiple-notifications/multiple-notifications.component.js b/ui/app/components/app/multiple-notifications/multiple-notifications.component.js new file mode 100644 index 000000000..95dbb5c9a --- /dev/null +++ b/ui/app/components/app/multiple-notifications/multiple-notifications.component.js @@ -0,0 +1,39 @@ +import React, { PureComponent } from 'react' +import classnames from 'classnames' +import PropTypes from 'prop-types' + +export default class MultipleNotifications extends PureComponent { + static propTypes = { + notifications: PropTypes.array, + classNames: PropTypes.array, + } + + state = { + showAll: false, + } + + render () { + const { showAll } = this.state + const { notifications, classNames = [] } = this.props + + return (
+ {notifications + .filter(notificationConfig => notificationConfig.shouldBeRendered) + .map(notificationConfig => notificationConfig.component) + } +
this.setState({ showAll: !showAll })} + > + {notifications.length > 1 ? : null} +
+
) + } +} diff --git a/ui/app/css/itcss/generic/index.scss b/ui/app/css/itcss/generic/index.scss index 771008205..aaf6c7c0e 100644 --- a/ui/app/css/itcss/generic/index.scss +++ b/ui/app/css/itcss/generic/index.scss @@ -130,3 +130,14 @@ input.form-control { overflow: hidden; text-overflow: ellipsis; } + +.pinned-to-bottom { + position: absolute; + bottom: 0px; +} + +.pinned-to-bottom-right { + position: absolute; + bottom: 0px; + right: 0; +} diff --git a/ui/app/pages/home/home.component.js b/ui/app/pages/home/home.component.js index e59106537..dca4c8540 100644 --- a/ui/app/pages/home/home.component.js +++ b/ui/app/pages/home/home.component.js @@ -3,15 +3,16 @@ import PropTypes from 'prop-types' import Media from 'react-media' import { Redirect } from 'react-router-dom' import HomeNotification from '../../components/app/home-notification' +import MultipleNotifications from '../../components/app/multiple-notifications' import WalletView from '../../components/app/wallet-view' import TransactionView from '../../components/app/transaction-view' import ProviderApproval from '../provider-approval' -import BackupNotification from '../../components/app/backup-notification' import { RESTORE_VAULT_ROUTE, CONFIRM_TRANSACTION_ROUTE, CONFIRM_ADD_SUGGESTED_TOKEN_ROUTE, + INITIALIZE_SEED_PHRASE_ROUTE, } from '../../helpers/constants/routes' export default class Home extends PureComponent { @@ -20,15 +21,17 @@ export default class Home extends PureComponent { } static defaultProps = { - activeTab: null, + activeTab: {}, unsetMigratedPrivacyMode: null, forceApproveProviderRequestByOrigin: null, } static propTypes = { activeTab: PropTypes.shape({ - title: PropTypes.string.isRequired, - url: PropTypes.string.isRequired, + origin: PropTypes.string, + protocol: PropTypes.string, + title: PropTypes.string, + url: PropTypes.string, }), history: PropTypes.object, forgottenPassword: PropTypes.bool, @@ -40,6 +43,8 @@ export default class Home extends PureComponent { viewingUnconnectedDapp: PropTypes.bool.isRequired, forceApproveProviderRequestByOrigin: PropTypes.func, shouldShowSeedPhraseReminder: PropTypes.bool, + showSeedPhraseBackupAfterOnboarding: PropTypes.bool, + rejectProviderRequestByOrigin: PropTypes.func, } componentWillMount () { @@ -77,6 +82,8 @@ export default class Home extends PureComponent { viewingUnconnectedDapp, forceApproveProviderRequestByOrigin, shouldShowSeedPhraseReminder, + showSeedPhraseBackupAfterOnboarding, + rejectProviderRequestByOrigin, } = this.props if (forgottenPassword) { @@ -98,39 +105,49 @@ export default class Home extends PureComponent { { !history.location.pathname.match(/^\/confirm-transaction/) ? ( - { - showPrivacyModeNotification - ? ( - { window.open('https://medium.com/metamask/42549d4870fa', '_blank', 'noopener') unsetMigratedPrivacyMode() }} - /> - ) - : null - } - { - viewingUnconnectedDapp - ? ( - , + }, + { + shouldBeRendered: viewingUnconnectedDapp, + component: { forceApproveProviderRequestByOrigin(activeTab.origin) }} + ignoreText={t('dismiss')} + onIgnore={() => rejectProviderRequestByOrigin(activeTab.origin)} infoText={t('shareAddressInfo', [activeTab.origin])} - /> - ) - : null - } - { - shouldShowSeedPhraseReminder - ? () - : null - } + key="home-shareAddressToConnect" + />, + }, + { + shouldBeRendered: shouldShowSeedPhraseReminder, + component: { + showSeedPhraseBackupAfterOnboarding() + history.push(INITIALIZE_SEED_PHRASE_ROUTE) + }} + infoText={t('backupApprovalInfo')} + key="home-backupApprovalNotice" + />, + }, + ]}/> ) : null } diff --git a/ui/app/pages/home/home.container.js b/ui/app/pages/home/home.container.js index 064c914cd..434d4b7e3 100644 --- a/ui/app/pages/home/home.container.js +++ b/ui/app/pages/home/home.container.js @@ -7,6 +7,8 @@ import { getCurrentEthBalance } from '../../selectors/selectors' import { forceApproveProviderRequestByOrigin, unsetMigratedPrivacyMode, + showSeedPhraseBackupAfterOnboarding, + rejectProviderRequestByOrigin, } from '../../store/actions' import { getEnvironmentType } from '../../../../app/scripts/lib/util' import { ENVIRONMENT_TYPE_POPUP } from '../../../../app/scripts/lib/enums' @@ -17,6 +19,7 @@ const mapStateToProps = state => { const { activeTab, metamask, appState } = state const { approvedOrigins, + dismissedOrigins, lostAccounts, suggestedTokens, providerRequests, @@ -34,7 +37,8 @@ const mapStateToProps = state => { activeTab && activeTabDappProtocols.includes(activeTab.protocol) && privacyMode && - !approvedOrigins[activeTab.origin] + !approvedOrigins[activeTab.origin] && + !dismissedOrigins[activeTab.origin] ) const isPopup = getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_POPUP @@ -48,12 +52,15 @@ const mapStateToProps = state => { activeTab, viewingUnconnectedDapp: isUnconnected && isPopup, shouldShowSeedPhraseReminder: !seedPhraseBackedUp && (parseInt(accountBalance, 16) > 0 || tokens.length > 0), + isPopup, } } const mapDispatchToProps = (dispatch) => ({ unsetMigratedPrivacyMode: () => dispatch(unsetMigratedPrivacyMode()), forceApproveProviderRequestByOrigin: (origin) => dispatch(forceApproveProviderRequestByOrigin(origin)), + rejectProviderRequestByOrigin: origin => dispatch(rejectProviderRequestByOrigin(origin)), + showSeedPhraseBackupAfterOnboarding: () => dispatch(showSeedPhraseBackupAfterOnboarding()), }) export default compose(