Skip to content

Commit

Permalink
Alert user upon switching to unconnected account
Browse files Browse the repository at this point in the history
An alert is now shown when the user switches from an account that is
connected to the active tab to an account that is not connected. The
alert prompts the user to dismiss the alert or connect the account
they're switching to.

The "loading" state is handled by disabling the buttons, and the error
state is handled by displaying a generic error message and disabling
the connect button.

The new reducer for this alert has been created with `createSlice` from
the Redux Toolkit. This utility is recommended by the Redux team, and
represents a new style of writing reducers that I hope we will use more
in the future (or at least something similar). `createSlice` constructs
a reducer, actions, and action creators automatically. The reducer is
constructed using their `createReducer` helper, which uses Immer to
allow directly mutating the state in the reducer but exposing these
changes as immutable.
  • Loading branch information
Gudahtt committed Apr 17, 2020
1 parent 15616a3 commit 08e71d0
Show file tree
Hide file tree
Showing 19 changed files with 291 additions and 8 deletions.
12 changes: 12 additions & 0 deletions app/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,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"
},
Expand Down Expand Up @@ -603,6 +606,9 @@
"failed": {
"message": "Failed"
},
"failureMessage": {
"message": "Something went wrong, and we were unable to complete the action"
},
"fast": {
"message": "Fast"
},
Expand Down Expand Up @@ -1549,6 +1555,12 @@
"unapproved": {
"message": "Unapproved"
},
"unconnectedAccountAlertTitle": {
"message": "Not connected"
},
"unconnectedAccountAlertDescription": {
"message": "This account is not connected to this site"
},
"units": {
"message": "units"
},
Expand Down
3 changes: 3 additions & 0 deletions development/states/confirm-sig-requests.json
Original file line number Diff line number Diff line change
Expand Up @@ -523,5 +523,8 @@
],
"priceAndTimeEstimatesLastRetrieved": 1541527901281,
"errors": {}
},
"unconnectedAccount": {
"state": "CLOSED"
}
}
3 changes: 3 additions & 0 deletions development/states/currency-localization.json
Original file line number Diff line number Diff line change
Expand Up @@ -474,5 +474,8 @@
],
"priceAndTimeEstimatesLastRetrieved": 1541527901281,
"errors": {}
},
"unconnectedAccount": {
"state": "CLOSED"
}
}
5 changes: 4 additions & 1 deletion development/states/tx-list-items.json
Original file line number Diff line number Diff line change
Expand Up @@ -1405,5 +1405,8 @@
"priceAndTimeEstimatesLastRetrieved": 1541527901281,
"errors": {}
},
"confirmTransaction": {}
"confirmTransaction": {},
"unconnectedAccount": {
"state": "CLOSED"
}
}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
"@metamask/eth-ledger-bridge-keyring": "^0.2.6",
"@metamask/eth-token-tracker": "^2.0.0",
"@metamask/etherscan-link": "^1.1.0",
"@reduxjs/toolkit": "^1.3.2",
"@sentry/browser": "^5.11.1",
"@sentry/integrations": "^5.11.1",
"@zxing/library": "^0.8.0",
Expand Down
12 changes: 6 additions & 6 deletions test/unit/ui/app/actions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -367,19 +367,19 @@ describe('Actions', function () {
importAccountWithStrategySpy.restore()
})

