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 28, 2020
1 parent da37371 commit fb02c58
Show file tree
Hide file tree
Showing 19 changed files with 282 additions and 4 deletions.
9 changes: 9 additions & 0 deletions app/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,9 @@
"failed": {
"message": "Failed"
},
"failureMessage": {
"message": "Something went wrong, and we were unable to complete the action"
},
"fast": {
"message": "Fast"
},
Expand Down Expand Up @@ -1530,6 +1533,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 @@ -521,5 +521,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 @@ -472,5 +472,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 @@ -1403,5 +1403,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 @@ -76,6 +76,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
4 changes: 2 additions & 2 deletions test/unit/ui/app/actions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -979,7 +979,7 @@ describe('Actions', function () {
it('#showAccountDetail', async function () {
setSelectedAddressSpy = sinon.stub(background, 'setSelectedAddress')
.callsArgWith(1, null)
const store = mockStore()
const store = mockStore({ metamask: { selectedAddress: '0x123' } })

await store.dispatch(actions.showAccountDetail())
assert(setSelectedAddressSpy.calledOnce)
Expand All @@ -988,7 +988,7 @@ describe('Actions', function () {
it('displays warning if setSelectedAddress throws', async function () {
setSelectedAddressSpy = sinon.stub(background, 'setSelectedAddress')
.callsArgWith(1, new Error('error'))
const store = mockStore()
const store = mockStore({ metamask: { selectedAddress: '0x123' } })
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 _unconnectedAccountAlertIsOpen = useSelector(unconnectedAccountAlertIsOpen)

if (_unconnectedAccountAlertIsOpen) {
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,63 @@
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"
/>
)
}

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) => {
// close 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
17 changes: 16 additions & 1 deletion ui/app/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ import { setCustomGasLimit } from '../ducks/gas/gas.duck'
import txHelper from '../../lib/tx-helper'
import { getEnvironmentType } from '../../../app/scripts/lib/util'
import actionConstants from './actionConstants'
import {
getPermittedAccountsForCurrentTab,
getSelectedAddress,
} from '../selectors/selectors'
import { switchedToUnconnectedAccount } from '../ducks/alerts/unconnected-account'

let background = null
let promisifiedBackground = null
Expand Down Expand Up @@ -1161,10 +1166,17 @@ export function setSelectedAddress (address) {
}

export function showAccountDetail (address) {
return async (dispatch) => {
return async (dispatch, getState) => {
dispatch(showLoadingIndication())
log.debug(`background.setSelectedAddress`)

const state = getState()
const selectedAddress = getSelectedAddress(state)
const permittedAccountsForCurrentTab = getPermittedAccountsForCurrentTab(state)
const currentTabIsConnectedToPreviousAddress = permittedAccountsForCurrentTab.includes(selectedAddress)
const currentTabIsConnectedToNextAddress = permittedAccountsForCurrentTab.includes(address)
const switchingToUnconnectedAddress = currentTabIsConnectedToPreviousAddress && !currentTabIsConnectedToNextAddress

let tokens
try {
tokens = await promisifiedBackground.setSelectedAddress(address)
Expand All @@ -1179,6 +1191,9 @@ export function showAccountDetail (address) {
type: actionConstants.SHOW_ACCOUNT_DETAIL,
value: address,
})
if (switchingToUnconnectedAddress) {
dispatch(switchedToUnconnectedAccount(address))
}
dispatch(setSelectedToken())
}
}
Expand Down
Loading

0 comments on commit fb02c58

Please sign in to comment.