Skip to content

Commit

Permalink
fix: loading spinner for plugins (DHIS2-8117) (#587)
Browse files Browse the repository at this point in the history
  • Loading branch information
martinkrulltott committed Feb 7, 2020
1 parent 75b6ae4 commit f8be30b
Show file tree
Hide file tree
Showing 15 changed files with 139 additions and 78 deletions.
14 changes: 13 additions & 1 deletion packages/app/src/actions/__tests__/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ import {
SET_UI_RIGHT_SIDEBAR_OPEN,
SET_UI_INTERPRETATION,
} from '../../reducers/ui'
import { SET_LOAD_ERROR, CLEAR_LOAD_ERROR } from '../../reducers/loader'
import {
SET_LOAD_ERROR,
CLEAR_LOAD_ERROR,
SET_PLUGIN_LOADING,
} from '../../reducers/loader'
import {
RECEIVED_SNACKBAR_MESSAGE,
CLOSE_SNACKBAR,
Expand Down Expand Up @@ -56,6 +60,10 @@ describe('index', () => {
Promise.resolve({ visualization: vis })

const expectedActions = [
{
type: SET_PLUGIN_LOADING,
value: true,
},
{
type: SET_VISUALIZATION,
value: vis,
Expand Down Expand Up @@ -94,6 +102,10 @@ describe('index', () => {
Promise.resolve({ visualization: vis })

const expectedActions = [
{
type: SET_PLUGIN_LOADING,
value: true,
},
{
type: SET_UI_INTERPRETATION,
value: interpretation,
Expand Down
1 change: 1 addition & 0 deletions packages/app/src/actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export const tDoLoadVisualization = ({
ouLevels,
}) => async (dispatch, getState, engine) => {
const onSuccess = async response => {
dispatch(fromLoader.acSetPluginLoading(true))
const visualization = convertOuLevelsToUids(
ouLevels,
response.visualization
Expand Down
6 changes: 6 additions & 0 deletions packages/app/src/actions/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
SET_LOAD_ERROR,
CLEAR_LOAD_ERROR,
SET_LOADING,
SET_PLUGIN_LOADING,
} from '../reducers/loader'

export const acSetLoadError = value => ({
Expand All @@ -15,3 +16,8 @@ export const acSetLoading = value => ({
type: SET_LOADING,
value,
})

export const acSetPluginLoading = value => ({
type: SET_PLUGIN_LOADING,
value,
})
1 change: 1 addition & 0 deletions packages/app/src/components/App.css
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,5 @@ body {

.main-center-canvas {
overflow: hidden;
position: relative;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { connect } from 'react-redux'
import { sGetCurrent, sGetCurrentFromUi } from '../../reducers/current'
import * as fromActions from '../../actions'
import { validateLayout } from '../../modules/layoutValidation'
import { acSetLoadError, acClearLoadError } from '../../actions/loader'
import {
acSetLoadError,
acClearLoadError,
acSetPluginLoading,
} from '../../actions/loader'
import history from '../../modules/history'
import { CURRENT_AO_KEY } from '../../api/userDataStore'
import { GenericClientError } from '../../modules/error'
Expand All @@ -15,6 +19,7 @@ const UpdateVisualizationContainer = ({
onUpdate,
acSetLoadError,
acClearLoadError,
onLoadingStart,
}) => {
// validate layout on update
// validation error message will be shown without loading the plugin first
Expand All @@ -27,6 +32,8 @@ const UpdateVisualizationContainer = ({
acSetLoadError(error || new GenericClientError())
}

onLoadingStart()

onUpdate()

const urlContainsCurrentAOKey =
Expand Down Expand Up @@ -55,6 +62,7 @@ const mapDispatchToProps = {
onUpdate: fromActions.fromCurrent.tSetCurrentFromUi,
acSetLoadError,
acClearLoadError,
onLoadingStart: () => acSetPluginLoading(true),
}

export default connect(null, mapDispatchToProps)(UpdateVisualizationContainer)
File renamed without changes.
5 changes: 2 additions & 3 deletions packages/app/src/components/Visualization/StartScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ const StartScreen = ({ error, classes }) => {
</div>
)

const getErrorContent = () => {
return error instanceof VisualizationError ? (
const getErrorContent = () =>
error instanceof VisualizationError ? (
<div style={styles.errorContainer}>
<div style={styles.errorIcon}>{error.icon()}</div>
<p style={styles.errorTitle}>{error.title}</p>
Expand All @@ -106,7 +106,6 @@ const StartScreen = ({ error, classes }) => {
<p style={styles.errorDescription}>{error.message || error}</p>
</div>
)
}

return (
<div style={styles.outer}>
Expand Down
63 changes: 39 additions & 24 deletions packages/app/src/components/Visualization/Visualization.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { Component } from 'react'
import React, { Component, Fragment } from 'react'
import PropTypes from 'prop-types'
import { connect } from 'react-redux'
import { createSelector } from 'reselect'
Expand All @@ -9,11 +9,11 @@ import VisualizationPlugin from '@dhis2/data-visualizer-plugin'
import { sGetVisualization } from '../../reducers/visualization'
import { sGetCurrent } from '../../reducers/current'
import { sGetUiRightSidebarOpen, sGetUiInterpretation } from '../../reducers/ui'
import { sGetLoadError } from '../../reducers/loader'
import { sGetLoadError, sGetIsPluginLoading } from '../../reducers/loader'

import { acAddMetadata } from '../../actions/metadata'
import { acSetChart } from '../../actions/chart'
import { acSetLoadError } from '../../actions/loader'
import { acSetLoadError, acSetPluginLoading } from '../../actions/loader'

import StartScreen from './StartScreen'
import {
Expand All @@ -22,6 +22,7 @@ import {
EmptyResponseError,
AssignedCategoriesAsFilterError,
} from '../../modules/error'
import LoadingMask from './LoadingMask'

export class Visualization extends Component {
constructor(props) {
Expand All @@ -46,10 +47,10 @@ export class Visualization extends Component {
error = new GenericServerError()
}

this.props.acSetLoadError(error)
this.props.setLoadError(error)
}

onChartGenerated = svg => this.props.acSetChart(svg)
onChartGenerated = svg => this.props.setChart(svg)

onResponsesReceived = responses => {
const forMetadata = {}
Expand All @@ -67,7 +68,7 @@ export class Visualization extends Component {
})
})

this.props.acAddMetadata(forMetadata)
this.props.addMetadata(forMetadata)
}

getNewRenderId = () =>
Expand Down Expand Up @@ -105,16 +106,24 @@ export class Visualization extends Component {
return !visConfig || error ? (
<StartScreen />
) : (
<VisualizationPlugin
id={renderId}
d2={this.context.d2}
config={visConfig}
filters={visFilters}
onChartGenerated={this.onChartGenerated}
onResponsesReceived={this.onResponsesReceived}
onError={this.onError}
style={styles.chartCanvas}
/>
<Fragment>
{this.props.isLoading ? (
<div style={styles.loadingCover}>
<LoadingMask />
</div>
) : null}
<VisualizationPlugin
id={renderId}
d2={this.context.d2}
config={visConfig}
filters={visFilters}
onChartGenerated={this.onChartGenerated}
onLoadingComplete={this.props.onLoadingComplete}
onResponsesReceived={this.onResponsesReceived}
onError={this.onError}
style={styles.chartCanvas}
/>
</Fragment>
)
}
}
Expand All @@ -124,13 +133,15 @@ Visualization.contextTypes = {
}

Visualization.propTypes = {
acAddMetadata: PropTypes.func,
acSetChart: PropTypes.func,
acSetLoadError: PropTypes.func,
addMetadata: PropTypes.func,
error: PropTypes.object,
isLoading: PropTypes.bool,
rightSidebarOpen: PropTypes.bool,
setChart: PropTypes.func,
setLoadError: PropTypes.func,
visConfig: PropTypes.object,
visFilters: PropTypes.object,
onLoadingComplete: PropTypes.func,
}

export const visConfigSelector = createSelector(
Expand All @@ -152,10 +163,14 @@ const mapStateToProps = state => ({
visFilters: visFiltersSelector(state),
rightSidebarOpen: sGetUiRightSidebarOpen(state),
error: sGetLoadError(state),
isLoading: sGetIsPluginLoading(state),
})

export default connect(mapStateToProps, {
acAddMetadata,
acSetChart,
acSetLoadError,
})(Visualization)
const mapDispatchToProps = dispatch => ({
onLoadingComplete: () => dispatch(acSetPluginLoading(false)),
addMetadata: metadata => dispatch(acAddMetadata(metadata)),
setChart: chart => dispatch(acSetChart(chart)),
setLoadError: error => dispatch(acSetLoadError(error)),
})

export default connect(mapStateToProps, mapDispatchToProps)(Visualization)
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from '../Visualization'
import StartScreen from '../StartScreen'
import { GenericServerError } from '../../../modules/error'
import LoadingMask from '../LoadingMask'

jest.mock('@dhis2/data-visualizer-plugin', () => () => <div />)

Expand All @@ -28,10 +29,11 @@ describe('Visualization', () => {
visFilters: null,
error: null,
rightSidebarOpen: false,
acAddMetadata: jest.fn(),
acSetChart: jest.fn(),
acClearLoadError: jest.fn(),
acSetLoadError: jest.fn(),
addMetadata: jest.fn(),
setChart: jest.fn(),
clearLoadError: jest.fn(),
setLoadError: jest.fn(),
onLoadingComplete: jest.fn(),
}

shallowVisualization = undefined
Expand All @@ -43,6 +45,24 @@ describe('Visualization', () => {
expect(vis().find(StartScreen).length).toEqual(1)
})

it('renders the loading indicator when loading', () => {
props.isLoading = true
expect(
vis()
.find(LoadingMask)
.exists()
).toBeTruthy()
})

it('hides the loading indicator when not loading', () => {
props.isLoading = false
expect(
vis()
.find(LoadingMask)
.exists()
).toBeFalsy()
})

it('renders a VisualizationPlugin when no error and visConfig available', () => {
expect(vis().find(VisualizationPlugin).length).toEqual(1)
})
Expand All @@ -54,21 +74,23 @@ describe('Visualization', () => {
c: { id: 'c', name: 'c' },
}

vis().simulate('responsesReceived', [
{ metaData: { items }, rows: [1, 2, 3] },
])
vis()
.instance()
.onResponsesReceived([{ metaData: { items }, rows: [1, 2, 3] }])

expect(props.acAddMetadata).toHaveBeenCalled()
expect(props.acAddMetadata).toHaveBeenCalledWith(items)
expect(props.addMetadata).toHaveBeenCalled()
expect(props.addMetadata).toHaveBeenCalledWith(items)
})

it('triggers setChart action when chart has been generated', () => {
const svg = 'coolChart'

vis().simulate('chartGenerated', svg)
vis()
.instance()
.onChartGenerated(svg)

expect(props.acSetChart).toHaveBeenCalled()
expect(props.acSetChart).toHaveBeenCalledWith(svg)
expect(props.setChart).toHaveBeenCalled()
expect(props.setChart).toHaveBeenCalledWith(svg)
})

it('renders visualization with new id when rightSidebarOpen prop changes', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,13 @@ export default {
alignItems: 'flex-start',
height: '100%',
},
loadingCover: {
position: 'absolute',
height: '100%',
width: '100%',
left: 0,
top: 0,
zIndex: 100,
background: '#ffffffab',
},
}
6 changes: 5 additions & 1 deletion packages/app/src/reducers/__tests__/loader.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ describe('reducer: loader', () => {

it(`${CLEAR_LOAD_ERROR}: should clear the loadError`, () => {
const actualState = reducer(
{ isLoading: false, loadingError: errorMsg },
{
isLoading: false,
loadingError: errorMsg,
isPluginLoading: true,
},
{ type: CLEAR_LOAD_ERROR }
)

Expand Down
8 changes: 8 additions & 0 deletions packages/app/src/reducers/loader.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
export const SET_LOAD_ERROR = 'SET_LOAD_ERROR'
export const CLEAR_LOAD_ERROR = 'CLEAR_LOAD_ERROR'
export const SET_LOADING = 'SET_LOADING'
export const SET_PLUGIN_LOADING = 'SET_PLUGIN_LOADING'

export const DEFAULT_LOADING = {
isLoading: false,
loadingError: null,
isPluginLoading: true,
}

export default (state = DEFAULT_LOADING, action) => {
Expand All @@ -25,6 +27,11 @@ export default (state = DEFAULT_LOADING, action) => {
...state,
isLoading: action.value,
}
case SET_PLUGIN_LOADING:
return {
...state,
isPluginLoading: action.value,
}
default:
return state
}
Expand All @@ -33,3 +40,4 @@ export default (state = DEFAULT_LOADING, action) => {
// Selectors
export const sGetLoadError = state => state.loader.loadingError
export const sGetIsLoading = state => state.loader.isLoading
export const sGetIsPluginLoading = state => state.loader.isPluginLoading
Loading

0 comments on commit f8be30b

Please sign in to comment.