it('calls importAccountWithStrategies in background', function () {
const store = mockStore()
it('calls importAccountWithStrategies in background', async function () {
const store = mockStore({ metamask: devState })

importAccountWithStrategySpy = sinon.spy(background, 'importAccountWithStrategy')

const importPrivkey = 'c87509a1c067bbde78beb793e6fa76530b6382a4c0241e5e4a9ec0a0f44dc0d3'

store.dispatch(actions.importNewAccount('Private Key', [ importPrivkey ]))
await store.dispatch(actions.importNewAccount('Private Key', [ importPrivkey ]))
assert(importAccountWithStrategySpy.calledOnce)
})

it('displays warning error message when importAccount in background callback errors', async function () {
const store = mockStore()
const store = mockStore({ metamask: devState })

const expectedActions = [
{ type: 'SHOW_LOADING_INDICATION', value: 'This may take a while, please be patient.' },
Expand Down Expand Up @@ -1055,14 +1055,14 @@ describe('Actions', function () {
})

it('#showAccountDetail', function () {
const store = mockStore()
const store = mockStore({ metamask: devState })

store.dispatch(actions.showAccountDetail())
assert(setSelectedAddressSpy.calledOnce)
})

it('displays warning if setSelectedAddress throws', function () {
const store = mockStore()
const store = mockStore({ metamask: devState })
const expectedActions = [
{ type: 'SHOW_LOADING_INDICATION', value: undefined },
{ type: 'HIDE_LOADING_INDICATION' },
Expand Down
19 changes: 19 additions & 0 deletions ui/app/components/app/alerts/alerts.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import React from 'react'
import { useSelector } from 'react-redux'

import UnconnectedAccountAlert from './unconnected-account-alert'
import { alertIsOpen as unconnectedAccountAlertIsOpen } from '../../../ducks/alerts/unconnected-account'

const Alerts = () => {
const isOpen = useSelector(unconnectedAccountAlertIsOpen)

if (isOpen) {
return (
<UnconnectedAccountAlert />
)
}

return null
}

export default Alerts
1 change: 1 addition & 0 deletions ui/app/components/app/alerts/alerts.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import './unconnected-account-alert/unconnected-account-alert.scss'
1 change: 1 addition & 0 deletions ui/app/components/app/alerts/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from './alerts'
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from './unconnected-account-alert'
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import React, { useContext } from 'react'
import { useDispatch, useSelector } from 'react-redux'

import {
ALERT_STATE,
connectAccount,
dismissAlert,
getAlertState,
} from '../../../../ducks/alerts/unconnected-account'
import { I18nContext } from '../../../../contexts/i18n'
import Popover from '../../../ui/popover'
import Button from '../../../ui/button'

const {
ERROR,
LOADING,
} = ALERT_STATE

const SwitchToUnconnectedAccountAlert = () => {
const t = useContext(I18nContext)
const dispatch = useDispatch()
const alertState = useSelector(getAlertState)

return (
<Popover
title={t('unconnectedAccountAlertTitle')}
subtitle={t('unconnectedAccountAlertDescription')}
onClose={() => dispatch(dismissAlert())}
footer={(
<>
{
alertState === ERROR
? (
<div className="unconnected-account-alert__error">
{ t('failureMessage') }
</div>
)
: null
}
<div className="unconnected-account-alert__footer-buttons">
<Button
disabled={alertState === LOADING}
onClick={() => dispatch(dismissAlert())}
type="secondary"
>
{ t('dismiss') }
</Button>
<Button
disabled={alertState === LOADING || alertState === ERROR}
onClick={() => dispatch(connectAccount())}
type="primary"
>
{ t('connect') }
</Button>
</div>
</>
)}
footerClassName="unconnected-account-alert__footer"
>
</Popover>
)
}

export default SwitchToUnconnectedAccountAlert
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
.unconnected-account-alert {
&__footer {
flex-direction: column;

:only-child {
margin: 0;
}
}

&__footer-buttons {
display: flex;
flex-direction: row;

button:first-child {
margin-right: 24px;
}
}

&__error {
margin-bottom: 16px;
padding: 16px;
font-size: 14px;
border: 1px solid #D73A49;
background: #F8EAE8;
border-radius: 3px;
}
}
2 changes: 2 additions & 0 deletions ui/app/components/app/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

@import 'add-token-button/index';

@import 'alerts/alerts.scss';

@import 'app-header/index';

@import 'asset-list/asset-list.scss';
Expand Down
5 changes: 5 additions & 0 deletions ui/app/ducks/alerts/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import unconnectedAccount from './unconnected-account'

export default {
unconnectedAccount,
}
101 changes: 101 additions & 0 deletions ui/app/ducks/alerts/unconnected-account.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import { createSlice } from '@reduxjs/toolkit'
import { captureException } from '@sentry/browser'

import actionConstants from '../../store/actionConstants'
import { addPermittedAccount } from '../../store/actions'
import { getOriginOfCurrentTab } from '../../selectors/selectors'

// Constants

export const ALERT_STATE = {
CLOSED: 'CLOSED',
ERROR: 'ERROR',
LOADING: 'LOADING',
OPEN: 'OPEN',
}

const name = 'unconnectedAccount'

const initialState = {
state: ALERT_STATE.CLOSED,
address: null,
}

// Slice (reducer plus auto-generated actions and action creators)

const slice = createSlice({
name,
initialState,
reducers: {
connectAccountFailed: (state) => {
state.state = ALERT_STATE.ERROR
},
connectAccountRequested: (state) => {
state.state = ALERT_STATE.LOADING
},
connectAccountSucceeded: (state) => {
state.state = ALERT_STATE.CLOSED
state.address = null
},
dismissAlert: (state) => {
state.state = ALERT_STATE.CLOSED
state.address = null
},
switchedToUnconnectedAccount: (state, action) => {
state.state = ALERT_STATE.OPEN
state.address = action.payload
},
},
extraReducers: {
[actionConstants.UPDATE_METAMASK_STATE]: (state, action) => {
// lose the alert if the account is switched while it's open
if (
state.state === ALERT_STATE.OPEN &&
state.address !== action.value.selectedAddress
) {
state.state = ALERT_STATE.CLOSED
state.address = null
}
},
},
})

const { actions, reducer } = slice

export default reducer

// Selectors

export const getAlertState = (state) => state[name].state

export const getAlertAccountAddress = (state) => state[name].address

export const alertIsOpen = (state) => state[name].state !== ALERT_STATE.CLOSED


// Actions / action-creators

export const {
connectAccountFailed,
connectAccountRequested,
connectAccountSucceeded,
dismissAlert,
switchedToUnconnectedAccount,
} = actions

export const connectAccount = () => {
return async (dispatch, getState) => {
const state = getState()
const address = getAlertAccountAddress(state)
const origin = getOriginOfCurrentTab(state)
try {
await dispatch(connectAccountRequested())
await dispatch(addPermittedAccount(origin, address))
await dispatch(connectAccountSucceeded())
} catch (error) {
console.error(error)
captureException(error)
await dispatch(connectAccountFailed())
}
}
}
2 changes: 2 additions & 0 deletions ui/app/ducks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ import sendReducer from './send/send.duck'
import appStateReducer from './app/app'
import confirmTransactionReducer from './confirm-transaction/confirm-transaction.duck'
import gasReducer from './gas/gas.duck'
import alerts from './alerts'

export default combineReducers({
...alerts,
activeTab: (s) => (s === undefined ? null : s),
metamask: metamaskReducer,
appState: appStateReducer,
Expand Down
2 changes: 2 additions & 0 deletions ui/app/pages/routes/routes.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { Modal } from '../../components/app/modals'
import Alert from '../../components/ui/alert'
import AppHeader from '../../components/app/app-header'
import UnlockPage from '../unlock-page'
import Alerts from '../../components/app/alerts'

import {
ADD_TOKEN_ROUTE,
Expand Down Expand Up @@ -251,6 +252,7 @@ export default class Routes extends Component {
{ !isLoading && isLoadingNetwork && <LoadingNetwork /> }
{ this.renderRoutes() }
</div>
<Alerts />
</div>
)
}
Expand Down
Loading

0 comments on commit 08e71d0

Please sign in to comment.