From 54be7c3ded0bce21087015f0a69fb3e9a2a21b97 Mon Sep 17 00:00:00 2001 From: Jen Jones Arnesen Date: Thu, 24 Sep 2020 13:44:06 +0200 Subject: [PATCH] fix: dashboard filter - filter dialog incorrectly shows filter as selected even though it was removed [DHIS2-9560] (#1074) The FilterDialog component was always rendered, because FilterSelector is always rendered in the ViewTitleBar. That meant that the filter state was never reset, causing previous item filter to still show. Fix includes: * only render the FilterDialog if there is a selected dimension * move item handling (onSelect, onDeselect...) to the FilterDialog. Since FilterDialog is removed when closed, it will be able to pick up the current state from redux in the useState statement when reopened. --- src/actions/selected.js | 12 +- .../ItemHeader/styles/ItemHeader.module.css | 7 +- .../Item/PrintTitlePageItem/Item.js | 27 +++- src/components/ItemFilter/FilterDialog.js | 147 +++++++++++------- src/components/ItemFilter/FilterSelector.js | 68 +------- src/components/TitleBar/ViewTitleBar.js | 11 +- src/reducers/dashboards.js | 2 + src/reducers/itemFilters.js | 2 - src/reducers/printDashboard.js | 6 + 9 files changed, 146 insertions(+), 136 deletions(-) diff --git a/src/actions/selected.js b/src/actions/selected.js index 818ab5177..c2da6de6b 100644 --- a/src/actions/selected.js +++ b/src/actions/selected.js @@ -1,4 +1,8 @@ -import { getCustomDashboards, sGetDashboardById } from '../reducers/dashboards' +import { + getCustomDashboards, + sGetDashboardById, + EMPTY_DASHBOARD, +} from '../reducers/dashboards' import { SET_SELECTED_ID, SET_SELECTED_ISLOADING, @@ -33,7 +37,6 @@ import { EVENT_CHART, MESSAGES, } from '../modules/itemTypes' -import { orObject } from '../modules/util' // actions @@ -57,8 +60,9 @@ export const tSetSelectedDashboardById = id => async (dispatch, getState) => { dispatch(acSetSelectedIsLoading(true)) const snackbarTimeout = setTimeout(() => { - const dashboardName = orObject(sGetDashboardById(getState(), id)) - .displayName + const dashboardName = ( + sGetDashboardById(getState(), id) || EMPTY_DASHBOARD + ).displayName if (sGetSelectedIsLoading(getState()) && dashboardName) { loadingDashboardMsg.name = dashboardName diff --git a/src/components/Item/ItemHeader/styles/ItemHeader.module.css b/src/components/Item/ItemHeader/styles/ItemHeader.module.css index 37d1719a4..9c58160e9 100644 --- a/src/components/Item/ItemHeader/styles/ItemHeader.module.css +++ b/src/components/Item/ItemHeader/styles/ItemHeader.module.css @@ -1,10 +1,6 @@ -.itemWrap { - padding: var(--spacers-dp8); -} - .itemHeaderWrap { display: flex; - margin: var(--spacers-dp4) var(--spacers-dp4) var(--spacers-dp8) + margin: var(--spacers-dp8) var(--spacers-dp4) var(--spacers-dp8) var(--spacers-dp8); } @@ -12,7 +8,6 @@ font-weight: 700; color: var(--colors-grey800); font-size: 14px; - line-height: 28px; padding: 0; margin: 0; align-self: center; diff --git a/src/components/Item/PrintTitlePageItem/Item.js b/src/components/Item/PrintTitlePageItem/Item.js index d29b1da67..cc3ad6bc3 100644 --- a/src/components/Item/PrintTitlePageItem/Item.js +++ b/src/components/Item/PrintTitlePageItem/Item.js @@ -3,14 +3,20 @@ import { connect } from 'react-redux' import PropTypes from 'prop-types' import i18n from '@dhis2/d2-i18n' -import { orObject } from '../../../modules/util' - import { sGetSelectedId, sGetSelectedShowDescription, } from '../../../reducers/selected' -import { sGetDashboardById } from '../../../reducers/dashboards' +import { + sGetDashboardById, + EMPTY_DASHBOARD, +} from '../../../reducers/dashboards' import { sGetNamedItemFilters } from '../../../reducers/itemFilters' +import { sGetIsEditing } from '../../../reducers/editDashboard' +import { + sGetPrintDashboardName, + sGetPrintDashboardDescription, +} from '../../../reducers/printDashboard' import classes from './styles/Item.module.css' @@ -63,12 +69,21 @@ PrintTitlePageItem.defaultProps = { const mapStateToProps = state => { const id = sGetSelectedId(state) - const dashboard = orObject(sGetDashboardById(state, id)) + const isEditMode = sGetIsEditing(state) + const viewDashboard = sGetDashboardById(state, id) || EMPTY_DASHBOARD + + const name = isEditMode + ? sGetPrintDashboardName(state) || i18n.t('Untitled dashboard') + : viewDashboard.displayName + + const description = isEditMode + ? sGetPrintDashboardDescription(state) + : viewDashboard.displayDescription return { - name: dashboard.displayName, + name, + description, itemFilters: sGetNamedItemFilters(state), - description: dashboard.displayDescription, showDescription: sGetSelectedShowDescription(state), } } diff --git a/src/components/ItemFilter/FilterDialog.js b/src/components/ItemFilter/FilterDialog.js index e5c030f2c..adadfe089 100644 --- a/src/components/ItemFilter/FilterDialog.js +++ b/src/components/ItemFilter/FilterDialog.js @@ -1,6 +1,7 @@ -import React, { Component } from 'react' +import React, { useState } from 'react' import PropTypes from 'prop-types' - +import { connect } from 'react-redux' +import i18n from '@dhis2/d2-i18n' import { Button, Modal, @@ -9,9 +10,6 @@ import { ModalActions, ButtonStrip, } from '@dhis2/ui' - -import i18n from '@dhis2/d2-i18n' - import { PeriodDimension, DynamicDimension, @@ -20,21 +18,71 @@ import { DIMENSION_ID_ORGUNIT, } from '@dhis2/analytics' -class FilterDialog extends Component { - onConfirm = id => () => this.props.onConfirm(id) +import { acAddItemFilter, acRemoveItemFilter } from '../../actions/itemFilters' +import { sGetItemFiltersRoot } from '../../reducers/itemFilters' + +const FilterDialog = ( + { + displayNameProperty, + dimension, + initiallySelectedItems, + addItemFilter, + removeItemFilter, + onClose, + }, + context +) => { + const [filters, setFilters] = useState(initiallySelectedItems) + + const onSelectItems = ({ dimensionId, items }) => { + setFilters({ [dimensionId]: items }) + } + + const onDeselectItems = ({ dimensionId, itemIdsToRemove }) => { + const oldList = filters[dimensionId] || [] + const newList = oldList.filter( + item => !itemIdsToRemove.includes(item.id) + ) + + setFilters({ ...filters, [dimensionId]: newList }) + } + + const onReorderItems = ({ dimensionId, itemIds }) => { + const oldList = filters[dimensionId] || [] + const reorderedList = itemIds.map(id => + oldList.find(item => item.id === id) + ) + + setFilters({ ...filters, [dimensionId]: reorderedList }) + } + + const saveFilter = () => { + const id = dimension.id + const filterItems = filters[id] + + if (filterItems && filterItems.length) { + addItemFilter({ + id, + value: [...filterItems], + }) + } else { + removeItemFilter(id) + } - renderDialogContent() { - const { displayNameProperty, dimension, selectedItems } = this.props - const dialogId = dimension.id + onClose(id) + } + const renderDialogContent = () => { const commonProps = { - d2: this.context.d2, - onSelect: this.props.onSelect, - onDeselect: this.props.onDeselect, - onReorder: this.props.onReorder, + d2: context.d2, + onSelect: onSelectItems, + onDeselect: onDeselectItems, + onReorder: onReorderItems, } - switch (dialogId) { + const selectedItems = filters[dimension.id] || [] + + switch (dimension.id) { case DIMENSION_ID_PERIOD: { return ( @@ -63,51 +111,46 @@ class FilterDialog extends Component { } } - render() { - const { dimension, onClose } = this.props - const dialogId = dimension.id - - return ( - <> - {dialogId && ( - - {dimension.name} - - {this.renderDialogContent()} - - - - - - - - - )} - - ) - } + return ( + <> + {dimension.id && ( + + {dimension.name} + {renderDialogContent()} + + + + + + + + )} + + ) } FilterDialog.propTypes = { + addItemFilter: PropTypes.func, dimension: PropTypes.object, displayNameProperty: PropTypes.string, - selectedItems: PropTypes.array, + initiallySelectedItems: PropTypes.object, + removeItemFilter: PropTypes.func, onClose: PropTypes.func, - onConfirm: PropTypes.func, - onDeselect: PropTypes.func, - onReorder: PropTypes.func, - onSelect: PropTypes.func, } FilterDialog.contextTypes = { d2: PropTypes.object, } -export default FilterDialog +const mapStateToProps = state => ({ + initiallySelectedItems: sGetItemFiltersRoot(state), +}) + +export default connect(mapStateToProps, { + addItemFilter: acAddItemFilter, + removeItemFilter: acRemoveItemFilter, +})(FilterDialog) diff --git a/src/components/ItemFilter/FilterSelector.js b/src/components/ItemFilter/FilterSelector.js index 8b5c9a159..ee8e9b87d 100644 --- a/src/components/ItemFilter/FilterSelector.js +++ b/src/components/ItemFilter/FilterSelector.js @@ -2,6 +2,7 @@ import React, { useState, useRef } from 'react' import PropTypes from 'prop-types' import { connect } from 'react-redux' import ArrowDropDownIcon from '@material-ui/icons/ArrowDropDown' +import isEmpty from 'lodash/isEmpty' import i18n from '@dhis2/d2-i18n' import { DimensionsPanel } from '@dhis2/analytics' @@ -12,11 +13,7 @@ import FilterDialog from './FilterDialog' import { sGetSettingsDisplayNameProperty } from '../../reducers/settings' import { sGetActiveModalDimension } from '../../reducers/activeModalDimension' import { sGetDimensions } from '../../reducers/dimensions' -import { - sGetItemFiltersRoot, - sGetFiltersKeys, -} from '../../reducers/itemFilters' -import { acAddItemFilter, acRemoveItemFilter } from '../../actions/itemFilters' +import { sGetItemFiltersRoot } from '../../reducers/itemFilters' import { acClearActiveModalDimension, acSetActiveModalDimension, @@ -24,14 +21,11 @@ import { const FilterSelector = props => { const [showPopover, setShowPopover] = useState(false) - const [filters, setFilters] = useState(props.initiallySelectedItems) const ref = useRef(null) - const closePanel = () => setShowPopover(false) - const onCloseDialog = () => { - closePanel() + setShowPopover(false) props.clearActiveModalDimension() } @@ -42,45 +36,6 @@ const FilterSelector = props => { ) } - const onSelectItems = ({ dimensionId, items }) => { - setFilters({ ...filters, [dimensionId]: items }) - } - - const onDeselectItems = ({ dimensionId, itemIdsToRemove }) => { - const oldList = filters[dimensionId] || [] - const newList = oldList.filter( - item => !itemIdsToRemove.includes(item.id) - ) - - const list = newList.length ? newList : [] - - setFilters({ ...filters, [dimensionId]: list }) - } - - const onReorderItems = ({ dimensionId, itemIds }) => { - const oldList = filters[dimensionId] || [] - const reorderedList = itemIds.map(id => - oldList.find(item => item.id === id) - ) - - setFilters({ ...filters, [dimensionId]: reorderedList }) - } - - const saveFilter = id => { - const filterItems = filters[id] - - if (filterItems && filterItems.length) { - props.addItemFilter({ - id, - value: [...filterItems], - }) - } else { - props.removeItemFilter(id) - } - - onCloseDialog() - } - return ( <> @@ -91,7 +46,7 @@ const FilterSelector = props => { {showPopover && ( { style={{ width: '320px' }} dimensions={props.dimensions} onDimensionClick={selectDimension} - selectedIds={props.selectedDimensions} + selectedIds={Object.keys(props.initiallySelectedItems)} /> )} - {props.dimension ? ( + {!isEmpty(props.dimension) ? ( ) : null} @@ -125,24 +75,18 @@ const mapStateToProps = state => ({ dimension: sGetActiveModalDimension(state), dimensions: sGetDimensions(state), initiallySelectedItems: sGetItemFiltersRoot(state), - selectedDimensions: sGetFiltersKeys(state), }) FilterSelector.propTypes = { - addItemFilter: PropTypes.func, clearActiveModalDimension: PropTypes.func, dimension: PropTypes.object, dimensions: PropTypes.array, displayNameProperty: PropTypes.string, initiallySelectedItems: PropTypes.object, - removeItemFilter: PropTypes.func, - selectedDimensions: PropTypes.array, setActiveModalDimension: PropTypes.func, } export default connect(mapStateToProps, { clearActiveModalDimension: acClearActiveModalDimension, setActiveModalDimension: acSetActiveModalDimension, - addItemFilter: acAddItemFilter, - removeItemFilter: acRemoveItemFilter, })(FilterSelector) diff --git a/src/components/TitleBar/ViewTitleBar.js b/src/components/TitleBar/ViewTitleBar.js index cdbca529c..a2ef407ab 100644 --- a/src/components/TitleBar/ViewTitleBar.js +++ b/src/components/TitleBar/ViewTitleBar.js @@ -21,6 +21,7 @@ import { import { sGetDashboardById, sGetDashboardItems, + EMPTY_DASHBOARD, } from '../../reducers/dashboards' import classes from './styles/ViewTitleBar.module.css' @@ -82,6 +83,8 @@ const ViewTitleBar = (props, context) => { const buttonRef = createRef() + const userAccess = orObject(access) + return ( <>
@@ -94,7 +97,7 @@ const ViewTitleBar = (props, context) => {
- {access.update ? ( + {userAccess.update ? ( { ) : null} - {access.manage ? ( + {userAccess.manage ? ( @@ -201,7 +204,7 @@ ViewTitleBar.contextTypes = { const mapStateToProps = state => { const id = sGetSelectedId(state) - const dashboard = orObject(sGetDashboardById(state, id)) + const dashboard = sGetDashboardById(state, id) || EMPTY_DASHBOARD return { id, @@ -210,7 +213,7 @@ const mapStateToProps = state => { dashboardItems: sGetDashboardItems(state), showDescription: sGetSelectedShowDescription(state), starred: dashboard.starred, - access: orObject(dashboard.access), + access: dashboard.access, } } diff --git a/src/reducers/dashboards.js b/src/reducers/dashboards.js index 6735a1346..747a3ae36 100644 --- a/src/reducers/dashboards.js +++ b/src/reducers/dashboards.js @@ -17,6 +17,8 @@ export const DEFAULT_STATE_DASHBOARDS = { items: [], } +export const EMPTY_DASHBOARD = {} + // reducer helper functions const updateDashboardProp = ({ state, dashboardId, prop, value }) => ({ diff --git a/src/reducers/itemFilters.js b/src/reducers/itemFilters.js index a26433eeb..e56801510 100644 --- a/src/reducers/itemFilters.js +++ b/src/reducers/itemFilters.js @@ -38,8 +38,6 @@ export default (state = DEFAULT_STATE_ITEM_FILTERS, action) => { export const sGetItemFiltersRoot = state => state.itemFilters -export const sGetFiltersKeys = state => Object.keys(sGetItemFiltersRoot(state)) - // simplify the filters structure to: // [{ id: 'pe', name: 'Period', values: [{ id: 2019: name: '2019' }, {id: 'LAST_MONTH', name: 'Last month' }]}, ...] export const sGetNamedItemFilters = createSelector( diff --git a/src/reducers/printDashboard.js b/src/reducers/printDashboard.js index 9c1d4c3cb..320b26c33 100644 --- a/src/reducers/printDashboard.js +++ b/src/reducers/printDashboard.js @@ -130,6 +130,12 @@ export const sGetPrintDashboardRoot = state => state.printDashboard export const sGetIsPrinting = state => !isEmpty(state.printDashboard) +export const sGetPrintDashboardName = state => + sGetPrintDashboardRoot(state).name + +export const sGetPrintDashboardDescription = state => + sGetPrintDashboardRoot(state).description + export const sGetPrintDashboardItems = state => { return ( sGetPrintDashboardRoot(state)?.dashboardItems || DEFAULT_DASHBOARD_ITEMS