From af86dd0f431343a059c7f5eeb07335e94ea23a5d Mon Sep 17 00:00:00 2001 From: Tom Kirkpatrick Date: Mon, 7 Jan 2019 22:20:03 +0100 Subject: [PATCH] fix(ui): ensure error messages are visible Enhance the error reducer and GlobalError component to allow for multiple messages and ensure that error messages are visible for 10 seconds or until they are manually cleared. Fix #1206 --- app/components/GlobalError/GlobalError.js | 68 ++++++++--------------- app/components/Home/WalletLauncher.js | 4 +- app/components/UI/Notification.js | 14 ++--- app/containers/Root.js | 8 +-- app/reducers/error.js | 39 +++++++++---- 5 files changed, 64 insertions(+), 69 deletions(-) diff --git a/app/components/GlobalError/GlobalError.js b/app/components/GlobalError/GlobalError.js index d2a668c8fe3..15e5d098432 100644 --- a/app/components/GlobalError/GlobalError.js +++ b/app/components/GlobalError/GlobalError.js @@ -7,57 +7,37 @@ import { Notification } from 'components/UI' class GlobalError extends React.Component { static propTypes = { - error: PropTypes.oneOfType([PropTypes.string, PropTypes.array, PropTypes.object]), + errors: PropTypes.array, clearError: PropTypes.func.isRequired } - componentDidUpdate(prevProps) { - const { clearError, error } = this.props - if (!prevProps.error && error) { - setTimeout(clearError, 10000) - } - } - render() { - const { error, clearError } = this.props - - let errors = [] - - if (error) { - if (Array.isArray(error)) { - errors = error - } else if (typeof error === 'string') { - errors.push(error) - } else if (typeof error === 'object') { - Object.keys(error).forEach(key => { - errors.push(`${key}: ${error[key]}`) - }) - } - } + const { errors, clearError } = this.props return ( - - {show => - show && - (springStyles => ( - - - {errors.map(error => ( - - {errorToUserFriendly(error)} + + {errors.map(item => ( + + {show => + show && + (springStyles => ( + + clearError(item.id)} mb={2}> + {errorToUserFriendly(item.message)} - ))} - - - )) - } - + + )) + } + + ))} + ) } } diff --git a/app/components/Home/WalletLauncher.js b/app/components/Home/WalletLauncher.js index 61e9bbb9457..ac6f51cf90d 100644 --- a/app/components/Home/WalletLauncher.js +++ b/app/components/Home/WalletLauncher.js @@ -29,7 +29,7 @@ class WalletLauncher extends React.Component { // If there are lnd start errors, show as a global error. if (startLndError) { - setError(startLndError) + Object.keys(startLndError).forEach(key => setError(startLndError[key])) setStartLndError(null) } } @@ -50,7 +50,7 @@ class WalletLauncher extends React.Component { // If we got lnd start errors, show as a global error. if (startLndError && !prevProps.startLndError) { - setError(startLndError) + Object.keys(startLndError).forEach(key => setError(startLndError.startLndError[key])) setStartLndError(null) } diff --git a/app/components/UI/Notification.js b/app/components/UI/Notification.js index a71efd56194..7c6205a3560 100644 --- a/app/components/UI/Notification.js +++ b/app/components/UI/Notification.js @@ -27,23 +27,18 @@ class Notification extends React.Component { variant: PropTypes.string } - constructor(props) { - super(props) - this.state = { hover: false } - this.hoverOn = this.hoverOn.bind(this) - this.hoverOff = this.hoverOff.bind(this) - } + state = { hover: false } - hoverOn() { + hoverOn = () => { this.setState({ hover: true }) } - hoverOff() { + hoverOff = () => { this.setState({ hover: false }) } render() { - const { children, processing, variant } = this.props + const { children, processing, variant, ...rest } = this.props const { hover } = this.state return ( diff --git a/app/containers/Root.js b/app/containers/Root.js index e78686f0e72..0afae01cc06 100644 --- a/app/containers/Root.js +++ b/app/containers/Root.js @@ -34,7 +34,7 @@ class Root extends React.Component { hasWallets: PropTypes.bool, clearError: PropTypes.func.isRequired, theme: PropTypes.object, - error: PropTypes.oneOfType([PropTypes.string, PropTypes.array, PropTypes.object]), + errors: PropTypes.array.isRequired, history: PropTypes.object.isRequired, isLoading: PropTypes.bool.isRequired, @@ -76,7 +76,7 @@ class Root extends React.Component { } render() { - const { hasWallets, clearError, theme, error, history, isLoading } = this.props + const { hasWallets, clearError, theme, errors, history, isLoading } = this.props // Wait until we have loaded essential data before displaying anything. if (!theme) { @@ -89,7 +89,7 @@ class Root extends React.Component { - + @@ -125,7 +125,7 @@ class Root extends React.Component { const mapStateToProps = state => ({ hasWallets: walletSelectors.hasWallets(state), - error: errorSelectors.getErrorState(state), + errors: errorSelectors.getErrorState(state), theme: themeSelectors.currentThemeSettings(state), isLoading: appSelectors.isLoading(state) || state.lnd.startingLnd, isMounted: appSelectors.isMounted(state) diff --git a/app/reducers/error.js b/app/reducers/error.js index 3ceeb9e418f..627af866915 100644 --- a/app/reducers/error.js +++ b/app/reducers/error.js @@ -2,28 +2,41 @@ // Initial State // ------------------------------------ const initialState = { - error: null + errors: [] } // ------------------------------------ // Constants // ------------------------------------ +const ERROR_TIMEOUT = 10000 export const SET_ERROR = 'SET_ERROR' export const CLEAR_ERROR = 'CLEAR_ERROR' // ------------------------------------ // Actions // ------------------------------------ -export function setError(error) { - return { - type: SET_ERROR, - error +export const setError = error => dispatch => { + // Cooerce the error into an error item with a random id that we can use for clearning the error later. + const errorItem = { + id: Math.random() + .toString(36) + .substring(7), + message: error } + + // Set a timer to clear the error after 10 seconds. + setTimeout(() => dispatch(clearError(errorItem.id)), ERROR_TIMEOUT) + + return dispatch({ + type: SET_ERROR, + errorItem + }) } -export function clearError() { +export function clearError(id) { return { - type: CLEAR_ERROR + type: CLEAR_ERROR, + id } } @@ -31,8 +44,14 @@ export function clearError() { // Action Handlers // ------------------------------------ const ACTION_HANDLERS = { - [SET_ERROR]: (state, { error }) => ({ ...state, error }), - [CLEAR_ERROR]: () => initialState + [SET_ERROR]: (state, { errorItem }) => ({ + ...state, + errors: [...state.errors, errorItem] + }), + [CLEAR_ERROR]: (state, { id }) => ({ + ...state, + errors: state.errors.filter(item => item.id !== id) + }) } // ------------------------------------ @@ -40,7 +59,7 @@ const ACTION_HANDLERS = { // ------------------------------------ const errorSelectors = {} -errorSelectors.getErrorState = state => state.error.error +errorSelectors.getErrorState = state => state.error.errors export { errorSelectors }