From d0f0a3c45cd80e4fdd636e66a0695e2379651e4e Mon Sep 17 00:00:00 2001 From: Arina Date: Wed, 19 Aug 2026 13:35:59 +0200 Subject: [PATCH 01/13] feat(CalculationModal): enhance keyboard navigation --- i18n/en.pot | 55 ++- src/__demo__/CalculationModal.stories.js | 38 +- .../Calculation/CalculationModal.js | 411 ++++++++++++++---- .../Calculation/DataElementOption.js | 8 +- .../Calculation/DataElementSelector.js | 6 +- .../DataDimension/Calculation/DndContext.js | 22 +- .../DataDimension/Calculation/FormulaField.js | 77 ++-- .../DataDimension/Calculation/FormulaItem.js | 59 ++- .../Calculation/MathOperatorSelector.js | 12 +- .../DataDimension/Calculation/Operator.js | 15 +- .../styles/CalculationModal.style.js | 128 ++++-- .../Calculation/styles/FormulaField.style.js | 21 +- .../styles/MathOperatorSelector.style.js | 12 +- src/modules/__tests__/expressions.spec.js | 2 +- src/modules/expressions.js | 2 +- 15 files changed, 586 insertions(+), 282 deletions(-) diff --git a/i18n/en.pot b/i18n/en.pot index 69942d84a..9b6c51861 100644 --- a/i18n/en.pot +++ b/i18n/en.pot @@ -74,23 +74,53 @@ msgstr "This app could not retrieve required data." msgid "Network error" msgstr "Network error" +msgid "Add or select the focused item" +msgstr "Add or select the focused item" + +msgid "Move the selected item" +msgstr "Move the selected item" + +msgid "Insert an operator after the selected item" +msgstr "Insert an operator after the selected item" + +msgid "Keyboard navigation" +msgstr "Keyboard navigation" + +msgid "The formula is valid" +msgstr "The formula is valid" + +msgid "Could not validate the formula" +msgstr "Could not validate the formula" + msgid "Data / Edit calculation" msgstr "Data / Edit calculation" msgid "Data / New calculation" msgstr "Data / New calculation" +msgid "Calculation name" +msgstr "Calculation name" + +msgid "Shown in table headers and chart axes/legends" +msgstr "Shown in table headers and chart axes/legends" + +msgid "Formula" +msgstr "Formula" + msgid "Remove item" msgstr "Remove item" msgid "Check formula" msgstr "Check formula" -msgid "Calculation name" -msgstr "Calculation name" - -msgid "Shown in table headers and chart axes/legends" -msgstr "Shown in table headers and chart axes/legends" +msgid "" +"Drag or click a data element or operator to add it to the formula. Drag to " +"reorder. Select an item and click Remove item, or double-click, to delete " +"it." +msgstr "" +"Drag or click a data element or operator to add it to the formula. Drag to " +"reorder. Select an item and click Remove item, or double-click, to delete " +"it." msgid "Delete calculation" msgstr "Delete calculation" @@ -139,14 +169,11 @@ msgid "No data elements found" msgstr "No data elements found" msgid "" -"Drag items here, or double click in the list, to start building a " -"calculation formula" +"Drag a data element or operator here, or click one, to start building a " +"formula" msgstr "" -"Drag items here, or double click in the list, to start building a " -"calculation formula" - -msgid "Math operators" -msgstr "Math operators" +"Drag a data element or operator here, or click one, to start building a " +"formula" msgid "Expression description" msgstr "Expression description" @@ -1197,8 +1224,8 @@ msgstr "Option" msgid "Number" msgstr "Number" -msgid "Formula is empty. Add items to the formula from the lists on the left." -msgstr "Formula is empty. Add items to the formula from the lists on the left." +msgid "Formula is empty. Add a data element or operator to the formula." +msgstr "Formula is empty. Add a data element or operator to the formula." msgid "Consecutive math operators" msgstr "Consecutive math operators" diff --git a/src/__demo__/CalculationModal.stories.js b/src/__demo__/CalculationModal.stories.js index dfab5179e..f09b08d41 100644 --- a/src/__demo__/CalculationModal.stories.js +++ b/src/__demo__/CalculationModal.stories.js @@ -208,6 +208,19 @@ const DATA_ELEMENT_GROUPS = { ], } +const VALIDATION_OK = { + status: 'OK', + message: 'Valid', + description: 'ANC 1st visit / 10 * ANC 4th or more visits', +} + +const providerData = { + dataElements: DATA_ELEMENTS, + dataElementGroups: DATA_ELEMENT_GROUPS, + dataElementOperands: DATA_ELEMENT_OPERANDS, + 'indicators/expression/description': VALIDATION_OK, +} + const calculation = { id: 'calculationid', name: 'My calculation', @@ -226,13 +239,7 @@ export default { export const Default = () => { return ( - + { export const WithCalculation = () => { return ( - + { return ( - + { return ( ( + + {children} + + +) + +Key.propTypes = { + children: PropTypes.node.isRequired, +} + +const ShortcutsPopoverContent = () => ( +
+
    +
  • + + Enter + Space + + {i18n.t('Add or select the focused item')} +
    • +
  • + + ← + → + + {i18n.t('Move the selected item')} +
    • +
  • + + + + - + * + / + ( + ) + + {i18n.t('Insert an operator after the selected item')} +
    • +
+ +
+) + +const KeyboardNavigationHint = () => { + const [isOpen, setIsOpen] = useState(false) + const triggerRef = useRef() + + return ( + + + {isOpen && ( + + + + + + )} + + + ) +} const CalculationModal = ({ calculation = CALCULATION_PROP_DEFAULT, @@ -61,7 +144,12 @@ const CalculationModal = ({ const [doBackendValidation, { loading: isValidating }] = useDataMutation( validateIndicatorExpressionMutation, { - onError: (error) => showError(error), + onError: (error) => + showError( + error?.message || + error || + i18n.t('Could not validate the formula') + ), } ) @@ -124,7 +212,11 @@ const CalculationModal = ({ } }, [data, calculation.expression]) - const [newIdCount, setNewIdCount] = useState(1) + const nextItemIdRef = useRef(1) + // State is read through this ref instead of a closure, so the + // document-level keydown listener can be registered once on mount + // and still see fresh state on every keystroke. + const latestRef = useRef() const [validationOutput, setValidationOutput] = useState(null) const [expressionArray, setExpressionArray] = useState() @@ -136,11 +228,19 @@ const CalculationModal = ({ const [selectedItemId, setSelectedItemId] = useState(null) const expressionStatus = validationOutput?.status + const validationMessage = + expressionStatus === VALID_EXPRESSION + ? i18n.t('The formula is valid') + : validationOutput?.message const selectItem = (itemId) => - setSelectedItemId((prevSelected) => - prevSelected !== itemId ? itemId : null - ) + setSelectedItemId((prevSelected) => { + const next = prevSelected !== itemId ? itemId : null + if (latestRef.current) { + latestRef.current.selectedItemId = next + } + return next + }) const isLoading = isCreatingCalculation || @@ -149,49 +249,62 @@ const CalculationModal = ({ isSavingCalculation || isValidating - const addItem = ({ label, value, type, destIndex = LAST_POSITION }) => { - if (isLoading) { - return null + const addItem = ({ label, value, type, destIndex }) => { + if (isLoading || !expressionArray) { + return } - setValidationOutput() + setValidationOutput(null) const newItem = { - id: `${type}-${newIdCount}`, + id: `${type}-${nextItemIdRef.current++}`, value: type === EXPRESSION_TYPE_DATA ? `#{${value}}` : value, label, type, } - setNewIdCount(newIdCount + 1) + // Without an explicit destIndex, insert after the selected item + // instead of always appending. + const selectedId = latestRef.current?.selectedItemId + setExpressionArray((prevArray) => { + let insertAt = destIndex + if (insertAt === undefined) { + const selectedIndex = prevArray.findIndex( + (item) => item.id === selectedId + ) + insertAt = + selectedIndex === -1 ? prevArray.length : selectedIndex + 1 + } else if (insertAt === LAST_POSITION) { + insertAt = prevArray.length + } - if (destIndex === LAST_POSITION) { - setExpressionArray((prevArray) => prevArray.concat([newItem])) - } else if (destIndex === FIRST_POSITION) { - setExpressionArray((prevArray) => [newItem].concat(prevArray)) - } else { - const items = Array.from(expressionArray) - const newFormulaItems = [ - ...items.slice(0, destIndex), + return [ + ...prevArray.slice(0, insertAt), newItem, - ...items.slice(destIndex), + ...prevArray.slice(insertAt), ] - setExpressionArray(newFormulaItems) - } + }) if (newItem.type === EXPRESSION_TYPE_NUMBER) { setFocusItemId(newItem.id) } + + // Keep the newly added item selected so it becomes the anchor for + // the next typed operator or arrow-key move. + setSelectedItemId(newItem.id) + latestRef.current.selectedItemId = newItem.id } const moveItem = ({ sourceIndex, destIndex }) => { if (isLoading) { - return null + return } - setValidationOutput() - const sourceList = Array.from(expressionArray) - const [moved] = sourceList.splice(sourceIndex, 1) - sourceList.splice(destIndex, 0, moved) - setExpressionArray(sourceList) + setValidationOutput(null) + setExpressionArray((prevArray) => { + const sourceList = Array.from(prevArray) + const [moved] = sourceList.splice(sourceIndex, 1) + sourceList.splice(destIndex, 0, moved) + return sourceList + }) } const setItemValue = ({ itemId, value }) => { @@ -203,7 +316,7 @@ const CalculationModal = ({ const removeItem = (itemId) => { if (!isLoading && itemId !== null) { - setValidationOutput() + setValidationOutput(null) const index = expressionArray.findIndex( (item) => item.id === itemId ) @@ -214,6 +327,81 @@ const CalculationModal = ({ } } + latestRef.current = { + isLoading, + showDeletePrompt, + selectedItemId, + expressionArray, + addItem, + moveItem, + } + + useEffect(() => { + const handleKeyDown = (event) => { + const { + isLoading, + showDeletePrompt, + selectedItemId, + expressionArray, + addItem, + moveItem, + } = latestRef.current + + // On some layouts (e.g. German, French) operator characters + // like ( ) * are typed via AltGr, which browsers report as + // altKey/ctrlKey being set - don't let that block the shortcut. + const isAltGraph = event.getModifierState?.('AltGraph') + + if ( + isLoading || + showDeletePrompt || + event.metaKey || + (!isAltGraph && (event.ctrlKey || event.altKey)) || + isInteractiveElement(event.target) + ) { + return + } + + const operator = OPERATORS.find( + (op) => + op.type === EXPRESSION_TYPE_OPERATOR && + op.value === event.key + ) + + if (operator) { + event.preventDefault() + addItem(operator) + return + } + + if (!selectedItemId || !expressionArray) { + return + } + + const index = expressionArray.findIndex( + (item) => item.id === selectedItemId + ) + if (index === -1) { + return + } + + if (event.key === 'ArrowLeft' && index > 0) { + event.preventDefault() + moveItem({ sourceIndex: index, destIndex: index - 1 }) + } else if ( + event.key === 'ArrowRight' && + index < expressionArray.length - 1 + ) { + event.preventDefault() + moveItem({ sourceIndex: index, destIndex: index + 1 }) + } + } + + document.addEventListener('keydown', handleKeyDown) + + return () => document.removeEventListener('keydown', handleKeyDown) + }, []) + const addOrMoveDraggedItem = ({ item, destination }) => { const destContainerId = destination.containerId @@ -238,14 +426,31 @@ const CalculationModal = ({ } const validate = async () => { - setValidationOutput() + setValidationOutput(null) const expression = parseArrayToExpression(expressionArray) let result = validateExpression(expression) + if (!result) { - result = await doBackendValidation({ + const backendResult = await doBackendValidation({ expression, }) + + // useDataMutation never rejects; network/engine failures go to + // onError and this promise does not resolve. + if (!backendResult) { + return + } + + if (backendResult.status === INVALID_EXPRESSION) { + result = backendResult + } else { + result = { + ...backendResult, + status: VALID_EXPRESSION, + } + } } + setValidationOutput(result) return result?.status @@ -303,6 +508,20 @@ const CalculationModal = ({ : i18n.t('Data / New calculation')} +
+ + setName(value.substr(0, 50)) + } + value={name} + dataTest="calculation-label" + dense + /> +
setFocusItemId(null)} onDragEnd={addOrMoveDraggedItem} @@ -311,77 +530,77 @@ const CalculationModal = ({
-
- -
-
-
- -
-
- -
+
+

+ {i18n.t('Formula')} +

+ + +
+ +
- + {validationMessage && ( +
- {validationOutput?.message} - -
- - setName(value.substr(0, 50)) + + > + {validationMessage} +
+ )} +
+ + {i18n.t( + 'Drag or click a data element or operator to add it to the formula. Drag to reorder. Select an item and click Remove item, or double-click, to delete it.' + )} + +

+ +

diff --git a/src/components/DataDimension/Calculation/DataElementOption.js b/src/components/DataDimension/Calculation/DataElementOption.js index 7743ff3b3..332cfadb4 100644 --- a/src/components/DataDimension/Calculation/DataElementOption.js +++ b/src/components/DataDimension/Calculation/DataElementOption.js @@ -5,9 +5,10 @@ import React from 'react' import { DIMENSION_TYPE_DATA_ELEMENT } from '../../../modules/dataTypes.js' import { getIcon } from '../../../modules/dimensionListItem.js' import { EXPRESSION_TYPE_DATA } from '../../../modules/expressions.js' +import { onActivationKeydown } from './DndContext.js' import styles from './styles/DataElementOption.style.js' -const DataElementOption = ({ label, value, onDoubleClick }) => { +const DataElementOption = ({ label, value, onClick }) => { const data = { label, value, type: EXPRESSION_TYPE_DATA } const { attributes, listeners, setNodeRef, transform } = useSortable({ id: value, @@ -25,10 +26,11 @@ const DataElementOption = ({ label, value, onDoubleClick }) => { {...listeners} ref={setNodeRef} style={style} + onKeyDown={onActivationKeydown(() => onClick(data))} >
onDoubleClick(data)} + onClick={() => onClick(data)} data-test="data-element-option" > @@ -45,7 +47,7 @@ const DataElementOption = ({ label, value, onDoubleClick }) => { DataElementOption.propTypes = { label: PropTypes.string, value: PropTypes.string, - onDoubleClick: PropTypes.func, + onClick: PropTypes.func, } export default DataElementOption diff --git a/src/components/DataDimension/Calculation/DataElementSelector.js b/src/components/DataDimension/Calculation/DataElementSelector.js index 36a5b4539..0964d668d 100644 --- a/src/components/DataDimension/Calculation/DataElementSelector.js +++ b/src/components/DataDimension/Calculation/DataElementSelector.js @@ -127,7 +127,7 @@ DisaggregationSelector.propTypes = { const DataElementSelector = ({ displayNameProp, - onDoubleClick, + onClick, height = SCROLLBOX_HEIGHT, }) => { const dataEngine = useDataEngine() @@ -296,7 +296,7 @@ const DataElementSelector = ({ key={value} label={label} value={value} - onDoubleClick={onDoubleClick} + onClick={onClick} /> ))} {!loading && !options.length && ( @@ -325,7 +325,7 @@ const DataElementSelector = ({ DataElementSelector.propTypes = { displayNameProp: PropTypes.string.isRequired, - onDoubleClick: PropTypes.func.isRequired, + onClick: PropTypes.func.isRequired, height: PropTypes.string, } diff --git a/src/components/DataDimension/Calculation/DndContext.js b/src/components/DataDimension/Calculation/DndContext.js index 846835894..b802460d2 100644 --- a/src/components/DataDimension/Calculation/DndContext.js +++ b/src/components/DataDimension/Calculation/DndContext.js @@ -79,20 +79,18 @@ const rectIntersectionCustom = ({ return collisions.sort(sortCollisionsDesc) } -const isInteractiveElement = (el) => { - const interactiveElements = [ - 'button', - 'input', - 'textarea', - 'select', - 'option', - ] +const INTERACTIVE_SELECTOR = 'button, input, textarea, select, option' - if (interactiveElements.includes(el.tagName.toLowerCase())) { - return true - } +export const isInteractiveElement = (el) => + Boolean(el?.closest?.(INTERACTIVE_SELECTOR)) - return false +// Mirrors a click on Enter/Space, so draggable chips (which aren't +// natively clickable elements) can be activated from the keyboard. +export const onActivationKeydown = (callback) => (e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault() + callback() + } } // disable dragging if user is in an input diff --git a/src/components/DataDimension/Calculation/FormulaField.js b/src/components/DataDimension/Calculation/FormulaField.js index 3089da5f0..6f095e3fa 100644 --- a/src/components/DataDimension/Calculation/FormulaField.js +++ b/src/components/DataDimension/Calculation/FormulaField.js @@ -17,7 +17,7 @@ const Placeholder = () => ( {i18n.t( - 'Drag items here, or double click in the list, to start building a calculation formula' + 'Drag a data element or operator here, or click one, to start building a formula' )} @@ -44,45 +44,42 @@ const FormulaField = ({ const overLastDropZone = over?.id === LAST_DROPZONE_ID return ( -
-
-
- {loading && ( -
- -
- )} - {!loading && itemIds && ( - - - {!items.length && } - {Boolean(items.length) && - items.map(({ id, label, type, value }, index) => ( - - ))} - - )} -
+
+ {loading && ( +
+ +
+ )} + {!loading && ( + + + {!items.length && } + {Boolean(items.length) && + items.map(({ id, label, type, value }, index) => ( + + ))} + + )}
) diff --git a/src/components/DataDimension/Calculation/FormulaItem.js b/src/components/DataDimension/Calculation/FormulaItem.js index 45e1a1f88..7042c7f53 100644 --- a/src/components/DataDimension/Calculation/FormulaItem.js +++ b/src/components/DataDimension/Calculation/FormulaItem.js @@ -3,22 +3,21 @@ import { useSortable } from '@dnd-kit/sortable' import { CSS } from '@dnd-kit/utilities' import cx from 'classnames' import PropTypes from 'prop-types' -import React, { useState, useRef, useEffect } from 'react' +import React, { useRef, useEffect } from 'react' import { DIMENSION_TYPE_DATA_ELEMENT } from '../../../modules/dataTypes.js' import { getIcon } from '../../../modules/dimensionListItem.js' import { EXPRESSION_TYPE_NUMBER, EXPRESSION_TYPE_DATA, } from '../../../modules/expressions.js' +import { isInteractiveElement, onActivationKeydown } from './DndContext.js' import DragHandleIcon from './DragHandleIcon.js' import styles from './styles/FormulaItem.style.js' const BEFORE = 'BEFORE' const AFTER = 'AFTER' -const maxMsBetweenClicks = 300 - -const TAG_INPUT = 'INPUT' +const DOUBLE_CLICK_THRESHOLD_MS = 300 const FormulaItem = ({ id, @@ -49,8 +48,12 @@ const FormulaItem = ({ }) const inputRef = useRef(null) + const clickTimeoutRef = useRef(null) + const ignoreClickRef = useRef(false) - const [clickTimeoutId, setClickTimeoutId] = useState(null) + useEffect(() => { + return () => clearTimeout(clickTimeoutRef.current) + }, []) useEffect(() => { if (hasFocus && inputRef.current) { @@ -97,30 +100,45 @@ const FormulaItem = ({ } const handleClick = (e) => { - const tagname = e.target.tagName - clearTimeout(clickTimeoutId) - const to = setTimeout(function () { - if (tagname !== TAG_INPUT) { - onClick(id) - } else { - inputRef.current && inputRef.current.focus() - } - }, maxMsBetweenClicks) - setClickTimeoutId(to) + if (ignoreClickRef.current) { + ignoreClickRef.current = false + return + } + if (isInteractiveElement(e.target)) { + inputRef.current && inputRef.current.focus() + return + } + // Delay in case this click is the first of a double-click, so + // selecting the item doesn't flicker in between removing it. + clearTimeout(clickTimeoutRef.current) + clickTimeoutRef.current = setTimeout(() => { + onClick(id) + }, DOUBLE_CLICK_THRESHOLD_MS) } const handleDoubleClick = (e) => { - clearTimeout(clickTimeoutId) - setClickTimeoutId(null) - if (e.target.tagName !== TAG_INPUT) { - onDoubleClick(id) - } else { + if (isInteractiveElement(e.target)) { inputRef.current && inputRef.current.focus() + return } + clearTimeout(clickTimeoutRef.current) + onDoubleClick(id) } const handleChange = (e) => onChange({ itemId: id, value: e.target.value }) + const handleKeyDown = (e) => { + if (isInteractiveElement(e.target)) { + return + } + if (e.key === 'Enter' || e.key === ' ') { + // role="button" from dnd-kit also synthesizes a click on Enter/Space; + // ignore that click so selection is not toggled off 300ms later. + ignoreClickRef.current = true + } + onActivationKeydown(() => onClick(id))(e) + } + const getContent = () => { if (type === EXPRESSION_TYPE_NUMBER) { return ( @@ -196,6 +214,7 @@ const FormulaItem = ({ })} onClick={handleClick} onDoubleClick={handleDoubleClick} + onKeyDown={handleKeyDown} data-test={`formula-item-${id}`} > {getContent()} diff --git a/src/components/DataDimension/Calculation/MathOperatorSelector.js b/src/components/DataDimension/Calculation/MathOperatorSelector.js index d87689f6a..5319101b4 100644 --- a/src/components/DataDimension/Calculation/MathOperatorSelector.js +++ b/src/components/DataDimension/Calculation/MathOperatorSelector.js @@ -1,23 +1,23 @@ import PropTypes from 'prop-types' import React from 'react' -import i18n from '../../../locales/index.js' import { getOperators } from '../../../modules/expressions.js' import DraggableOperator from './Operator.js' import styles from './styles/MathOperatorSelector.style.js' -const MathOperatorSelector = ({ onDoubleClick }) => ( +const OPERATORS = getOperators() + +const MathOperatorSelector = ({ onClick }) => ( <>
-

{i18n.t('Math operators')}

- {getOperators().map(({ label, value, type }, index) => ( + {OPERATORS.map(({ label, value, type }, index) => ( ))}
@@ -27,7 +27,7 @@ const MathOperatorSelector = ({ onDoubleClick }) => ( ) MathOperatorSelector.propTypes = { - onDoubleClick: PropTypes.func.isRequired, + onClick: PropTypes.func.isRequired, } export default MathOperatorSelector diff --git a/src/components/DataDimension/Calculation/Operator.js b/src/components/DataDimension/Calculation/Operator.js index 2bd46479b..e27777c8c 100644 --- a/src/components/DataDimension/Calculation/Operator.js +++ b/src/components/DataDimension/Calculation/Operator.js @@ -7,10 +7,11 @@ import { EXPRESSION_TYPE_NUMBER, EXPRESSION_TYPE_OPERATOR, } from '../../../modules/expressions.js' +import { onActivationKeydown } from './DndContext.js' import formulaItemStyles from './styles/FormulaItem.style.js' import styles from './styles/Operator.style.js' -const Operator = ({ label, value, type, onDoubleClick }) => { +const Operator = ({ label, value, type, onClick }) => { const data = { label, value, type } const { attributes, listeners, setNodeRef, transform } = useSortable({ id: `operator-${label}`, @@ -21,14 +22,20 @@ const Operator = ({ label, value, type, onDoubleClick }) => { } return ( -
+
onClick(data))} + >
onDoubleClick(data)} + onClick={() => onClick(data)} > {label}
@@ -42,7 +49,7 @@ Operator.propTypes = { label: PropTypes.string.isRequired, type: PropTypes.string.isRequired, value: PropTypes.string.isRequired, - onDoubleClick: PropTypes.func.isRequired, + onClick: PropTypes.func.isRequired, } export default Operator diff --git a/src/components/DataDimension/Calculation/styles/CalculationModal.style.js b/src/components/DataDimension/Calculation/styles/CalculationModal.style.js index a15c19c14..3eecce012 100644 --- a/src/components/DataDimension/Calculation/styles/CalculationModal.style.js +++ b/src/components/DataDimension/Calculation/styles/CalculationModal.style.js @@ -1,68 +1,126 @@ -import { colors, spacers } from '@dhis2/ui' +import { colors, elevations, spacers } from '@dhis2/ui' import css from 'styled-jsx/css' export default css` - .header { - background: ${colors.grey200}; - padding: ${spacers.dp16}; - font-weight: normal; + .formula-section { + background: ${colors.white}; + border: 1px solid ${colors.grey400}; } - .header-icon { - padding: 0 ${spacers.dp8}; - vertical-align: text-bottom; - line-height: 14px; + .validation-notice { + margin-top: ${spacers.dp8}; } - .actions-wrapper { - margin-top: ${spacers.dp16}; - margin-bottom: ${spacers.dp16}; - margin-left: ${spacers.dp4}; + .formula-actions { + display: flex; + justify-content: space-between; + align-items: center; + padding: ${spacers.dp8}; } - .button-container { - display: inline-flex; + .delete-button { + margin-right: ${spacers.dp8}; } - .validate-button { - margin-bottom: ${spacers.dp4}; + .content { + display: flex; + gap: ${spacers.dp12}; } - .remove-button { - margin-right: ${spacers.dp8}; + .left-section { + width: 40%; } - .delete-button { - margin-right: ${spacers.dp8}; + .right-section { + width: 60%; + font-size: 14px; + display: flex; + flex-direction: column; } - .content { + .sub-header { + font-size: 14px; + font-weight: normal; + margin: ${spacers.dp4} ${spacers.dp8}; + } + + .name-field { + margin-bottom: ${spacers.dp16}; + } + + .usage-legend { display: flex; + flex-direction: column; + gap: ${spacers.dp4}; + padding-top: ${spacers.dp4}; } - .left-section { - width: 45%; + .see-also { + margin: 0; + font-size: 12px; + line-height: 14px; + color: ${colors.grey700}; } - .right-section { - width: 55%; - padding-left: ${spacers.dp8}; + .hint { + position: relative; + } + + .hint-trigger { + background: none; + border: none; + padding: 0; + font: inherit; + color: inherit; + text-decoration: underline dotted; + text-underline-offset: 2px; + cursor: help; + white-space: nowrap; + } + + .shortcuts { + background: ${colors.white}; + border-radius: 4px; + box-shadow: ${elevations.popover}; + padding: ${spacers.dp12} ${spacers.dp16}; + max-width: 340px; + color: ${colors.grey900}; font-size: 14px; } - .validation-message { - margin-left: ${spacers.dp8}; + .shortcuts ul { + margin: 0; + padding: 0; + list-style: none; + display: flex; + flex-direction: column; + gap: ${spacers.dp8}; } - .validation-error { - color: ${colors.red500}; + .shortcuts li { + margin: 0; + display: flex; + flex-direction: column; + gap: ${spacers.dp4}; } - .validation-success { - color: ${colors.green500}; + .shortcut-keys { + display: flex; + flex-wrap: wrap; + gap: ${spacers.dp4}; } - .name-input { - margin-top: ${spacers.dp12}; + .key { + display: inline-block; + min-width: 1.4em; + padding: 1px 5px; + border: 1px solid ${colors.grey400}; + border-radius: 3px; + background: ${colors.grey050}; + box-shadow: 0 1px 0 ${colors.grey400}; + font-family: monospace; + font-size: 12px; + line-height: 1.4; + text-align: center; } ` diff --git a/src/components/DataDimension/Calculation/styles/FormulaField.style.js b/src/components/DataDimension/Calculation/styles/FormulaField.style.js index 24b120ec5..0d65549e8 100644 --- a/src/components/DataDimension/Calculation/styles/FormulaField.style.js +++ b/src/components/DataDimension/Calculation/styles/FormulaField.style.js @@ -3,7 +3,8 @@ import css from 'styled-jsx/css' export default css` .formula-field { - border-right: 2px solid ${colors.grey200}; + border-top: 1px solid ${colors.grey400}; + border-bottom: 1px solid ${colors.grey400}; height: 180px; overflow: auto; padding: 6px 12px; @@ -16,23 +17,9 @@ export default css` width: 100%; } - .container { - position: relative; - } - - .border { - position: absolute; - top: 0; - left: 6px; - height: 180px; - width: calc(100% - 6px); - border-left: 2px solid ${colors.grey200}; - border-top: 2px solid ${colors.grey200}; - border-bottom: 2px solid ${colors.grey200}; - } - .placeholder { height: 100%; + width: 100%; display: flex; flex-direction: column; gap: ${spacers.dp8}; @@ -40,12 +27,14 @@ export default css` justify-content: center; margin-top: -28px; padding: 0 ${spacers.dp32}; + text-align: center; } .help-text { color: ${colors.grey600}; font-size: 14px; line-height: 19px; + text-align: center; user-select: none; } ` diff --git a/src/components/DataDimension/Calculation/styles/MathOperatorSelector.style.js b/src/components/DataDimension/Calculation/styles/MathOperatorSelector.style.js index 98c2214ee..712d958ea 100644 --- a/src/components/DataDimension/Calculation/styles/MathOperatorSelector.style.js +++ b/src/components/DataDimension/Calculation/styles/MathOperatorSelector.style.js @@ -3,21 +3,13 @@ import css from 'styled-jsx/css' export default css` .wrapper { - border: 1px solid ${colors.grey400}; - margin-top: ${spacers.dp8}; + border-bottom: 1px solid ${colors.grey400}; } .operators { display: flex; flex-wrap: wrap; gap: ${spacers.dp4}; - padding: ${spacers.dp4}; - border-top: 1px solid ${colors.grey400}; - } - - .sub-header { - font-size: 14px; - font-weight: normal; - margin: ${spacers.dp4} ${spacers.dp8}; + padding: ${spacers.dp8}; } ` diff --git a/src/modules/__tests__/expressions.spec.js b/src/modules/__tests__/expressions.spec.js index 4208a1eba..804e696fa 100644 --- a/src/modules/__tests__/expressions.spec.js +++ b/src/modules/__tests__/expressions.spec.js @@ -12,7 +12,7 @@ import { const invalidTestExpressions = [ { message: - 'Formula is empty. Add items to the formula from the lists on the left.', + 'Formula is empty. Add a data element or operator to the formula.', expressions: [''], }, // { diff --git a/src/modules/expressions.js b/src/modules/expressions.js index 99eba9dcf..9ea589c88 100644 --- a/src/modules/expressions.js +++ b/src/modules/expressions.js @@ -67,7 +67,7 @@ export const validateExpression = (expression) => { result = { status: INVALID_EXPRESSION, message: i18n.t( - 'Formula is empty. Add items to the formula from the lists on the left.' + 'Formula is empty. Add a data element or operator to the formula.' ), } // TODO: reimplement this but allow negative values, e.g. 10 / -5 From 4c1117f0be873704e6f202e5da5e646a922b5364 Mon Sep 17 00:00:00 2001 From: Arina Date: Mon, 24 Aug 2026 11:45:37 +0200 Subject: [PATCH 02/13] refactor(CalculationModal): improve usage hints and keyboard shortcuts layout --- .../Calculation/CalculationModal.js | 63 ++++++++++------- .../DataDimension/Calculation/FormulaField.js | 3 - .../DataDimension/Calculation/FormulaItem.js | 28 +------- .../styles/CalculationModal.style.js | 70 ++++++++++--------- 4 files changed, 77 insertions(+), 87 deletions(-) diff --git a/src/components/DataDimension/Calculation/CalculationModal.js b/src/components/DataDimension/Calculation/CalculationModal.js index e162d4ee1..d71095ee9 100644 --- a/src/components/DataDimension/Calculation/CalculationModal.js +++ b/src/components/DataDimension/Calculation/CalculationModal.js @@ -6,7 +6,7 @@ import { ModalContent, ModalActions, ButtonStrip, - Help, + IconQuestion16, InputField, NoticeBox, Popper, @@ -64,22 +64,42 @@ Key.propTypes = { const ShortcutsPopoverContent = () => (
+

{i18n.t('Usage tips')}

  • + {i18n.t( + 'Click or drag a data element or operator to add it to the formula.' + )} +
    • +
  • {i18n.t('Drag an item to reorder it.')}
    • +
  • + {i18n.t('Select an item, then click')}{' '} + {i18n.t('Remove item')}{' '} + {i18n.t('to delete it.')} +
    • +
+

{i18n.t('Keyboard shortcuts')}

+
    +
  • + {i18n.t('Press')}{' '} Enter + {i18n.t('or')} Space - - {i18n.t('Add or select the focused item')} + {' '} + {i18n.t('to add or select the focused item.')}
  • + {i18n.t('Press')}{' '} ← + {i18n.t('or')} → - - {i18n.t('Move the selected item')} + {' '} + {i18n.t('to move the selected item.')}
  • + {i18n.t('Press')}{' '} + - @@ -87,15 +107,15 @@ const ShortcutsPopoverContent = () => ( / ( ) - - {i18n.t('Insert an operator after the selected item')} + {' '} + {i18n.t('to insert an operator after the selected item.')}
) -const KeyboardNavigationHint = () => { +const UsageHint = () => { const [isOpen, setIsOpen] = useState(false) const triggerRef = useRef() @@ -105,17 +125,18 @@ const KeyboardNavigationHint = () => { type="button" className="hint-trigger" ref={triggerRef} - data-test="keyboard-navigation-hint" + data-test="usage-hint" + aria-label={i18n.t('Usage tips')} onMouseEnter={() => setIsOpen(true)} onMouseLeave={() => setIsOpen(false)} onFocus={() => setIsOpen(true)} onBlur={() => setIsOpen(false)} > - {i18n.t('Keyboard navigation')} + {isOpen && ( - + @@ -536,16 +557,18 @@ const CalculationModal = ({
-

- {i18n.t('Formula')} -

+
+

+ {i18n.t('Formula')} +

+ +
@@ -592,16 +615,6 @@ const CalculationModal = ({
)} -
- - {i18n.t( - 'Drag or click a data element or operator to add it to the formula. Drag to reorder. Select an item and click Remove item, or double-click, to delete it.' - )} - -

- -

-
diff --git a/src/components/DataDimension/Calculation/FormulaField.js b/src/components/DataDimension/Calculation/FormulaField.js index 6f095e3fa..12a7e0de9 100644 --- a/src/components/DataDimension/Calculation/FormulaField.js +++ b/src/components/DataDimension/Calculation/FormulaField.js @@ -32,7 +32,6 @@ const FormulaField = ({ focusItemId, onChange, onClick, - onDoubleClick, loading, }) => { const { over, setNodeRef: setLastDropzoneRef } = useDroppable({ @@ -74,7 +73,6 @@ const FormulaField = ({ isLast={index === items.length - 1} onChange={onChange} onClick={onClick} - onDoubleClick={onDoubleClick} overLastDropZone={overLastDropZone} /> ))} @@ -88,7 +86,6 @@ const FormulaField = ({ FormulaField.propTypes = { onChange: PropTypes.func.isRequired, onClick: PropTypes.func.isRequired, - onDoubleClick: PropTypes.func.isRequired, focusItemId: PropTypes.string, items: PropTypes.arrayOf( PropTypes.shape({ diff --git a/src/components/DataDimension/Calculation/FormulaItem.js b/src/components/DataDimension/Calculation/FormulaItem.js index 7042c7f53..6e7df109e 100644 --- a/src/components/DataDimension/Calculation/FormulaItem.js +++ b/src/components/DataDimension/Calculation/FormulaItem.js @@ -17,8 +17,6 @@ import styles from './styles/FormulaItem.style.js' const BEFORE = 'BEFORE' const AFTER = 'AFTER' -const DOUBLE_CLICK_THRESHOLD_MS = 300 - const FormulaItem = ({ id, label, @@ -29,7 +27,6 @@ const FormulaItem = ({ overLastDropZone, onChange, onClick, - onDoubleClick, hasFocus, }) => { const { @@ -48,13 +45,8 @@ const FormulaItem = ({ }) const inputRef = useRef(null) - const clickTimeoutRef = useRef(null) const ignoreClickRef = useRef(false) - useEffect(() => { - return () => clearTimeout(clickTimeoutRef.current) - }, []) - useEffect(() => { if (hasFocus && inputRef.current) { // setTimeout seems to be needed in order for the cursor @@ -108,21 +100,7 @@ const FormulaItem = ({ inputRef.current && inputRef.current.focus() return } - // Delay in case this click is the first of a double-click, so - // selecting the item doesn't flicker in between removing it. - clearTimeout(clickTimeoutRef.current) - clickTimeoutRef.current = setTimeout(() => { - onClick(id) - }, DOUBLE_CLICK_THRESHOLD_MS) - } - - const handleDoubleClick = (e) => { - if (isInteractiveElement(e.target)) { - inputRef.current && inputRef.current.focus() - return - } - clearTimeout(clickTimeoutRef.current) - onDoubleClick(id) + onClick(id) } const handleChange = (e) => onChange({ itemId: id, value: e.target.value }) @@ -133,7 +111,7 @@ const FormulaItem = ({ } if (e.key === 'Enter' || e.key === ' ') { // role="button" from dnd-kit also synthesizes a click on Enter/Space; - // ignore that click so selection is not toggled off 300ms later. + // ignore that click so selection is not toggled off right after. ignoreClickRef.current = true } onActivationKeydown(() => onClick(id))(e) @@ -213,7 +191,6 @@ const FormulaItem = ({ insertAfter: insertPosition === AFTER, })} onClick={handleClick} - onDoubleClick={handleDoubleClick} onKeyDown={handleKeyDown} data-test={`formula-item-${id}`} > @@ -231,7 +208,6 @@ FormulaItem.propTypes = { type: PropTypes.string.isRequired, onChange: PropTypes.func.isRequired, onClick: PropTypes.func.isRequired, - onDoubleClick: PropTypes.func.isRequired, hasFocus: PropTypes.bool, isHighlighted: PropTypes.bool, isLast: PropTypes.bool, diff --git a/src/components/DataDimension/Calculation/styles/CalculationModal.style.js b/src/components/DataDimension/Calculation/styles/CalculationModal.style.js index 3eecce012..6ca0ac579 100644 --- a/src/components/DataDimension/Calculation/styles/CalculationModal.style.js +++ b/src/components/DataDimension/Calculation/styles/CalculationModal.style.js @@ -38,44 +38,35 @@ export default css` flex-direction: column; } + .sub-header-row { + display: flex; + align-items: center; + margin: ${spacers.dp4} ${spacers.dp8}; + } + .sub-header { font-size: 14px; font-weight: normal; - margin: ${spacers.dp4} ${spacers.dp8}; + margin: 0; } .name-field { margin-bottom: ${spacers.dp16}; } - .usage-legend { - display: flex; - flex-direction: column; - gap: ${spacers.dp4}; - padding-top: ${spacers.dp4}; - } - - .see-also { - margin: 0; - font-size: 12px; - line-height: 14px; - color: ${colors.grey700}; - } - .hint { position: relative; + display: inline-flex; } .hint-trigger { + display: inline-flex; + align-items: center; + padding: ${spacers.dp4} ${spacers.dp4} ${spacers.dp4} 4px; background: none; border: none; - padding: 0; - font: inherit; - color: inherit; - text-decoration: underline dotted; - text-underline-offset: 2px; - cursor: help; - white-space: nowrap; + color: ${colors.grey600}; + cursor: default; } .shortcuts { @@ -83,31 +74,44 @@ export default css` border-radius: 4px; box-shadow: ${elevations.popover}; padding: ${spacers.dp12} ${spacers.dp16}; - max-width: 340px; + max-width: 364px; color: ${colors.grey900}; font-size: 14px; } + .shortcuts-header { + margin: ${spacers.dp12} 0 ${spacers.dp8}; + text-transform: uppercase; + font-size: 11px; + font-weight: 400; + letter-spacing: 0.3px; + color: ${colors.grey600}; + } + + .shortcuts-header:first-child { + margin-top: 0; + } + .shortcuts ul { margin: 0; - padding: 0; - list-style: none; - display: flex; - flex-direction: column; - gap: ${spacers.dp8}; + padding-left: ${spacers.dp16}; } .shortcuts li { - margin: 0; - display: flex; - flex-direction: column; - gap: ${spacers.dp4}; + margin: 0 0 ${spacers.dp8}; + line-height: 1.5; + } + + .shortcuts li:last-child { + margin-bottom: 0; } .shortcut-keys { - display: flex; + display: inline-flex; + align-items: center; flex-wrap: wrap; gap: ${spacers.dp4}; + vertical-align: middle; } .key { From 8e3bdd6e24a6659ac1afcb717d33d9b387e8c709 Mon Sep 17 00:00:00 2001 From: Arina Date: Mon, 24 Aug 2026 13:58:26 +0200 Subject: [PATCH 03/13] feat(CalculationModal): implement responsive modal width and add FormulaToolbar component --- .../Calculation/CalculationModal.js | 61 +++++++++++-------- .../Calculation/FormulaToolbar.js | 53 ++++++++++++++++ .../Calculation/MathOperatorSelector.js | 24 ++++---- .../styles/CalculationModal.style.js | 7 --- .../Calculation/styles/FormulaField.style.js | 1 - .../styles/FormulaToolbar.style.js | 18 ++++++ .../styles/MathOperatorSelector.style.js | 7 +-- .../InterpretationModal.js | 2 +- .../useModalContentWidth.js | 20 +++--- 9 files changed, 131 insertions(+), 62 deletions(-) create mode 100644 src/components/DataDimension/Calculation/FormulaToolbar.js create mode 100644 src/components/DataDimension/Calculation/styles/FormulaToolbar.style.js rename src/{components/Interpretations/InterpretationModal => modules}/useModalContentWidth.js (52%) diff --git a/src/components/DataDimension/Calculation/CalculationModal.js b/src/components/DataDimension/Calculation/CalculationModal.js index d71095ee9..3c5ab3501 100644 --- a/src/components/DataDimension/Calculation/CalculationModal.js +++ b/src/components/DataDimension/Calculation/CalculationModal.js @@ -14,6 +14,7 @@ import { } from '@dhis2/ui' import PropTypes from 'prop-types' import React, { useEffect, useRef, useState } from 'react' +import css from 'styled-jsx/css' import { createCalculationMutation, deleteCalculationMutation, @@ -33,6 +34,7 @@ import { VALID_EXPRESSION, getItemIdsFromExpression, } from '../../../modules/expressions.js' +import { useModalContentWidth } from '../../../modules/useModalContentWidth.js' import { OfflineTooltip as Tooltip } from '../../OfflineTooltip.js' import DataElementSelector from './DataElementSelector.js' import DndContext, { @@ -43,13 +45,25 @@ import FormulaField, { LAST_DROPZONE_ID, FORMULA_BOX_ID, } from './FormulaField.js' -import MathOperatorSelector from './MathOperatorSelector.js' +import FormulaToolbar from './FormulaToolbar.js' import styles from './styles/CalculationModal.style.js' const FIRST_POSITION = 0 const LAST_POSITION = -1 const CALCULATION_PROP_DEFAULT = {} const OPERATORS = getOperators() +// Matches the content width of the previous fixed `large` Modal size, so +// the modal never gets narrower than it used to on small windows. +const MODAL_MIN_CONTENT_WIDTH = 740 +// Caps how far the modal grows on wide screens, so the two columns don't +// stretch out further than is useful. +const MODAL_MAX_CONTENT_WIDTH = 1000 + +const getContentWidthCSS = (width) => css.resolve` + .content { + width: ${width}px; + } +` const Key = ({ children }) => ( @@ -248,6 +262,12 @@ const CalculationModal = ({ const [focusItemId, setFocusItemId] = useState(null) const [selectedItemId, setSelectedItemId] = useState(null) + const modalContentWidth = useModalContentWidth({ + minWidth: MODAL_MIN_CONTENT_WIDTH, + maxWidth: MODAL_MAX_CONTENT_WIDTH, + }) + const contentWidthCSS = getContentWidthCSS(modalContentWidth) + const expressionStatus = validationOutput?.status const validationMessage = expressionStatus === VALID_EXPRESSION @@ -522,7 +542,7 @@ const CalculationModal = ({ return ( <> - + {calculation.id ? i18n.t('Data / Edit calculation') @@ -547,7 +567,7 @@ const CalculationModal = ({ onDragStart={() => setFocusItemId(null)} onDragEnd={addOrMoveDraggedItem} > -
+
+ + removeItem(selectedItemId) + } + onValidate={validate} + canRemove={Boolean(selectedItemId)} + isValidating={isValidating} + isLoading={isLoading} + /> - -
- - -
{validationMessage && (
)} + {contentWidthCSS.styles} ) diff --git a/src/components/DataDimension/Calculation/FormulaToolbar.js b/src/components/DataDimension/Calculation/FormulaToolbar.js new file mode 100644 index 000000000..8db0e5203 --- /dev/null +++ b/src/components/DataDimension/Calculation/FormulaToolbar.js @@ -0,0 +1,53 @@ +import { Button, ButtonStrip } from '@dhis2/ui' +import PropTypes from 'prop-types' +import React from 'react' +import i18n from '../../../locales/index.js' +import MathOperatorSelector from './MathOperatorSelector.js' +import styles from './styles/FormulaToolbar.style.js' + +const FormulaToolbar = ({ + onAddOperator, + onRemove, + onValidate, + canRemove, + isValidating, + isLoading, +}) => ( +
+ + + + + + + +
+) + +FormulaToolbar.propTypes = { + onAddOperator: PropTypes.func.isRequired, + onRemove: PropTypes.func.isRequired, + onValidate: PropTypes.func.isRequired, + canRemove: PropTypes.bool, + isLoading: PropTypes.bool, + isValidating: PropTypes.bool, +} + +export default FormulaToolbar diff --git a/src/components/DataDimension/Calculation/MathOperatorSelector.js b/src/components/DataDimension/Calculation/MathOperatorSelector.js index 5319101b4..1b31985b6 100644 --- a/src/components/DataDimension/Calculation/MathOperatorSelector.js +++ b/src/components/DataDimension/Calculation/MathOperatorSelector.js @@ -8,19 +8,17 @@ const OPERATORS = getOperators() const MathOperatorSelector = ({ onClick }) => ( <> -
-
- {OPERATORS.map(({ label, value, type }, index) => ( - - ))} -
+
+ {OPERATORS.map(({ label, value, type }, index) => ( + + ))}
diff --git a/src/components/DataDimension/Calculation/styles/CalculationModal.style.js b/src/components/DataDimension/Calculation/styles/CalculationModal.style.js index 6ca0ac579..d4e7e16cc 100644 --- a/src/components/DataDimension/Calculation/styles/CalculationModal.style.js +++ b/src/components/DataDimension/Calculation/styles/CalculationModal.style.js @@ -11,13 +11,6 @@ export default css` margin-top: ${spacers.dp8}; } - .formula-actions { - display: flex; - justify-content: space-between; - align-items: center; - padding: ${spacers.dp8}; - } - .delete-button { margin-right: ${spacers.dp8}; } diff --git a/src/components/DataDimension/Calculation/styles/FormulaField.style.js b/src/components/DataDimension/Calculation/styles/FormulaField.style.js index 0d65549e8..4672f0253 100644 --- a/src/components/DataDimension/Calculation/styles/FormulaField.style.js +++ b/src/components/DataDimension/Calculation/styles/FormulaField.style.js @@ -4,7 +4,6 @@ import css from 'styled-jsx/css' export default css` .formula-field { border-top: 1px solid ${colors.grey400}; - border-bottom: 1px solid ${colors.grey400}; height: 180px; overflow: auto; padding: 6px 12px; diff --git a/src/components/DataDimension/Calculation/styles/FormulaToolbar.style.js b/src/components/DataDimension/Calculation/styles/FormulaToolbar.style.js new file mode 100644 index 000000000..2294f036b --- /dev/null +++ b/src/components/DataDimension/Calculation/styles/FormulaToolbar.style.js @@ -0,0 +1,18 @@ +import { colors, spacers } from '@dhis2/ui' +import css from 'styled-jsx/css' + +export default css` + .formula-toolbar { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: ${spacers.dp8}; + padding: ${spacers.dp8}; + } + + .divider { + align-self: stretch; + width: 1px; + background: ${colors.grey400}; + } +` diff --git a/src/components/DataDimension/Calculation/styles/MathOperatorSelector.style.js b/src/components/DataDimension/Calculation/styles/MathOperatorSelector.style.js index 712d958ea..dbac349ee 100644 --- a/src/components/DataDimension/Calculation/styles/MathOperatorSelector.style.js +++ b/src/components/DataDimension/Calculation/styles/MathOperatorSelector.style.js @@ -1,15 +1,10 @@ -import { colors, spacers } from '@dhis2/ui' +import { spacers } from '@dhis2/ui' import css from 'styled-jsx/css' export default css` - .wrapper { - border-bottom: 1px solid ${colors.grey400}; - } - .operators { display: flex; flex-wrap: wrap; gap: ${spacers.dp4}; - padding: ${spacers.dp8}; } ` diff --git a/src/components/Interpretations/InterpretationModal/InterpretationModal.js b/src/components/Interpretations/InterpretationModal/InterpretationModal.js index 73bc13c8c..689cc9780 100644 --- a/src/components/Interpretations/InterpretationModal/InterpretationModal.js +++ b/src/components/Interpretations/InterpretationModal/InterpretationModal.js @@ -15,12 +15,12 @@ import cx from 'classnames' import PropTypes from 'prop-types' import React, { useMemo } from 'react' import css from 'styled-jsx/css' +import { useModalContentWidth } from '../../../modules/useModalContentWidth.js' import { useActiveInterpretation, useInterpretationsCurrentUser, } from '../InterpretationsProvider/hooks.js' import { InterpretationThread } from './InterpretationThread.js' -import { useModalContentWidth } from './useModalContentWidth.js' const modalCSS = css.resolve` aside { diff --git a/src/components/Interpretations/InterpretationModal/useModalContentWidth.js b/src/modules/useModalContentWidth.js similarity index 52% rename from src/components/Interpretations/InterpretationModal/useModalContentWidth.js rename to src/modules/useModalContentWidth.js index 9ce9e1fde..9bc3c390a 100644 --- a/src/components/Interpretations/InterpretationModal/useModalContentWidth.js +++ b/src/modules/useModalContentWidth.js @@ -1,18 +1,22 @@ import { useState, useEffect } from 'react' -import { useDebounce } from '../../../modules/utils.js' +import { useDebounce } from './utils.js' const MODAL_SIDE_PADDING = 2 * 24 const MODAL_SIDE_MARGINS = 2 * 128 -const computeModalContentWidth = (windowWidth) => { - return windowWidth - MODAL_SIDE_MARGINS - MODAL_SIDE_PADDING +const computeModalContentWidth = (windowWidth, minWidth, maxWidth) => { + const width = windowWidth - MODAL_SIDE_MARGINS - MODAL_SIDE_PADDING + return Math.min(Math.max(width, minWidth), maxWidth) } -export const useModalContentWidth = () => { +export const useModalContentWidth = ({ + minWidth = 0, + maxWidth = Infinity, +} = {}) => { const [windowWidth, setWindowWidth] = useState(window.innerWidth) const debouncedWindowWidth = useDebounce(windowWidth, 150) const [modalContentWidth, setModalContentWidth] = useState( - computeModalContentWidth(windowWidth) + computeModalContentWidth(windowWidth, minWidth, maxWidth) ) useEffect(() => { @@ -27,8 +31,10 @@ export const useModalContentWidth = () => { }, []) useEffect(() => { - setModalContentWidth(computeModalContentWidth(debouncedWindowWidth)) - }, [debouncedWindowWidth]) + setModalContentWidth( + computeModalContentWidth(debouncedWindowWidth, minWidth, maxWidth) + ) + }, [debouncedWindowWidth, minWidth, maxWidth]) return modalContentWidth } From af9830cf6c7c2bde9c3ef772b9629396b14edb7c Mon Sep 17 00:00:00 2001 From: Arina Date: Mon, 24 Aug 2026 15:50:32 +0200 Subject: [PATCH 04/13] refactor(FormulaItem): remove Tooltip wrapper and adjust styles for better layout --- .../DataDimension/Calculation/FormulaItem.js | 25 ++++++++----------- .../styles/CalculationModal.style.js | 5 ++++ .../Calculation/styles/FormulaField.style.js | 3 ++- .../Calculation/styles/FormulaItem.style.js | 3 --- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/components/DataDimension/Calculation/FormulaItem.js b/src/components/DataDimension/Calculation/FormulaItem.js index 6e7df109e..85676a167 100644 --- a/src/components/DataDimension/Calculation/FormulaItem.js +++ b/src/components/DataDimension/Calculation/FormulaItem.js @@ -1,4 +1,3 @@ -import { Tooltip } from '@dhis2/ui' import { useSortable } from '@dnd-kit/sortable' import { CSS } from '@dnd-kit/utilities' import cx from 'classnames' @@ -147,19 +146,17 @@ const FormulaItem = ({ if (type === EXPRESSION_TYPE_DATA) { return ( - -
- - {getIcon(DIMENSION_TYPE_DATA_ELEMENT)} - - {label} - -
- +
+ + {getIcon(DIMENSION_TYPE_DATA_ELEMENT)} + + {label} + +
) } diff --git a/src/components/DataDimension/Calculation/styles/CalculationModal.style.js b/src/components/DataDimension/Calculation/styles/CalculationModal.style.js index d4e7e16cc..779344efc 100644 --- a/src/components/DataDimension/Calculation/styles/CalculationModal.style.js +++ b/src/components/DataDimension/Calculation/styles/CalculationModal.style.js @@ -5,6 +5,10 @@ export default css` .formula-section { background: ${colors.white}; border: 1px solid ${colors.grey400}; + display: flex; + flex-direction: column; + flex: 1; + min-height: 0; } .validation-notice { @@ -29,6 +33,7 @@ export default css` font-size: 14px; display: flex; flex-direction: column; + min-height: 0; } .sub-header-row { diff --git a/src/components/DataDimension/Calculation/styles/FormulaField.style.js b/src/components/DataDimension/Calculation/styles/FormulaField.style.js index 4672f0253..17baf06bc 100644 --- a/src/components/DataDimension/Calculation/styles/FormulaField.style.js +++ b/src/components/DataDimension/Calculation/styles/FormulaField.style.js @@ -4,7 +4,8 @@ import css from 'styled-jsx/css' export default css` .formula-field { border-top: 1px solid ${colors.grey400}; - height: 180px; + flex: 1; + min-height: 180px; overflow: auto; padding: 6px 12px; position: relative; diff --git a/src/components/DataDimension/Calculation/styles/FormulaItem.style.js b/src/components/DataDimension/Calculation/styles/FormulaItem.style.js index 6b0998960..d9ba3ca2c 100644 --- a/src/components/DataDimension/Calculation/styles/FormulaItem.style.js +++ b/src/components/DataDimension/Calculation/styles/FormulaItem.style.js @@ -45,9 +45,6 @@ export default css` } .data .label { - max-width: 280px; - text-overflow: ellipsis; - overflow: hidden; white-space: nowrap; } From aa30db2815ba57141a3df39eeb8b101d38a1511a Mon Sep 17 00:00:00 2001 From: Arina Date: Mon, 24 Aug 2026 16:26:07 +0200 Subject: [PATCH 05/13] feat(FormulaToolbar): add validation status and message display --- .../Calculation/CalculationModal.js | 22 +----- .../Calculation/FormulaToolbar.js | 78 +++++++++++++------ .../styles/CalculationModal.style.js | 4 - .../styles/FormulaToolbar.style.js | 22 +++++- 4 files changed, 77 insertions(+), 49 deletions(-) diff --git a/src/components/DataDimension/Calculation/CalculationModal.js b/src/components/DataDimension/Calculation/CalculationModal.js index 3c5ab3501..8afde455e 100644 --- a/src/components/DataDimension/Calculation/CalculationModal.js +++ b/src/components/DataDimension/Calculation/CalculationModal.js @@ -8,7 +8,6 @@ import { ButtonStrip, IconQuestion16, InputField, - NoticeBox, Popper, Portal, } from '@dhis2/ui' @@ -592,6 +591,8 @@ const CalculationModal = ({ canRemove={Boolean(selectedItemId)} isValidating={isValidating} isLoading={isLoading} + validationStatus={expressionStatus} + validationMessage={validationMessage} />
- {validationMessage && ( -
- - {validationMessage} - -
- )}
diff --git a/src/components/DataDimension/Calculation/FormulaToolbar.js b/src/components/DataDimension/Calculation/FormulaToolbar.js index 8db0e5203..a6e735f80 100644 --- a/src/components/DataDimension/Calculation/FormulaToolbar.js +++ b/src/components/DataDimension/Calculation/FormulaToolbar.js @@ -1,7 +1,15 @@ -import { Button, ButtonStrip } from '@dhis2/ui' +import { + Button, + ButtonStrip, + IconCheckmarkCircle16, + IconErrorFilled16, + colors, +} from '@dhis2/ui' +import cx from 'classnames' import PropTypes from 'prop-types' import React from 'react' import i18n from '../../../locales/index.js' +import { VALID_EXPRESSION } from '../../../modules/expressions.js' import MathOperatorSelector from './MathOperatorSelector.js' import styles from './styles/FormulaToolbar.style.js' @@ -12,31 +20,51 @@ const FormulaToolbar = ({ canRemove, isValidating, isLoading, + validationStatus, + validationMessage, }) => (
- - - - - - +
+ + + + + + +
+
+ {validationMessage && ( + + {validationStatus === VALID_EXPRESSION ? ( + + ) : ( + + )} + {validationMessage} + + )} +
) @@ -48,6 +76,8 @@ FormulaToolbar.propTypes = { canRemove: PropTypes.bool, isLoading: PropTypes.bool, isValidating: PropTypes.bool, + validationMessage: PropTypes.string, + validationStatus: PropTypes.string, } export default FormulaToolbar diff --git a/src/components/DataDimension/Calculation/styles/CalculationModal.style.js b/src/components/DataDimension/Calculation/styles/CalculationModal.style.js index 779344efc..d74214e25 100644 --- a/src/components/DataDimension/Calculation/styles/CalculationModal.style.js +++ b/src/components/DataDimension/Calculation/styles/CalculationModal.style.js @@ -11,10 +11,6 @@ export default css` min-height: 0; } - .validation-notice { - margin-top: ${spacers.dp8}; - } - .delete-button { margin-right: ${spacers.dp8}; } diff --git a/src/components/DataDimension/Calculation/styles/FormulaToolbar.style.js b/src/components/DataDimension/Calculation/styles/FormulaToolbar.style.js index 2294f036b..7723eda59 100644 --- a/src/components/DataDimension/Calculation/styles/FormulaToolbar.style.js +++ b/src/components/DataDimension/Calculation/styles/FormulaToolbar.style.js @@ -3,11 +3,14 @@ import css from 'styled-jsx/css' export default css` .formula-toolbar { + padding: ${spacers.dp8}; + } + + .buttons-row { display: flex; flex-wrap: wrap; align-items: center; gap: ${spacers.dp8}; - padding: ${spacers.dp8}; } .divider { @@ -15,4 +18,21 @@ export default css` width: 1px; background: ${colors.grey400}; } + + .status { + display: inline-flex; + align-items: center; + gap: ${spacers.dp4}; + margin-top: ${spacers.dp4}; + } + + .status-text { + color: ${colors.red700}; + font-size: 14px; + line-height: 19px; + } + + .valid .status-text { + color: ${colors.green700}; + } ` From 196486972cf5e2ad0fb4a9dd81efc804afb347b7 Mon Sep 17 00:00:00 2001 From: Joseph John Aas Cooper <33054985+cooper-joe@users.noreply.github.com> Date: Thu, 27 Aug 2026 09:58:32 +0200 Subject: [PATCH 06/13] fix: calculations formula changes --- .../Calculation/CalculationModal.js | 149 ++++++------------ .../Calculation/FormulaToolbar.js | 51 ++---- .../styles/CalculationModal.style.js | 108 +++++-------- .../Calculation/styles/FormulaField.style.js | 2 +- .../styles/FormulaToolbar.style.js | 27 +--- 5 files changed, 101 insertions(+), 236 deletions(-) diff --git a/src/components/DataDimension/Calculation/CalculationModal.js b/src/components/DataDimension/Calculation/CalculationModal.js index 8afde455e..fcd10ca20 100644 --- a/src/components/DataDimension/Calculation/CalculationModal.js +++ b/src/components/DataDimension/Calculation/CalculationModal.js @@ -6,11 +6,12 @@ import { ModalContent, ModalActions, ButtonStrip, - IconQuestion16, + IconCheckmarkCircle16, + IconErrorFilled16, InputField, - Popper, - Portal, + colors, } from '@dhis2/ui' +import cx from 'classnames' import PropTypes from 'prop-types' import React, { useEffect, useRef, useState } from 'react' import css from 'styled-jsx/css' @@ -64,101 +65,6 @@ const getContentWidthCSS = (width) => css.resolve` } ` -const Key = ({ children }) => ( - - {children} - - -) - -Key.propTypes = { - children: PropTypes.node.isRequired, -} - -const ShortcutsPopoverContent = () => ( -
-

{i18n.t('Usage tips')}

-
    -
  • - {i18n.t( - 'Click or drag a data element or operator to add it to the formula.' - )} -
    • -
  • {i18n.t('Drag an item to reorder it.')}
    • -
  • - {i18n.t('Select an item, then click')}{' '} - {i18n.t('Remove item')}{' '} - {i18n.t('to delete it.')} -
    • -
-

{i18n.t('Keyboard shortcuts')}

-
    -
  • - {i18n.t('Press')}{' '} - - Enter - {i18n.t('or')} - Space - {' '} - {i18n.t('to add or select the focused item.')} -
    • -
  • - {i18n.t('Press')}{' '} - - ← - {i18n.t('or')} - → - {' '} - {i18n.t('to move the selected item.')} -
    • -
  • - {i18n.t('Press')}{' '} - - + - - - * - / - ( - ) - {' '} - {i18n.t('to insert an operator after the selected item.')} -
    • -
- -
-) - -const UsageHint = () => { - const [isOpen, setIsOpen] = useState(false) - const triggerRef = useRef() - - return ( - - - {isOpen && ( - - - - - - )} - - - ) -} - const CalculationModal = ({ calculation = CALCULATION_PROP_DEFAULT, onSave, @@ -575,12 +481,53 @@ const CalculationModal = ({ />
-
+

{i18n.t('Formula')}

- +
+ {validationMessage && ( + + {expressionStatus === + VALID_EXPRESSION ? ( + + ) : ( + + )} + + {validationMessage} + + + )} +
(
- - + {canRemove && ( + + )}
-
- {validationMessage && ( - - {validationStatus === VALID_EXPRESSION ? ( - - ) : ( - - )} - {validationMessage} - - )} -
) @@ -76,8 +51,6 @@ FormulaToolbar.propTypes = { canRemove: PropTypes.bool, isLoading: PropTypes.bool, isValidating: PropTypes.bool, - validationMessage: PropTypes.string, - validationStatus: PropTypes.string, } export default FormulaToolbar diff --git a/src/components/DataDimension/Calculation/styles/CalculationModal.style.js b/src/components/DataDimension/Calculation/styles/CalculationModal.style.js index d74214e25..279133d42 100644 --- a/src/components/DataDimension/Calculation/styles/CalculationModal.style.js +++ b/src/components/DataDimension/Calculation/styles/CalculationModal.style.js @@ -1,16 +1,27 @@ -import { colors, elevations, spacers } from '@dhis2/ui' +import { colors, spacers } from '@dhis2/ui' import css from 'styled-jsx/css' export default css` .formula-section { + /* Match left column height; FormulaField scrolls inside. */ + position: absolute; + inset: 0; background: ${colors.white}; border: 1px solid ${colors.grey400}; display: flex; flex-direction: column; - flex: 1; + overflow: hidden; min-height: 0; } + .formula-section.valid { + border-color: ${colors.green500}; + } + + .formula-section.invalid { + border-color: ${colors.red500}; + } + .delete-button { margin-right: ${spacers.dp8}; } @@ -18,107 +29,64 @@ export default css` .content { display: flex; gap: ${spacers.dp12}; + align-items: stretch; } .left-section { width: 40%; + flex-shrink: 0; } .right-section { width: 60%; font-size: 14px; - display: flex; - flex-direction: column; + position: relative; min-height: 0; } .sub-header-row { display: flex; align-items: center; - margin: ${spacers.dp4} ${spacers.dp8}; + gap: ${spacers.dp8}; + padding: ${spacers.dp8} ${spacers.dp8} 0; + /* Reserve space for status text so the row doesn't jump when it appears */ + min-height: calc(${spacers.dp8} + 19px); + box-sizing: border-box; + flex-shrink: 0; } .sub-header { font-size: 14px; font-weight: normal; margin: 0; + flex-shrink: 0; } - .name-field { - margin-bottom: ${spacers.dp16}; - } - - .hint { - position: relative; - display: inline-flex; + .validation-status { + margin-left: auto; + min-width: 0; + display: flex; + justify-content: flex-end; } - .hint-trigger { + .status { display: inline-flex; align-items: center; - padding: ${spacers.dp4} ${spacers.dp4} ${spacers.dp4} 4px; - background: none; - border: none; - color: ${colors.grey600}; - cursor: default; + gap: ${spacers.dp4}; + min-width: 0; } - .shortcuts { - background: ${colors.white}; - border-radius: 4px; - box-shadow: ${elevations.popover}; - padding: ${spacers.dp12} ${spacers.dp16}; - max-width: 364px; - color: ${colors.grey900}; + .status-text { + color: ${colors.red700}; font-size: 14px; + line-height: 19px; } - .shortcuts-header { - margin: ${spacers.dp12} 0 ${spacers.dp8}; - text-transform: uppercase; - font-size: 11px; - font-weight: 400; - letter-spacing: 0.3px; - color: ${colors.grey600}; + .valid .status-text { + color: ${colors.green700}; } - .shortcuts-header:first-child { - margin-top: 0; - } - - .shortcuts ul { - margin: 0; - padding-left: ${spacers.dp16}; - } - - .shortcuts li { - margin: 0 0 ${spacers.dp8}; - line-height: 1.5; - } - - .shortcuts li:last-child { - margin-bottom: 0; - } - - .shortcut-keys { - display: inline-flex; - align-items: center; - flex-wrap: wrap; - gap: ${spacers.dp4}; - vertical-align: middle; - } - - .key { - display: inline-block; - min-width: 1.4em; - padding: 1px 5px; - border: 1px solid ${colors.grey400}; - border-radius: 3px; - background: ${colors.grey050}; - box-shadow: 0 1px 0 ${colors.grey400}; - font-family: monospace; - font-size: 12px; - line-height: 1.4; - text-align: center; + .name-field { + margin-bottom: ${spacers.dp16}; } ` diff --git a/src/components/DataDimension/Calculation/styles/FormulaField.style.js b/src/components/DataDimension/Calculation/styles/FormulaField.style.js index 17baf06bc..9233fedaa 100644 --- a/src/components/DataDimension/Calculation/styles/FormulaField.style.js +++ b/src/components/DataDimension/Calculation/styles/FormulaField.style.js @@ -5,7 +5,7 @@ export default css` .formula-field { border-top: 1px solid ${colors.grey400}; flex: 1; - min-height: 180px; + min-height: 0; overflow: auto; padding: 6px 12px; position: relative; diff --git a/src/components/DataDimension/Calculation/styles/FormulaToolbar.style.js b/src/components/DataDimension/Calculation/styles/FormulaToolbar.style.js index 7723eda59..01b048ae1 100644 --- a/src/components/DataDimension/Calculation/styles/FormulaToolbar.style.js +++ b/src/components/DataDimension/Calculation/styles/FormulaToolbar.style.js @@ -1,38 +1,17 @@ -import { colors, spacers } from '@dhis2/ui' +import { spacers } from '@dhis2/ui' import css from 'styled-jsx/css' export default css` .formula-toolbar { padding: ${spacers.dp8}; + flex-shrink: 0; } .buttons-row { display: flex; flex-wrap: wrap; + justify-content: space-between; align-items: center; gap: ${spacers.dp8}; } - - .divider { - align-self: stretch; - width: 1px; - background: ${colors.grey400}; - } - - .status { - display: inline-flex; - align-items: center; - gap: ${spacers.dp4}; - margin-top: ${spacers.dp4}; - } - - .status-text { - color: ${colors.red700}; - font-size: 14px; - line-height: 19px; - } - - .valid .status-text { - color: ${colors.green700}; - } ` From 85899db020b08ad4baa9c6923f7994881c540807 Mon Sep 17 00:00:00 2001 From: Joseph John Aas Cooper <33054985+cooper-joe@users.noreply.github.com> Date: Thu, 27 Aug 2026 13:59:21 +0200 Subject: [PATCH 07/13] fix: move validation output --- .../Calculation/CalculationModal.js | 82 +++++++++---------- .../styles/CalculationModal.style.js | 51 +++++++++--- .../Calculation/styles/FormulaField.style.js | 1 - 3 files changed, 80 insertions(+), 54 deletions(-) diff --git a/src/components/DataDimension/Calculation/CalculationModal.js b/src/components/DataDimension/Calculation/CalculationModal.js index fcd10ca20..e3a1ccff8 100644 --- a/src/components/DataDimension/Calculation/CalculationModal.js +++ b/src/components/DataDimension/Calculation/CalculationModal.js @@ -481,33 +481,47 @@ const CalculationModal = ({ />
-
+

{i18n.t('Formula')}

-
- {validationMessage && ( - +
+ + removeItem(selectedItemId) + } + onValidate={validate} + canRemove={Boolean(selectedItemId)} + isValidating={isValidating} + isLoading={isLoading} + /> +
+ + {validationMessage && ( +
+ {expressionStatus === VALID_EXPRESSION ? ( - )} -
+
+ )}
- - removeItem(selectedItemId) - } - onValidate={validate} - canRemove={Boolean(selectedItemId)} - isValidating={isValidating} - isLoading={isLoading} - /> -
diff --git a/src/components/DataDimension/Calculation/styles/CalculationModal.style.js b/src/components/DataDimension/Calculation/styles/CalculationModal.style.js index 279133d42..fbd12d306 100644 --- a/src/components/DataDimension/Calculation/styles/CalculationModal.style.js +++ b/src/components/DataDimension/Calculation/styles/CalculationModal.style.js @@ -14,12 +14,24 @@ export default css` min-height: 0; } - .formula-section.valid { - border-color: ${colors.green500}; + .formula-box { + position: relative; + flex: 1; + min-height: 0; + display: flex; + flex-direction: column; + overflow: hidden; + border-top: 1px solid ${colors.grey400}; } - .formula-section.invalid { - border-color: ${colors.red500}; + .formula-box.valid { + border-top-color: transparent; + box-shadow: inset 0 0 0 1px ${colors.green500}; + } + + .formula-box.invalid { + border-top-color: transparent; + box-shadow: inset 0 0 0 1px ${colors.red500}; } .delete-button { @@ -49,8 +61,6 @@ export default css` align-items: center; gap: ${spacers.dp8}; padding: ${spacers.dp8} ${spacers.dp8} 0; - /* Reserve space for status text so the row doesn't jump when it appears */ - min-height: calc(${spacers.dp8} + 19px); box-sizing: border-box; flex-shrink: 0; } @@ -62,11 +72,31 @@ export default css` flex-shrink: 0; } - .validation-status { - margin-left: auto; - min-width: 0; + /* Clear chips under the overlay bar when validation is shown */ + .formula-box.valid :global(.formula-field), + .formula-box.invalid :global(.formula-field) { + padding-bottom: 40px; + } + + .validation-bar { + position: absolute; + left: 0; + right: 0; + bottom: 0; + z-index: 1; display: flex; - justify-content: flex-end; + align-items: center; + gap: ${spacers.dp4}; + padding: ${spacers.dp8} ${spacers.dp12}; + box-sizing: border-box; + background: ${colors.red050}; + outline: 1px solid ${colors.red500}; + outline-offset: -1px; + } + + .formula-box.valid .validation-bar { + background: ${colors.green050}; + outline-color: ${colors.green500}; } .status { @@ -80,6 +110,7 @@ export default css` color: ${colors.red700}; font-size: 14px; line-height: 19px; + min-width: 0; } .valid .status-text { diff --git a/src/components/DataDimension/Calculation/styles/FormulaField.style.js b/src/components/DataDimension/Calculation/styles/FormulaField.style.js index 9233fedaa..47bee19d4 100644 --- a/src/components/DataDimension/Calculation/styles/FormulaField.style.js +++ b/src/components/DataDimension/Calculation/styles/FormulaField.style.js @@ -3,7 +3,6 @@ import css from 'styled-jsx/css' export default css` .formula-field { - border-top: 1px solid ${colors.grey400}; flex: 1; min-height: 0; overflow: auto; From 6537fd608cb7c9d9ecad281dda71a9982b03558b Mon Sep 17 00:00:00 2001 From: Arina Date: Mon, 7 Sep 2026 11:21:27 +0200 Subject: [PATCH 08/13] refactor(CalculationModal, FormulaItem): optimize performance with useMemo and clean up unused refs --- .../DataDimension/Calculation/CalculationModal.js | 9 +++++---- .../DataDimension/Calculation/FormulaItem.js | 10 ---------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/components/DataDimension/Calculation/CalculationModal.js b/src/components/DataDimension/Calculation/CalculationModal.js index e3a1ccff8..b09fca67e 100644 --- a/src/components/DataDimension/Calculation/CalculationModal.js +++ b/src/components/DataDimension/Calculation/CalculationModal.js @@ -13,7 +13,7 @@ import { } from '@dhis2/ui' import cx from 'classnames' import PropTypes from 'prop-types' -import React, { useEffect, useRef, useState } from 'react' +import React, { useEffect, useMemo, useRef, useState } from 'react' import css from 'styled-jsx/css' import { createCalculationMutation, @@ -171,7 +171,10 @@ const CalculationModal = ({ minWidth: MODAL_MIN_CONTENT_WIDTH, maxWidth: MODAL_MAX_CONTENT_WIDTH, }) - const contentWidthCSS = getContentWidthCSS(modalContentWidth) + const contentWidthCSS = useMemo( + () => getContentWidthCSS(modalContentWidth), + [modalContentWidth] + ) const expressionStatus = validationOutput?.status const validationMessage = @@ -381,8 +384,6 @@ const CalculationModal = ({ expression, }) - // useDataMutation never rejects; network/engine failures go to - // onError and this promise does not resolve. if (!backendResult) { return } diff --git a/src/components/DataDimension/Calculation/FormulaItem.js b/src/components/DataDimension/Calculation/FormulaItem.js index 85676a167..ad2ca06a7 100644 --- a/src/components/DataDimension/Calculation/FormulaItem.js +++ b/src/components/DataDimension/Calculation/FormulaItem.js @@ -44,7 +44,6 @@ const FormulaItem = ({ }) const inputRef = useRef(null) - const ignoreClickRef = useRef(false) useEffect(() => { if (hasFocus && inputRef.current) { @@ -91,10 +90,6 @@ const FormulaItem = ({ } const handleClick = (e) => { - if (ignoreClickRef.current) { - ignoreClickRef.current = false - return - } if (isInteractiveElement(e.target)) { inputRef.current && inputRef.current.focus() return @@ -108,11 +103,6 @@ const FormulaItem = ({ if (isInteractiveElement(e.target)) { return } - if (e.key === 'Enter' || e.key === ' ') { - // role="button" from dnd-kit also synthesizes a click on Enter/Space; - // ignore that click so selection is not toggled off right after. - ignoreClickRef.current = true - } onActivationKeydown(() => onClick(id))(e) } From fe7dbfd2a4e80fda54084c2f367d93bb2dc0634b Mon Sep 17 00:00:00 2001 From: Arina Date: Mon, 7 Sep 2026 11:25:27 +0200 Subject: [PATCH 09/13] chore(i18n): update translation template dates and add missing entries --- i18n/en.pot | 41 ++++++++++------------------------------- 1 file changed, 10 insertions(+), 31 deletions(-) diff --git a/i18n/en.pot b/i18n/en.pot index 4fe60a2df..68c6cb165 100644 --- a/i18n/en.pot +++ b/i18n/en.pot @@ -5,8 +5,8 @@ msgstr "" "Content-Type: text/plain; charset=utf-8\n" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=(n != 1)\n" -"POT-Creation-Date: 2026-08-27T11:56:55.720Z\n" -"PO-Revision-Date: 2026-08-27T11:56:55.720Z\n" +"POT-Creation-Date: 2026-09-07T09:22:41.250Z\n" +"PO-Revision-Date: 2026-09-07T09:22:41.251Z\n" msgid "view only" msgstr "view only" @@ -79,24 +79,12 @@ msgstr "This app could not retrieve required data." msgid "Network error" msgstr "Network error" -msgid "Add or select the focused item" -msgstr "Add or select the focused item" - -msgid "Move the selected item" -msgstr "Move the selected item" - -msgid "Insert an operator after the selected item" -msgstr "Insert an operator after the selected item" - -msgid "Keyboard navigation" -msgstr "Keyboard navigation" +msgid "Could not validate the formula" +msgstr "Could not validate the formula" msgid "The formula is valid" msgstr "The formula is valid" -msgid "Could not validate the formula" -msgstr "Could not validate the formula" - msgid "Data / Edit calculation" msgstr "Data / Edit calculation" @@ -112,21 +100,6 @@ msgstr "Shown in table headers and chart axes/legends" msgid "Formula" msgstr "Formula" -msgid "Remove item" -msgstr "Remove item" - -msgid "Check formula" -msgstr "Check formula" - -msgid "" -"Drag or click a data element or operator to add it to the formula. Drag to " -"reorder. Select an item and click Remove item, or double-click, to delete " -"it." -msgstr "" -"Drag or click a data element or operator to add it to the formula. Drag to " -"reorder. Select an item and click Remove item, or double-click, to delete " -"it." - msgid "Delete calculation" msgstr "Delete calculation" @@ -180,6 +153,12 @@ msgstr "" "Drag a data element or operator here, or click one, to start building a " "formula" +msgid "Remove item" +msgstr "Remove item" + +msgid "Check formula" +msgstr "Check formula" + msgid "Expression description" msgstr "Expression description" From 7baeb5717f9fa55cdae31d5a5ebb74f180e2e572 Mon Sep 17 00:00:00 2001 From: Arina Date: Tue, 8 Sep 2026 09:00:16 +0200 Subject: [PATCH 10/13] refactor(CalculationModal): clean up unused constants and improve modal content width handling --- src/__demo__/CalculationModal.stories.js | 2 - .../Calculation/CalculationModal.js | 60 ++++++++----------- .../Calculation/MathOperatorSelector.js | 4 +- .../styles/CalculationModal.style.js | 2 +- 4 files changed, 26 insertions(+), 42 deletions(-) diff --git a/src/__demo__/CalculationModal.stories.js b/src/__demo__/CalculationModal.stories.js index f09b08d41..1d4605839 100644 --- a/src/__demo__/CalculationModal.stories.js +++ b/src/__demo__/CalculationModal.stories.js @@ -210,8 +210,6 @@ const DATA_ELEMENT_GROUPS = { const VALIDATION_OK = { status: 'OK', - message: 'Valid', - description: 'ANC 1st visit / 10 * ANC 4th or more visits', } const providerData = { diff --git a/src/components/DataDimension/Calculation/CalculationModal.js b/src/components/DataDimension/Calculation/CalculationModal.js index b09fca67e..56c66a775 100644 --- a/src/components/DataDimension/Calculation/CalculationModal.js +++ b/src/components/DataDimension/Calculation/CalculationModal.js @@ -51,15 +51,11 @@ import styles from './styles/CalculationModal.style.js' const FIRST_POSITION = 0 const LAST_POSITION = -1 const CALCULATION_PROP_DEFAULT = {} -const OPERATORS = getOperators() -// Matches the content width of the previous fixed `large` Modal size, so -// the modal never gets narrower than it used to on small windows. -const MODAL_MIN_CONTENT_WIDTH = 740 -// Caps how far the modal grows on wide screens, so the two columns don't -// stretch out further than is useful. +// Matches the content width of the previous fixed `large` Modal +const MODAL_MIN_CONTENT_WIDTH = 752 const MODAL_MAX_CONTENT_WIDTH = 1000 -const getContentWidthCSS = (width) => css.resolve` +const getModalContentCSS = (width) => css.resolve` .content { width: ${width}px; } @@ -86,9 +82,7 @@ const CalculationModal = ({ { onError: (error) => showError( - error?.message || - error || - i18n.t('Could not validate the formula') + error?.message || i18n.t('Could not validate the formula') ), } ) @@ -153,9 +147,6 @@ const CalculationModal = ({ }, [data, calculation.expression]) const nextItemIdRef = useRef(1) - // State is read through this ref instead of a closure, so the - // document-level keydown listener can be registered once on mount - // and still see fresh state on every keystroke. const latestRef = useRef() const [validationOutput, setValidationOutput] = useState(null) @@ -171,8 +162,8 @@ const CalculationModal = ({ minWidth: MODAL_MIN_CONTENT_WIDTH, maxWidth: MODAL_MAX_CONTENT_WIDTH, }) - const contentWidthCSS = useMemo( - () => getContentWidthCSS(modalContentWidth), + const modalContentCSS = useMemo( + () => getModalContentCSS(modalContentWidth), [modalContentWidth] ) @@ -182,14 +173,15 @@ const CalculationModal = ({ ? i18n.t('The formula is valid') : validationOutput?.message - const selectItem = (itemId) => - setSelectedItemId((prevSelected) => { - const next = prevSelected !== itemId ? itemId : null - if (latestRef.current) { - latestRef.current.selectedItemId = next - } - return next - }) + const selectItem = (itemId) => { + const prevSelected = latestRef.current?.selectedItemId + const next = prevSelected !== itemId ? itemId : null + + if (latestRef.current) { + latestRef.current.selectedItemId = next + } + setSelectedItemId(next) + } const isLoading = isCreatingCalculation || @@ -211,8 +203,6 @@ const CalculationModal = ({ type, } - // Without an explicit destIndex, insert after the selected item - // instead of always appending. const selectedId = latestRef.current?.selectedItemId setExpressionArray((prevArray) => { let insertAt = destIndex @@ -237,8 +227,6 @@ const CalculationModal = ({ setFocusItemId(newItem.id) } - // Keep the newly added item selected so it becomes the anchor for - // the next typed operator or arrow-key move. setSelectedItemId(newItem.id) latestRef.current.selectedItemId = newItem.id } @@ -276,6 +264,8 @@ const CalculationModal = ({ } } + // Mirrored on every render so the keydown listener below, which is + // registered once on mount, still sees fresh values on every keystroke. latestRef.current = { isLoading, showDeletePrompt, @@ -296,22 +286,18 @@ const CalculationModal = ({ moveItem, } = latestRef.current - // On some layouts (e.g. German, French) operator characters - // like ( ) * are typed via AltGr, which browsers report as - // altKey/ctrlKey being set - don't let that block the shortcut. - const isAltGraph = event.getModifierState?.('AltGraph') - if ( isLoading || showDeletePrompt || event.metaKey || - (!isAltGraph && (event.ctrlKey || event.altKey)) || + event.ctrlKey || + event.altKey || isInteractiveElement(event.target) ) { return } - const operator = OPERATORS.find( + const operator = getOperators().find( (op) => op.type === EXPRESSION_TYPE_OPERATOR && op.value === event.key @@ -473,7 +459,9 @@ const CalculationModal = ({ onDragStart={() => setFocusItemId(null)} onDragEnd={addOrMoveDraggedItem} > -
+
)} - {contentWidthCSS.styles} + {modalContentCSS.styles} ) diff --git a/src/components/DataDimension/Calculation/MathOperatorSelector.js b/src/components/DataDimension/Calculation/MathOperatorSelector.js index 1b31985b6..97c06c781 100644 --- a/src/components/DataDimension/Calculation/MathOperatorSelector.js +++ b/src/components/DataDimension/Calculation/MathOperatorSelector.js @@ -4,12 +4,10 @@ import { getOperators } from '../../../modules/expressions.js' import DraggableOperator from './Operator.js' import styles from './styles/MathOperatorSelector.style.js' -const OPERATORS = getOperators() - const MathOperatorSelector = ({ onClick }) => ( <>
- {OPERATORS.map(({ label, value, type }, index) => ( + {getOperators().map(({ label, value, type }, index) => ( Date: Tue, 8 Sep 2026 09:28:33 +0200 Subject: [PATCH 11/13] refactor(DataElementOption, Operator): streamline click handling by consolidating onClick event logic --- .../DataDimension/Calculation/DataElementOption.js | 7 ++----- src/components/DataDimension/Calculation/Operator.js | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/components/DataDimension/Calculation/DataElementOption.js b/src/components/DataDimension/Calculation/DataElementOption.js index 332cfadb4..cdf8c2a8c 100644 --- a/src/components/DataDimension/Calculation/DataElementOption.js +++ b/src/components/DataDimension/Calculation/DataElementOption.js @@ -26,13 +26,10 @@ const DataElementOption = ({ label, value, onClick }) => { {...listeners} ref={setNodeRef} style={style} + onClick={() => onClick(data)} onKeyDown={onActivationKeydown(() => onClick(data))} > -
onClick(data)} - data-test="data-element-option" - > +
{getIcon(DIMENSION_TYPE_DATA_ELEMENT)} diff --git a/src/components/DataDimension/Calculation/Operator.js b/src/components/DataDimension/Calculation/Operator.js index e27777c8c..c49324e58 100644 --- a/src/components/DataDimension/Calculation/Operator.js +++ b/src/components/DataDimension/Calculation/Operator.js @@ -27,6 +27,7 @@ const Operator = ({ label, value, type, onClick }) => { {...listeners} ref={setNodeRef} style={style} + onClick={() => onClick(data)} onKeyDown={onActivationKeydown(() => onClick(data))} >
{ number: type === EXPRESSION_TYPE_NUMBER, })} data-test="operator" - onClick={() => onClick(data)} > {label}
From 2c5d17953aef8df85dc1c68c591849a9bdc929ac Mon Sep 17 00:00:00 2001 From: Arina Date: Tue, 8 Sep 2026 10:09:04 +0200 Subject: [PATCH 12/13] feat(DataElementOption, Operator): enhance accessibility by adding role and tabindex for keyboard navigation --- src/components/DataDimension/Calculation/DataElementOption.js | 2 ++ src/components/DataDimension/Calculation/Operator.js | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/components/DataDimension/Calculation/DataElementOption.js b/src/components/DataDimension/Calculation/DataElementOption.js index cdf8c2a8c..a274615a4 100644 --- a/src/components/DataDimension/Calculation/DataElementOption.js +++ b/src/components/DataDimension/Calculation/DataElementOption.js @@ -22,6 +22,8 @@ const DataElementOption = ({ label, value, onClick }) => {
{ return (
Date: Tue, 8 Sep 2026 12:43:37 +0200 Subject: [PATCH 13/13] refactor(DataElementOption, Operator): replace div with button for improved accessibility and add draggable styles --- .../Calculation/DataElementOption.js | 12 +++++------ .../DataDimension/Calculation/DndContext.js | 17 +++++++++++---- .../DataDimension/Calculation/Operator.js | 13 ++++++------ .../styles/DataElementOption.style.js | 5 ----- .../styles/DraggableChipButton.style.js | 21 +++++++++++++++++++ 5 files changed, 47 insertions(+), 21 deletions(-) create mode 100644 src/components/DataDimension/Calculation/styles/DraggableChipButton.style.js diff --git a/src/components/DataDimension/Calculation/DataElementOption.js b/src/components/DataDimension/Calculation/DataElementOption.js index a274615a4..5839aef4a 100644 --- a/src/components/DataDimension/Calculation/DataElementOption.js +++ b/src/components/DataDimension/Calculation/DataElementOption.js @@ -5,8 +5,8 @@ import React from 'react' import { DIMENSION_TYPE_DATA_ELEMENT } from '../../../modules/dataTypes.js' import { getIcon } from '../../../modules/dimensionListItem.js' import { EXPRESSION_TYPE_DATA } from '../../../modules/expressions.js' -import { onActivationKeydown } from './DndContext.js' import styles from './styles/DataElementOption.style.js' +import draggableChipButtonStyles from './styles/DraggableChipButton.style.js' const DataElementOption = ({ label, value, onClick }) => { const data = { label, value, type: EXPRESSION_TYPE_DATA } @@ -20,16 +20,15 @@ const DataElementOption = ({ label, value, onClick }) => { return (
-
onClick(data)} - onKeyDown={onActivationKeydown(() => onClick(data))} >
@@ -37,7 +36,8 @@ const DataElementOption = ({ label, value, onClick }) => { {label}
-
+ +
) diff --git a/src/components/DataDimension/Calculation/DndContext.js b/src/components/DataDimension/Calculation/DndContext.js index b802460d2..574e9f1a4 100644 --- a/src/components/DataDimension/Calculation/DndContext.js +++ b/src/components/DataDimension/Calculation/DndContext.js @@ -81,11 +81,20 @@ const rectIntersectionCustom = ({ const INTERACTIVE_SELECTOR = 'button, input, textarea, select, option' -export const isInteractiveElement = (el) => - Boolean(el?.closest?.(INTERACTIVE_SELECTOR)) +// Chips are real ) } diff --git a/src/components/DataDimension/Calculation/styles/DataElementOption.style.js b/src/components/DataDimension/Calculation/styles/DataElementOption.style.js index f3333d726..eb09ccf13 100644 --- a/src/components/DataDimension/Calculation/styles/DataElementOption.style.js +++ b/src/components/DataDimension/Calculation/styles/DataElementOption.style.js @@ -9,11 +9,6 @@ export default css` .wrapper:last-child { margin-bottom: ${spacers.dp4}; } - .draggable-item { - cursor: pointer; - display: inline-flex; - } - .chip { display: inline-flex; background: ${colors.grey200}; diff --git a/src/components/DataDimension/Calculation/styles/DraggableChipButton.style.js b/src/components/DataDimension/Calculation/styles/DraggableChipButton.style.js new file mode 100644 index 000000000..330f948e3 --- /dev/null +++ b/src/components/DataDimension/Calculation/styles/DraggableChipButton.style.js @@ -0,0 +1,21 @@ +import { theme } from '@dhis2/ui' +import css from 'styled-jsx/css' + +export default css` + .draggable-item { + all: unset; + display: inline-flex; + cursor: pointer; + border-radius: 3px; + } + + .draggable-item:focus { + outline: 2px solid ${theme.focus}; + outline-offset: -2px; + } + + /* Prevent focus styles when mouse clicking */ + .draggable-item:focus:not(:focus-visible) { + outline: none; + } +`