Skip to content

Commit

Permalink
fix: fixes for DND between axes (#554)
Browse files Browse the repository at this point in the history 
Use the right method to ensure layout rules are checked.

Disable DND on locked dimensions

Check against layout rules when adding/moving dimensions to avoid dropping where is not allowed (ie. axis with max 1 dimension
where there is already a locked dimension that cannot be moved to
filters).

Use getTransferableDimension from analytics instead of looking at 1 locked dimension which does not necessarily
prevent a add/move of a new dimension.
Example: a case with max limit > 1 and 1 locked dimension present.
  • Loading branch information
@edoardo
edoardo committed Jan 24, 2020
1 parent 94ad11d commit 886de23
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 61 deletions.
2 changes: 1 addition & 1 deletion packages/app/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"redux-mock-store": "^1.5.3"
},
"dependencies": {
"@dhis2/analytics": "2.8.3",
"@dhis2/analytics": "^2.8.5",
"@dhis2/d2-ui-core": "^6.5.0",
"@dhis2/d2-ui-file-menu": "^6.5.0",
"@dhis2/d2-ui-interpretations": "^6.5.0",
Expand Down
6 changes: 2 additions & 4 deletions packages/app/src/components/Layout/Chip.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import {
getFixedDimensionProp,
getAxisMaxNumberOfItems,
hasAxisTooManyItems,
getAxisPerLockedDimension,
getDisplayNameByVisType,
getAxisName,
DIMENSION_ID_ASSIGNED_CATEGORIES,
isDimensionLocked,
} from '@dhis2/analytics'
import PropTypes from 'prop-types'

Expand Down Expand Up @@ -47,9 +47,7 @@ class Chip extends React.Component {

timeout = null

isLocked =
getAxisPerLockedDimension(this.props.type, this.props.dimensionId) ===
this.props.axisId
isLocked = isDimensionLocked(this.props.type, this.props.dimensionId)

axisMaxNumberOfItems = getAxisMaxNumberOfItems(
this.props.type,
Expand Down
64 changes: 44 additions & 20 deletions packages/app/src/components/Layout/DefaultLayout/DefaultAxis.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,19 @@ import React from 'react'
import PropTypes from 'prop-types'
import { connect } from 'react-redux'
import { Droppable, Draggable } from 'react-beautiful-dnd'
import { getAxisName } from '@dhis2/analytics'
import {
getAxisName,
isDimensionLocked,
canDimensionBeAddedToAxis,
} from '@dhis2/analytics'

import Chip from '../Chip'
import { sGetUi, sGetUiItems, sGetUiType } from '../../../reducers/ui'
import {
sGetUi,
sGetUiLayout,
sGetUiItems,
sGetUiType,
} from '../../../reducers/ui'
import { decodeDataTransfer } from '../../../modules/dnd'
import {
acAddUiLayoutDimensions,
Expand All @@ -22,47 +31,60 @@ class Axis extends React.Component {
onDrop = e => {
e.preventDefault()

const {
type,
layout,
axisId,
itemsByDimension,
onAddDimension,
onDropWithoutItems,
} = this.props
const { dimensionId, source } = decodeDataTransfer(e)

this.props.onAddDimension({
[dimensionId]: this.props.axisId,
})
if (canDimensionBeAddedToAxis(type, layout[axisId], axisId)) {
onAddDimension({
[dimensionId]: axisId,
})

const items = this.props.itemsByDimension[dimensionId]
const hasNoItems = Boolean(!items || !items.length)
const items = itemsByDimension[dimensionId]
const hasNoItems = Boolean(!items || !items.length)

if (source === SOURCE_DIMENSIONS && hasNoItems) {
this.props.onDropWithoutItems(dimensionId)
if (source === SOURCE_DIMENSIONS && hasNoItems) {
onDropWithoutItems(dimensionId)
}
}
}

render() {
const { axisId, axis, style, type, getOpenHandler } = this.props

return (
<div
id={this.props.axisId}
style={{ ...styles.axisContainer, ...this.props.style }}
id={axisId}
style={{ ...styles.axisContainer, ...style }}
onDragOver={this.onDragOver}
onDrop={this.onDrop}
>
<div style={styles.label}>{getAxisName(this.props.axisId)}</div>
<Droppable
droppableId={this.props.axisId}
direction="horizontal"
>
<div style={styles.label}>{getAxisName(axisId)}</div>
<Droppable droppableId={axisId} direction="horizontal">
{provided => (
<div
style={styles.content}
ref={provided.innerRef}
{...provided.droppableProps}
>
{this.props.axis.map((dimensionId, index) => {
const key = `${this.props.axisId}-${dimensionId}`
{axis.map((dimensionId, index) => {
const key = `${axisId}-${dimensionId}`

return (
<Draggable
key={key}
draggableId={key}
index={index}
isDragDisabled={isDimensionLocked(
type,
dimensionId
)}
>
{provided => (
<div
Expand All @@ -71,10 +93,10 @@ class Axis extends React.Component {
{...provided.dragHandleProps}
>
<Chip
onClick={this.props.getOpenHandler(
onClick={getOpenHandler(
dimensionId
)}
axisId={this.props.axisId}
axisId={axisId}
dimensionId={dimensionId}
/>
</div>
Expand All @@ -98,6 +120,7 @@ Axis.propTypes = {
getOpenHandler: PropTypes.func,
getRemoveHandler: PropTypes.func,
itemsByDimension: PropTypes.object,
layout: PropTypes.object,
style: PropTypes.object,
type: PropTypes.string,
ui: PropTypes.object,
Expand All @@ -109,6 +132,7 @@ Axis.propTypes = {
const mapStateToProps = state => ({
ui: sGetUi(state),
type: sGetUiType(state),
layout: sGetUiLayout(state),
itemsByDimension: sGetUiItems(state),
})

Expand Down
29 changes: 18 additions & 11 deletions packages/app/src/components/Layout/Layout.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@ import {
LAYOUT_TYPE_PIE,
LAYOUT_TYPE_YEAR_OVER_YEAR,
getLayoutTypeByVisType,
canDimensionBeAddedToAxis,
} from '@dhis2/analytics'

import DefaultLayout from './DefaultLayout/DefaultLayout'
import YearOverYearLayout from './YearOverYearLayout/YearOverYearLayout'
import PieLayout from './PieLayout/PieLayout'
import { sGetUiLayout, sGetUiType } from '../../reducers/ui'
import { acSetUiLayout } from '../../actions/ui'
import { acAddUiLayoutDimensions, acSetUiLayout } from '../../actions/ui'

const componentMap = {
[LAYOUT_TYPE_DEFAULT]: DefaultLayout,
Expand All @@ -22,7 +23,9 @@ const componentMap = {
}

const Layout = props => {
const layoutType = getLayoutTypeByVisType(props.visType)
const { visType, layout } = props

const layoutType = getLayoutTypeByVisType(visType)
const LayoutComponent = componentMap[layoutType]

const onDragEnd = result => {
Expand All @@ -32,21 +35,23 @@ const Layout = props => {
return
}

const sourceList = Array.from(props.layout[source.droppableId])
const sourceList = Array.from(layout[source.droppableId])
const [moved] = sourceList.splice(source.index, 1)
const reorderedDimensions = {}

if (source.droppableId === destination.droppableId) {
sourceList.splice(destination.index, 0, moved)
reorderedDimensions[source.droppableId] = sourceList

props.onReorderDimensions({
...layout,
[source.droppableId]: sourceList,
})
} else {
const destList = Array.from(props.layout[destination.droppableId])
destList.splice(destination.index, 0, moved)
reorderedDimensions[destination.droppableId] = destList
reorderedDimensions[source.droppableId] = sourceList
}
const axisId = destination.droppableId

props.onReorderDimensions({ ...props.layout, ...reorderedDimensions })
if (canDimensionBeAddedToAxis(visType, layout[axisId], axisId)) {
props.onAddDimensions({ [moved]: destination.droppableId })
}
}
}

return (
Expand All @@ -59,6 +64,7 @@ const Layout = props => {
Layout.propTypes = {
layout: PropTypes.object,
visType: PropTypes.string,
onAddDimensions: PropTypes.func,
onReorderDimensions: PropTypes.func,
}

Expand All @@ -69,6 +75,7 @@ const mapStateToProps = state => ({

const mapDispatchToProps = dispatch => ({
onReorderDimensions: layout => dispatch(acSetUiLayout(layout)),
onAddDimensions: map => dispatch(acAddUiLayoutDimensions(map)),
})

export default connect(mapStateToProps, mapDispatchToProps)(Layout)
2 changes: 1 addition & 1 deletion packages/app/src/components/Visualization/StartScreen.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const StartScreen = ({ error, classes }) => {
setMostViewedVisualizations(result.visualization)
}
fetchData(engine)
}, [])
}, []) // eslint-disable-line react-hooks/exhaustive-deps

const getContent = () =>
error ? (
Expand Down
30 changes: 21 additions & 9 deletions packages/app/src/modules/layout.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {
AXIS_ID_FILTERS,
getAxisMaxNumberOfDimensions,
getAvailableAxes,
isAxisFull,
getTransferableDimension,
} from '@dhis2/analytics'

// Names for dnd sources
Expand Down Expand Up @@ -67,16 +69,26 @@ export const getRetransfer = (layout, transfer, visType) => {
const destinationAxisId = transfer[id]
const dimensionsAtDestination = layout[destinationAxisId] || []

const axisIsFull =
dimensionsAtDestination.length ===
getAxisMaxNumberOfDimensions(visType, destinationAxisId)
if (
isAxisFull(
visType,
destinationAxisId,
dimensionsAtDestination.length
)
) {
const transferableDimension = getTransferableDimension(
visType,
destinationAxisId,
layout[destinationAxisId]
)

if (axisIsFull) {
retransfer[dimensionsAtDestination[0]] = sourceAxis
? sourceAxis
: getAvailableAxes(visType).find(
axis => !getAxisMaxNumberOfDimensions(visType, axis)
)
if (transferableDimension) {
retransfer[transferableDimension] = sourceAxis
? sourceAxis
: getAvailableAxes(visType).find(
axis => !getAxisMaxNumberOfDimensions(visType, axis)
)
}
}
})

Expand Down
20 changes: 14 additions & 6 deletions packages/app/src/reducers/ui.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
DIMENSION_ID_ORGUNIT,
VIS_TYPE_COLUMN,
DIMENSION_ID_ASSIGNED_CATEGORIES,
canDimensionBeAddedToAxis,
} from '@dhis2/analytics'

import {
Expand Down Expand Up @@ -101,15 +102,22 @@ export default (state = DEFAULT_UI, action) => {
...getRetransfer(state.layout, action.value, state.type),
}

// Filter out transferred dimension ids (remove from source)
const newLayout = getFilteredLayout(
state.layout,
Object.keys(transfers)
)
let newLayout = state.layout

// Add dimension ids to destination (axisId === null means remove from layout)
Object.entries(transfers).forEach(([dimensionId, axisId]) => {
newLayout[axisId] && newLayout[axisId].push(dimensionId)
if (
newLayout[axisId] &&
canDimensionBeAddedToAxis(
state.type,
newLayout[axisId],
axisId
)
) {
// Filter out transferred dimension id (remove from source)
newLayout = getFilteredLayout(newLayout, [dimensionId])
newLayout[axisId].push(dimensionId)
}
})

return {
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"module": "./build/es/lib.js",
"license": "BSD-3-Clause",
"dependencies": {
"@dhis2/analytics": "2.8.3",
"@dhis2/analytics": "^2.8.5",
"@material-ui/core": "^3.1.2",
"d2-analysis": "33.2.11",
"lodash-es": "^4.17.11",
Expand Down
16 changes: 8 additions & 8 deletions yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1243,10 +1243,10 @@
debug "^3.1.0"
lodash.once "^4.1.1"

"@dhis2/analytics@2.8.3":
version "2.8.3"
resolved "https://registry.yarnpkg.com/@dhis2/analytics/-/analytics-2.8.3.tgz#062118d257c29d5e28cf4bf035842f112b9eef41"
integrity sha512-bF38HNa6vL9JcJApSPE1KvLAb8M3p1nh/ymtcD6ULIhnTTqKy9eOaiz8hulH2NYRG+aJkXFNtp46OhmQGTZ7Ng==
"@dhis2/analytics@^2.1.0":
version "2.8.1"
resolved "https://registry.yarnpkg.com/@dhis2/analytics/-/analytics-2.8.1.tgz#228af6524f35c864689e28678752f1c8f42072e8"
integrity sha512-uIiQA8kIxUdRZEC+y0cwUDQUxBqgq1yVMRdEuevkm1zTuFJxKaNrJ6nZT9WyrVtnxpzYQneK5LsXsjMMo4vl8g==
dependencies:
"@dhis2/d2-i18n" "^1.0.4"
"@dhis2/d2-ui-org-unit-dialog" "^6.3.2"
Expand All @@ -1262,10 +1262,10 @@
react-beautiful-dnd "^10.1.1"
styled-jsx "^3.2.1"

"@dhis2/analytics@^2.1.0":
version "2.8.1"
resolved "https://registry.yarnpkg.com/@dhis2/analytics/-/analytics-2.8.1.tgz#228af6524f35c864689e28678752f1c8f42072e8"
integrity sha512-uIiQA8kIxUdRZEC+y0cwUDQUxBqgq1yVMRdEuevkm1zTuFJxKaNrJ6nZT9WyrVtnxpzYQneK5LsXsjMMo4vl8g==
"@dhis2/analytics@^2.8.5":
version "2.8.5"
resolved "https://registry.yarnpkg.com/@dhis2/analytics/-/analytics-2.8.5.tgz#8c0ae9a5c3b63d49901f895630a080fd5e5358bf"
integrity sha512-+M8WSHvt7Jscuc5cS/HPsLQRRTKayf3GSQDuVIGToaYt7paskJBSK9gA8ldu6pcRkdYho7JPg7C852JEbM8Y4g==
dependencies:
"@dhis2/d2-i18n" "^1.0.4"
"@dhis2/d2-ui-org-unit-dialog" "^6.3.2"
Expand Down

0 comments on commit 886de23

Please sign in to comment.