fix: fixes for options with numeric values and toggleable Select

packages/app/src/components/VisualizationOptions/Options/AggregationType.js

@@ -13,23 +13,23 @@ const AggregationType = () => (
         option={{
             name: 'aggregationType',
             items: [
-                { id: 'DEFAULT', label: i18n.t('By data element') },
-                { id: 'COUNT', label: i18n.t('Count') },
-                { id: 'AVERAGE', label: i18n.t('Average') },
+                { value: 'DEFAULT', label: i18n.t('By data element') },
+                { value: 'COUNT', label: i18n.t('Count') },
+                { value: 'AVERAGE', label: i18n.t('Average') },
                 {
-                    id: 'AVERAGE_SUM_ORG_UNIT',
+                    value: 'AVERAGE_SUM_ORG_UNIT',
                     label: i18n.t('Average (sum in org unit hierarchy)'),
                 },
-                { id: 'SUM', label: i18n.t('Sum') },
-                { id: 'LAST', label: i18n.t('Last value') },
+                { value: 'SUM', label: i18n.t('Sum') },
+                { value: 'LAST', label: i18n.t('Last value') },
                 {
-                    id: 'LAST_AVERAGE_ORG_UNIT',
+                    value: 'LAST_AVERAGE_ORG_UNIT',
                     label: i18n.t('Last value (average in org unit hierarchy)'),
                 },
-                { id: 'MIN', label: i18n.t('Min') },
-                { id: 'MAX', label: i18n.t('Max') },
-                { id: 'STDDEV', label: i18n.t('Standard deviation') },
-                { id: 'VARIANCE', label: i18n.t('Variance') },
+                { value: 'MIN', label: i18n.t('Min') },
+                { value: 'MAX', label: i18n.t('Max') },
+                { value: 'STDDEV', label: i18n.t('Standard deviation') },
+                { value: 'VARIANCE', label: i18n.t('Variance') },
             ],
         }}
     />

packages/app/src/components/VisualizationOptions/Options/DigitGroupSeparator.js

@@ -10,9 +10,9 @@ const DigitGroupSeparator = () => (
         option={{
             name: 'digitGroupSeparator',
             items: [
-                { id: 'NONE', label: i18n.t('None') },
-                { id: 'SPACE', label: i18n.t('Space') },
-                { id: 'COMMA', label: i18n.t('Comma') },
+                { value: 'NONE', label: i18n.t('None') },
+                { value: 'SPACE', label: i18n.t('Space') },
+                { value: 'COMMA', label: i18n.t('Comma') },
             ],
         }}
     />

packages/app/src/components/VisualizationOptions/Options/DisplayDensity.js

@@ -10,9 +10,9 @@ const DisplayDensity = () => (
         option={{
             name: 'displayDensity',
             items: [
-                { id: 'COMFORTABLE', label: i18n.t('Comfortable') },
-                { id: 'NORMAL', label: i18n.t('Normal') },
-                { id: 'COMPACT', label: i18n.t('Compact') },
+                { value: 'COMFORTABLE', label: i18n.t('Comfortable') },
+                { value: 'NORMAL', label: i18n.t('Normal') },
+                { value: 'COMPACT', label: i18n.t('Compact') },
             ],
         }}
     />

packages/app/src/components/VisualizationOptions/Options/FontSize.js

@@ -10,9 +10,9 @@ const FontSize = () => (
         option={{
             name: 'fontSize',
             items: [
-                { id: 'LARGE', label: i18n.t('Large') },
-                { id: 'NORMAL', label: i18n.t('Normal') },
-                { id: 'SMALL', label: i18n.t('Small') },
+                { value: 'LARGE', label: i18n.t('Large') },
+                { value: 'NORMAL', label: i18n.t('Normal') },
+                { value: 'SMALL', label: i18n.t('Small') },
             ],
         }}
     />

packages/app/src/components/VisualizationOptions/Options/HideEmptyRowItems.js

@@ -16,14 +16,13 @@ const HideEmptyRowItems = () => (
             name: optionName,
             defaultValue: defaultValue,
             items: [
-                { id: 'NONE', label: i18n.t('None') },
-                { id: 'BEFORE_FIRST', label: i18n.t('Before first') },
-                { id: 'AFTER_LAST', label: i18n.t('After last') },
+                { value: 'BEFORE_FIRST', label: i18n.t('Before first') },
+                { value: 'AFTER_LAST', label: i18n.t('After last') },
                 {
-                    id: 'BEFORE_FIRST_AFTER_LAST',
+                    value: 'BEFORE_FIRST_AFTER_LAST',
                     label: i18n.t('Before first and after last'),
                 },
-                { id: 'ALL', label: i18n.t('All') },
+                { value: 'ALL', label: i18n.t('All') },
             ],
         }}
     />

packages/app/src/components/VisualizationOptions/Options/LegendSet.js

@@ -25,7 +25,8 @@ import {
 } from '../styles/VisualizationOptions.style.js'
 
 const LegendSelect = ({ value, loading, options, onFocus, onChange }) => {
-    const selected = value ? { value: value.id, label: value.displayName } : {}
+    const selected =
+        value && value.id ? { value: value.id, label: value.displayName } : {}
 
     return (
         <SingleSelectField
@@ -47,8 +48,8 @@ const LegendSelect = ({ value, loading, options, onFocus, onChange }) => {
                 })
             }
         >
-            {options.map(({ id, label }) => (
-                <SingleSelectOption key={id} value={String(id)} label={label} />
+            {options.map(({ value, label }) => (
+                <SingleSelectOption key={value} value={value} label={label} />
             ))}
         </SingleSelectField>
     )
@@ -68,9 +69,12 @@ const LegendSetup = ({ value, onChange }) => {
     const [options, setOptions] = useState([])
     const [isLoaded, setIsLoaded] = useState(false)
 
-    if (value) {
-        if (!options.find(option => option.id === value.id)) {
-            setOptions([...options, { id: value.id, label: value.displayName }])
+    if (value && value.id) {
+        if (!options.find(option => option.value === value.id)) {
+            setOptions([
+                ...options,
+                { value: value.id, label: value.displayName },
+            ])
         }
     }
 
@@ -91,7 +95,7 @@ const LegendSetup = ({ value, onChange }) => {
 
             if (legendSets) {
                 const options = legendSets.legendSets.map(legendSet => ({
-                    id: legendSet.id,
+                    value: legendSet.id,
                     label: legendSet.name,
                 }))
 
@@ -119,7 +123,7 @@ LegendSetup.propTypes = {
 }
 
 const LegendSet = ({ value, legendDisplayStrategy, onChange }) => {
-    const [legendSetEnabled, setLegendSetEnabled] = useState(Boolean(value))
+    const [legendSetEnabled, setLegendSetEnabled] = useState(Boolean(value.id))
 
     const onCheckboxChange = ({ checked }) => {
         setLegendSetEnabled(checked)
@@ -189,7 +193,7 @@ LegendSet.propTypes = {
 }
 
 const mapStateToProps = state => ({
-    value: sGetUiOptions(state).legendSet,
+    value: sGetUiOptions(state).legendSet || {},
     legendDisplayStrategy: sGetUiOptions(state).legendDisplayStrategy,
 })
 

packages/app/src/components/VisualizationOptions/Options/NumberType.js

@@ -11,13 +11,13 @@ const NumberType = () => (
         option={{
             name: 'numberType',
             items: [
-                { id: 'VALUE', label: i18n.t('Value') },
+                { value: 'VALUE', label: i18n.t('Value') },
                 {
-                    id: 'ROW_PERCENTAGE',
+                    value: 'ROW_PERCENTAGE',
                     label: i18n.t('Percentage of row'),
                 },
                 {
-                    id: 'COLUMN_PERCENTAGE',
+                    value: 'COLUMN_PERCENTAGE',
                     label: i18n.t('Percentage of column'),
                 },
             ],

packages/app/src/components/VisualizationOptions/Options/RegressionType.js

@@ -16,10 +16,10 @@ const RegressionType = () => (
             name: optionName,
             defaultValue: defaultValue,
             items: [
-                { id: 'NONE', label: i18n.t('None') },
-                { id: 'LINEAR', label: i18n.t('Linear') },
-                { id: 'POLYNOMIAL', label: i18n.t('Polynomial') },
-                { id: 'LOESS', label: i18n.t('Loess') },
+                { value: 'NONE', label: i18n.t('None') },
+                { value: 'LINEAR', label: i18n.t('Linear') },
+                { value: 'POLYNOMIAL', label: i18n.t('Polynomial') },
+                { value: 'LOESS', label: i18n.t('Loess') },
             ],
         }}
     />

packages/app/src/components/VisualizationOptions/Options/SelectBaseOption.js

@@ -1,4 +1,4 @@
-import React from 'react'
+import React, { useState } from 'react'
 import PropTypes from 'prop-types'
 import { connect } from 'react-redux'
 
@@ -16,13 +16,21 @@ export const SelectBaseOption = ({
     option,
     label,
     helpText,
-    enabled,
     toggleable,
     value,
     onChange,
-    onToggle,
 }) => {
-    const selected = option.items.find(item => item.id === value)
+    const defaultValue = option.defaultValue
+
+    const [enabled, setEnabled] = useState(value !== defaultValue)
+
+    const selected = option.items.find(item => item.value === value) || {}
+
+    const onToggle = checked => {
+        setEnabled(checked)
+
+        onChange(checked ? option.items[0].value : defaultValue)
+    }
 
     return (
         <div
@@ -46,21 +54,18 @@ export const SelectBaseOption = ({
                     }
                 >
                     <SingleSelectField
-                        name={option.name}
+                        name={`${option.name}-select`}
                         label={toggleable ? '' : label}
                         onChange={({ selected }) => onChange(selected.value)}
-                        selected={{
-                            value: selected.id,
-                            label: selected.label,
-                        }}
+                        selected={selected}
                         helpText={helpText}
                         inputWidth="280px"
                         dense
                     >
-                        {option.items.map(({ id, label }) => (
+                        {option.items.map(({ value, label }) => (
                             <SingleSelectOption
-                                key={id}
-                                value={id}
+                                key={value}
+                                value={value}
                                 label={label}
                             />
                         ))}
@@ -75,29 +80,18 @@ SelectBaseOption.propTypes = {
     option: PropTypes.object.isRequired,
     value: PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired,
     onChange: PropTypes.func.isRequired,
-    enabled: PropTypes.bool,
     helpText: PropTypes.string,
     label: PropTypes.string,
     toggleable: PropTypes.bool,
-    onToggle: PropTypes.func,
 }
 
 const mapStateToProps = (state, ownProps) => ({
     value: sGetUiOptions(state)[ownProps.option.name],
-    enabled: sGetUiOptions(state)[ownProps.option.name] !== undefined,
 })
 
 const mapDispatchToProps = (dispatch, ownProps) => ({
     onChange: value =>
         dispatch(acSetUiOptions({ [ownProps.option.name]: value })),
-    onToggle: checked =>
-        dispatch(
-            acSetUiOptions({
-                [ownProps.option.name]: checked
-                    ? ownProps.option.defaultValue
-                    : undefined,
-            })
-        ),
 })
 
 export default connect(mapStateToProps, mapDispatchToProps)(SelectBaseOption)

packages/app/src/components/VisualizationOptions/Options/SortOrder.js

@@ -1,89 +1,25 @@
-import React, { useState } from 'react'
-import PropTypes from 'prop-types'
-import { connect } from 'react-redux'
+import React from 'react'
 
 import i18n from '@dhis2/d2-i18n'
 
-import { Checkbox, SingleSelectField, SingleSelectOption } from '@dhis2/ui-core'
-
-import { sGetUiOptions } from '../../../reducers/ui'
-import { acSetUiOptions } from '../../../actions/ui'
-
-import {
-    tabSectionOptionItem,
-    tabSectionOptionToggleable,
-} from '../styles/VisualizationOptions.style.js'
-
+import SelectBaseOption from './SelectBaseOption'
 import { options } from '../../../modules/options'
 
 const optionName = 'sortOrder'
 const defaultValue = options[optionName].defaultValue
 
-const SortOrder = ({ value, onChange }) => {
-    const [enabled, setEnabled] = useState(Boolean(value))
-
-    const options = [
-        { value: -1, label: i18n.t('Low to high') },
-        { value: 1, label: i18n.t('High to low') },
-    ]
-
-    const selected = options.find(option => option.value === value) || {}
-
-    const onToggle = checked => {
-        setEnabled(checked)
-
-        onChange({
-            sortOrder: checked ? -1 : defaultValue,
-        })
-    }
-
-    return (
-        <div className={enabled ? '' : tabSectionOptionItem.className}>
-            <Checkbox
-                checked={enabled}
-                label={i18n.t('Custom sort order')}
-                name="sortOrder-toggle"
-                onChange={({ checked }) => onToggle(checked)}
-                dense
-            />
-            {enabled ? (
-                <div className={tabSectionOptionToggleable.className}>
-                    <SingleSelectField
-                        name="sortOrder-select"
-                        selected={selected}
-                        onChange={({ selected }) =>
-                            onChange({
-                                sortOrder: selected.value,
-                            })
-                        }
-                        inputWidth="280px"
-                        dense
-                    >
-                        {options.map(({ value, label }) => (
-                            <SingleSelectOption
-                                key={value}
-                                value={value}
-                                label={label}
-                            />
-                        ))}
-                    </SingleSelectField>
-                </div>
-            ) : null}
-        </div>
-    )
-}
-
-SortOrder.propTypes = {
-    value: PropTypes.number.isRequired,
-    onChange: PropTypes.func.isRequired,
-}
-
-const mapStateToProps = state => ({
-    value: sGetUiOptions(state)[optionName] || defaultValue,
-})
-
-const mapDispatchToProps = dispatch => ({
-    onChange: value => dispatch(acSetUiOptions(value)),
-})
-
-export default connect(mapStateToProps, mapDispatchToProps)(SortOrder)
+const SortOrder = () => (
+    <SelectBaseOption
+        label={i18n.t('Custom sort order')}
+        toggleable={true}
+        option={{
+            name: optionName,
+            defaultValue: defaultValue,
+            items: [
+                { value: '-1', label: i18n.t('Low to high') },
+                { value: '1', label: i18n.t('High to low') },
+            ],
+        }}
+    />
+)
+export default SortOrder