From 6797dc3168e0f828074cc4b02211049aed54ed0e Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Mon, 7 Sep 2026 17:12:47 +0200 Subject: [PATCH 1/8] feat: multi-layer data table, shared rendering architecture, and cross-cutting fixes Split out from the original feat/datatable-pr7-multilayer, which bundled this together with the Combined data table. This half is everything not strictly Combined-specific, including groundwork built in preparation for it: - Multi-layer support: openIds replaces the old single active-layer id, with isPanelVisible and activeLayerId now persisted in Redux so closing and reopening the panel (or the menu-bar Data Table shortcut) restores exactly what was open and which tab was active, instead of resetting. - LayerSelectorControl replaces the old single-name display with a dropdown covering every eligible layer. - Shared rendering/interaction architecture extracted for reuse by the upcoming Combined table: CellValue/SelectionCheckboxColumn/ SortableColumnHeader components, useSortState/useRowClickSelection/ useRowContextMenuHighlight hooks, and a shared cell-text formatter (util/cellValue.js) with a unified "-" placeholder for empty values. - getNextSorting gained a defaultSortField/defaultSortDirection option so a column's third click resets to the table's actual default sort instead of an unsorted state. - Selection now survives closing the data table or switching layer tabs, instead of clearing. - Per-layer "clear filters" action added directly to each layer's map toolbar (FilterActiveIcon, LayerToolbar). - Event layer: incremental "top-up" loading of newly-visible columns instead of a full reload when more columns need to display. - Tracked entity layer: resolve option-set-coded values to display names for the data table's categorical columns. - Various smaller fixes carried over from the original branch: row sorting/selection cleanup, column picker onChange now takes the caller (not an internal layerId+dispatch), row-count digit grouping, org unit id casing. Co-Authored-By: Claude Sonnet 5 --- cypress/integration/dataTable.cy.js | 54 +-- i18n/en.pot | 41 +- src/actions/__tests__/dataTable.spec.js | 19 + src/actions/dataTable.js | 9 + src/components/app/App.jsx | 5 +- src/components/app/AppMenu.jsx | 2 + src/components/app/FileMenu.jsx | 5 +- src/components/core/FilterActiveIcon.jsx | 12 + src/components/core/icons.jsx | 31 ++ src/components/core/index.js | 2 + .../core/styles/FilterActiveIcon.module.css | 40 ++ .../core/styles/IconButton.module.css | 10 + src/components/datatable/BottomPanel.jsx | 77 +++- src/components/datatable/CellValue.jsx | 73 ++++ src/components/datatable/DataTable.jsx | 398 +++++++++--------- src/components/datatable/DataTableButton.jsx | 47 +++ .../datatable/DateGroupFilterInput.jsx | 16 +- .../datatable/FilterDropdownPopover.jsx | 29 +- src/components/datatable/FilterInput.jsx | 98 ++--- .../datatable/OrgUnitGroupFilterInput.jsx | 36 +- .../datatable/SelectionCheckboxColumn.jsx | 128 ++++++ .../datatable/SortableColumnHeader.jsx | 48 +++ src/components/datatable/TableContextMenu.jsx | 8 +- .../datatable/__tests__/BottomPanel.spec.jsx | 106 ++++- .../datatable/__tests__/CellValue.spec.jsx | 112 +++++ .../__tests__/ColumnPickerControl.spec.jsx | 36 +- .../__tests__/DataTableButton.spec.jsx | 71 ++++ .../__tests__/DateGroupFilterInput.spec.jsx | 157 +++---- .../datatable/__tests__/FilterInput.spec.jsx | 53 ++- .../__tests__/LayerSelectorControl.spec.jsx | 40 ++ .../OrgUnitGroupFilterInput.spec.jsx | 122 +++--- .../__tests__/TableContextMenu.spec.jsx | 5 +- .../__tests__/useRowClickSelection.spec.js | 61 +++ .../useRowContextMenuHighlight.spec.js | 98 +++++ .../__tests__/useRowSelection.spec.js | 72 ++++ .../datatable/__tests__/useSortState.spec.js | 62 +++ .../datatable/__tests__/useTableData.spec.jsx | 43 +- .../controls/ClearFiltersControl.jsx | 10 +- .../controls/ColumnPickerControl.jsx | 24 +- .../datatable/controls/ColumnRow.jsx | 6 +- .../controls/LayerSelectorControl.jsx | 33 ++ .../datatable/controls/RowCountControl.jsx | 24 +- .../datatable/controls/ShowInViewControl.jsx | 2 + .../__tests__/RowCountControl.spec.jsx | 44 ++ .../styles/ColumnPickerControl.module.css | 5 +- .../datatable/styles/BottomPanel.module.css | 19 + .../styles/DataTableButton.module.css | 34 ++ .../datatable/useGroupFilterInput.js | 28 +- .../datatable/useRowClickSelection.js | 36 ++ .../datatable/useRowContextMenuHighlight.js | 38 ++ src/components/datatable/useRowSelection.js | 24 +- src/components/datatable/useSortState.js | 25 ++ src/components/datatable/useTableData.js | 56 +-- .../layers/overlays/OverlayCard.jsx | 116 ++--- .../overlays/__tests__/OverlayCard.spec.jsx | 43 +- .../layers/toolbar/LayerToolbar.jsx | 28 +- .../layers/toolbar/LayerToolbarMoreMenu.jsx | 13 +- .../toolbar/__tests__/LayerToolbar.spec.jsx | 30 ++ .../__tests__/LayerToolbarMoreMenu.spec.jsx | 47 ++- .../__snapshots__/LayerToolbar.spec.jsx.snap | 107 ++++- .../toolbar/styles/LayerToolbar.module.css | 8 +- src/components/map/MapPosition.jsx | 5 +- src/constants/actionTypes.js | 2 + src/hooks/__tests__/useLayersLoader.spec.js | 25 +- src/hooks/useLayersLoader.js | 8 +- src/loaders/__tests__/eventLoader.spec.js | 181 ++++++++ .../__tests__/trackedEntityLoader.spec.js | 25 ++ src/loaders/eventLoader.js | 150 ++++++- src/loaders/trackedEntityLoader.js | 11 + src/reducers/__tests__/dataTable.spec.js | 166 ++++++-- src/reducers/__tests__/selection.spec.js | 48 ++- src/reducers/dataTable.js | 46 +- src/reducers/selection.js | 52 +-- src/util/__tests__/analytics.spec.js | 11 + src/util/__tests__/cellValue.spec.js | 95 +++++ src/util/__tests__/dataTable.spec.js | 118 +++++- src/util/__tests__/filter.spec.js | 136 +++++- src/util/__tests__/styleByDataItem.spec.js | 4 + src/util/__tests__/tableColumns.spec.js | 46 ++ src/util/__tests__/tableHeaders.spec.js | 111 ++++- src/util/__tests__/tableRows.spec.js | 4 +- src/util/analytics.js | 1 + src/util/cellValue.js | 41 ++ src/util/dataTable.js | 20 +- src/util/filter.js | 50 +-- src/util/styleByDataItem.js | 9 +- src/util/tableColumns.js | 38 +- src/util/tableHeaders.js | 107 +++-- src/util/tableRows.js | 3 - 89 files changed, 3614 insertions(+), 925 deletions(-) create mode 100644 src/components/core/FilterActiveIcon.jsx create mode 100644 src/components/core/styles/FilterActiveIcon.module.css create mode 100644 src/components/datatable/CellValue.jsx create mode 100644 src/components/datatable/DataTableButton.jsx create mode 100644 src/components/datatable/SelectionCheckboxColumn.jsx create mode 100644 src/components/datatable/SortableColumnHeader.jsx create mode 100644 src/components/datatable/__tests__/CellValue.spec.jsx create mode 100644 src/components/datatable/__tests__/DataTableButton.spec.jsx create mode 100644 src/components/datatable/__tests__/LayerSelectorControl.spec.jsx create mode 100644 src/components/datatable/__tests__/useRowClickSelection.spec.js create mode 100644 src/components/datatable/__tests__/useRowContextMenuHighlight.spec.js create mode 100644 src/components/datatable/__tests__/useRowSelection.spec.js create mode 100644 src/components/datatable/__tests__/useSortState.spec.js create mode 100644 src/components/datatable/controls/LayerSelectorControl.jsx create mode 100644 src/components/datatable/controls/__tests__/RowCountControl.spec.jsx create mode 100644 src/components/datatable/styles/DataTableButton.module.css create mode 100644 src/components/datatable/useRowClickSelection.js create mode 100644 src/components/datatable/useRowContextMenuHighlight.js create mode 100644 src/components/datatable/useSortState.js create mode 100644 src/util/__tests__/cellValue.spec.js create mode 100644 src/util/cellValue.js diff --git a/cypress/integration/dataTable.cy.js b/cypress/integration/dataTable.cy.js index cfc3698411..ee4468fac0 100644 --- a/cypress/integration/dataTable.cy.js +++ b/cypress/integration/dataTable.cy.js @@ -83,7 +83,7 @@ describe('data table', () => { // Check number of columns cy.getByDataTest('bottom-panel') .findByDataTest('dhis2-uicore-datatablecellhead') - .should('have.length', 10) + .should('have.length', 7) // Filter by Org unit cy.getByDataTest('data-table-column-filter-search-Org unit') @@ -97,8 +97,8 @@ describe('data table', () => { .should('have.length', 7) // Confirm that the sort order is initially ascending by Name - checkTableCell({ row: 0, column: 2, expectedContent: 'Bargbe' }) - checkTableCell({ row: 6, column: 2, expectedContent: 'Upper Bambara' }) + checkTableCell({ row: 0, column: 1, expectedContent: 'Bargbe' }) + checkTableCell({ row: 6, column: 1, expectedContent: 'Upper Bambara' }) // Sort by name (descending) cy.getByDataTest('data-table-column-sort-button-Org unit').click() @@ -110,8 +110,8 @@ describe('data table', () => { cy.get('[data-testid="virtuoso-scroller"]').scrollTo('top') // Confirm that the rows are sorted by Name descending - checkTableCell({ row: 0, column: 2, expectedContent: 'Upper Bambara' }) - checkTableCell({ row: 6, column: 2, expectedContent: 'Bargbe' }) + checkTableCell({ row: 0, column: 1, expectedContent: 'Upper Bambara' }) + checkTableCell({ row: 6, column: 1, expectedContent: 'Bargbe' }) // Filter by Value (numeric) cy.getByDataTest('data-table-column-filter-search-Value') @@ -131,8 +131,8 @@ describe('data table', () => { cy.get('[data-testid="virtuoso-scroller"]').scrollTo('top') // Check that the rows are sorted by Value ascending - checkTableCell({ row: 0, column: 5, expectedContent: '35' }) - checkTableCell({ row: 4, column: 5, expectedContent: '76' }) + checkTableCell({ row: 0, column: 3, expectedContent: '35' }) + checkTableCell({ row: 4, column: 3, expectedContent: '76' }) // Right-click a row and select "View profile" cy.getByDataTest('bottom-panel') @@ -196,7 +196,7 @@ describe('data table', () => { // Check number of columns cy.getByDataTest('bottom-panel') .findByDataTest('dhis2-uicore-datatablecellhead') - .should('have.length', 13) + .should('have.length', 9) cy.getByDataTest('bottom-panel') .findByDataTest('dhis2-uicore-datatablecellhead') @@ -210,8 +210,8 @@ describe('data table', () => { .type(`${ouName}{enter}`) // Check that all the rows have Org unit Moyowa - checkTableCell({ row: 0, column: 3, expectedContent: ouName }) - checkTableCell({ row: 2, column: 3, expectedContent: ouName }) + checkTableCell({ row: 0, column: 1, expectedContent: ouName }) + checkTableCell({ row: 2, column: 1, expectedContent: ouName }) cy.getByDataTest('bottom-panel') .findByDataTest('dhis2-uicore-tablebody') @@ -256,8 +256,8 @@ describe('data table', () => { // Confirm that the rows are sorted by Age in years ascending // (the first click on a new column always sorts ascending) - checkTableCell({ row: 0, column: 10, expectedContent: '6' }) - checkTableCell({ row: 1, column: 10, expectedContent: '32' }) + checkTableCell({ row: 0, column: 7, expectedContent: '6' }) + checkTableCell({ row: 1, column: 7, expectedContent: '32' }) // Right-click a row: Event layers have no profile to view cy.getByDataTest('bottom-panel') @@ -318,7 +318,7 @@ describe('data table', () => { cy.getByDataTest('layers-toggle-button').click() // Confirm that the sort order is initially ascending by Name - checkTableCell({ row: 0, column: 2, expectedContent: 'Bendu CHC' }) + checkTableCell({ row: 0, column: 1, expectedContent: 'Bendu CHC' }) // First click on a new column always sorts ascending cy.getByDataTest('data-table-column-sort-button-Value').click() @@ -327,15 +327,15 @@ describe('data table', () => { cy.get('[data-testid="virtuoso-scroller"]').scrollTo('top') // Check that first row has Tihun CHC with value 28.63 - checkTableCell({ row: 0, column: 2, expectedContent: 'Tihun CHC' }) - checkTableCell({ row: 0, column: 5, expectedContent: '28.63' }) + checkTableCell({ row: 0, column: 1, expectedContent: 'Tihun CHC' }) + checkTableCell({ row: 0, column: 3, expectedContent: '28.63' }) // Check that row 5 has Gbamgbama CHC with value 117.98 - checkTableCell({ row: 5, column: 2, expectedContent: 'Gbamgbama CHC' }) - checkTableCell({ row: 5, column: 5, expectedContent: '117.98' }) + checkTableCell({ row: 5, column: 1, expectedContent: 'Gbamgbama CHC' }) + checkTableCell({ row: 5, column: 3, expectedContent: '117.98' }) // Check that row 6 has no value (undefined) - checkTableCell({ row: 6, column: 5, expectedContent: '' }) + checkTableCell({ row: 6, column: 3, expectedContent: '' }) // Sort descending by Value cy.getByDataTest('data-table-column-sort-button-Value').click() @@ -343,13 +343,13 @@ describe('data table', () => { // Reset scroll position after sorting - see comment above cy.get('[data-testid="virtuoso-scroller"]').scrollTo('top') - checkTableCell({ row: 0, column: 2, expectedContent: 'Gbamgbama CHC' }) - checkTableCell({ row: 0, column: 5, expectedContent: '117.98' }) + checkTableCell({ row: 0, column: 1, expectedContent: 'Gbamgbama CHC' }) + checkTableCell({ row: 0, column: 3, expectedContent: '117.98' }) - checkTableCell({ row: 5, column: 2, expectedContent: 'Tihun CHC' }) - checkTableCell({ row: 5, column: 5, expectedContent: '28.63' }) + checkTableCell({ row: 5, column: 1, expectedContent: 'Tihun CHC' }) + checkTableCell({ row: 5, column: 3, expectedContent: '28.63' }) - checkTableCell({ row: 6, column: 5, expectedContent: '' }) + checkTableCell({ row: 6, column: 3, expectedContent: '' }) // Third click on the same column cycles back to natural (unsorted) // order - there's no dedicated Index column/button any more @@ -359,7 +359,7 @@ describe('data table', () => { cy.get('[data-testid="virtuoso-scroller"]').scrollTo('top') // Check that row 0 range value is empty - checkTableCell({ row: 0, column: 7, expectedContent: '' }) + checkTableCell({ row: 0, column: 5, expectedContent: '' }) // Sort by range, which is a string cy.getByDataTest('data-table-column-sort-button-Range').click() @@ -368,12 +368,12 @@ describe('data table', () => { cy.get('[data-testid="virtuoso-scroller"]').scrollTo('top') // Check that row 0 range value has value '0-40' - checkTableCell({ row: 0, column: 7, expectedContent: '0 – 40' }) + checkTableCell({ row: 0, column: 5, expectedContent: '0 – 40' }) // Check that row 5 range value has value '90 - 120' - checkTableCell({ row: 5, column: 7, expectedContent: '90 – 120' }) + checkTableCell({ row: 5, column: 5, expectedContent: '90 – 120' }) // Check that row 6 range value is empty - checkTableCell({ row: 6, column: 7, expectedContent: '' }) + checkTableCell({ row: 6, column: 5, expectedContent: '' }) }) }) diff --git a/i18n/en.pot b/i18n/en.pot index 4aa2ba8ed9..3065b61474 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-09-07T10:22:12.476Z\n" -"PO-Revision-Date: 2026-09-07T10:22:12.476Z\n" +"POT-Creation-Date: 2026-09-07T15:11:13.032Z\n" +"PO-Revision-Date: 2026-09-07T15:11:13.032Z\n" msgid "2020" msgstr "2020" @@ -155,21 +155,12 @@ msgstr "Operator" msgid "Date" msgstr "Date" -msgid "Select all visible rows" -msgstr "Select all visible rows" - -msgid "Reverse selection of visible rows" -msgstr "Reverse selection of visible rows" - -msgid "Sort by Selected" -msgstr "Sort by Selected" - -msgid "Sort by {{column}}" -msgstr "Sort by {{column}}" - msgid "Edit layer" msgstr "Edit layer" +msgid "Data table" +msgstr "Data table" + msgid "Select a year, month, day or hour" msgstr "Select a year, month, day or hour" @@ -242,6 +233,15 @@ msgstr "to match the rows under it, or type to search" msgid "Select matches" msgstr "Select matches" +msgid "Select all visible rows" +msgstr "Select all visible rows" + +msgid "Reverse selection of visible rows" +msgstr "Reverse selection of visible rows" + +msgid "Sort by Selected" +msgstr "Sort by Selected" + msgid "Selected" msgstr "Selected" @@ -251,6 +251,9 @@ msgstr "Not selected" msgid "All" msgstr "All" +msgid "Sort by {{column}}" +msgstr "Sort by {{column}}" + msgid "Drill up one level" msgstr "Drill up one level" @@ -323,6 +326,9 @@ msgstr "Search all columns" msgid "Highlight color" msgstr "Highlight color" +msgid "Choose a data table to view" +msgstr "Choose a data table to view" + msgid "{{filtered}} of {{total}} rows" msgstr "{{filtered}} of {{total}} rows" @@ -815,6 +821,9 @@ msgstr "Layer is invalid" msgid "Set layer opacity" msgstr "Set layer opacity" +msgid "Clear filters applied in this layer’s table" +msgstr "Clear filters applied in this layer’s table" + msgid "More actions" msgstr "More actions" @@ -2080,8 +2089,8 @@ msgstr "GroupSet used for styling was not found" msgid "Id" msgstr "Id" -msgid "Org unit Id" -msgstr "Org unit Id" +msgid "Org unit id" +msgstr "Org unit id" msgid "Org unit" msgstr "Org unit" diff --git a/src/actions/__tests__/dataTable.spec.js b/src/actions/__tests__/dataTable.spec.js index 217c86a66c..07954d4e2d 100644 --- a/src/actions/__tests__/dataTable.spec.js +++ b/src/actions/__tests__/dataTable.spec.js @@ -1,7 +1,9 @@ import * as types from '../../constants/actionTypes.js' import { closeDataTable, + openDataTable, toggleDataTable, + setActiveDataTableLayer, resizeDataTable, setActiveTimelinePeriod, } from '../dataTable.js' @@ -14,6 +16,14 @@ describe('closeDataTable', () => { }) }) +describe('openDataTable', () => { + it('creates a DATA_TABLE_OPEN action', () => { + expect(openDataTable()).toEqual({ + type: types.DATA_TABLE_OPEN, + }) + }) +}) + describe('toggleDataTable', () => { it('creates a DATA_TABLE_TOGGLE action', () => { expect(toggleDataTable('layer1')).toEqual({ @@ -23,6 +33,15 @@ describe('toggleDataTable', () => { }) }) +describe('setActiveDataTableLayer', () => { + it('creates a DATA_TABLE_ACTIVE_LAYER_SET action', () => { + expect(setActiveDataTableLayer('layer1')).toEqual({ + type: types.DATA_TABLE_ACTIVE_LAYER_SET, + id: 'layer1', + }) + }) +}) + describe('resizeDataTable', () => { it('creates a DATA_TABLE_RESIZE action', () => { expect(resizeDataTable(300)).toEqual({ diff --git a/src/actions/dataTable.js b/src/actions/dataTable.js index 281c7e9cef..94375753aa 100644 --- a/src/actions/dataTable.js +++ b/src/actions/dataTable.js @@ -4,11 +4,20 @@ export const closeDataTable = () => ({ type: types.DATA_TABLE_CLOSE, }) +export const openDataTable = () => ({ + type: types.DATA_TABLE_OPEN, +}) + export const toggleDataTable = (id) => ({ type: types.DATA_TABLE_TOGGLE, id, }) +export const setActiveDataTableLayer = (id) => ({ + type: types.DATA_TABLE_ACTIVE_LAYER_SET, + id, +}) + export const resizeDataTable = (height) => ({ type: types.DATA_TABLE_RESIZE, height, diff --git a/src/components/app/App.jsx b/src/components/app/App.jsx index 91a8f4ab13..116bc033d0 100644 --- a/src/components/app/App.jsx +++ b/src/components/app/App.jsx @@ -2,6 +2,7 @@ import cx from 'classnames' import React, { useEffect, useState } from 'react' import { useSelector } from 'react-redux' import { useLayersLoader } from '../../hooks/useLayersLoader.js' +import { isDataTableOpen } from '../../util/dataTable.js' import BottomPanel from '../datatable/BottomPanel.jsx' import DownloadModeMenu from '../download/DownloadMenubar.jsx' import DownloadSettings from '../download/DownloadSettings.jsx' @@ -35,7 +36,9 @@ const App = () => { const [interpretationsRenderCount, setInterpretationsRenderCount] = useState(1) - const dataTableOpen = useSelector((state) => !!state.dataTable) + const dataTableOpen = useSelector((state) => + isDataTableOpen(state.dataTable) + ) const downloadModeOpen = useSelector((state) => !!state.ui.downloadMode) const detailsPanelOpen = useSelector( (state) => state.ui.rightPanelOpen && !state.orgUnitProfile diff --git a/src/components/app/AppMenu.jsx b/src/components/app/AppMenu.jsx index d0ac79f722..2eb8b5666f 100644 --- a/src/components/app/AppMenu.jsx +++ b/src/components/app/AppMenu.jsx @@ -1,6 +1,7 @@ import { Toolbar, HoverMenuBar } from '@dhis2/analytics' import PropTypes from 'prop-types' import React from 'react' +import DataTableButton from '../datatable/DataTableButton.jsx' import DownloadButton from '../download/DownloadButton.jsx' import InterpretationsToggle from '../interpretations/InterpretationsToggle.jsx' import AddLayerButton from '../layers/overlays/AddLayerButton.jsx' @@ -12,6 +13,7 @@ const AppMenu = ({ onFileMenuAction }) => ( + diff --git a/src/components/app/FileMenu.jsx b/src/components/app/FileMenu.jsx index cfd1b6c188..4ec06b4464 100644 --- a/src/components/app/FileMenu.jsx +++ b/src/components/app/FileMenu.jsx @@ -70,7 +70,8 @@ const FileMenu = ({ onFileMenuAction }) => { const { serverVersion } = useConfig() const { systemSettings, currentUser } = useCachedData() const defaultBasemap = systemSettings.keyDefaultBaseMap - //alerts + + // Alerts const saveAlert = useAlert(ALERT_MESSAGE_DYNAMIC, ALERT_OPTIONS_DYNAMIC) const renameFailedAlert = useAlert(ALERT_MESSAGE_DYNAMIC, ALERT_WARNING) const renameSuccessAlert = useAlert( @@ -145,7 +146,7 @@ const FileMenu = ({ onFileMenuAction }) => { } const onRename = async ({ name, description }) => { - // fetch the original Map + // Fetch the original Map const fetchedMap = await fetchMap({ id: map.id, engine, diff --git a/src/components/core/FilterActiveIcon.jsx b/src/components/core/FilterActiveIcon.jsx new file mode 100644 index 0000000000..437c5b7ddb --- /dev/null +++ b/src/components/core/FilterActiveIcon.jsx @@ -0,0 +1,12 @@ +import { IconFilter16 } from '@dhis2/ui' +import React from 'react' +import styles from './styles/FilterActiveIcon.module.css' + +const FilterActiveIcon = () => ( + + + + +) + +export default FilterActiveIcon diff --git a/src/components/core/icons.jsx b/src/components/core/icons.jsx index 1a067ea7bd..d7eb5b53ec 100644 --- a/src/components/core/icons.jsx +++ b/src/components/core/icons.jsx @@ -49,6 +49,37 @@ export const IconZoomIn16 = () => ( ) +export const IconLayersStack16 = () => ( + + + + + + + +) + export const IconDrag = () => ( span { + display: flex; + align-items: center; + justify-content: center; + line-height: 0; +} + .iconButton svg { color: var(--colors-grey700); } diff --git a/src/components/datatable/BottomPanel.jsx b/src/components/datatable/BottomPanel.jsx index 85e78eae04..458f65b264 100644 --- a/src/components/datatable/BottomPanel.jsx +++ b/src/components/datatable/BottomPanel.jsx @@ -1,6 +1,7 @@ import React, { useRef, useCallback, + useMemo, useState, useEffect, useLayoutEffect, @@ -13,17 +14,24 @@ import { toggleShowOnlyFeaturesInView, setSelectionFilter, setHighlightColor, + toggleDataTable, + setDataTableColumnConfig, + setActiveDataTableLayer, } from '../../actions/dataTable.js' import useDebouncedValue from '../../hooks/useDebouncedValue.js' import useKeyDown from '../../hooks/useKeyDown.js' -import { hasActiveDataTableFilters } from '../../util/dataTable.js' -import ActiveLayerControl from './controls/ActiveLayerControl.jsx' +import { + getEligibleDataTableLayers, + hasActiveDataTableFilters, +} from '../../util/dataTable.js' +import { useCachedData } from '../cachedDataProvider/CachedDataProvider.jsx' import ClearFiltersControl from './controls/ClearFiltersControl.jsx' import CloseControl from './controls/CloseControl.jsx' import CollapseControl from './controls/CollapseControl.jsx' import ColumnPickerControl from './controls/ColumnPickerControl.jsx' import GlobalSearchControl from './controls/GlobalSearchControl.jsx' import HighlightColorControl from './controls/HighlightColorControl.jsx' +import LayerSelectorControl from './controls/LayerSelectorControl.jsx' import ResizeHandleControl from './controls/ResizeHandleControl.jsx' import RowCountControl from './controls/RowCountControl.jsx' import ShowInViewControl from './controls/ShowInViewControl.jsx' @@ -36,10 +44,27 @@ const MIN_HEIGHT = 50 const EMPTY_FILTERS = {} const BottomPanel = () => { - const activeLayerId = useSelector((state) => state.dataTable) - const activeLayer = useSelector((state) => - state.map.mapViews.find((l) => l.id === activeLayerId) + const { + systemSettings: { keyAnalysisDigitGroupSeparator }, + } = useCachedData() + const { openIds, activeLayerId: storedActiveLayerId } = useSelector( + (state) => state.dataTable ) + const mapViews = useSelector((state) => state.map.mapViews) + const activeLayerId = + storedActiveLayerId && openIds.includes(storedActiveLayerId) + ? storedActiveLayerId + : openIds[openIds.length - 1] ?? null + + const eligibleLayers = useMemo(() => { + const loaded = getEligibleDataTableLayers(mapViews).reverse() + const stillOpen = openIds + .map((id) => mapViews.find((l) => l.id === id)) + .filter((l) => l && !loaded.some((el) => el.id === l.id)) + return [...loaded, ...stillOpen] + }, [mapViews, openIds]) + + const activeLayer = mapViews.find((l) => l.id === activeLayerId) const dataFilters = activeLayer?.dataFilters ?? EMPTY_FILTERS const showOnlyFeaturesInView = useSelector( (state) => state.ui.showOnlyFeaturesInView @@ -76,7 +101,10 @@ const BottomPanel = () => { const onControlsDoubleClick = useCallback( (e) => { - if (e.target.closest('button, input, label')) { + if ( + e.target.closest('button, input, label, select') || + !e.currentTarget.contains(e.target) + ) { return } toggleCollapsed() @@ -138,12 +166,14 @@ const BottomPanel = () => { const onClearFilters = useCallback(() => { dispatch(clearDataFilters(activeLayerId)) - dispatch(setSelectionFilter([])) - setSearchInputValue('') if (showOnlyFeaturesInView) { dispatch(toggleShowOnlyFeaturesInView()) } - }, [dispatch, activeLayerId, showOnlyFeaturesInView]) + if (selectionFilter?.length) { + dispatch(setSelectionFilter([])) + } + setSearchInputValue('') + }, [dispatch, activeLayerId, showOnlyFeaturesInView, selectionFilter]) const onToggleShowOnlyFeaturesInView = useCallback(() => { dispatch(toggleShowOnlyFeaturesInView()) @@ -178,9 +208,13 @@ const BottomPanel = () => { useEffect(() => { const observer = new ResizeObserver(() => { - if (panelRef.current) { - setPanelWidth(panelRef.current.getBoundingClientRect().width) + if (!panelRef.current) { + return } + const width = Math.round( + panelRef.current.getBoundingClientRect().width + ) + setPanelWidth((prev) => (prev === width ? prev : width)) }) if (panelRef.current) { observer.observe(panelRef.current) @@ -205,16 +239,29 @@ const BottomPanel = () => { onClick={toggleCollapsed} /> - + { + dispatch(setActiveDataTableLayer(id)) + if (!openIds.includes(id)) { + dispatch(toggleDataTable(id)) + } + }} + /> + dispatch( + setDataTableColumnConfig(activeLayerId, config) + ) + } /> { {
({ + isColorCell: renderer === RENDERER_COLOR, + isIconCell: renderer === RENDERER_ICON, + isDateCell: renderer === RENDERER_DATE, + isDateOnlyCell: type === TYPE_DATE, + isOrgUnitHierarchyCell: renderer === RENDERER_ORG_UNIT, + isOrgUnitNameCell: renderer === RENDERER_ORG_UNIT_NAME, + isBooleanCell: renderer === RENDERER_BOOLEAN, +}) + +const NO_VALUE = '—' + +const CellValue = ({ + value, + renderer, + type, + orgUnitIdToName, + keyAnalysisDigitGroupSeparator, +}) => { + if (value == null) { + return NO_VALUE + } + + const { isIconCell } = getCellRendererFlags(renderer, type) + + if (isIconCell) { + return ( + { + e.target.style.visibility = 'hidden' + }} + /> + ) + } + + return formatCellText(value, { + renderer, + type, + orgUnitIdToName, + keyAnalysisDigitGroupSeparator, + }) +} + +CellValue.propTypes = { + keyAnalysisDigitGroupSeparator: PropTypes.string, + orgUnitIdToName: PropTypes.instanceOf(Map), + renderer: PropTypes.string, + type: PropTypes.string, + value: PropTypes.oneOfType([ + PropTypes.string, + PropTypes.number, + PropTypes.bool, + ]), +} + +export default CellValue diff --git a/src/components/datatable/DataTable.jsx b/src/components/datatable/DataTable.jsx index 1e10612949..e88295e101 100644 --- a/src/components/datatable/DataTable.jsx +++ b/src/components/datatable/DataTable.jsx @@ -1,43 +1,37 @@ import i18n from '@dhis2/d2-i18n' import { DataTableRow, - DataTableColumnHeader, + DataTableCell, ComponentCover, CenteredContent, CircularLoader, - IconSync16, } from '@dhis2/ui' import cx from 'classnames' import PropTypes from 'prop-types' -import React, { - useReducer, - useCallback, - useMemo, - useEffect, - useRef, - useState, -} from 'react' +import React, { useCallback, useMemo, useEffect, useRef, useState } from 'react' import { useSelector, useDispatch } from 'react-redux' import { TableVirtuoso } from 'react-virtuoso' +import { setDataFilter, clearDataFilter } from '../../actions/dataFilters.js' import { setSelectionFilter } from '../../actions/dataTable.js' import { highlightFeature } from '../../actions/feature.js' import { editLayer, setForceClientCluster } from '../../actions/layers.js' import { toggleFeatureSelection, selectFeatureRange, + selectAllFeatures, + clearSelection, } from '../../actions/selection.js' import { SENTINEL_SELECTED_ROW, - SORT_ASCENDING, + ORG_UNIT_ID_DATA_KEY, } from '../../constants/dataTable.js' +import { isDarkColor } from '../../util/colors.js' import { buildFeatureIndex, - getNextSorting, - getRowClickAction, + getLayerSelectedIds, getRowId, hasActiveDataTableFilters, isFilterable, - shouldClearFeatureHighlight, } from '../../util/dataTable.js' import { getPinnedCellProps, @@ -46,22 +40,29 @@ import { getVisibleHeaders, } from '../../util/tableColumns.js' import { useCachedData } from '../cachedDataProvider/CachedDataProvider.jsx' -import { SortIcon } from '../core/icons.jsx' +import CellValue, { getCellRendererFlags } from './CellValue.jsx' import FilterInput from './FilterInput.jsx' -import RowCells from './RowCells.jsx' +import { + SelectionCheckboxHeaderCell, + SelectionCheckboxCell, +} from './SelectionCheckboxColumn.jsx' import SelectionFilterButton from './SelectionFilterButton.jsx' +import SortableColumnHeader from './SortableColumnHeader.jsx' import styles from './styles/DataTable.module.css' import TableContextMenu from './TableContextMenu.jsx' import TableComponents from './TableVirtuosoComponents.jsx' -import TopTooltip from './TopTooltip.jsx' import { useColumnWidths } from './useColumnWidths.js' +import { useRowClickSelection } from './useRowClickSelection.js' +import { useRowContextMenuHighlight } from './useRowContextMenuHighlight.js' import { useRowSelection } from './useRowSelection.js' +import { useSortState } from './useSortState.js' import { useTableData } from './useTableData.js' const TABLE_STYLE = { height: '100%', width: '100%' } const VIEWPORT_OVERSCAN = { top: 400, bottom: 400 } const Table = ({ + activeLayerId, availableWidth, onCountChange, onHeadersChange, @@ -74,7 +75,6 @@ const Table = ({ const virtuosoRef = useRef(null) const { mapViews } = useSelector((state) => state.map) - const activeLayerId = useSelector((state) => state.dataTable) const dispatch = useDispatch() const feature = useSelector((state) => state.feature) @@ -84,23 +84,10 @@ const Table = ({ ) const mapBounds = useSelector((state) => state.ui.mapBounds) const selectionFilter = useSelector((state) => state.ui.selectionFilter) - const [{ sortField, sortDirection }, setSorting] = useReducer( - (sorting, newSorting) => ({ ...sorting, ...newSorting }), - { - sortField: 'name', - sortDirection: SORT_ASCENDING, - } - ) + const { sortField, sortDirection, sortData } = useSortState('name') const layer = mapViews.find((l) => l.id === activeLayerId) - const sortData = useCallback( - ({ name }) => { - setSorting(getNextSorting(name, { sortField, sortDirection })) - }, - [sortField, sortDirection] - ) - // Read via ref rather than a dependency, so this callback stays stable // across hovers instead of getting a new identity on every single mouse-enter const featureRef = useRef(feature) @@ -127,14 +114,15 @@ const Table = ({ }, [dispatch, layer.id] ) - const clearFeatureHighlight = useCallback( - (event) => { - if (shouldClearFeatureHighlight(event)) { - dispatch(highlightFeature(null)) - } - }, + const onClearHighlight = useCallback( + () => dispatch(highlightFeature(null)), [dispatch] ) + const { onContextMenuOpen, guardedClear, onMenuClose } = + useRowContextMenuHighlight({ + onPin: setFeatureHighlight, + onClear: onClearHighlight, + }) const featureById = useMemo( () => buildFeatureIndex(layer.data), @@ -146,6 +134,7 @@ const Table = ({ const onRowContextMenu = useCallback( (e, row) => { e.preventDefault() + onContextMenuOpen(row) const id = getRowId(row) const feature = featureById.get(id) setTableContextMenu({ @@ -154,11 +143,11 @@ const Table = ({ featureProps: feature?.properties ?? { id }, }) }, - [featureById] + [featureById, onContextMenuOpen] ) const selectedIds = useMemo( - () => (selection.layerId === layer.id ? selection.ids : []), + () => getLayerSelectedIds(selection, layer.id), [selection, layer.id] ) const selectedIdSet = useMemo(() => new Set(selectedIds), [selectedIds]) @@ -233,37 +222,19 @@ const Table = ({ onCountChange?.(totalCount, filteredCount) }, [onCountChange, totalCount, filteredCount]) - const lastClickedRowIndexRef = useRef(null) - - const onRowClick = useCallback( - (row, event) => { - const id = getRowId(row) - - if (!id || !rows) { - return - } - - const rowIndex = rows.findIndex((r) => getRowId(r) === id) - const action = getRowClickAction(event, { - id, - rowIndex, - rows, - lastClickedRowIndex: lastClickedRowIndexRef.current, - }) - - if (!action) { - return - } - - if (action.type === 'range') { - dispatch(selectFeatureRange(action.ids, layer.id)) - } else { - dispatch(toggleFeatureSelection(action.id, layer.id)) - } - lastClickedRowIndexRef.current = rowIndex - }, - [dispatch, layer.id, rows] + const onToggleRow = useCallback( + (id) => dispatch(toggleFeatureSelection(id, layer.id)), + [dispatch, layer.id] ) + const onSelectRowRange = useCallback( + (ids) => dispatch(selectFeatureRange(ids, layer.id)), + [dispatch, layer.id] + ) + const onRowClick = useRowClickSelection({ + rows, + onToggle: onToggleRow, + onSelectRange: onSelectRowRange, + }) const onRowDoubleClick = useCallback( (row) => { @@ -303,7 +274,7 @@ const Table = ({ const tableContext = useMemo( () => ({ onMouseEnter: setFeatureHighlight, - onMouseLeave: clearFeatureHighlight, + onMouseLeave: guardedClear, onContextMenu: onRowContextMenu, onRowClick, onRowDoubleClick, @@ -316,7 +287,7 @@ const Table = ({ }), [ setFeatureHighlight, - clearFeatureHighlight, + guardedClear, onRowContextMenu, onRowClick, onRowDoubleClick, @@ -359,12 +330,23 @@ const Table = ({ [rows] ) + const onSelectionChange = useCallback( + (nextIds) => { + if (nextIds.length) { + dispatch(selectAllFeatures(nextIds, layer.id)) + } else { + dispatch(clearSelection()) + } + }, + [dispatch, layer.id] + ) + const { isAllSelected, onToggleSelectAll, onReverseSelection } = useRowSelection({ selectedIds, selectedIdSet, allRowIds, - layerId: layer.id, + onChange: onSelectionChange, }) const computeItemKey = useCallback( @@ -375,11 +357,18 @@ const Table = ({ const fixedHeaderContent = useCallback( () => ( - + sortData({ name: SENTINEL_SELECTED_ROW }) + } onFilterIconClick={Function.prototype} showFilter={true} filter={ @@ -390,53 +379,7 @@ const Table = ({ } /> } - > -
- - - - - - - - - -
- + /> {visibleHeaders.map( ({ name, dataKey, type, optionSet, renderer }, index) => { const { fixed, left, isLastPinned } = @@ -446,11 +389,17 @@ const Table = ({ columnWidths, }) return ( - + dispatch( + setDataFilter( + activeLayerId, + dataKey, + value + ) + ) + } + onClear={() => + dispatch( + clearDataFilter( + activeLayerId, + dataKey + ) + ) + } /> ) } @@ -477,43 +453,14 @@ const Table = ({ ? `${columnWidths[index]}px` : 'auto' } - > - - - {name} - - - - - - + /> ) } )} ), [ + activeLayerId, isCheckboxColumnPinned, selectionFilter, dispatch, @@ -524,6 +471,8 @@ const Table = ({ sortDirection, visibleHeaders, pinnedLeftOffsets, + layer.dataFilters, + layer.optionSetOptionsByCode, pinnedColumnCount, columnWidths, columnOptions, @@ -534,47 +483,6 @@ const Table = ({ ] ) - const onToggleSelection = useCallback( - (rowId) => dispatch(toggleFeatureSelection(rowId, layer.id)), - [dispatch, layer.id] - ) - - const itemContent = useCallback( - (_, row) => ( - - ), - [ - visibleHeaders, - selectedIdSet, - feature, - layer.id, - isCheckboxColumnPinned, - pinnedLeftOffsets, - pinnedColumnCount, - columnWidths, - rendererByDataKey, - typeByDataKey, - keyAnalysisDigitGroupSeparator, - orgUnitIdToName, - onToggleSelection, - ] - ) - if (error) { return (

@@ -601,7 +509,93 @@ const Table = ({ computeItemKey={computeItemKey} increaseViewportBy={VIEWPORT_OVERSCAN} fixedHeaderContent={fixedHeaderContent} - itemContent={itemContent} + itemContent={(_, row) => { + const rowId = getRowId(row) + const isSelected = !!rowId && selectedIdSet.has(rowId) + const isHovered = + !!rowId && + feature?.id === rowId && + feature?.layerId === layer.id + + const cellsByDataKey = new Map( + row.map((cell) => [cell.dataKey, cell]) + ) + + return ( + <> + rowId && onToggleRow(rowId)} + /> + {visibleHeaders.map(({ dataKey }, index) => { + const cell = cellsByDataKey.get(dataKey) + if (!cell) { + return null + } + const { value, align } = cell + const { fixed, left, width, isLastPinned } = + getPinnedCellProps(dataKey, index, { + pinnedLeftOffsets, + pinnedColumnCount, + columnWidths, + }) + const renderer = rendererByDataKey.get(dataKey) + const type = typeByDataKey.get(dataKey) + const { isColorCell } = getCellRendererFlags( + renderer, + type + ) + return ( + + + + ) + })} + + ) + }} /> {(isLoading || layer?.isLoaded === false || layer?.isLoading) && ( @@ -622,13 +616,17 @@ const Table = ({ layer={layer} selectedIds={selectedIds} filteredIds={hasActiveFilters ? allRowIds : null} - onClose={() => setTableContextMenu(null)} + onClose={(highlightChanged) => { + setTableContextMenu(null) + onMenuClose(highlightChanged) + }} /> ) } Table.propTypes = { + activeLayerId: PropTypes.string, availableWidth: PropTypes.number, globalSearch: PropTypes.string, onClearFilters: PropTypes.func, diff --git a/src/components/datatable/DataTableButton.jsx b/src/components/datatable/DataTableButton.jsx new file mode 100644 index 0000000000..39232a5c88 --- /dev/null +++ b/src/components/datatable/DataTableButton.jsx @@ -0,0 +1,47 @@ +import i18n from '@dhis2/d2-i18n' +import React from 'react' +import { useDispatch, useSelector } from 'react-redux' +import { + closeDataTable, + openDataTable, + toggleDataTable, +} from '../../actions/dataTable.js' +import { + getEligibleDataTableLayers, + isDataTableOpen, +} from '../../util/dataTable.js' +import styles from './styles/DataTableButton.module.css' + +const DataTableButton = () => { + const dispatch = useDispatch() + const dataTable = useSelector((state) => state.dataTable) + const mapViews = useSelector((state) => state.map.mapViews) + const eligibleLayers = getEligibleDataTableLayers(mapViews) + + const onClick = () => { + if (isDataTableOpen(dataTable)) { + dispatch(closeDataTable()) + return + } + if (dataTable.openIds.length > 0) { + dispatch(openDataTable()) + return + } + if (eligibleLayers.length >= 1) { + dispatch(toggleDataTable(eligibleLayers[0].id)) + } + } + + return ( + + ) +} + +export default DataTableButton diff --git a/src/components/datatable/DateGroupFilterInput.jsx b/src/components/datatable/DateGroupFilterInput.jsx index 3c7131c764..dda9c29aba 100644 --- a/src/components/datatable/DateGroupFilterInput.jsx +++ b/src/components/datatable/DateGroupFilterInput.jsx @@ -1,7 +1,6 @@ import i18n from '@dhis2/d2-i18n' import PropTypes from 'prop-types' import React, { useCallback } from 'react' -import { setDataFilter } from '../../actions/dataFilters.js' import { DATE_GROUPS_GRANULARITY } from '../../constants/dataTable.js' import { buildDateGroupTree, @@ -29,13 +28,12 @@ const parseFilterValue = (filterValue) => ({ const sanitizeInput = (value) => value.replace(DATE_INPUT_DISALLOWED, '') -const commitSearch = (text, { dispatch, layerId, dataKey }) => - dispatch(setDataFilter(layerId, dataKey, text)) +const commitSearch = (text, { onChange }) => onChange(text) const DateGroupFilterInput = ({ - dataKey, name, - layerId, + onChange, + onClear, filterValue, options, type, @@ -46,8 +44,8 @@ const DateGroupFilterInput = ({ ) const groupFilter = useGroupFilterInput({ - dataKey, - layerId, + onChange, + onClear, filterValue, options, granularity: DATE_GROUPS_GRANULARITY, @@ -70,17 +68,17 @@ const DateGroupFilterInput = ({ } DateGroupFilterInput.propTypes = { - dataKey: PropTypes.string.isRequired, name: PropTypes.string.isRequired, options: PropTypes.arrayOf(PropTypes.shape({ value: PropTypes.string })) .isRequired, type: PropTypes.string.isRequired, + onChange: PropTypes.func.isRequired, + onClear: PropTypes.func.isRequired, filterValue: PropTypes.oneOfType([ PropTypes.string, PropTypes.arrayOf(PropTypes.string), PropTypes.object, ]), - layerId: PropTypes.string, } export default DateGroupFilterInput diff --git a/src/components/datatable/FilterDropdownPopover.jsx b/src/components/datatable/FilterDropdownPopover.jsx index ae2f257c0c..676b95cddc 100644 --- a/src/components/datatable/FilterDropdownPopover.jsx +++ b/src/components/datatable/FilterDropdownPopover.jsx @@ -1,6 +1,7 @@ import { Layer, Popper } from '@dhis2/ui' import PropTypes from 'prop-types' import React from 'react' +import useKeyDown from '../../hooks/useKeyDown.js' const ESTIMATED_POPOVER_HEIGHT = 340 // Rough popover height used to flip the dropdown when there isn't room to open downward @@ -28,18 +29,22 @@ export const FilterDropdownPopover = ({ onClickOutside, className, children, -}) => ( - - - {children} - - -) +}) => { + useKeyDown('Escape', onClickOutside) + + return ( + + + {children} + + + ) +} FilterDropdownPopover.propTypes = { children: PropTypes.node.isRequired, diff --git a/src/components/datatable/FilterInput.jsx b/src/components/datatable/FilterInput.jsx index 846f23acae..a05337e8e3 100644 --- a/src/components/datatable/FilterInput.jsx +++ b/src/components/datatable/FilterInput.jsx @@ -3,9 +3,7 @@ import { Input, IconFilter16, IconSync16 } from '@dhis2/ui' import cx from 'classnames' import PropTypes from 'prop-types' import React, { useCallback, useMemo, useRef, useState } from 'react' -import { useDispatch, useSelector } from 'react-redux' import { Virtuoso } from 'react-virtuoso' -import { setDataFilter, clearDataFilter } from '../../actions/dataFilters.js' import { SENTINEL_ANY_VALUE, SENTINEL_NO_VALUE, @@ -80,15 +78,15 @@ const NUMERIC_INPUT_DISALLOWED = /[^0-9.\-<>=,&\s]/g const SearchableFilterPopover = React.memo(function SearchableFilterPopover({ dataKey, name, - layerId, filterValue, options, resolveLabel, type, renderer, allowCustomFilter = true, + onChange, + onClear, }) { - const dispatch = useDispatch() const anchorRef = useRef(null) const listRef = useRef(null) const [isOpen, setIsOpen] = useState(false) @@ -109,10 +107,7 @@ const SearchableFilterPopover = React.memo(function SearchableFilterPopover({ const { dropdownPlacement, dropdownSide, tooltipPlacement } = getDropdownPlacement(anchorRect) - const applyValues = (next) => - next.length - ? dispatch(setDataFilter(layerId, dataKey, next)) - : dispatch(clearDataFilter(layerId, dataKey)) + const applyValues = (next) => (next.length ? onChange(next) : onClear()) const toggleValue = (value) => { const next = selected.includes(value) @@ -126,7 +121,7 @@ const SearchableFilterPopover = React.memo(function SearchableFilterPopover({ const applyCustomFilter = (text) => { if (!text) { - dispatch(clearDataFilter(layerId, dataKey)) + onClear() return } if (isOrgUnitRenderer) { @@ -134,16 +129,14 @@ const SearchableFilterPopover = React.memo(function SearchableFilterPopover({ const values = realValues.filter((value) => resolveLabel(value).toLowerCase().includes(lower) ) - dispatch( - setDataFilter(layerId, dataKey, { - values, - searchDerived: true, - searchText: text, - }) - ) + onChange({ + values, + searchDerived: true, + searchText: text, + }) return } - dispatch(setDataFilter(layerId, dataKey, text)) + onChange(text) } const isIconColumn = renderer === RENDERER_ICON @@ -234,7 +227,7 @@ const SearchableFilterPopover = React.memo(function SearchableFilterPopover({ const trimmed = sanitized.trim() if (trimmed === '') { if (hasActiveFilter) { - dispatch(clearDataFilter(layerId, dataKey)) + onClear() } return } @@ -296,10 +289,6 @@ const SearchableFilterPopover = React.memo(function SearchableFilterPopover({ onEnterKey() closePopover() break - case 'Escape': - event.preventDefault() - closePopover() - break default: break } @@ -505,12 +494,13 @@ SearchableFilterPopover.propTypes = { .isRequired, resolveLabel: PropTypes.func.isRequired, type: PropTypes.string.isRequired, + onChange: PropTypes.func.isRequired, + onClear: PropTypes.func.isRequired, allowCustomFilter: PropTypes.bool, filterValue: PropTypes.oneOfType([ PropTypes.string, PropTypes.arrayOf(PropTypes.string), ]), - layerId: PropTypes.string, renderer: PropTypes.string, } @@ -553,18 +543,27 @@ PlainSearchableFilter.propTypes = { type: PropTypes.string, } -const OptionSetSearchableFilter = ({ optionSetId, ...props }) => { - const { optionSet } = useOptionSet(optionSetId) +const OptionSetSearchableFilter = ({ + optionSetId, + resolvedOptionNames, + ...props +}) => { + const { optionSet } = useOptionSet( + resolvedOptionNames ? undefined : optionSetId + ) const optionByCode = useMemo(() => { + if (resolvedOptionNames) { + return new Map(Object.entries(resolvedOptionNames)) + } const map = new Map() - optionSet?.options.forEach((o) => map.set(o.code, o)) + optionSet?.options.forEach((o) => map.set(o.code, o.name)) return map - }, [optionSet]) + }, [resolvedOptionNames, optionSet]) const resolveLabel = useCallback( (value) => value === SENTINEL_NO_VALUE ? i18n.t('No value') - : optionByCode.get(value)?.name ?? value, + : optionByCode.get(value) ?? value, [optionByCode] ) return ( @@ -578,6 +577,7 @@ const OptionSetSearchableFilter = ({ optionSetId, ...props }) => { OptionSetSearchableFilter.propTypes = { optionSetId: PropTypes.string.isRequired, + resolvedOptionNames: PropTypes.object, } const FilterInput = React.memo(function FilterInput({ @@ -586,33 +586,22 @@ const FilterInput = React.memo(function FilterInput({ name, options, optionSetId, + resolvedOptionNames, renderer, orgUnitIdToName, + filterValue, + onChange, + onClear, }) { - const dataTable = useSelector((state) => state.dataTable) - const map = useSelector((state) => state.map) - - const overlay = - dataTable && map.mapViews.find((layer) => layer.id === dataTable) - - let layerId - let filters - if (overlay) { - layerId = overlay.id - filters = overlay.dataFilters || {} - } - - const filterValue = filters?.[dataKey] - const isDateType = type === TYPE_DATE || type === TYPE_DATETIME || type === TYPE_TIME if (isDateType) { return ( ) } @@ -652,12 +643,13 @@ const FilterInput = React.memo(function FilterInput({ ) }) @@ -666,10 +658,18 @@ FilterInput.propTypes = { dataKey: PropTypes.string.isRequired, name: PropTypes.string.isRequired, type: PropTypes.string.isRequired, + onChange: PropTypes.func.isRequired, + onClear: PropTypes.func.isRequired, + filterValue: PropTypes.oneOfType([ + PropTypes.string, + PropTypes.arrayOf(PropTypes.string), + PropTypes.object, + ]), optionSetId: PropTypes.string, options: PropTypes.arrayOf(PropTypes.shape({ value: PropTypes.string })), orgUnitIdToName: PropTypes.instanceOf(Map), renderer: PropTypes.string, + resolvedOptionNames: PropTypes.object, } export default FilterInput diff --git a/src/components/datatable/OrgUnitGroupFilterInput.jsx b/src/components/datatable/OrgUnitGroupFilterInput.jsx index 84198946f9..b658831854 100644 --- a/src/components/datatable/OrgUnitGroupFilterInput.jsx +++ b/src/components/datatable/OrgUnitGroupFilterInput.jsx @@ -1,7 +1,6 @@ import i18n from '@dhis2/d2-i18n' import PropTypes from 'prop-types' import React, { useCallback } from 'react' -import { setDataFilter, clearDataFilter } from '../../actions/dataFilters.js' import { ORG_UNIT_GROUPS_GRANULARITY } from '../../constants/dataTable.js' import { isOrgUnitGroupFilter } from '../../util/filter.js' import { @@ -36,9 +35,9 @@ const parseFilterValue = (filterValue) => ({ }) const OrgUnitGroupFilterInput = ({ - dataKey, name, - layerId, + onChange, + onClear, filterValue, options, idToName, @@ -50,10 +49,7 @@ const OrgUnitGroupFilterInput = ({ ) const commitSearch = useCallback( - ( - text, - { tree, dispatch, layerId: layerIdArg, dataKey: dataKeyArg } - ) => { + (text, { tree, onChange: onChangeArg }) => { const matches = getOrgUnitSearchMatches( tree, text.toLowerCase(), @@ -66,25 +62,19 @@ const OrgUnitGroupFilterInput = ({ .map((key) => nodeByKey.get(key)) .filter(Boolean) .map((node) => node.prefix) - if (!matchedPrefixes.length) { - dispatch(clearDataFilter(layerIdArg, dataKeyArg)) - return - } - dispatch( - setDataFilter(layerIdArg, dataKeyArg, { - granularity: ORG_UNIT_GROUPS_GRANULARITY, - prefixes: matchedPrefixes, - searchDerived: true, - searchText: text, - }) - ) + onChangeArg({ + granularity: ORG_UNIT_GROUPS_GRANULARITY, + prefixes: matchedPrefixes, + searchDerived: true, + searchText: text, + }) }, [idToName] ) const groupFilter = useGroupFilterInput({ - dataKey, - layerId, + onChange, + onClear, filterValue, options, granularity: ORG_UNIT_GROUPS_GRANULARITY, @@ -106,17 +96,17 @@ const OrgUnitGroupFilterInput = ({ } OrgUnitGroupFilterInput.propTypes = { - dataKey: PropTypes.string.isRequired, idToName: PropTypes.instanceOf(Map).isRequired, name: PropTypes.string.isRequired, options: PropTypes.arrayOf(PropTypes.shape({ value: PropTypes.string })) .isRequired, + onChange: PropTypes.func.isRequired, + onClear: PropTypes.func.isRequired, filterValue: PropTypes.oneOfType([ PropTypes.string, PropTypes.arrayOf(PropTypes.string), PropTypes.object, ]), - layerId: PropTypes.string, } export default OrgUnitGroupFilterInput diff --git a/src/components/datatable/SelectionCheckboxColumn.jsx b/src/components/datatable/SelectionCheckboxColumn.jsx new file mode 100644 index 0000000000..22a9171d12 --- /dev/null +++ b/src/components/datatable/SelectionCheckboxColumn.jsx @@ -0,0 +1,128 @@ +import i18n from '@dhis2/d2-i18n' +import { DataTableColumnHeader, DataTableCell, IconSync16 } from '@dhis2/ui' +import cx from 'classnames' +import PropTypes from 'prop-types' +import React from 'react' +import { SENTINEL_SELECTED_ROW } from '../../constants/dataTable.js' +import { SortIcon } from '../core/icons.jsx' +import styles from './styles/DataTable.module.css' +import TopTooltip from './TopTooltip.jsx' + +export const SelectionCheckboxHeaderCell = ({ + fixed, + left, + isAllSelected, + onToggleSelectAll, + onReverseSelection, + disabled, + sortField, + sortDirection, + onSortBySelected, + filter, + showFilter, + onFilterIconClick, +}) => ( + +

+ + + + + + + {onSortBySelected && ( + + + + )} +
+ +) + +SelectionCheckboxHeaderCell.propTypes = { + disabled: PropTypes.bool, + filter: PropTypes.node, + fixed: PropTypes.bool, + isAllSelected: PropTypes.bool, + left: PropTypes.string, + showFilter: PropTypes.bool, + sortDirection: PropTypes.string, + sortField: PropTypes.string, + onFilterIconClick: PropTypes.func, + onReverseSelection: PropTypes.func, + onSortBySelected: PropTypes.func, + onToggleSelectAll: PropTypes.func, +} + +export const SelectionCheckboxCell = ({ + fixed, + left, + width, + className, + isSelected, + isHovered, + onToggle, +}) => ( + + e.stopPropagation()} + /> + +) + +SelectionCheckboxCell.propTypes = { + className: PropTypes.string, + fixed: PropTypes.bool, + isHovered: PropTypes.bool, + isSelected: PropTypes.bool, + left: PropTypes.string, + width: PropTypes.string, + onToggle: PropTypes.func, +} diff --git a/src/components/datatable/SortableColumnHeader.jsx b/src/components/datatable/SortableColumnHeader.jsx new file mode 100644 index 0000000000..9f2b4e3573 --- /dev/null +++ b/src/components/datatable/SortableColumnHeader.jsx @@ -0,0 +1,48 @@ +import i18n from '@dhis2/d2-i18n' +import { DataTableColumnHeader } from '@dhis2/ui' +import PropTypes from 'prop-types' +import React from 'react' +import { SortIcon } from '../core/icons.jsx' +import styles from './styles/DataTable.module.css' +import TopTooltip from './TopTooltip.jsx' + +const SortableColumnHeader = ({ + name, + dataKey, + sortField, + sortDirection, + onSort, + dataTestPrefix, + ...columnHeaderProps +}) => ( + + + {name} + + + + + +) + +SortableColumnHeader.propTypes = { + dataKey: PropTypes.string.isRequired, + dataTestPrefix: PropTypes.string.isRequired, + name: PropTypes.string.isRequired, + onSort: PropTypes.func.isRequired, + sortDirection: PropTypes.string, + sortField: PropTypes.string, +} + +export default SortableColumnHeader diff --git a/src/components/datatable/TableContextMenu.jsx b/src/components/datatable/TableContextMenu.jsx index eb69c3e55d..e62296b6b5 100644 --- a/src/components/datatable/TableContextMenu.jsx +++ b/src/components/datatable/TableContextMenu.jsx @@ -167,7 +167,7 @@ const TableContextMenu = ({ zoom: true, }) ) - onClose() + onClose(true) }} /> )} @@ -183,7 +183,7 @@ const TableContextMenu = ({ zoom: true, }) ) - onClose() + onClose(true) }} /> diff --git a/src/components/datatable/__tests__/BottomPanel.spec.jsx b/src/components/datatable/__tests__/BottomPanel.spec.jsx index 314397bfae..4dbbe13d8b 100644 --- a/src/components/datatable/__tests__/BottomPanel.spec.jsx +++ b/src/components/datatable/__tests__/BottomPanel.spec.jsx @@ -1,7 +1,8 @@ -import { render, fireEvent } from '@testing-library/react' +import { render, fireEvent, screen } from '@testing-library/react' import React from 'react' import { Provider } from 'react-redux' import configureMockStore from 'redux-mock-store' +import { THEMATIC_LAYER } from '../../../constants/layers.js' import WindowDimensionsProvider from '../../WindowDimensionsProvider.jsx' import BottomPanel from '../BottomPanel.jsx' @@ -11,6 +12,12 @@ jest.mock('../DataTable.jsx', () => { return DataTableMock }) +jest.mock('../../cachedDataProvider/CachedDataProvider.jsx', () => ({ + useCachedData: () => ({ + systemSettings: { keyAnalysisDigitGroupSeparator: ',' }, + }), +})) + const mockStore = configureMockStore() // jsdom doesn't implement pointer capture or ResizeObserver @@ -26,7 +33,7 @@ beforeAll(() => { const DATA_TABLE_HEIGHT = 300 -const renderBottomPanel = () => { +const renderBottomPanel = ({ dataTable, mapViews } = {}) => { const store = mockStore({ ui: { dataTableHeight: DATA_TABLE_HEIGHT, @@ -34,8 +41,14 @@ const renderBottomPanel = () => { selectionFilter: [], highlightColor: null, }, - dataTable: 'layer1', - map: { mapViews: [{ id: 'layer1', name: 'Layer 1' }] }, + dataTable: dataTable ?? { + openIds: ['layer1'], + activeLayerId: 'layer1', + isPanelVisible: true, + }, + map: { + mapViews: mapViews ?? [{ id: 'layer1', name: 'Layer 1' }], + }, }) const { container } = render( @@ -44,7 +57,7 @@ const renderBottomPanel = () => { ) - return { handle: container.querySelector('.resizeHandle') } + return { handle: container.querySelector('.resizeHandle'), store } } const getDisplayHeight = () => @@ -79,3 +92,86 @@ describe('BottomPanel resize cancel', () => { expect(getDisplayHeight()).toBe(`${DATA_TABLE_HEIGHT}px`) }) }) + +describe('BottomPanel layer selection', () => { + const eligibleLayer = (id, name) => ({ + id, + name, + layer: THEMATIC_LAYER, + isLoaded: true, + data: [{}], + }) + const mapViews = [ + eligibleLayer('layer1', 'Layer 1'), + eligibleLayer('layer2', 'Layer 2'), + ] + + test('restores the stored activeLayerId as the selected value on mount', () => { + renderBottomPanel({ + dataTable: { + openIds: ['layer1', 'layer2'], + activeLayerId: 'layer2', + isPanelVisible: true, + }, + mapViews, + }) + + expect(screen.getByTestId('data-table-layer-selector')).toHaveValue( + 'layer2' + ) + }) + + test('falls back to the last open tab when the stored activeLayerId is stale (e.g. its layer was removed)', () => { + renderBottomPanel({ + dataTable: { + openIds: ['layer1', 'layer2'], + activeLayerId: 'removed-layer', + isPanelVisible: true, + }, + mapViews, + }) + + expect(screen.getByTestId('data-table-layer-selector')).toHaveValue( + 'layer2' + ) + }) + + test('selecting an already-open layer dispatches setActiveDataTableLayer only', () => { + const { store } = renderBottomPanel({ + dataTable: { + openIds: ['layer1', 'layer2'], + activeLayerId: 'layer1', + isPanelVisible: true, + }, + mapViews, + }) + + fireEvent.change(screen.getByTestId('data-table-layer-selector'), { + target: { value: 'layer2' }, + }) + + expect(store.getActions()).toEqual([ + { type: 'DATA_TABLE_ACTIVE_LAYER_SET', id: 'layer2' }, + ]) + }) + + test('selecting an eligible-but-not-open layer also opens it', () => { + const { store } = renderBottomPanel({ + dataTable: { + openIds: ['layer1'], + activeLayerId: 'layer1', + isPanelVisible: true, + }, + mapViews, + }) + + fireEvent.change(screen.getByTestId('data-table-layer-selector'), { + target: { value: 'layer2' }, + }) + + expect(store.getActions()).toEqual([ + { type: 'DATA_TABLE_ACTIVE_LAYER_SET', id: 'layer2' }, + { type: 'DATA_TABLE_TOGGLE', id: 'layer2' }, + ]) + }) +}) diff --git a/src/components/datatable/__tests__/CellValue.spec.jsx b/src/components/datatable/__tests__/CellValue.spec.jsx new file mode 100644 index 0000000000..9c979af62b --- /dev/null +++ b/src/components/datatable/__tests__/CellValue.spec.jsx @@ -0,0 +1,112 @@ +import { render, screen } from '@testing-library/react' +import React from 'react' +import { + RENDERER_COLOR, + RENDERER_ICON, + RENDERER_DATE, + RENDERER_ORG_UNIT, + RENDERER_ORG_UNIT_NAME, + RENDERER_BOOLEAN, + TYPE_DATE, +} from '../../../constants/dataTable.js' +import CellValue, { getCellRendererFlags } from '../CellValue.jsx' + +describe('getCellRendererFlags', () => { + test('flags exactly one renderer at a time', () => { + expect(getCellRendererFlags(RENDERER_COLOR)).toMatchObject({ + isColorCell: true, + isIconCell: false, + isDateCell: false, + isBooleanCell: false, + }) + expect(getCellRendererFlags(RENDERER_BOOLEAN)).toMatchObject({ + isColorCell: false, + isBooleanCell: true, + }) + }) + + test('isDateOnlyCell is driven by type, independent of renderer', () => { + expect(getCellRendererFlags(RENDERER_DATE, TYPE_DATE)).toMatchObject({ + isDateCell: true, + isDateOnlyCell: true, + }) + expect(getCellRendererFlags(RENDERER_DATE, 'datetime')).toMatchObject({ + isDateCell: true, + isDateOnlyCell: false, + }) + }) +}) + +describe('CellValue', () => { + test('formats a plain number with the digit group separator', () => { + render( + + ) + expect(screen.getByText('1,234,567')).toBeInTheDocument() + }) + + test('leaves a plain string untouched', () => { + render() + expect(screen.getByText('Bo')).toBeInTheDocument() + }) + + test('renders an em-dash placeholder for a missing value, regardless of renderer', () => { + render() + expect(screen.getByText('—')).toBeInTheDocument() + }) + + test('renders an em-dash placeholder for an undefined value on a renderer-tagged column', () => { + render() + expect(screen.getByText('—')).toBeInTheDocument() + }) + + test('lowercases a color value instead of formatting it as a number', () => { + render() + expect(screen.getByText('#abcdef')).toBeInTheDocument() + }) + + test('renders an icon thumbnail for an icon column', () => { + const { container } = render( + + ) + expect(container.querySelector('img')).toHaveAttribute( + 'src', + 'https://server/icons/marker.png' + ) + }) + + test('formats a boolean-renderer value as Yes/No', () => { + render() + expect(screen.getByText('Yes')).toBeInTheDocument() + }) + + test('formats an org-unit-hierarchy value as a breadcrumb', () => { + const idToName = new Map([ + ['country1', 'Country'], + ['ou1', 'Facility'], + ]) + render( + + ) + expect(screen.getByText('Country / Facility')).toBeInTheDocument() + }) + + test("formats an org-unit-name value as just the feature's own name", () => { + const idToName = new Map([['ou1', 'Facility']]) + render( + + ) + expect(screen.getByText('Facility')).toBeInTheDocument() + }) +}) diff --git a/src/components/datatable/__tests__/ColumnPickerControl.spec.jsx b/src/components/datatable/__tests__/ColumnPickerControl.spec.jsx index 9df0ad1b3c..c79452a634 100644 --- a/src/components/datatable/__tests__/ColumnPickerControl.spec.jsx +++ b/src/components/datatable/__tests__/ColumnPickerControl.spec.jsx @@ -2,6 +2,7 @@ import { render, fireEvent, screen } from '@testing-library/react' import React from 'react' import { Provider } from 'react-redux' import configureMockStore from 'redux-mock-store' +import { setDataTableColumnConfig } from '../../../actions/dataTable.js' import { DATA_TABLE_COLUMN_CONFIG_SET } from '../../../constants/actionTypes.js' import ColumnPickerControl from '../controls/ColumnPickerControl.jsx' @@ -18,8 +19,10 @@ const renderColumnPicker = (props) => { const result = render( + store.dispatch(setDataTableColumnConfig('layer1', config)) + } {...props} /> @@ -65,6 +68,16 @@ describe('ColumnPicker trigger', () => { expect(screen.getByLabelText('Value')).toBeChecked() expect(screen.getByLabelText('Legend')).toBeChecked() }) + + test('pressing Escape closes the popover', () => { + renderColumnPicker() + openPicker() + expect(screen.getByLabelText('Name')).toBeInTheDocument() + + fireEvent.keyDown(window, { key: 'Escape' }) + + expect(screen.queryByLabelText('Name')).not.toBeInTheDocument() + }) }) describe('ColumnPicker visibility toggling', () => { @@ -356,6 +369,27 @@ describe('ColumnPicker search', () => { }) }) +describe('ColumnPicker configName (timeline current-period columns)', () => { + test('shows configName instead of the period-specific name, when present', () => { + const headersWithConfigName = [ + ...headers, + { + name: 'Range (Jan 2023)', + configName: 'Range (Current period)', + dataKey: 'range', + }, + ] + renderColumnPicker({ allHeaders: headersWithConfigName }) + openPicker() + expect( + screen.getByLabelText('Range (Current period)') + ).toBeInTheDocument() + expect( + screen.queryByLabelText('Range (Jan 2023)') + ).not.toBeInTheDocument() + }) +}) + describe('ColumnPicker defaultHidden headers (e.g. period columns)', () => { const headersWithHiddenColumn = [ ...headers, diff --git a/src/components/datatable/__tests__/DataTableButton.spec.jsx b/src/components/datatable/__tests__/DataTableButton.spec.jsx new file mode 100644 index 0000000000..e77d3677ec --- /dev/null +++ b/src/components/datatable/__tests__/DataTableButton.spec.jsx @@ -0,0 +1,71 @@ +import { render, fireEvent, screen } from '@testing-library/react' +import React from 'react' +import { Provider } from 'react-redux' +import configureMockStore from 'redux-mock-store' +import { THEMATIC_LAYER, EXTERNAL_LAYER } from '../../../constants/layers.js' +import DataTableButton from '../DataTableButton.jsx' + +const mockStore = configureMockStore() + +const layer = (id, overrides = {}) => ({ + id, + name: id, + layer: THEMATIC_LAYER, + isLoaded: true, + data: [{}], + ...overrides, +}) + +const renderButton = ({ dataTable, mapViews }) => { + const store = mockStore({ + dataTable, + map: { mapViews }, + }) + const result = render( + + + + ) + return { ...result, store } +} + +const CLOSED = { openIds: [] } + +describe('DataTableButton', () => { + test('is disabled when the map has no eligible layers', () => { + renderButton({ + dataTable: CLOSED, + mapViews: [layer('a', { layer: EXTERNAL_LAYER })], + }) + expect(screen.getByText('Data table')).toBeDisabled() + }) + + test('opens the first eligible layer when none is open yet', () => { + const { store } = renderButton({ + dataTable: CLOSED, + mapViews: [layer('a'), layer('b')], + }) + fireEvent.click(screen.getByText('Data table')) + expect(store.getActions()).toEqual([ + { type: 'DATA_TABLE_TOGGLE', id: 'a' }, + ]) + }) + + test('closes the panel when a table is already open', () => { + const { store } = renderButton({ + dataTable: { openIds: ['a'], isPanelVisible: true }, + mapViews: [layer('a'), layer('b')], + }) + fireEvent.click(screen.getByText('Data table')) + expect(store.getActions()).toEqual([{ type: 'DATA_TABLE_CLOSE' }]) + }) + + test('reopens (without changing what is open) when a table was open but the panel is hidden', () => { + const { store } = renderButton({ + dataTable: { openIds: ['a'], isPanelVisible: false }, + mapViews: [layer('a'), layer('b')], + }) + fireEvent.click(screen.getByText('Data table')) + expect(store.getActions()).toEqual([{ type: 'DATA_TABLE_OPEN' }]) + }) +}) diff --git a/src/components/datatable/__tests__/DateGroupFilterInput.spec.jsx b/src/components/datatable/__tests__/DateGroupFilterInput.spec.jsx index f3b50feaff..643e89ffb6 100644 --- a/src/components/datatable/__tests__/DateGroupFilterInput.spec.jsx +++ b/src/components/datatable/__tests__/DateGroupFilterInput.spec.jsx @@ -1,12 +1,6 @@ import { render, fireEvent, screen } from '@testing-library/react' import React from 'react' -import { Provider } from 'react-redux' import { VirtuosoMockContext } from 'react-virtuoso' -import configureMockStore from 'redux-mock-store' -import { - DATA_FILTER_SET, - DATA_FILTER_CLEAR, -} from '../../../constants/actionTypes.js' import { SENTINEL_ANY_VALUE, SENTINEL_NO_VALUE, @@ -17,8 +11,6 @@ import { } from '../../../constants/dataTable.js' import DateGroupFilterInput from '../DateGroupFilterInput.jsx' -const mockStore = configureMockStore() - const DATETIME_VALUES = [ { value: '2023-05-15 09:00:00.0' }, { value: '2023-05-15 14:00:00.0' }, @@ -26,24 +18,23 @@ const DATETIME_VALUES = [ ] const renderDateGroupFilter = (props) => { - const store = mockStore({}) + const onChange = jest.fn() + const onClear = jest.fn() const result = render( - - - - - + + + ) - return { ...result, store } + return { ...result, onChange, onClear } } const getInput = () => @@ -92,24 +83,19 @@ describe('DateGroupFilterInput - default (collapsed) tree', () => { }) }) -describe('DateGroupFilterInput - selection dispatches', () => { - test('checking a year dispatches the full date-group filter shape', () => { - const { store } = renderDateGroupFilter() +describe('DateGroupFilterInput - selection calls onChange/onClear', () => { + test('checking a year calls onChange with the full date-group filter shape', () => { + const { onChange } = renderDateGroupFilter() openPopover() fireEvent.click(screen.getByLabelText('2023')) - expect(store.getActions()).toContainEqual({ - type: DATA_FILTER_SET, - layerId: 'layer1', - fieldId: 'eventdate', - filter: { - granularity: DATE_GROUPS_GRANULARITY, - prefixes: ['2023'], - }, + expect(onChange).toHaveBeenCalledWith({ + granularity: DATE_GROUPS_GRANULARITY, + prefixes: ['2023'], }) }) - test('unchecking the only selected prefix dispatches DATA_FILTER_CLEAR', () => { - const { store } = renderDateGroupFilter({ + test('unchecking the only selected prefix calls onClear', () => { + const { onClear } = renderDateGroupFilter({ filterValue: { granularity: DATE_GROUPS_GRANULARITY, prefixes: ['2023'], @@ -117,15 +103,11 @@ describe('DateGroupFilterInput - selection dispatches', () => { }) openPopover() fireEvent.click(screen.getByLabelText('2023')) - expect(store.getActions()).toContainEqual({ - type: DATA_FILTER_CLEAR, - layerId: 'layer1', - fieldId: 'eventdate', - }) + expect(onClear).toHaveBeenCalled() }) test('checking a month drops the now-redundant year-level ancestor selection scenario in reverse: checking a day under an unrelated selected month keeps both', () => { - const { store } = renderDateGroupFilter({ + const { onChange } = renderDateGroupFilter({ filterValue: { granularity: DATE_GROUPS_GRANULARITY, prefixes: ['2024'], @@ -134,14 +116,9 @@ describe('DateGroupFilterInput - selection dispatches', () => { openPopover() fireEvent.click(screen.getByLabelText('Expand 2023')) fireEvent.click(screen.getByLabelText('May')) - expect(store.getActions()).toContainEqual({ - type: DATA_FILTER_SET, - layerId: 'layer1', - fieldId: 'eventdate', - filter: { - granularity: DATE_GROUPS_GRANULARITY, - prefixes: ['2024', '2023-05'], - }, + expect(onChange).toHaveBeenCalledWith({ + granularity: DATE_GROUPS_GRANULARITY, + prefixes: ['2024', '2023-05'], }) }) }) @@ -193,8 +170,8 @@ describe('DateGroupFilterInput - "Any value" / "No value"', () => { expect(screen.queryByLabelText('No value')).not.toBeInTheDocument() }) - test('checking "Any value" dispatches the sentinel and clears prior selections', () => { - const { store } = renderDateGroupFilter({ + test('checking "Any value" calls onChange with the sentinel and clears prior selections', () => { + const { onChange } = renderDateGroupFilter({ options: [...DATETIME_VALUES, { value: SENTINEL_NO_VALUE }], filterValue: { granularity: DATE_GROUPS_GRANULARITY, @@ -203,19 +180,14 @@ describe('DateGroupFilterInput - "Any value" / "No value"', () => { }) openPopover() fireEvent.click(screen.getByLabelText('Any value')) - expect(store.getActions()).toContainEqual({ - type: DATA_FILTER_SET, - layerId: 'layer1', - fieldId: 'eventdate', - filter: { - granularity: DATE_GROUPS_GRANULARITY, - prefixes: [SENTINEL_ANY_VALUE], - }, + expect(onChange).toHaveBeenCalledWith({ + granularity: DATE_GROUPS_GRANULARITY, + prefixes: [SENTINEL_ANY_VALUE], }) }) test('checking "No value" preserves an existing tree selection alongside it', () => { - const { store } = renderDateGroupFilter({ + const { onChange } = renderDateGroupFilter({ options: [...DATETIME_VALUES, { value: SENTINEL_NO_VALUE }], filterValue: { granularity: DATE_GROUPS_GRANULARITY, @@ -224,19 +196,14 @@ describe('DateGroupFilterInput - "Any value" / "No value"', () => { }) openPopover() fireEvent.click(screen.getByLabelText('No value')) - expect(store.getActions()).toContainEqual({ - type: DATA_FILTER_SET, - layerId: 'layer1', - fieldId: 'eventdate', - filter: { - granularity: DATE_GROUPS_GRANULARITY, - prefixes: ['2023', SENTINEL_NO_VALUE], - }, + expect(onChange).toHaveBeenCalledWith({ + granularity: DATE_GROUPS_GRANULARITY, + prefixes: ['2023', SENTINEL_NO_VALUE], }) }) test('clicking a tree node while "Any value" is active is a no-op (v1 scope boundary)', () => { - const { store } = renderDateGroupFilter({ + const { onChange, onClear } = renderDateGroupFilter({ filterValue: { granularity: DATE_GROUPS_GRANULARITY, prefixes: [SENTINEL_ANY_VALUE], @@ -244,13 +211,14 @@ describe('DateGroupFilterInput - "Any value" / "No value"', () => { }) openPopover() fireEvent.click(screen.getByLabelText('2023')) - expect(store.getActions()).toEqual([]) + expect(onChange).not.toHaveBeenCalled() + expect(onClear).not.toHaveBeenCalled() }) }) describe('DateGroupFilterInput - clearing via the input’s clear ("x") button', () => { - test('clearing the closed trigger (showing "N selected") clears the whole filter', () => { - const { store } = renderDateGroupFilter({ + test('clearing the closed trigger (showing "N selected") calls onClear', () => { + const { onClear } = renderDateGroupFilter({ filterValue: { granularity: DATE_GROUPS_GRANULARITY, prefixes: ['2023'], @@ -258,15 +226,11 @@ describe('DateGroupFilterInput - clearing via the input’s clear ("x") button', }) expect(getInput()).toHaveValue('1 selected') fireEvent.change(getInput(), { target: { value: '' } }) - expect(store.getActions()).toContainEqual({ - type: DATA_FILTER_CLEAR, - layerId: 'layer1', - fieldId: 'eventdate', - }) + expect(onClear).toHaveBeenCalled() }) - test('clearing a typed search narrow while a selection is active also clears the selection (mirrors the flat filter variant)', () => { - const { store } = renderDateGroupFilter({ + test('clearing a typed search narrow while a selection is active also calls onClear (mirrors the flat filter variant)', () => { + const { onClear } = renderDateGroupFilter({ filterValue: { granularity: DATE_GROUPS_GRANULARITY, prefixes: ['2023'], @@ -277,18 +241,15 @@ describe('DateGroupFilterInput - clearing via the input’s clear ("x") button', expect(screen.queryByLabelText('2024')).not.toBeInTheDocument() fireEvent.change(getInput(), { target: { value: '' } }) - expect(store.getActions()).toContainEqual({ - type: DATA_FILTER_CLEAR, - layerId: 'layer1', - fieldId: 'eventdate', - }) + expect(onClear).toHaveBeenCalled() }) - test('clearing empty search text with no active filter dispatches nothing', () => { - const { store } = renderDateGroupFilter() + test('clearing empty search text with no active filter calls neither onChange nor onClear', () => { + const { onChange, onClear } = renderDateGroupFilter() openPopover() fireEvent.change(getInput(), { target: { value: '' } }) - expect(store.getActions()).toEqual([]) + expect(onChange).not.toHaveBeenCalled() + expect(onClear).not.toHaveBeenCalled() }) }) @@ -312,34 +273,24 @@ describe('DateGroupFilterInput - search', () => { }) test('typing text with no exact tree match shows a live-applying "Contains" custom filter row', () => { - const { store } = renderDateGroupFilter() + const { onChange } = renderDateGroupFilter() openPopover() fireEvent.change(getInput(), { target: { value: '2023-05-15 09:0' } }) expect( screen.getByTestId('data-table-column-filter-custom-Event date') ).toBeInTheDocument() - expect(store.getActions()).toContainEqual({ - type: DATA_FILTER_SET, - layerId: 'layer1', - fieldId: 'eventdate', - filter: '2023-05-15 09:0', - }) + expect(onChange).toHaveBeenCalledWith('2023-05-15 09:0') }) test('stays shown and keeps live-applying even when the typed text exactly matches a tree node prefix (e.g. a full year)', () => { - const { store } = renderDateGroupFilter() + const { onChange } = renderDateGroupFilter() openPopover() fireEvent.change(getInput(), { target: { value: '202' } }) fireEvent.change(getInput(), { target: { value: '2023' } }) expect( screen.getByTestId('data-table-column-filter-custom-Event date') ).toBeInTheDocument() - expect(store.getActions()).toContainEqual({ - type: DATA_FILTER_SET, - layerId: 'layer1', - fieldId: 'eventdate', - filter: '2023', - }) + expect(onChange).toHaveBeenCalledWith('2023') }) }) diff --git a/src/components/datatable/__tests__/FilterInput.spec.jsx b/src/components/datatable/__tests__/FilterInput.spec.jsx index 19fe751014..0543065656 100644 --- a/src/components/datatable/__tests__/FilterInput.spec.jsx +++ b/src/components/datatable/__tests__/FilterInput.spec.jsx @@ -3,6 +3,7 @@ import React from 'react' import { Provider } from 'react-redux' import { VirtuosoMockContext } from 'react-virtuoso' import configureMockStore from 'redux-mock-store' +import { setDataFilter, clearDataFilter } from '../../../actions/dataFilters.js' import { DATA_FILTER_SET, DATA_FILTER_CLEAR, @@ -30,11 +31,12 @@ const mockStore = configureMockStore() const renderFilterInput = (props, dataFilters) => { const store = mockStore({ - dataTable: 'layer1', map: { mapViews: [{ id: 'layer1', dataFilters: dataFilters || {} }], }, }) + const dataKey = props?.dataKey ?? 'name' + const filterValue = (dataFilters || {})[dataKey] // The checkbox list is virtualized (react-virtuoso) const result = render( @@ -45,6 +47,13 @@ const renderFilterInput = (props, dataFilters) => { dataKey="name" name="Name" type="string" + filterValue={filterValue} + onChange={(value) => + store.dispatch(setDataFilter('layer1', dataKey, value)) + } + onClear={() => + store.dispatch(clearDataFilter('layer1', dataKey)) + } {...props} /> @@ -354,6 +363,48 @@ describe('FilterInput multi-select path (optionSetId)', () => { }) }) +describe('FilterInput multi-select path (resolvedOptionNames pre-resolved)', () => { + const options = [{ value: 'CONFIRMED' }, { value: 'PROBABLE' }] + + beforeEach(() => { + useOptionSet.mockReturnValue({ optionSet: null }) + }) + + test('uses resolvedOptionNames directly without calling useOptionSet', () => { + renderFilterInput({ + dataKey: 'caseType', + name: 'Case classification', + options, + optionSetId: 'optionSet1', + resolvedOptionNames: { + CONFIRMED: 'Confirmed case', + PROBABLE: 'Probable case', + }, + }) + openPopover('Case classification') + expect(screen.getByLabelText('Confirmed case')).toBeInTheDocument() + expect(screen.getByLabelText('Probable case')).toBeInTheDocument() + expect(useOptionSet).toHaveBeenCalledWith(undefined) + }) + + test('falls back to useOptionSet when resolvedOptionNames is not provided', () => { + useOptionSet.mockReturnValue({ + optionSet: { + options: [{ code: 'CONFIRMED', name: 'Confirmed case' }], + }, + }) + renderFilterInput({ + dataKey: 'caseType', + name: 'Case classification', + options, + optionSetId: 'optionSet1', + }) + openPopover('Case classification') + expect(screen.getByLabelText('Confirmed case')).toBeInTheDocument() + expect(useOptionSet).toHaveBeenCalledWith('optionSet1') + }) +}) + describe('FilterInput searchable popover — search', () => { const options = [{ value: 'High' }, { value: 'Low' }] diff --git a/src/components/datatable/__tests__/LayerSelectorControl.spec.jsx b/src/components/datatable/__tests__/LayerSelectorControl.spec.jsx new file mode 100644 index 0000000000..a69e422fe2 --- /dev/null +++ b/src/components/datatable/__tests__/LayerSelectorControl.spec.jsx @@ -0,0 +1,40 @@ +import { render, fireEvent, screen } from '@testing-library/react' +import React from 'react' +import LayerSelectorControl from '../controls/LayerSelectorControl.jsx' + +const layers = [ + { id: 'layer1', name: 'Layer 1' }, + { id: 'layer2', name: 'Layer 2' }, +] + +const renderControl = (props) => + render( + + ) + +const getSelect = () => screen.getByTestId('data-table-layer-selector') + +describe('LayerSelectorControl', () => { + test('lists every eligible layer by name', () => { + renderControl() + expect(screen.getByText('Layer 1')).toBeInTheDocument() + expect(screen.getByText('Layer 2')).toBeInTheDocument() + }) + + test('shows the active layer id as the selected value', () => { + renderControl({ activeLayerId: 'layer2' }) + expect(getSelect()).toHaveValue('layer2') + }) + + test('selecting a different layer calls onSelectLayer with its id', () => { + const onSelectLayer = jest.fn() + renderControl({ onSelectLayer }) + fireEvent.change(getSelect(), { target: { value: 'layer2' } }) + expect(onSelectLayer).toHaveBeenCalledWith('layer2') + }) +}) diff --git a/src/components/datatable/__tests__/OrgUnitGroupFilterInput.spec.jsx b/src/components/datatable/__tests__/OrgUnitGroupFilterInput.spec.jsx index 813687c49a..eaa013406c 100644 --- a/src/components/datatable/__tests__/OrgUnitGroupFilterInput.spec.jsx +++ b/src/components/datatable/__tests__/OrgUnitGroupFilterInput.spec.jsx @@ -1,12 +1,6 @@ import { render, fireEvent, screen } from '@testing-library/react' import React from 'react' -import { Provider } from 'react-redux' import { VirtuosoMockContext } from 'react-virtuoso' -import configureMockStore from 'redux-mock-store' -import { - DATA_FILTER_SET, - DATA_FILTER_CLEAR, -} from '../../../constants/actionTypes.js' import { SENTINEL_ANY_VALUE, SENTINEL_NO_VALUE, @@ -14,8 +8,6 @@ import { } from '../../../constants/dataTable.js' import OrgUnitGroupFilterInput from '../OrgUnitGroupFilterInput.jsx' -const mockStore = configureMockStore() - const ORG_UNIT_VALUES = [ { value: '/country1/region1/facility1' }, { value: '/country1/region2/facility2' }, @@ -23,24 +15,23 @@ const ORG_UNIT_VALUES = [ ] const renderOrgUnitGroupFilter = (props) => { - const store = mockStore({}) + const onChange = jest.fn() + const onClear = jest.fn() const result = render( - - - - - + + + ) - return { ...result, store } + return { ...result, onChange, onClear } } const getInput = () => @@ -104,24 +95,19 @@ describe('OrgUnitGroupFilterInput - label resolution', () => { }) }) -describe('OrgUnitGroupFilterInput - selection dispatches', () => { - test('checking a root node dispatches the full org-unit-group filter shape', () => { - const { store } = renderOrgUnitGroupFilter() +describe('OrgUnitGroupFilterInput - selection calls onChange/onClear', () => { + test('checking a root node calls onChange with the full org-unit-group filter shape', () => { + const { onChange } = renderOrgUnitGroupFilter() openPopover() fireEvent.click(screen.getByLabelText('country1')) - expect(store.getActions()).toContainEqual({ - type: DATA_FILTER_SET, - layerId: 'layer1', - fieldId: 'orgUnitPath', - filter: { - granularity: ORG_UNIT_GROUPS_GRANULARITY, - prefixes: ['/country1'], - }, + expect(onChange).toHaveBeenCalledWith({ + granularity: ORG_UNIT_GROUPS_GRANULARITY, + prefixes: ['/country1'], }) }) - test('unchecking the only selected prefix dispatches DATA_FILTER_CLEAR', () => { - const { store } = renderOrgUnitGroupFilter({ + test('unchecking the only selected prefix calls onClear', () => { + const { onClear } = renderOrgUnitGroupFilter({ filterValue: { granularity: ORG_UNIT_GROUPS_GRANULARITY, prefixes: ['/country1'], @@ -129,11 +115,7 @@ describe('OrgUnitGroupFilterInput - selection dispatches', () => { }) openPopover() fireEvent.click(screen.getByLabelText('country1')) - expect(store.getActions()).toContainEqual({ - type: DATA_FILTER_CLEAR, - layerId: 'layer1', - fieldId: 'orgUnitPath', - }) + expect(onClear).toHaveBeenCalled() }) }) @@ -184,8 +166,8 @@ describe('OrgUnitGroupFilterInput - "Any value" / "No value"', () => { expect(screen.queryByLabelText('No value')).not.toBeInTheDocument() }) - test('checking "Any value" dispatches the sentinel and clears prior selections', () => { - const { store } = renderOrgUnitGroupFilter({ + test('checking "Any value" calls onChange with the sentinel and clears prior selections', () => { + const { onChange } = renderOrgUnitGroupFilter({ options: [...ORG_UNIT_VALUES, { value: SENTINEL_NO_VALUE }], filterValue: { granularity: ORG_UNIT_GROUPS_GRANULARITY, @@ -194,19 +176,14 @@ describe('OrgUnitGroupFilterInput - "Any value" / "No value"', () => { }) openPopover() fireEvent.click(screen.getByLabelText('Any value')) - expect(store.getActions()).toContainEqual({ - type: DATA_FILTER_SET, - layerId: 'layer1', - fieldId: 'orgUnitPath', - filter: { - granularity: ORG_UNIT_GROUPS_GRANULARITY, - prefixes: [SENTINEL_ANY_VALUE], - }, + expect(onChange).toHaveBeenCalledWith({ + granularity: ORG_UNIT_GROUPS_GRANULARITY, + prefixes: [SENTINEL_ANY_VALUE], }) }) test('clicking a tree node while "Any value" is active is a no-op', () => { - const { store } = renderOrgUnitGroupFilter({ + const { onChange, onClear } = renderOrgUnitGroupFilter({ filterValue: { granularity: ORG_UNIT_GROUPS_GRANULARITY, prefixes: [SENTINEL_ANY_VALUE], @@ -214,7 +191,8 @@ describe('OrgUnitGroupFilterInput - "Any value" / "No value"', () => { }) openPopover() fireEvent.click(screen.getByLabelText('country1')) - expect(store.getActions()).toEqual([]) + expect(onChange).not.toHaveBeenCalled() + expect(onClear).not.toHaveBeenCalled() }) }) @@ -246,39 +224,33 @@ describe('OrgUnitGroupFilterInput - search', () => { expect(screen.queryByLabelText('country2')).not.toBeInTheDocument() }) - test('typing text with no tree match shows the custom filter row but clears rather than filtering by the raw id/path', () => { - const { store } = renderOrgUnitGroupFilter() + test('typing text with no tree match shows the custom filter row and applies a filter matching nothing, rather than clearing back to unfiltered', () => { + const { onChange, onClear } = renderOrgUnitGroupFilter() openPopover() fireEvent.change(getInput(), { target: { value: 'Nairobi' } }) expect( screen.getByTestId('data-table-column-filter-custom-Org unit') ).toBeInTheDocument() - expect(store.getActions()).toContainEqual({ - type: DATA_FILTER_CLEAR, - layerId: 'layer1', - fieldId: 'orgUnitPath', + expect(onChange).toHaveBeenCalledWith({ + granularity: ORG_UNIT_GROUPS_GRANULARITY, + prefixes: [], + searchDerived: true, + searchText: 'Nairobi', }) - expect(store.getActions()).not.toContainEqual( - expect.objectContaining({ type: DATA_FILTER_SET }) - ) + expect(onClear).not.toHaveBeenCalled() }) - test('committing a name-matched custom filter dispatches the matched nodes’ prefixes, not a raw substring match against the id path', () => { - const { store } = renderOrgUnitGroupFilter({ + test('committing a name-matched custom filter calls onChange with the matched nodes’ prefixes, not a raw substring match against the id path', () => { + const { onChange } = renderOrgUnitGroupFilter({ idToName: new Map([['country1', 'Sierra Leone']]), }) openPopover() fireEvent.change(getInput(), { target: { value: 'Sierra' } }) - expect(store.getActions()).toContainEqual({ - type: DATA_FILTER_SET, - layerId: 'layer1', - fieldId: 'orgUnitPath', - filter: { - granularity: ORG_UNIT_GROUPS_GRANULARITY, - prefixes: ['/country1'], - searchDerived: true, - searchText: 'Sierra', - }, + expect(onChange).toHaveBeenCalledWith({ + granularity: ORG_UNIT_GROUPS_GRANULARITY, + prefixes: ['/country1'], + searchDerived: true, + searchText: 'Sierra', }) }) diff --git a/src/components/datatable/__tests__/TableContextMenu.spec.jsx b/src/components/datatable/__tests__/TableContextMenu.spec.jsx index 0384092cdc..d0ba24e7ca 100644 --- a/src/components/datatable/__tests__/TableContextMenu.spec.jsx +++ b/src/components/datatable/__tests__/TableContextMenu.spec.jsx @@ -55,8 +55,10 @@ describe('TableContextMenu — view profile menu item', () => { }) test('dispatches setOrgUnitProfile with the row id for a layer type that supports it', () => { + const onClose = jest.fn() const { store } = renderMenu({ contextMenu: { x: 10, y: 10, featureProps: { id: 'ou1' } }, + onClose, }) fireEvent.click( screen @@ -67,6 +69,7 @@ describe('TableContextMenu — view profile menu item', () => { type: ORGANISATION_UNIT_PROFILE_SET, payload: 'ou1', }) + expect(onClose).toHaveBeenCalledWith() }) }) @@ -97,6 +100,6 @@ describe('TableContextMenu — zoom to filtered features', () => { zoom: true, }, }) - expect(onClose).toHaveBeenCalled() + expect(onClose).toHaveBeenCalledWith(true) }) }) diff --git a/src/components/datatable/__tests__/useRowClickSelection.spec.js b/src/components/datatable/__tests__/useRowClickSelection.spec.js new file mode 100644 index 0000000000..0b7be75f09 --- /dev/null +++ b/src/components/datatable/__tests__/useRowClickSelection.spec.js @@ -0,0 +1,61 @@ +import { renderHook } from '@testing-library/react' +import { useRowClickSelection } from '../useRowClickSelection.js' + +const row = (id) => [{ dataKey: 'id', value: id, align: 'left' }] + +describe('useRowClickSelection', () => { + test('does nothing on a plain click (no modifier)', () => { + const onToggle = jest.fn() + const onSelectRange = jest.fn() + const rows = [row('a'), row('b')] + const { result } = renderHook(() => + useRowClickSelection({ rows, onToggle, onSelectRange }) + ) + + result.current(row('a'), { ctrlKey: false, shiftKey: false }) + + expect(onToggle).not.toHaveBeenCalled() + expect(onSelectRange).not.toHaveBeenCalled() + }) + + test('toggles a single row on ctrl/cmd-click', () => { + const onToggle = jest.fn() + const onSelectRange = jest.fn() + const rows = [row('a'), row('b')] + const { result } = renderHook(() => + useRowClickSelection({ rows, onToggle, onSelectRange }) + ) + + result.current(row('b'), { ctrlKey: true }) + + expect(onToggle).toHaveBeenCalledWith('b') + expect(onSelectRange).not.toHaveBeenCalled() + }) + + test('selects a range on shift-click after a prior click', () => { + const onToggle = jest.fn() + const onSelectRange = jest.fn() + const rows = [row('a'), row('b'), row('c'), row('d')] + const { result } = renderHook(() => + useRowClickSelection({ rows, onToggle, onSelectRange }) + ) + + result.current(row('a'), { ctrlKey: true }) + result.current(row('c'), { shiftKey: true }) + + expect(onSelectRange).toHaveBeenCalledWith(['a', 'b', 'c']) + }) + + test('does nothing when the row has no id', () => { + const onToggle = jest.fn() + const onSelectRange = jest.fn() + const rows = [row(null)] + const { result } = renderHook(() => + useRowClickSelection({ rows, onToggle, onSelectRange }) + ) + + result.current(row(null), { ctrlKey: true }) + + expect(onToggle).not.toHaveBeenCalled() + }) +}) diff --git a/src/components/datatable/__tests__/useRowContextMenuHighlight.spec.js b/src/components/datatable/__tests__/useRowContextMenuHighlight.spec.js new file mode 100644 index 0000000000..2b0af0a8ea --- /dev/null +++ b/src/components/datatable/__tests__/useRowContextMenuHighlight.spec.js @@ -0,0 +1,98 @@ +import { renderHook } from '@testing-library/react' +import { useRowContextMenuHighlight } from '../useRowContextMenuHighlight.js' + +const leaveEvent = (relatedTagName) => ({ + relatedTarget: relatedTagName ? { tagName: relatedTagName } : null, +}) + +describe('useRowContextMenuHighlight', () => { + test('opening the context menu pins the row via onPin', () => { + const onPin = jest.fn() + const onClear = jest.fn() + const { result } = renderHook(() => + useRowContextMenuHighlight({ onPin, onClear }) + ) + const row = { id: 'row1' } + + result.current.onContextMenuOpen(row) + + expect(onPin).toHaveBeenCalledWith(row) + }) + + test('a mouseleave while the menu is open is ignored, even when it would normally clear the highlight', () => { + const onPin = jest.fn() + const onClear = jest.fn() + const { result } = renderHook(() => + useRowContextMenuHighlight({ onPin, onClear }) + ) + + result.current.onContextMenuOpen({ id: 'row1' }) + result.current.guardedClear(leaveEvent('DIV')) + + expect(onClear).not.toHaveBeenCalled() + }) + + test('mouseleave clears the highlight normally when no menu is open', () => { + const onPin = jest.fn() + const onClear = jest.fn() + const { result } = renderHook(() => + useRowContextMenuHighlight({ onPin, onClear }) + ) + + result.current.guardedClear(leaveEvent('DIV')) + + expect(onClear).toHaveBeenCalledTimes(1) + }) + + test('mouseleave between cells of the same row (relatedTarget is a TD) still never clears, menu or no menu', () => { + const onPin = jest.fn() + const onClear = jest.fn() + const { result } = renderHook(() => + useRowContextMenuHighlight({ onPin, onClear }) + ) + + result.current.guardedClear(leaveEvent('TD')) + + expect(onClear).not.toHaveBeenCalled() + }) + + test('closing the menu without a superseding highlight clears it', () => { + const onPin = jest.fn() + const onClear = jest.fn() + const { result } = renderHook(() => + useRowContextMenuHighlight({ onPin, onClear }) + ) + + result.current.onContextMenuOpen({ id: 'row1' }) + result.current.onMenuClose(false) + + expect(onClear).toHaveBeenCalledTimes(1) + }) + + test('closing the menu after a "Zoom to ..." action (highlightChanged=true) preserves the new highlight', () => { + const onPin = jest.fn() + const onClear = jest.fn() + const { result } = renderHook(() => + useRowContextMenuHighlight({ onPin, onClear }) + ) + + result.current.onContextMenuOpen({ id: 'row1' }) + result.current.onMenuClose(true) + + expect(onClear).not.toHaveBeenCalled() + }) + + test('after the menu closes, mouseleave clearing resumes normally', () => { + const onPin = jest.fn() + const onClear = jest.fn() + const { result } = renderHook(() => + useRowContextMenuHighlight({ onPin, onClear }) + ) + + result.current.onContextMenuOpen({ id: 'row1' }) + result.current.onMenuClose(true) + result.current.guardedClear(leaveEvent('DIV')) + + expect(onClear).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/components/datatable/__tests__/useRowSelection.spec.js b/src/components/datatable/__tests__/useRowSelection.spec.js new file mode 100644 index 0000000000..61d7a78225 --- /dev/null +++ b/src/components/datatable/__tests__/useRowSelection.spec.js @@ -0,0 +1,72 @@ +import { renderHook } from '@testing-library/react' +import { useRowSelection } from '../useRowSelection.js' + +describe('useRowSelection', () => { + test('selects every visible row when nothing is selected yet', () => { + const onChange = jest.fn() + const { result } = renderHook(() => + useRowSelection({ + selectedIds: [], + selectedIdSet: new Set(), + allRowIds: ['a', 'b', 'c'], + onChange, + }) + ) + + expect(result.current.isAllSelected).toBe(false) + + result.current.onToggleSelectAll() + + expect(onChange).toHaveBeenCalledWith(['a', 'b', 'c']) + }) + + test('deselects every visible row when all are already selected', () => { + const onChange = jest.fn() + const { result } = renderHook(() => + useRowSelection({ + selectedIds: ['a', 'b', 'c'], + selectedIdSet: new Set(['a', 'b', 'c']), + allRowIds: ['a', 'b', 'c'], + onChange, + }) + ) + + expect(result.current.isAllSelected).toBe(true) + + result.current.onToggleSelectAll() + + expect(onChange).toHaveBeenCalledWith([]) + }) + + test('preserves ids selected outside the current view when toggling off', () => { + const onChange = jest.fn() + const { result } = renderHook(() => + useRowSelection({ + selectedIds: ['a', 'b', 'z'], + selectedIdSet: new Set(['a', 'b', 'z']), + allRowIds: ['a', 'b'], + onChange, + }) + ) + + result.current.onToggleSelectAll() + + expect(onChange).toHaveBeenCalledWith(['z']) + }) + + test('reverses the visible selection via onChange', () => { + const onChange = jest.fn() + const { result } = renderHook(() => + useRowSelection({ + selectedIds: ['a'], + selectedIdSet: new Set(['a']), + allRowIds: ['a', 'b', 'c'], + onChange, + }) + ) + + result.current.onReverseSelection() + + expect(onChange).toHaveBeenCalledWith(['b', 'c']) + }) +}) diff --git a/src/components/datatable/__tests__/useSortState.spec.js b/src/components/datatable/__tests__/useSortState.spec.js new file mode 100644 index 0000000000..a84711fdad --- /dev/null +++ b/src/components/datatable/__tests__/useSortState.spec.js @@ -0,0 +1,62 @@ +import { act, renderHook } from '@testing-library/react' +import { useSortState } from '../useSortState.js' + +describe('useSortState', () => { + test('starts sorted by the initial field, ascending - matching what the table shows before any interaction', () => { + const { result } = renderHook(() => useSortState('name')) + + expect(result.current.sortField).toBe('name') + expect(result.current.sortDirection).toBe('asc') + }) + + test('cycling a different column 3 times returns to the same field/direction the table started with - not an unsorted state', () => { + const { result } = renderHook(() => useSortState('name')) + + act(() => result.current.sortData({ name: 'type' })) + expect(result.current).toMatchObject({ + sortField: 'type', + sortDirection: 'asc', + }) + + act(() => result.current.sortData({ name: 'type' })) + expect(result.current).toMatchObject({ + sortField: 'type', + sortDirection: 'desc', + }) + + act(() => result.current.sortData({ name: 'type' })) + expect(result.current).toMatchObject({ + sortField: 'name', + sortDirection: 'asc', + }) + }) + + test('cycling the initial/default column itself is a 2-state toggle, since it already is the default', () => { + const { result } = renderHook(() => useSortState('name')) + + act(() => result.current.sortData({ name: 'name' })) + expect(result.current).toMatchObject({ + sortField: 'name', + sortDirection: 'desc', + }) + + act(() => result.current.sortData({ name: 'name' })) + expect(result.current).toMatchObject({ + sortField: 'name', + sortDirection: 'asc', + }) + }) + + test('respects a custom initial sort field as the reset target', () => { + const { result } = renderHook(() => useSortState('level')) + + act(() => result.current.sortData({ name: 'name' })) + act(() => result.current.sortData({ name: 'name' })) + act(() => result.current.sortData({ name: 'name' })) + + expect(result.current).toMatchObject({ + sortField: 'level', + sortDirection: 'asc', + }) + }) +}) diff --git a/src/components/datatable/__tests__/useTableData.spec.jsx b/src/components/datatable/__tests__/useTableData.spec.jsx index 83d85a83e7..0a87d6eb13 100644 --- a/src/components/datatable/__tests__/useTableData.spec.jsx +++ b/src/components/datatable/__tests__/useTableData.spec.jsx @@ -63,7 +63,7 @@ describe('useTableData headers', () => { const { headers, rows, isLoading } = result.current expect(headers).toHaveLength(5) expect(headers).toMatchObject([ - { name: 'Org unit Id', dataKey: 'id', type: 'string' }, + { name: 'Org unit id', dataKey: 'id', type: 'string' }, { name: 'Org unit', dataKey: 'orgUnitOwn', type: 'string' }, { name: 'Org unit level', dataKey: 'level', type: 'number' }, { @@ -221,7 +221,7 @@ describe('useTableData headers', () => { const { headers, rows, isLoading } = result.current expect(headers).toHaveLength(5) expect(headers).toMatchObject([ - { name: 'Org unit Id', dataKey: 'id', type: 'string' }, + { name: 'Org unit id', dataKey: 'id', type: 'string' }, { name: 'Org unit', dataKey: 'orgUnitOwn', type: 'string' }, { name: 'Org unit level', dataKey: 'level', type: 'number' }, { @@ -283,7 +283,7 @@ describe('useTableData headers', () => { const { headers, rows, isLoading } = result.current expect(headers).toHaveLength(9) expect(headers).toMatchObject([ - { name: 'Org unit Id', dataKey: 'id', type: 'string' }, + { name: 'Org unit id', dataKey: 'id', type: 'string' }, { name: 'Org unit', dataKey: 'orgUnitOwn', type: 'string' }, { name: 'Org unit level', dataKey: 'level', type: 'number' }, { @@ -369,7 +369,7 @@ describe('useTableData headers', () => { ) const { headers, rows } = result.current expect(headers).toMatchObject([ - { name: 'Org unit Id', dataKey: 'id' }, + { name: 'Org unit id', dataKey: 'id' }, { name: 'Org unit', dataKey: 'orgUnitOwn' }, { name: 'Org unit level', dataKey: 'level' }, { name: 'Org unit hierarchy', dataKey: 'orgUnitPath' }, @@ -391,7 +391,7 @@ describe('useTableData headers', () => { ) }) - test('adds a defaultHidden raw-value-only column for every other period, for a timeline thematic layer', () => { + test('adds a defaultHidden raw-value-only column for every period, including the current one, for a timeline thematic layer', () => { const store = { aggregations: {}, ui: { @@ -434,21 +434,30 @@ describe('useTableData headers', () => { } ) const { headers, rows } = result.current - expect(headers).not.toContainEqual( - expect.objectContaining({ dataKey: 'period_202302_rawValue' }) - ) expect(headers).toContainEqual({ name: 'Value (January 2023)', dataKey: 'period_202301_rawValue', type: 'number', defaultHidden: true, }) + expect(headers).toContainEqual({ + name: 'Value (February 2023)', + dataKey: 'period_202302_rawValue', + type: 'number', + defaultHidden: true, + }) expect(rows[0]).toContainEqual( expect.objectContaining({ value: 100, dataKey: 'period_202301_rawValue', }) ) + expect(rows[0]).toContainEqual( + expect.objectContaining({ + value: 200, + dataKey: 'period_202302_rawValue', + }) + ) }) test('split-by-period thematic layer has no default current-period column, only defaultHidden period columns', () => { @@ -486,7 +495,7 @@ describe('useTableData headers', () => { ) const { headers, rows } = result.current expect(headers).toMatchObject([ - { name: 'Org unit Id', dataKey: 'id' }, + { name: 'Org unit id', dataKey: 'id' }, { name: 'Org unit', dataKey: 'orgUnitOwn' }, { name: 'Org unit level', dataKey: 'level' }, { name: 'Org unit hierarchy', dataKey: 'orgUnitPath' }, @@ -578,7 +587,7 @@ describe('useTableData headers', () => { expect(headers).toHaveLength(10) expect(headers).toMatchObject([ { name: 'Event Id', dataKey: 'id', type: 'string' }, - { name: 'Org unit Id', dataKey: 'orgUnitId', type: 'string' }, + { name: 'Org unit id', dataKey: 'orgUnitId', type: 'string' }, { name: 'Org unit', dataKey: 'orgUnitOwn', type: 'string' }, { name: 'Org unit level', dataKey: 'level', type: 'number' }, { @@ -724,7 +733,7 @@ describe('useTableData headers', () => { expect(headers).toHaveLength(11) expect(headers).toMatchObject([ { name: 'Tracked entity Id', dataKey: 'id', type: 'string' }, - { name: 'Org unit Id', dataKey: 'orgUnitId', type: 'string' }, + { name: 'Org unit id', dataKey: 'orgUnitId', type: 'string' }, { name: 'Org unit', dataKey: 'orgUnitOwn', type: 'string' }, { name: 'Org unit level', dataKey: 'level', type: 'number' }, { @@ -1094,7 +1103,7 @@ describe('useTableData headers', () => { expect(headers).toHaveLength(7) expect(headers).toMatchObject([ - { name: 'Org unit Id', dataKey: 'id', type: 'string' }, + { name: 'Org unit id', dataKey: 'id', type: 'string' }, { name: 'Org unit', dataKey: 'orgUnitOwn', type: 'string' }, { name: 'Org unit level', dataKey: 'level', type: 'number' }, { @@ -1257,7 +1266,7 @@ describe('useTableData headers', () => { expect(headers).toHaveLength(7) expect(headers).toMatchObject([ - { name: 'Org unit Id', dataKey: 'id', type: 'string' }, + { name: 'Org unit id', dataKey: 'id', type: 'string' }, { name: 'Org unit', dataKey: 'orgUnitOwn', type: 'string' }, { name: 'Org unit level', dataKey: 'level', type: 'number' }, { @@ -2193,7 +2202,7 @@ describe('useTableData globalSearch', () => { ).toBe('evt1') }) - test('matches a custom ORGANISATION_UNIT-valued attribute on a Tracked entity layer only by its raw stored value, not its resolved name - only "Org unit hierarchy" gets name-aware global search', () => { + test('matches a custom ORGANISATION_UNIT-valued attribute on a Tracked entity layer by its resolved name, same as the "Org unit hierarchy" column - fixed an earlier asymmetry where custom org-unit fields were only searchable by their raw stored id', () => { useOrgUnitAncestorNames.mockReturnValue({ idToName: new Map([['facility9', 'Referral Hospital']]), loading: false, @@ -2233,13 +2242,13 @@ describe('useTableData globalSearch', () => { } ).result - expect(renderTeiTableData('referral').current.rows).toHaveLength(0) - - const { current } = renderTeiTableData('facility9') + const { current } = renderTeiTableData('referral') expect(current.rows).toHaveLength(1) expect(current.rows[0].find((c) => c.dataKey === 'id').value).toBe( 'tei1' ) + + expect(renderTeiTableData('addis ababa').current.rows).toHaveLength(0) }) test('shows no rows when nothing matches', () => { diff --git a/src/components/datatable/controls/ClearFiltersControl.jsx b/src/components/datatable/controls/ClearFiltersControl.jsx index 902e0fc7ed..31d42db860 100644 --- a/src/components/datatable/controls/ClearFiltersControl.jsx +++ b/src/components/datatable/controls/ClearFiltersControl.jsx @@ -1,20 +1,18 @@ import i18n from '@dhis2/d2-i18n' -import { IconFilter16 } from '@dhis2/ui' import PropTypes from 'prop-types' import React from 'react' -import styles from './styles/ClearFiltersControl.module.css' +import { FilterActiveIcon } from '../../core/index.js' import ToolbarIconButton from './ToolbarIconButton.jsx' const ClearFiltersControl = ({ disabled, onClick }) => ( - - - - + ) diff --git a/src/components/datatable/controls/ColumnPickerControl.jsx b/src/components/datatable/controls/ColumnPickerControl.jsx index fe7ca0a70f..34c4d1657a 100644 --- a/src/components/datatable/controls/ColumnPickerControl.jsx +++ b/src/components/datatable/controls/ColumnPickerControl.jsx @@ -26,8 +26,6 @@ import React, { useState, } from 'react' import { createPortal } from 'react-dom' -import { useDispatch } from 'react-redux' -import { setDataTableColumnConfig } from '../../../actions/dataTable.js' import { filterHeadersByName, getDefaultVisibleKeys, @@ -49,11 +47,10 @@ const EMPTY_HEADERS = [] const EMPTY_KEYS = [] const ColumnPickerControl = React.memo(function ColumnPickerControl({ - layerId, allHeaders, columnConfig, + onChange, }) { - const dispatch = useDispatch() const anchorRef = useRef(null) const [isOpen, setIsOpen] = useState(false) const [activeId, setActiveId] = useState(null) @@ -101,14 +98,12 @@ const ColumnPickerControl = React.memo(function ColumnPickerControl({ ) const updateConfig = (partial) => - dispatch( - setDataTableColumnConfig(layerId, { - visibleKeys, - pinnedKeys, - orderedKeys, - ...partial, - }) - ) + onChange({ + visibleKeys, + pinnedKeys, + orderedKeys, + ...partial, + }) const onToggleVisible = (dataKey, checked) => updateConfig({ @@ -129,8 +124,7 @@ const ColumnPickerControl = React.memo(function ColumnPickerControl({ visibleKeys: reverseVisibleKeys(headers, visibleKeys), }) - const onResetToDefaults = () => - dispatch(setDataTableColumnConfig(layerId, undefined)) + const onResetToDefaults = () => onChange(undefined) const filteredHeaders = useMemo( () => @@ -312,7 +306,7 @@ const ColumnPickerControl = React.memo(function ColumnPickerControl({ }) ColumnPickerControl.propTypes = { - layerId: PropTypes.string.isRequired, + onChange: PropTypes.func.isRequired, allHeaders: PropTypes.arrayOf( PropTypes.shape({ dataKey: PropTypes.string, diff --git a/src/components/datatable/controls/ColumnRow.jsx b/src/components/datatable/controls/ColumnRow.jsx index 28628fc7b9..d944040f05 100644 --- a/src/components/datatable/controls/ColumnRow.jsx +++ b/src/components/datatable/controls/ColumnRow.jsx @@ -53,7 +53,9 @@ export const ColumnRowFields = ({ {header.name} + + {header.configName ?? header.name} + } checked={isVisible} onChange={(checked) => onToggleVisible(header.dataKey, checked)} @@ -85,6 +87,7 @@ ColumnRowFields.propTypes = { header: PropTypes.shape({ dataKey: PropTypes.string.isRequired, name: PropTypes.string.isRequired, + configName: PropTypes.string, }).isRequired, isPinned: PropTypes.bool.isRequired, isVisible: PropTypes.bool.isRequired, @@ -145,6 +148,7 @@ ColumnRow.propTypes = { header: PropTypes.shape({ dataKey: PropTypes.string.isRequired, name: PropTypes.string.isRequired, + configName: PropTypes.string, }).isRequired, isDragActive: PropTypes.bool.isRequired, isPinned: PropTypes.bool.isRequired, diff --git a/src/components/datatable/controls/LayerSelectorControl.jsx b/src/components/datatable/controls/LayerSelectorControl.jsx new file mode 100644 index 0000000000..def564c737 --- /dev/null +++ b/src/components/datatable/controls/LayerSelectorControl.jsx @@ -0,0 +1,33 @@ +import i18n from '@dhis2/d2-i18n' +import PropTypes from 'prop-types' +import React from 'react' +import styles from '../styles/BottomPanel.module.css' + +const LayerSelectorControl = ({ layers, activeLayerId, onSelectLayer }) => ( + +) + +LayerSelectorControl.propTypes = { + layers: PropTypes.arrayOf( + PropTypes.shape({ + id: PropTypes.string.isRequired, + name: PropTypes.string, + }) + ).isRequired, + onSelectLayer: PropTypes.func.isRequired, + activeLayerId: PropTypes.string, +} + +export default LayerSelectorControl diff --git a/src/components/datatable/controls/RowCountControl.jsx b/src/components/datatable/controls/RowCountControl.jsx index 274ab4fd1d..2284b416d5 100644 --- a/src/components/datatable/controls/RowCountControl.jsx +++ b/src/components/datatable/controls/RowCountControl.jsx @@ -1,26 +1,38 @@ import i18n from '@dhis2/d2-i18n' import PropTypes from 'prop-types' import React from 'react' +import { formatWithSeparator } from '../../../util/numbers.js' import styles from './styles/RowCountControl.module.css' -const RowCountControl = ({ totalCount, filteredCount }) => { +const RowCountControl = ({ + totalCount, + filteredCount, + keyAnalysisDigitGroupSeparator, +}) => { if (totalCount === null || filteredCount === null) { return null } + const total = formatWithSeparator( + totalCount, + keyAnalysisDigitGroupSeparator + ) + const filtered = formatWithSeparator( + filteredCount, + keyAnalysisDigitGroupSeparator + ) + const label = filteredCount < totalCount - ? i18n.t('{{filtered}} of {{total}} rows', { - filtered: filteredCount, - total: totalCount, - }) - : i18n.t('{{total}} rows', { total: totalCount }) + ? i18n.t('{{filtered}} of {{total}} rows', { filtered, total }) + : i18n.t('{{total}} rows', { total }) return {label} } RowCountControl.propTypes = { filteredCount: PropTypes.number, + keyAnalysisDigitGroupSeparator: PropTypes.string, totalCount: PropTypes.number, } diff --git a/src/components/datatable/controls/ShowInViewControl.jsx b/src/components/datatable/controls/ShowInViewControl.jsx index a1297b0ae7..0922cca3d0 100644 --- a/src/components/datatable/controls/ShowInViewControl.jsx +++ b/src/components/datatable/controls/ShowInViewControl.jsx @@ -7,6 +7,8 @@ import ToolbarIconButton from './ToolbarIconButton.jsx' const ShowInViewControl = ({ active, onClick }) => ( diff --git a/src/components/datatable/controls/__tests__/RowCountControl.spec.jsx b/src/components/datatable/controls/__tests__/RowCountControl.spec.jsx new file mode 100644 index 0000000000..7c177dbc72 --- /dev/null +++ b/src/components/datatable/controls/__tests__/RowCountControl.spec.jsx @@ -0,0 +1,44 @@ +import { render, screen } from '@testing-library/react' +import React from 'react' +import RowCountControl from '../RowCountControl.jsx' + +describe('RowCountControl', () => { + test('renders nothing while counts are not yet known', () => { + const { container } = render( + + ) + expect(container).toBeEmptyDOMElement() + }) + + test('shows just the total when nothing is filtered out', () => { + render() + expect(screen.getByText('12345 rows')).toBeInTheDocument() + }) + + test('shows filtered/total when rows have been filtered out', () => { + render() + expect(screen.getByText('42 of 12345 rows')).toBeInTheDocument() + }) + + test('applies the digit-group separator to both numbers', () => { + render( + + ) + expect(screen.getByText('42 of 12,345 rows')).toBeInTheDocument() + }) + + test('applies the digit-group separator to the total-only label too', () => { + render( + + ) + expect(screen.getByText('12,345 rows')).toBeInTheDocument() + }) +}) diff --git a/src/components/datatable/controls/styles/ColumnPickerControl.module.css b/src/components/datatable/controls/styles/ColumnPickerControl.module.css index fa974e6fd3..3732301418 100644 --- a/src/components/datatable/controls/styles/ColumnPickerControl.module.css +++ b/src/components/datatable/controls/styles/ColumnPickerControl.module.css @@ -1,9 +1,6 @@ .columnPickerPopover { - padding: var(--spacers-dp8); + composes: popoverPanel from './PopoverPanel.module.css'; min-width: 190px; - background-color: var(--colors-white); - border-radius: 4px; - box-shadow: var(--elevations-popover); } .searchInput { diff --git a/src/components/datatable/styles/BottomPanel.module.css b/src/components/datatable/styles/BottomPanel.module.css index e0002b0700..26a8c3cd0a 100644 --- a/src/components/datatable/styles/BottomPanel.module.css +++ b/src/components/datatable/styles/BottomPanel.module.css @@ -40,3 +40,22 @@ background-color: var(--colors-grey300); flex-shrink: 0; } + +.layerSelect { + max-width: 220px; + height: 24px; + padding: 0 var(--spacers-dp4); + font-size: 12px; + font-weight: 500; + border: 1px solid var(--colors-grey500); + border-radius: 3px; + background-color: var(--colors-white); + flex: 0 1 auto; + min-width: 0; +} + +.layerSelect:focus { + outline: none; + border-color: var(--colors-blue600); + box-shadow: inset 0 0 0 2px var(--colors-blue600); +} diff --git a/src/components/datatable/styles/DataTableButton.module.css b/src/components/datatable/styles/DataTableButton.module.css new file mode 100644 index 0000000000..13c63b6e8b --- /dev/null +++ b/src/components/datatable/styles/DataTableButton.module.css @@ -0,0 +1,34 @@ +/* Based on https: //github.com/dhis2/analytics/blob/master/src/components/Toolbar/MenuButton.styles.js */ + +.button { + all: unset; + display: inline-flex; + align-items: center; + justify-content: center; + font-size: 14px; + line-height: 14px; + padding: 0 var(--spacers-dp12); + color: var(--colors-grey900); + transition: background-color 250ms cubic-bezier(0.4, 0, 0.2, 1) 0ms; + cursor: pointer; +} + +.button:hover:enabled, +.button:active { + background-color: var(--colors-grey200); +} + +.button:focus { + outline: 3px solid var(--theme-focus); + outline-offset: -3px; +} + +/* Prevent focus styles when mouse clicking */ +.button:focus:not(:focus-visible) { + outline: none; +} + +.button:disabled { + color: var(--colors-grey500); + cursor: not-allowed; +} diff --git a/src/components/datatable/useGroupFilterInput.js b/src/components/datatable/useGroupFilterInput.js index 4ccdc0d7ef..87c4d62d17 100644 --- a/src/components/datatable/useGroupFilterInput.js +++ b/src/components/datatable/useGroupFilterInput.js @@ -1,6 +1,4 @@ import { useCallback, useMemo, useRef, useState } from 'react' -import { useDispatch } from 'react-redux' -import { setDataFilter, clearDataFilter } from '../../actions/dataFilters.js' import { SENTINEL_ANY_VALUE, SENTINEL_NO_VALUE, @@ -22,8 +20,8 @@ import { getDropdownPlacement } from './FilterDropdownPopover.jsx' const identity = (value) => value const useGroupFilterInput = ({ - dataKey, - layerId, + onChange, + onClear, filterValue, options, granularity, @@ -33,7 +31,6 @@ const useGroupFilterInput = ({ commitSearch, sanitizeInput = identity, }) => { - const dispatch = useDispatch() const anchorRef = useRef(null) const listRef = useRef(null) const [isOpen, setIsOpen] = useState(false) @@ -63,14 +60,9 @@ const useGroupFilterInput = ({ const applyValues = useCallback( (nextPrefixes) => nextPrefixes.length - ? dispatch( - setDataFilter(layerId, dataKey, { - granularity, - prefixes: nextPrefixes, - }) - ) - : dispatch(clearDataFilter(layerId, dataKey)), - [dispatch, layerId, dataKey, granularity] + ? onChange({ granularity, prefixes: nextPrefixes }) + : onClear(), + [onChange, onClear, granularity] ) const hasNotSetOption = options.some( @@ -152,10 +144,10 @@ const useGroupFilterInput = ({ const applyCustomFilter = (text) => { if (!text) { - dispatch(clearDataFilter(layerId, dataKey)) + onClear() return } - commitSearch(text, { tree, dispatch, layerId, dataKey }) + commitSearch(text, { tree, onChange }) } const onSearchChange = ({ value }) => { @@ -166,7 +158,7 @@ const useGroupFilterInput = ({ const trimmed = sanitized.trim() if (trimmed === '') { if (hasActiveFilter) { - dispatch(clearDataFilter(layerId, dataKey)) + onClear() } return } @@ -238,10 +230,6 @@ const useGroupFilterInput = ({ onEnterKey() closePopover() break - case 'Escape': - event.preventDefault() - closePopover() - break default: break } diff --git a/src/components/datatable/useRowClickSelection.js b/src/components/datatable/useRowClickSelection.js new file mode 100644 index 0000000000..713c7908cb --- /dev/null +++ b/src/components/datatable/useRowClickSelection.js @@ -0,0 +1,36 @@ +import { useCallback, useRef } from 'react' +import { getRowClickAction, getRowId } from '../../util/dataTable.js' + +export const useRowClickSelection = ({ rows, onToggle, onSelectRange }) => { + const lastClickedRowIndexRef = useRef(null) + + return useCallback( + (row, event) => { + const id = getRowId(row) + + if (!id || !rows) { + return + } + + const rowIndex = rows.findIndex((r) => getRowId(r) === id) + const action = getRowClickAction(event, { + id, + rowIndex, + rows, + lastClickedRowIndex: lastClickedRowIndexRef.current, + }) + + if (!action) { + return + } + + if (action.type === 'range') { + onSelectRange(action.ids) + } else { + onToggle(action.id) + } + lastClickedRowIndexRef.current = rowIndex + }, + [rows, onToggle, onSelectRange] + ) +} diff --git a/src/components/datatable/useRowContextMenuHighlight.js b/src/components/datatable/useRowContextMenuHighlight.js new file mode 100644 index 0000000000..77a307ac05 --- /dev/null +++ b/src/components/datatable/useRowContextMenuHighlight.js @@ -0,0 +1,38 @@ +import { useCallback, useRef } from 'react' +import { shouldClearFeatureHighlight } from '../../util/dataTable.js' + +export const useRowContextMenuHighlight = ({ onPin, onClear }) => { + const menuOpenRef = useRef(false) + + const onContextMenuOpen = useCallback( + (row) => { + menuOpenRef.current = true + onPin(row) + }, + [onPin] + ) + + const guardedClear = useCallback( + (event) => { + if (menuOpenRef.current) { + return + } + if (shouldClearFeatureHighlight(event)) { + onClear() + } + }, + [onClear] + ) + + const onMenuClose = useCallback( + (highlightChanged) => { + menuOpenRef.current = false + if (!highlightChanged) { + onClear() + } + }, + [onClear] + ) + + return { onContextMenuOpen, guardedClear, onMenuClose } +} diff --git a/src/components/datatable/useRowSelection.js b/src/components/datatable/useRowSelection.js index 870bda6bd1..18380b7d49 100644 --- a/src/components/datatable/useRowSelection.js +++ b/src/components/datatable/useRowSelection.js @@ -1,6 +1,4 @@ import { useCallback, useMemo } from 'react' -import { useDispatch } from 'react-redux' -import { selectAllFeatures, clearSelection } from '../../actions/selection.js' export const getReversedSelection = (selectedIds, allRowIds) => { const selectedIdSet = new Set(selectedIds) @@ -14,10 +12,8 @@ export const useRowSelection = ({ selectedIds, selectedIdSet, allRowIds, - layerId, + onChange, }) => { - const dispatch = useDispatch() - const allRowIdSet = useMemo(() => new Set(allRowIds), [allRowIds]) const isAllSelected = useMemo( @@ -32,22 +28,12 @@ export const useRowSelection = ({ ? selectedIds.filter((id) => !allRowIdSet.has(id)) : [...new Set([...selectedIds, ...allRowIds])] - if (nextIds.length) { - dispatch(selectAllFeatures(nextIds, layerId)) - } else { - dispatch(clearSelection()) - } - }, [dispatch, isAllSelected, allRowIds, allRowIdSet, selectedIds, layerId]) + onChange(nextIds) + }, [isAllSelected, allRowIds, allRowIdSet, selectedIds, onChange]) const onReverseSelection = useCallback(() => { - const nextIds = getReversedSelection(selectedIds, allRowIds) - - if (nextIds.length) { - dispatch(selectAllFeatures(nextIds, layerId)) - } else { - dispatch(clearSelection()) - } - }, [dispatch, selectedIds, allRowIds, layerId]) + onChange(getReversedSelection(selectedIds, allRowIds)) + }, [selectedIds, allRowIds, onChange]) return { isAllSelected, diff --git a/src/components/datatable/useSortState.js b/src/components/datatable/useSortState.js new file mode 100644 index 0000000000..2a77e15cc8 --- /dev/null +++ b/src/components/datatable/useSortState.js @@ -0,0 +1,25 @@ +import { useCallback, useReducer } from 'react' +import { SORT_ASCENDING } from '../../constants/dataTable.js' +import { getNextSorting } from '../../util/dataTable.js' + +export const useSortState = (initialSortField = 'name') => { + const [{ sortField, sortDirection }, setSorting] = useReducer( + (sorting, newSorting) => ({ ...sorting, ...newSorting }), + { sortField: initialSortField, sortDirection: SORT_ASCENDING } + ) + + const sortData = useCallback( + ({ name }) => { + setSorting( + getNextSorting( + name, + { sortField, sortDirection }, + { defaultSortField: initialSortField } + ) + ) + }, + [sortField, sortDirection, initialSortField] + ) + + return { sortField, sortDirection, sortData } +} diff --git a/src/components/datatable/useTableData.js b/src/components/datatable/useTableData.js index 55f62b606d..91a12fc49b 100644 --- a/src/components/datatable/useTableData.js +++ b/src/components/datatable/useTableData.js @@ -3,8 +3,6 @@ import { useDeferredValue, useMemo, useRef } from 'react' import { useSelector } from 'react-redux' import { SENTINEL_SELECTED_ROW, - SORT_ASCENDING, - TYPE_ORG_UNIT, RENDERER_ORG_UNIT, RENDERER_ORG_UNIT_NAME, } from '../../constants/dataTable.js' @@ -25,14 +23,14 @@ import { buildKnownOrgUnitNames } from '../../util/orgUnits.js' import { buildRowCells, getColumnDistinctValues, + sortColumnOptions, } from '../../util/tableColumns.js' import { - TYPE_STRING, ERROR_NON_HOMOGENOUS_FEATURES, getHeadersForLayer, } from '../../util/tableHeaders.js' import { ERROR_NO_VALID_DATA, buildTableData } from '../../util/tableRows.js' -import { compareColumnOptionValues, compareRows } from '../../util/tableSort.js' +import { compareRows } from '../../util/tableSort.js' const ERROR_NO_HEADERS = 'NO_HEADERS' @@ -82,6 +80,8 @@ export const useTableData = ({ legend, styleDataItem, countEventsOutsideOrgUnits, + bands, + band, data, dataWithoutCoords, dataFilters, @@ -177,6 +177,8 @@ export const useTableData = ({ countEventsOutsideOrgUnits, aggregationType, legend, + bands, + band, data: dataWithAggregations, rawData: data, } @@ -200,6 +202,8 @@ export const useTableData = ({ legend, styleDataItem, countEventsOutsideOrgUnits, + bands, + band, dataWithAggregations, data, layerHeaders, @@ -217,30 +221,14 @@ export const useTableData = ({ ) // Cheap: just re-orders each column's already-known distinct-value list - const columnOptions = useMemo(() => { - if (!columnDistinctValues) { - return EMPTY_COLUMN_OPTIONS - } - - const result = {} - Object.entries(columnDistinctValues).forEach( - ([dataKey, { values, type }]) => { - const direction = - dataKey === sortField ? sortDirection : SORT_ASCENDING - result[dataKey] = [...values] - .sort((a, b) => - compareColumnOptionValues(a, b, { - dataKey, - type, - direction, - }) - ) - .map((value) => ({ value })) - } - ) - - return Object.keys(result).length ? result : EMPTY_COLUMN_OPTIONS - }, [columnDistinctValues, sortField, sortDirection]) + const columnOptions = useMemo( + () => + sortColumnOptions(columnDistinctValues, { + sortField, + sortDirection, + }) ?? EMPTY_COLUMN_OPTIONS, + [columnDistinctValues, sortField, sortDirection] + ) const orgUnitPathValues = useMemo( () => @@ -277,16 +265,10 @@ export const useTableData = ({ let filteredData = filterData(dataWithAggregations, dataFilters) if (globalSearch?.trim()) { - const stringDataKeys = headers - .filter((h) => h.type === TYPE_STRING) - .map((h) => h.dataKey) - const orgUnitDataKeys = headers - .filter((h) => h.type === TYPE_ORG_UNIT) - .map((h) => h.dataKey) filteredData = filterByGlobalSearch(filteredData, globalSearch, { - stringDataKeys, - orgUnitDataKeys, - idToName: orgUnitIdToName, + headers, + orgUnitIdToName, + keyAnalysisDigitGroupSeparator, }) } diff --git a/src/components/layers/overlays/OverlayCard.jsx b/src/components/layers/overlays/OverlayCard.jsx index a490f28cda..ecf981a97a 100644 --- a/src/components/layers/overlays/OverlayCard.jsx +++ b/src/components/layers/overlays/OverlayCard.jsx @@ -5,6 +5,7 @@ import i18n from '@dhis2/d2-i18n' import PropTypes from 'prop-types' import React, { useState } from 'react' import { connect } from 'react-redux' +import { clearDataFilters } from '../../../actions/dataFilters.js' import { toggleDataTable } from '../../../actions/dataTable.js' import { editLayer, @@ -36,6 +37,46 @@ import DataDownloadDialog from '../download/DataDownloadDialog.jsx' import LayerCard from '../LayerCard.jsx' import styles from './styles/OverlayCard.module.css' +const getCardContent = ({ loadError, legend }) => { + if (loadError) { + return ( +
+ +
+ ) + } + return ( + legend && ( +
+ +
+ ) + ) +} + +const getOpenAsHandler = (layer, baseUrl, setCurrentAO) => async (type) => { + const currentAO = getAnalyticalObjectFromThematicLayer(layer) + + // Store AO in user data store + await setCurrentAO(currentAO) + + // Open it in another app + window.open( + `${baseUrl}/${APP_URLS[type]}/#/currentAnalyticalObject`, + '_blank' + ) +} + +const getTitle = (isLoaded, name) => + isLoaded ? name : i18n.t('Loading layer') + '...' + +const getSubtitle = (isLoaded, legend) => + isLoaded && legend?.period ? legend.period : null + +const ifAllowed = (allowed, handler) => (allowed ? handler : undefined) + const OverlayCard = ({ layer, editLayer, @@ -45,6 +86,7 @@ const OverlayCard = ({ toggleLayerExpand, toggleLayerVisibility, toggleDataTable, + clearDataFilters, }) => { const [showDataDownloadDialog, setShowDataDownloadDialog] = useState(false) const { baseUrl } = useConfig() @@ -61,52 +103,33 @@ const OverlayCard = ({ layer: layerType, isLoaded, loadError, + dataFilters, } = layer const canEdit = layerType !== EXTERNAL_LAYER const canToggleDataTable = DATA_TABLE_LAYER_TYPES.includes(layerType) const canDownload = DOWNLOADABLE_LAYER_TYPES.includes(layerType) const canOpenAs = OPEN_AS_LAYER_TYPES.includes(layerType) - - const getCardContent = () => { - if (loadError) { - return ( -
- -
- ) - } - return ( - legend && ( -
- -
- ) - ) - } + const hasDataFilters = Object.keys(dataFilters ?? {}).length > 0 return ( <> toggleLayerExpand(id)} - onEdit={canEdit ? () => editLayer(layer) : undefined} - toggleDataTable={ - canToggleDataTable ? () => toggleDataTable(id) : undefined - } + onEdit={ifAllowed(canEdit, () => editLayer(layer))} + toggleDataTable={ifAllowed(canToggleDataTable, () => + toggleDataTable(id) + )} + onClearDataFilters={ifAllowed(hasDataFilters, () => + clearDataFilters(id) + )} toggleLayerVisibility={() => toggleLayerVisibility(id)} onOpacityChange={(newOpacity) => changeLayerOpacity(id, newOpacity) @@ -118,31 +141,16 @@ const OverlayCard = ({ msg: i18n.t('{{- name}} deleted.', { name }), }) }} - downloadData={ - canDownload - ? () => setShowDataDownloadDialog(true) - : undefined - } - openAs={ - canOpenAs - ? async (type) => { - const currentAO = - getAnalyticalObjectFromThematicLayer(layer) - - // Store AO in user data store - await set(currentAO) - - // Open it in another app - window.open( - `${baseUrl}/${APP_URLS[type]}/#/currentAnalyticalObject`, - '_blank' - ) - } - : undefined - } + downloadData={ifAllowed(canDownload, () => + setShowDataDownloadDialog(true) + )} + openAs={ifAllowed( + canOpenAs, + getOpenAsHandler(layer, baseUrl, set) + )} hasError={!!loadError} > - {getCardContent()} + {getCardContent({ loadError, legend })} {showDataDownloadDialog && ( ({ const mockStore = configureMockStore() describe('OverlayCard', () => { - const renderCard = (name) => - render( - + const renderCard = (name, layerOverrides = {}) => { + const store = mockStore({ + dataTable: { openIds: [] }, + aggregations: {}, + }) + const rendered = render( + { isExpanded: true, isVisible: true, opacity: 1, + ...layerOverrides, }} /> ) + return { ...rendered, store } + } - // Regression test for DHIS2-19998: special characters in the layer name - // must not be HTML-escaped in the "deleted" alert (default i18next - // interpolation escapes "<" to "<"). test('shows the raw layer name with special characters in the removal alert', async () => { renderCard('Children < 5y & "others"') @@ -58,4 +62,31 @@ describe('OverlayCard', () => { msg: 'Children < 5y & "others" deleted.', }) }) + + test('does not show a clear-filters button when the layer has no active dataFilters', () => { + const { container } = renderCard('Layer 1') + expect( + container.querySelector( + '[data-test="layer-clear-data-filters-button"]' + ) + ).not.toBeInTheDocument() + }) + + test('shows a clear-filters button when the layer has active dataFilters, and dispatches clearDataFilters on click', () => { + const { container, store } = renderCard('Layer 1', { + dataFilters: { population: '>100' }, + }) + + const button = container.querySelector( + '[data-test="layer-clear-data-filters-button"]' + ) + expect(button).toBeInTheDocument() + + fireEvent.click(button) + + expect(store.getActions()).toContainEqual({ + type: 'DATA_FILTERS_CLEAR_ALL', + layerId: 'layer1', + }) + }) }) diff --git a/src/components/layers/toolbar/LayerToolbar.jsx b/src/components/layers/toolbar/LayerToolbar.jsx index 476fcc999e..a19ebdaa29 100644 --- a/src/components/layers/toolbar/LayerToolbar.jsx +++ b/src/components/layers/toolbar/LayerToolbar.jsx @@ -3,7 +3,7 @@ import { Tooltip, IconEdit24, IconView24, IconViewOff24 } from '@dhis2/ui' import cx from 'classnames' import PropTypes from 'prop-types' import React from 'react' -import { IconButton } from '../../core/index.js' +import { FilterActiveIcon, IconButton } from '../../core/index.js' import LayerToolbarMoreMenu from './LayerToolbarMoreMenu.jsx' import OpacitySlider from './OpacitySlider.jsx' import styles from './styles/LayerToolbar.module.css' @@ -14,6 +14,7 @@ const LayerToolbar = ({ isVisible, onOpacityChange, toggleLayerVisibility, + onClearDataFilters, hasError, ...expansionMenuProps }) => { @@ -60,11 +61,25 @@ const LayerToolbar = ({ />
-
- +
+ {onClearDataFilters && ( + + + + )} +
+ +
) @@ -77,6 +92,7 @@ LayerToolbar.propTypes = { hasOpacity: PropTypes.bool, isVisible: PropTypes.bool, opacity: PropTypes.number, + onClearDataFilters: PropTypes.func, onEdit: PropTypes.func, } diff --git a/src/components/layers/toolbar/LayerToolbarMoreMenu.jsx b/src/components/layers/toolbar/LayerToolbarMoreMenu.jsx index 4092711c2c..b774fed51f 100644 --- a/src/components/layers/toolbar/LayerToolbarMoreMenu.jsx +++ b/src/components/layers/toolbar/LayerToolbarMoreMenu.jsx @@ -27,7 +27,7 @@ const LayerToolbarMoreMenu = ({ toggleDataTable, openAs, downloadData, - dataTableOpen, + openIds, hasOrgUnitData, isLoading, hasError, @@ -43,8 +43,7 @@ const LayerToolbarMoreMenu = ({ return null } - const showDataTableDisabled = - !hasOrgUnitData && (!dataTableOpen || dataTableOpen !== layer.id) + const showDataTableDisabled = !hasOrgUnitData && !openIds.includes(layer.id) return ( <> @@ -71,7 +70,7 @@ const LayerToolbarMoreMenu = ({ {toggleDataTable && ( { const isEarthEngine = layer.layer === EARTH_ENGINE_LAYER @@ -179,6 +178,6 @@ export default connect( const isLoading = isEarthEngine && hasOrgUnitData && !aggregations[layer.id] - return { dataTableOpen, hasOrgUnitData, isLoading } + return { openIds, hasOrgUnitData, isLoading } } )(LayerToolbarMoreMenu) diff --git a/src/components/layers/toolbar/__tests__/LayerToolbar.spec.jsx b/src/components/layers/toolbar/__tests__/LayerToolbar.spec.jsx index df7b3323ad..624b403f3a 100644 --- a/src/components/layers/toolbar/__tests__/LayerToolbar.spec.jsx +++ b/src/components/layers/toolbar/__tests__/LayerToolbar.spec.jsx @@ -93,4 +93,34 @@ describe('LayerToolbar', () => { expect(toggleVisibleFn).toHaveBeenCalledTimes(1) expect(editFn).toHaveBeenCalledTimes(1) }) + + it('Should not render a clear-filters button when the layer has no active dataFilters', () => { + const { container } = render() + expect( + container.querySelector( + '[data-test="layer-clear-data-filters-button"]' + ) + ).not.toBeInTheDocument() + }) + + it('Should render a clear-filters button when onClearDataFilters is provided', () => { + const { container } = render( + + ) + expect(container).toMatchSnapshot() + }) + + it('Should call onClearDataFilters callback on button press', async () => { + const clearDataFiltersFn = jest.fn() + const { container } = render( + + ) + + await fireEvent.click( + container.querySelector( + '[data-test="layer-clear-data-filters-button"]' + ) + ) + expect(clearDataFiltersFn).toHaveBeenCalledTimes(1) + }) }) diff --git a/src/components/layers/toolbar/__tests__/LayerToolbarMoreMenu.spec.jsx b/src/components/layers/toolbar/__tests__/LayerToolbarMoreMenu.spec.jsx index f524fa7bce..cdcca16158 100644 --- a/src/components/layers/toolbar/__tests__/LayerToolbarMoreMenu.spec.jsx +++ b/src/components/layers/toolbar/__tests__/LayerToolbarMoreMenu.spec.jsx @@ -8,7 +8,7 @@ const mockStore = configureMockStore() describe('LayerToolbarMoreMenu', () => { test('does not render if no props passed', () => { - const store = {} + const store = { dataTable: { openIds: [] } } const { container } = render( @@ -20,7 +20,7 @@ describe('LayerToolbarMoreMenu', () => { test('renders menu with Remove layer only', async () => { const store = { - dataTable: null, + dataTable: { openIds: [] }, aggregations: {}, } @@ -51,7 +51,7 @@ describe('LayerToolbarMoreMenu', () => { test('renders menu with Remove layer and Edit layer options', async () => { const store = { - dataTable: null, + dataTable: { openIds: [] }, aggregations: {}, } @@ -86,6 +86,7 @@ describe('LayerToolbarMoreMenu', () => { test('renders two MenuItems with no divider if only passed toggleDataTable and downloadData', async () => { const store = { + dataTable: { openIds: [] }, aggregations: {}, } @@ -120,6 +121,7 @@ describe('LayerToolbarMoreMenu', () => { test('renders only toggleDataTable menu', async () => { const store = { + dataTable: { openIds: [] }, aggregations: {}, } @@ -150,8 +152,42 @@ describe('LayerToolbarMoreMenu', () => { }) }) + test('shows "Hide data table" and keeps it enabled when this layer\'s tab is already open', async () => { + const store = { + dataTable: { openIds: ['someOtherLayer', 'rainbowdash'] }, + aggregations: {}, + } + + const layer = { + id: 'rainbowdash', + data: 'hasdata', + } + + render( + + + + ) + + fireEvent.click(screen.getByLabelText('Toggle layer menu')) + + await waitFor(() => { + expect(screen.queryByText('Hide data table')).toBeTruthy() + expect( + screen + .queryByText('Hide data table') + .closest('li') + .classList.contains('disabled') + ).toBe(false) + }) + }) + test('enables Show data table for a server-clustered event layer with no data yet', async () => { const store = { + dataTable: { openIds: [] }, aggregations: {}, } @@ -185,6 +221,7 @@ describe('LayerToolbarMoreMenu', () => { test('also enables Download data for a server-clustered event layer with no data yet', async () => { const store = { + dataTable: { openIds: [] }, aggregations: {}, } @@ -219,6 +256,7 @@ describe('LayerToolbarMoreMenu', () => { test('renders three MenuItems WITH divider if passed toggleDataTable, onEdit, and onRemove', async () => { const store = { + dataTable: { openIds: [] }, aggregations: {}, } @@ -255,6 +293,7 @@ describe('LayerToolbarMoreMenu', () => { test('renders four MenuItems WITH divider if passed toggleDataTable, downloadData, onEdit, and onRemove', async () => { const store = { + dataTable: { openIds: [] }, aggregations: {}, } @@ -291,6 +330,7 @@ describe('LayerToolbarMoreMenu', () => { test('renders Duplicate layer item between Edit layer and Remove layer', async () => { const store = { + dataTable: { openIds: [] }, aggregations: {}, } @@ -328,6 +368,7 @@ describe('LayerToolbarMoreMenu', () => { test('renders disabled menu items if there was an error', async () => { const store = { + dataTable: { openIds: [] }, aggregations: {}, } diff --git a/src/components/layers/toolbar/__tests__/__snapshots__/LayerToolbar.spec.jsx.snap b/src/components/layers/toolbar/__tests__/__snapshots__/LayerToolbar.spec.jsx.snap index f3fc0eb58f..b0ee8dc2bd 100644 --- a/src/components/layers/toolbar/__tests__/__snapshots__/LayerToolbar.spec.jsx.snap +++ b/src/components/layers/toolbar/__tests__/__snapshots__/LayerToolbar.spec.jsx.snap @@ -1,5 +1,82 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`LayerToolbar Should render a clear-filters button when onClearDataFilters is provided 1`] = ` +
+
+ +
+
+
+ +
+
+
+
+ +
+
+ LayerToolbarMoreMenu +
+
+
+
+
+`; + exports[`LayerToolbar Should render edit button 1`] = `
-
- LayerToolbarMoreMenu +
+
+ LayerToolbarMoreMenu +
@@ -101,10 +182,14 @@ exports[`LayerToolbar Should render only a visibility toggle and opacity slider
-
- LayerToolbarMoreMenu +
+
+ LayerToolbarMoreMenu +
@@ -148,10 +233,14 @@ exports[`LayerToolbar Should show SvgViewOff24 when not visible 1`] = `
-
- LayerToolbarMoreMenu +
+
+ LayerToolbarMoreMenu +
diff --git a/src/components/layers/toolbar/styles/LayerToolbar.module.css b/src/components/layers/toolbar/styles/LayerToolbar.module.css index f875e57066..73a3f138c6 100644 --- a/src/components/layers/toolbar/styles/LayerToolbar.module.css +++ b/src/components/layers/toolbar/styles/LayerToolbar.module.css @@ -16,6 +16,12 @@ margin-top: 1px; } -.menuButton { +.trailingActions { + display: flex; + align-items: center; margin-left: auto; } + +.clearDataFiltersButton { + margin-right: var(--spacers-dp8); +} diff --git a/src/components/map/MapPosition.jsx b/src/components/map/MapPosition.jsx index 0fba1fd211..17e4cc0163 100644 --- a/src/components/map/MapPosition.jsx +++ b/src/components/map/MapPosition.jsx @@ -2,6 +2,7 @@ import cx from 'classnames' import React, { useState, useEffect, useRef } from 'react' import { useSelector, useDispatch } from 'react-redux' import { setMapBounds } from '../../actions/dataTable.js' +import { isDataTableOpen } from '../../util/dataTable.js' import { getSplitViewLayer } from '../../util/helpers.js' import DownloadMapInfo from '../download/DownloadMapInfo.jsx' import NorthArrow from '../download/NorthArrow.jsx' @@ -26,7 +27,9 @@ const MapPosition = () => { const { id: mapId, mapViews: layers } = useSelector((state) => state.map) const { downloadMode, layersPanelOpen, rightPanelOpen, dataTableHeight } = useSelector((state) => state.ui) - const dataTableOpen = useSelector((state) => !!state.dataTable) + const dataTableOpen = useSelector((state) => + isDataTableOpen(state.dataTable) + ) const downloadMapInfoOpen = downloadMode && diff --git a/src/constants/actionTypes.js b/src/constants/actionTypes.js index cc05ef1559..8387f1e066 100644 --- a/src/constants/actionTypes.js +++ b/src/constants/actionTypes.js @@ -39,7 +39,9 @@ export const LAYER_FORCE_CLIENT_CLUSTER_SET = 'LAYER_FORCE_CLIENT_CLUSTER_SET' /* DATA TABLE */ export const DATA_TABLE_CLOSE = 'DATA_TABLE_CLOSE' +export const DATA_TABLE_OPEN = 'DATA_TABLE_OPEN' export const DATA_TABLE_TOGGLE = 'DATA_TABLE_TOGGLE' +export const DATA_TABLE_ACTIVE_LAYER_SET = 'DATA_TABLE_ACTIVE_LAYER_SET' export const DATA_TABLE_RESIZE = 'DATA_TABLE_RESIZE' export const MAP_BOUNDS_CHANGED = 'MAP_BOUNDS_CHANGED' export const TOGGLE_SHOW_ONLY_IN_VIEW = 'TOGGLE_SHOW_ONLY_IN_VIEW' diff --git a/src/hooks/__tests__/useLayersLoader.spec.js b/src/hooks/__tests__/useLayersLoader.spec.js index 19fec7e93b..d8de99c618 100644 --- a/src/hooks/__tests__/useLayersLoader.spec.js +++ b/src/hooks/__tests__/useLayersLoader.spec.js @@ -79,7 +79,7 @@ describe('useLayersLoader - data table reload trigger', () => { { ...baseLayer, serverCluster: true, isExtended: false }, ], }, - dataTable: 'a', + dataTable: { openIds: ['a'] }, }) expect(store.getActions()).toEqual([]) @@ -97,7 +97,7 @@ describe('useLayersLoader - data table reload trigger', () => { }, ], }, - dataTable: 'a', + dataTable: { openIds: ['a'] }, }) expect(store.getActions()).toEqual([ @@ -117,7 +117,7 @@ describe('useLayersLoader - data table reload trigger', () => { }, ], }, - dataTable: 'a', + dataTable: { openIds: ['a'] }, }) expect(store.getActions()).toEqual([]) @@ -130,13 +130,26 @@ describe('useLayersLoader - data table reload trigger', () => { { ...baseLayer, serverCluster: false, isExtended: false }, ], }, - dataTable: 'a', + dataTable: { openIds: ['a'] }, }) expect(store.getActions()).toEqual([ { type: 'LAYER_LOADING_SET', id: 'a' }, ]) }) + + test('does not reload a layer whose own tab is closed, even when another layer tab is open', () => { + const { store } = renderWithStore({ + map: { + mapViews: [ + { ...baseLayer, serverCluster: false, isExtended: false }, + ], + }, + dataTable: { openIds: ['someOtherLayer'] }, + }) + + expect(store.getActions()).toEqual([]) + }) }) describe('useLayersLoader - spatialSupport plumbing', () => { @@ -147,7 +160,7 @@ describe('useLayersLoader - spatialSupport plumbing', () => { map: { mapViews: [{ ...baseLayer, isLoaded: false }], }, - dataTable: null, + dataTable: { openIds: [] }, }) expect(eventLoader).toHaveBeenCalledWith( @@ -162,7 +175,7 @@ describe('useLayersLoader - spatialSupport plumbing', () => { map: { mapViews: [{ ...baseLayer, isLoaded: false }], }, - dataTable: null, + dataTable: { openIds: [] }, }) expect(eventLoader).toHaveBeenCalledWith( diff --git a/src/hooks/useLayersLoader.js b/src/hooks/useLayersLoader.js index 66b1083b56..ee16701a85 100644 --- a/src/hooks/useLayersLoader.js +++ b/src/hooks/useLayersLoader.js @@ -38,7 +38,7 @@ export const useLayersLoader = () => { } = useCachedData() const { showAlerts } = useLoaderAlerts() const allLayers = useSelector((state) => state.map.mapViews) - const dataTable = useSelector((state) => state.dataTable) + const openIds = useSelector((state) => state.dataTable.openIds) const dispatch = useDispatch() const { show: showLoaderAlert } = useAlert( ({ layer }) => `Could not load layer ${layer}`, @@ -65,7 +65,7 @@ export const useLayersLoader = () => { analyticsEngine, // Thematic and Event loader periodTypeData, // Thematic and Event loader serverVersion, // Tracked entity loader - loadExtended: !!dataTable, // Event loader + loadExtended: openIds.includes(config.id), // Event loader spatialSupport, // Event loader }) if (result.alerts) { @@ -87,7 +87,7 @@ export const useLayersLoader = () => { // event extended data hasn't been loaded yet - so load it if ( layer.layer === EVENT_LAYER && - layer.id === dataTable && + openIds.includes(layer.id) && !layer.isExtended && (!layer.serverCluster || layer.forceClientCluster) ) { @@ -128,7 +128,7 @@ export const useLayersLoader = () => { showAlerts, showLoaderAlert, baseUrl, - dataTable, + openIds, serverVersion, spatialSupport, ]) diff --git a/src/loaders/__tests__/eventLoader.spec.js b/src/loaders/__tests__/eventLoader.spec.js index 976864bef1..62bc98f52e 100644 --- a/src/loaders/__tests__/eventLoader.spec.js +++ b/src/loaders/__tests__/eventLoader.spec.js @@ -981,4 +981,185 @@ describe('eventLoader - isExtended vs serverCluster', () => { // actually loaded (capped), not the raw analytics total. expect(result.legend.items[0].count).toBe(0) }) + + test('attaches optionSetOptionsByCode from the analytics response metadata, matched by code (not by the metaData.items key)', async () => { + const args = makeArgs({ ...baseConfig(), eventClustering: false }) + args.analyticsEngine.events.getQuery = jest.fn().mockResolvedValue({ + headers: [ + { name: 'psi', valueType: 'TEXT' }, + { name: 'de1', valueType: 'TEXT', optionSet: { id: 'os1' } }, + ], + metaData: { + items: { + optUid1: { name: 'Male', code: 'M' }, + optUid2: { name: 'Female', code: 'F' }, + prog1: { name: 'Program 1' }, // no `code` - must be ignored + }, + pager: { total: 0 }, + }, + rows: [], + }) + + const result = await eventLoader(args) + + expect(result.optionSetOptionsByCode).toEqual({ + os1: { M: 'Male', F: 'Female' }, + }) + }) + + test('does not set optionSetOptionsByCode when no header has an optionSet', async () => { + const result = await eventLoader( + makeArgs({ ...baseConfig(), eventClustering: false }) + ) + + expect(result.optionSetOptionsByCode).toBeUndefined() + }) +}) + +describe('eventLoader - extended column top-up', () => { + const NEW_DE_UID = 'dataElemUID' // 11-char valid UID + + const loadedConfig = (overrides = {}) => ({ + program: { id: 'prog1' }, + programStage: { id: 'stage1', name: 'Stage 1' }, + columns: [{ dimension: 'de1' }], + filters: [], + rows: [], + eventClustering: false, + startDate: '2024-01-01', + endDate: '2024-01-31', + isLoaded: true, + isExtended: false, + serverCluster: false, + headers: [ + { name: 'psi', valueType: 'TEXT' }, + { name: 'de1', valueType: 'NUMBER' }, + ], + data: [ + { + type: 'Feature', + id: 'event1', + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { id: 'event1', de1: 5 }, + }, + ], + ...overrides, + }) + + const makeArgs = (config, { engineQueryImpl, getQueryImpl } = {}) => ({ + config, + engine: { + query: jest.fn().mockImplementation( + engineQueryImpl ?? + (() => + Promise.resolve({ + programStage: { + programStageDataElements: [ + { + displayInReports: true, + dataElement: { + id: NEW_DE_UID, + name: 'New DE', + valueType: 'NUMBER', + }, + }, + ], + }, + })) + ), + }, + keyAnalysisDisplayProperty: 'name', + keyAnalysisDigitGroupSeparator: 'NONE', + analyticsEngine: { + request: FakeAnalyticsRequest, + events: { + getCount: jest + .fn() + .mockResolvedValue({ count: 0, extent: null }), + getQuery: + getQueryImpl ?? + jest.fn().mockResolvedValue({ + headers: [ + { name: 'psi', valueType: 'TEXT' }, + { name: NEW_DE_UID, valueType: 'NUMBER' }, + ], + metaData: { items: {}, pager: { total: 1 } }, + rows: [['event1', '10']], + }), + }, + }, + periodTypeData: undefined, + loadExtended: true, + spatialSupport: true, + }) + + test('fetches only the metadata + delta query, merges the new column by id, and never re-runs clustering count', async () => { + const args = makeArgs(loadedConfig()) + + const result = await eventLoader(args) + + expect(args.engine.query).toHaveBeenCalledTimes(1) + expect(args.analyticsEngine.events.getQuery).toHaveBeenCalledTimes(1) + expect(args.analyticsEngine.events.getCount).not.toHaveBeenCalled() + + expect(result.isExtended).toBe(true) + expect(result.headers.map((h) => h.name)).toEqual([ + 'psi', + 'de1', + NEW_DE_UID, + ]) + expect(result.data[0].properties.de1).toBe(5) // pre-existing, untouched + expect(result.data[0].properties[NEW_DE_UID]).toBe(10) // new, parsed to a number + }) + + test('does nothing further when every "display in reports" column is already present', async () => { + const args = makeArgs(loadedConfig(), { + engineQueryImpl: () => + Promise.resolve({ + programStage: { + programStageDataElements: [ + { + displayInReports: true, + dataElement: { + id: 'de1', + name: 'DE 1', + valueType: 'NUMBER', + }, + }, + ], + }, + }), + }) + + const result = await eventLoader(args) + + expect(args.analyticsEngine.events.getQuery).not.toHaveBeenCalled() + expect(result.isExtended).toBe(true) + expect(result.headers).toEqual(loadedConfig().headers) + }) + + test('falls back to a full reload when there is no prior client dataset (e.g. was previously server-clustered)', async () => { + const args = makeArgs( + loadedConfig({ headers: undefined, data: undefined }) + ) + + await eventLoader(args) + + expect(args.analyticsEngine.events.getCount).not.toHaveBeenCalled() // eventClustering is false here + expect(args.analyticsEngine.events.getQuery).toHaveBeenCalledTimes(1) + }) + + test('runs a full load, not the top-up path, on a genuine first load even if the table is already open', async () => { + const args = makeArgs( + loadedConfig({ + isLoaded: undefined, + headers: undefined, + data: undefined, + }) + ) + + await eventLoader(args) + + expect(args.analyticsEngine.events.getQuery).toHaveBeenCalledTimes(1) + }) }) diff --git a/src/loaders/__tests__/trackedEntityLoader.spec.js b/src/loaders/__tests__/trackedEntityLoader.spec.js index 0968ca1327..cf137be6e0 100644 --- a/src/loaders/__tests__/trackedEntityLoader.spec.js +++ b/src/loaders/__tests__/trackedEntityLoader.spec.js @@ -3,6 +3,7 @@ import { getAttributeProperties, applyParsedConfig, toGeoJson, + toOptionSetOptionsByCode, } from '../trackedEntityLoader.js' jest.mock('../../components/map/MapApi.js', () => ({ @@ -288,3 +289,27 @@ describe('toGeoJson', () => { expect(result[0].properties.genderUid).toBe('Male') }) }) + +describe('toOptionSetOptionsByCode', () => { + it('converts a Map> to a plain nested object', () => { + const optionNamesByOptionSet = new Map([ + [ + 'os1', + new Map([ + ['M', 'Male'], + ['F', 'Female'], + ]), + ], + ['os2', new Map([['Y', 'Yes']])], + ]) + + expect(toOptionSetOptionsByCode(optionNamesByOptionSet)).toEqual({ + os1: { M: 'Male', F: 'Female' }, + os2: { Y: 'Yes' }, + }) + }) + + it('returns an empty object for an empty map', () => { + expect(toOptionSetOptionsByCode(new Map())).toEqual({}) + }) +}) diff --git a/src/loaders/eventLoader.js b/src/loaders/eventLoader.js index 2a96c1bbf7..8cc7527222 100644 --- a/src/loaders/eventLoader.js +++ b/src/loaders/eventLoader.js @@ -24,7 +24,11 @@ import { import { cssColor, getContrastColor } from '../util/colors.js' import { parseJsonConfig } from '../util/config.js' import { loadEventCoordinateFieldName } from '../util/coordinatesName.js' -import { getAnalyticsRequest, loadData } from '../util/event.js' +import { + getAnalyticsRequest, + getEventColumns, + loadData, +} from '../util/event.js' import { getBounds, getContainingOrgUnit, @@ -128,18 +132,35 @@ const eventLoader = async ({ ? 'displayName' : 'displayShortName' + // A layer already loaded once only needs the incremental columns + const isTopUpLoad = + loadExtended && + config.isLoaded && + !config.isExtended && + !config.serverCluster && + Array.isArray(config.headers) + try { - await loadEventLayer({ - config, - engine, - displayNameProp, - keyAnalysisDisplayProperty, - userOrgUnitIdsByKeyword, - analyticsEngine, - periodTypeData, - loadExtended, - spatialSupport, - }) + if (isTopUpLoad) { + await loadExtendedEventColumns({ + config, + engine, + displayNameProp, + analyticsEngine, + }) + } else { + await loadEventLayer({ + config, + engine, + displayNameProp, + keyAnalysisDisplayProperty, + userOrgUnitIdsByKeyword, + analyticsEngine, + periodTypeData, + loadExtended, + spatialSupport, + }) + } } catch (e) { if ( e.details?.httpStatusCode === 403 || @@ -162,6 +183,93 @@ const eventLoader = async ({ return config } +// Merges only the new "display in reports" columns +const loadExtendedEventColumns = async ({ + config, + engine, + displayNameProp, + analyticsEngine, +}) => { + const { programStage } = config + + const displayColumns = await getEventColumns( + { programStage }, + { engine, nameProperty: displayNameProp } + ) + + const newColumns = displayColumns.filter( + (col) => !config.headers.some((header) => header.name === col.dimension) + ) + + if (!newColumns.length) { + config.isExtended = true + return + } + + const deltaRequest = await getAnalyticsRequest( + { + ...config, + columns: newColumns, + styleDataItem: undefined, + labelDataItem: undefined, + isExtended: false, // the delta is resolved here - don't recurse + }, + { analyticsEngine, nameProperty: displayNameProp, engine } + ) + + const { + data: deltaData, + dataWithoutCoords: deltaDataWithoutCoords, + response, + } = await loadData({ + request: deltaRequest, + config: { ...config, outputIdScheme: 'ID' }, + analyticsEngine, + }) + + const deltaById = new Map( + [...deltaData, ...(deltaDataWithoutCoords ?? [])].map((f) => [ + f.id, + f.properties, + ]) + ) + const mergeDelta = (features) => + features.map((f) => ({ + ...f, + properties: { ...f.properties, ...deltaById.get(f.id) }, + })) + + config.data = mergeDelta(config.data) + if (config.dataWithoutCoords?.length) { + config.dataWithoutCoords = mergeDelta(config.dataWithoutCoords) + } + + const newHeaders = response.headers.filter((header) => + newColumns.some((col) => col.dimension === header.name) + ) + config.headers = [...config.headers, ...newHeaders] + + const numericNewHeaders = newHeaders.filter( + (header) => + isValidUid(header.name) && + numberValueTypes.includes(header.valueType) && + !header.optionSet + ) + if (numericNewHeaders.length) { + config.data = config.data.map((d) => { + const newD = { ...d } + numericNewHeaders.forEach((header) => { + newD.properties[header.name] = parseWithSeparator( + d.properties[header.name] + ) + }) + return newD + }) + } + + config.isExtended = true +} + const loadEventLayer = async ({ config, engine, @@ -397,6 +505,24 @@ const loadEventLayer = async ({ config.headers = response.headers + const optionSetIds = [ + ...new Set( + config.headers + .map((header) => header.optionSet?.id) + .filter(Boolean) + ), + ] + if (optionSetIds.length) { + const optionCodeToName = Object.fromEntries( + Object.values(response.metaData.items) + .filter((item) => item.code) + .map((item) => [item.code, item.name]) + ) + config.optionSetOptionsByCode = Object.fromEntries( + optionSetIds.map((id) => [id, optionCodeToName]) + ) + } + const numericDataItemHeaders = config.headers.filter( (header) => isValidUid(header.name) && diff --git a/src/loaders/trackedEntityLoader.js b/src/loaders/trackedEntityLoader.js index 0311742dc6..08c2c39262 100644 --- a/src/loaders/trackedEntityLoader.js +++ b/src/loaders/trackedEntityLoader.js @@ -214,6 +214,14 @@ const fetchOptionSetIdByAttribute = async ( ) } +export const toOptionSetOptionsByCode = (optionNamesByOptionSet) => + Object.fromEntries( + [...optionNamesByOptionSet].map(([id, codeMap]) => [ + id, + Object.fromEntries(codeMap), + ]) + ) + const fetchOptionNamesByOptionSet = async (engine, optionSetIds) => { const entries = await Promise.all( optionSetIds.map(async (id) => { @@ -501,6 +509,9 @@ const trackedEntityLoader = async ({ name, data, headers, + optionSetOptionsByCode: toOptionSetOptionsByCode( + optionNamesByOptionSet + ), keyAnalysisDigitGroupSeparator, relationships, secondaryData, diff --git a/src/reducers/__tests__/dataTable.spec.js b/src/reducers/__tests__/dataTable.spec.js index 2dcf5180b8..41412d16d0 100644 --- a/src/reducers/__tests__/dataTable.spec.js +++ b/src/reducers/__tests__/dataTable.spec.js @@ -1,52 +1,168 @@ import * as types from '../../constants/actionTypes.js' import dataTable from '../dataTable.js' +const initialState = { + openIds: [], + isPanelVisible: false, + activeLayerId: null, +} + describe('dataTable reducer', () => { - it('returns null by default', () => { - expect(dataTable(undefined, {})).toBe(null) + it('returns the initial state by default', () => { + expect(dataTable(undefined, {})).toEqual(initialState) }) it.each([ - types.DATA_TABLE_CLOSE, types.MAP_NEW, types.MAP_SET, types.DOWNLOAD_MODE_CLOSE, types.DOWNLOAD_MODE_OPEN, - ])('clears the open data table on %s', (type) => { - expect(dataTable('layer1', { type })).toBe(null) + ])('resets fully to the initial state on %s', (type) => { + const state = { + openIds: ['layer1', 'layer2'], + isPanelVisible: true, + activeLayerId: 'layer1', + } + + expect(dataTable(state, { type })).toEqual(initialState) }) - it('closes the data table when toggling the currently open layer', () => { - expect( - dataTable('layer1', { - type: types.DATA_TABLE_TOGGLE, + describe('DATA_TABLE_CLOSE', () => { + it('hides the panel without touching openIds or activeLayerId', () => { + const state = { + openIds: ['layer1', 'layer2'], + isPanelVisible: true, + activeLayerId: 'layer1', + } + + const nextState = dataTable(state, { type: types.DATA_TABLE_CLOSE }) + + expect(nextState).toEqual({ ...state, isPanelVisible: false }) + }) + }) + + describe('DATA_TABLE_OPEN', () => { + it('shows the panel without touching openIds or activeLayerId', () => { + const state = { + openIds: ['layer1'], + isPanelVisible: false, + activeLayerId: 'layer1', + } + + const nextState = dataTable(state, { type: types.DATA_TABLE_OPEN }) + + expect(nextState).toEqual({ ...state, isPanelVisible: true }) + }) + }) + + describe('DATA_TABLE_ACTIVE_LAYER_SET', () => { + it('sets activeLayerId', () => { + const state = dataTable(initialState, { + type: types.DATA_TABLE_ACTIVE_LAYER_SET, id: 'layer1', }) - ).toBe(null) + + expect(state.activeLayerId).toBe('layer1') + }) }) - it('opens the data table when toggling a different layer', () => { - expect( - dataTable('layer1', { + describe('DATA_TABLE_TOGGLE', () => { + it('opens a layer tab that was not open', () => { + const state = dataTable(initialState, { type: types.DATA_TABLE_TOGGLE, - id: 'layer2', + id: 'layer1', }) - ).toBe('layer2') - }) - it('clears the open data table when its layer is removed', () => { - expect( - dataTable('layer1', { type: types.LAYER_REMOVE, id: 'layer1' }) - ).toBe(null) + expect(state.openIds).toEqual(['layer1']) + }) + + it('appends to openIds without closing other tabs', () => { + const state = dataTable( + { ...initialState, openIds: ['layer1'] }, + { type: types.DATA_TABLE_TOGGLE, id: 'layer2' } + ) + + expect(state.openIds).toEqual(['layer1', 'layer2']) + }) + + it('closes an already-open tab', () => { + const state = dataTable( + { ...initialState, openIds: ['layer1', 'layer2'] }, + { type: types.DATA_TABLE_TOGGLE, id: 'layer1' } + ) + + expect(state.openIds).toEqual(['layer2']) + }) + + it('makes the panel visible when opening a tab, even from a hidden state', () => { + const state = dataTable( + { ...initialState, isPanelVisible: false }, + { type: types.DATA_TABLE_TOGGLE, id: 'layer1' } + ) + + expect(state.isPanelVisible).toBe(true) + }) + + it('does not forcibly clear panel visibility when closing a tab', () => { + const prevState = { + ...initialState, + openIds: ['layer1'], + isPanelVisible: true, + } + + const state = dataTable(prevState, { + type: types.DATA_TABLE_TOGGLE, + id: 'layer1', + }) + + expect(state.isPanelVisible).toBe(true) + }) }) - it('keeps the open data table when a different layer is removed', () => { - expect( - dataTable('layer1', { type: types.LAYER_REMOVE, id: 'layer2' }) - ).toBe('layer1') + describe('LAYER_REMOVE', () => { + it('removes the layer from openIds', () => { + const state = dataTable( + { ...initialState, openIds: ['layer1', 'layer2'] }, + { type: types.LAYER_REMOVE, id: 'layer1' } + ) + + expect(state.openIds).toEqual(['layer2']) + }) + + it('clears activeLayerId when the removed layer was the active one', () => { + const prevState = { + ...initialState, + openIds: ['layer1'], + activeLayerId: 'layer1', + } + + const state = dataTable(prevState, { + type: types.LAYER_REMOVE, + id: 'layer1', + }) + + expect(state.activeLayerId).toBeNull() + }) + + it('leaves activeLayerId untouched when a different layer is removed', () => { + const prevState = { + ...initialState, + openIds: ['layer1', 'layer2'], + activeLayerId: 'layer1', + } + + const state = dataTable(prevState, { + type: types.LAYER_REMOVE, + id: 'layer2', + }) + + expect(state.activeLayerId).toBe('layer1') + }) }) it('returns the current state for unknown actions', () => { - expect(dataTable('layer1', { type: 'UNKNOWN' })).toBe('layer1') + const state = { ...initialState, openIds: ['layer1'] } + + expect(dataTable(state, { type: 'UNKNOWN' })).toBe(state) }) }) diff --git a/src/reducers/__tests__/selection.spec.js b/src/reducers/__tests__/selection.spec.js index ddb81237ae..2a0b589664 100644 --- a/src/reducers/__tests__/selection.spec.js +++ b/src/reducers/__tests__/selection.spec.js @@ -107,19 +107,43 @@ describe('selection reducer', () => { expect(state).toEqual({ layerId: 'layer-2', ids: ['x', 'y'] }) }) - it.each([ - types.SELECTION_CLEAR, - types.MAP_NEW, - types.MAP_SET, - types.DATA_TABLE_CLOSE, - types.DATA_TABLE_TOGGLE, - ])('resets to default state on %s', (type) => { - const state = selection( - { layerId: 'layer-1', ids: ['a', 'b'] }, - { type } - ) + it.each([types.SELECTION_CLEAR, types.MAP_NEW, types.MAP_SET])( + 'resets to default state on %s', + (type) => { + const state = selection( + { layerId: 'layer-1', ids: ['a', 'b'] }, + { type } + ) + + expect(state).toEqual({ layerId: null, ids: [] }) + } + ) + + it('keeps the selection when the data table panel is closed', () => { + const prevState = { layerId: 'layer-1', ids: ['a', 'b'] } + const state = selection(prevState, { type: types.DATA_TABLE_CLOSE }) - expect(state).toEqual({ layerId: null, ids: [] }) + expect(state).toBe(prevState) + }) + + it("keeps the selection when the selected layer's own data table tab is toggled", () => { + const prevState = { layerId: 'layer-1', ids: ['a', 'b'] } + const state = selection(prevState, { + type: types.DATA_TABLE_TOGGLE, + id: 'layer-1', + }) + + expect(state).toBe(prevState) + }) + + it("keeps the selection when a different layer's data table tab is toggled", () => { + const prevState = { layerId: 'layer-1', ids: ['a', 'b'] } + const state = selection(prevState, { + type: types.DATA_TABLE_TOGGLE, + id: 'layer-2', + }) + + expect(state).toBe(prevState) }) it('resets to default state when the selected layer is removed', () => { diff --git a/src/reducers/dataTable.js b/src/reducers/dataTable.js index bfd2bf6a0c..e138686c3b 100644 --- a/src/reducers/dataTable.js +++ b/src/reducers/dataTable.js @@ -1,19 +1,49 @@ import * as types from '../constants/actionTypes.js' -const dataTable = (state = null, action) => { +const initialState = { + openIds: [], + isPanelVisible: false, + activeLayerId: null, +} + +const dataTable = (state = initialState, action) => { switch (action.type) { - case types.DATA_TABLE_CLOSE: - case types.MAP_NEW: - case types.MAP_SET: case types.DOWNLOAD_MODE_CLOSE: case types.DOWNLOAD_MODE_OPEN: - return null + case types.MAP_NEW: + case types.MAP_SET: + return initialState + + case types.DATA_TABLE_CLOSE: + return { ...state, isPanelVisible: false } + + case types.DATA_TABLE_OPEN: + return { ...state, isPanelVisible: true } + + case types.DATA_TABLE_ACTIVE_LAYER_SET: + return { ...state, activeLayerId: action.id } - case types.DATA_TABLE_TOGGLE: - return state === action.id ? null : action.id + case types.DATA_TABLE_TOGGLE: { + const isOpening = !state.openIds.includes(action.id) + const openIds = isOpening + ? [...state.openIds, action.id] + : state.openIds.filter((id) => id !== action.id) + return { + ...state, + openIds, + isPanelVisible: isOpening ? true : state.isPanelVisible, + } + } case types.LAYER_REMOVE: - return state === action.id ? null : state + return { + ...state, + openIds: state.openIds.filter((id) => id !== action.id), + activeLayerId: + state.activeLayerId === action.id + ? null + : state.activeLayerId, + } default: return state diff --git a/src/reducers/selection.js b/src/reducers/selection.js index 338d8a952b..e8b84f0d66 100644 --- a/src/reducers/selection.js +++ b/src/reducers/selection.js @@ -2,40 +2,44 @@ import * as types from '../constants/actionTypes.js' const defaultState = { layerId: null, ids: [] } -const selection = (state = defaultState, action) => { - switch (action.type) { - case types.FEATURE_TOGGLE_SELECTION: { - if (state.layerId !== action.layerId) { - return { layerId: action.layerId, ids: [action.id] } - } +const toggleFeatureSelection = (state, action) => { + if (state.layerId !== action.layerId) { + return { layerId: action.layerId, ids: [action.id] } + } - const alreadySelected = state.ids.includes(action.id) + const alreadySelected = state.ids.includes(action.id) - return { - layerId: action.layerId, - ids: alreadySelected - ? state.ids.filter((id) => id !== action.id) - : [...state.ids, action.id], - } - } + return { + layerId: action.layerId, + ids: alreadySelected + ? state.ids.filter((id) => id !== action.id) + : [...state.ids, action.id], + } +} + +const addSelectionRange = (state, action) => { + const ids = state.layerId === action.layerId ? state.ids : [] + + return { + layerId: action.layerId, + ids: [...new Set([...ids, ...action.ids])], + } +} + +const selection = (state = defaultState, action) => { + switch (action.type) { + case types.FEATURE_TOGGLE_SELECTION: + return toggleFeatureSelection(state, action) case types.SELECTION_SET_ALL: return { layerId: action.layerId, ids: action.ids } - case types.SELECTION_ADD_RANGE: { - const ids = state.layerId === action.layerId ? state.ids : [] - - return { - layerId: action.layerId, - ids: [...new Set([...ids, ...action.ids])], - } - } + case types.SELECTION_ADD_RANGE: + return addSelectionRange(state, action) case types.SELECTION_CLEAR: case types.MAP_NEW: case types.MAP_SET: - case types.DATA_TABLE_CLOSE: - case types.DATA_TABLE_TOGGLE: return defaultState case types.LAYER_REMOVE: diff --git a/src/util/__tests__/analytics.spec.js b/src/util/__tests__/analytics.spec.js index 454b604951..e946a99d23 100644 --- a/src/util/__tests__/analytics.spec.js +++ b/src/util/__tests__/analytics.spec.js @@ -56,6 +56,17 @@ describe('setDataItemInColumns', () => { const dataItem = { id: 'item1', name: 'Item 1' } expect(setDataItemInColumns(dataItem, 'invalid')).toEqual([]) }) + + it('stores aggregationType alongside the data item when present', () => { + const dataItem = { + id: 'item1', + name: 'Item 1', + aggregationType: 'AVERAGE', + } + const result = setDataItemInColumns(dataItem, 'dataElement') + + expect(result[0].items[0].aggregationType).toBe('AVERAGE') + }) }) describe('getOrgUnitsFromRows', () => { diff --git a/src/util/__tests__/cellValue.spec.js b/src/util/__tests__/cellValue.spec.js new file mode 100644 index 0000000000..a9bbd7e385 --- /dev/null +++ b/src/util/__tests__/cellValue.spec.js @@ -0,0 +1,95 @@ +import { + RENDERER_COLOR, + RENDERER_DATE, + RENDERER_ORG_UNIT, + RENDERER_ORG_UNIT_NAME, + RENDERER_BOOLEAN, + TYPE_DATE, + TYPE_DATETIME, +} from '../../constants/dataTable.js' +import { formatCellText, NO_VALUE_TEXT } from '../cellValue.js' + +describe('formatCellText', () => { + it('returns the em-dash placeholder for a missing value, regardless of renderer', () => { + expect(formatCellText(null)).toBe(NO_VALUE_TEXT) + expect(formatCellText(undefined, { renderer: RENDERER_BOOLEAN })).toBe( + NO_VALUE_TEXT + ) + }) + + it('lowercases a color value', () => { + expect(formatCellText('#ABCDEF', { renderer: RENDERER_COLOR })).toBe( + '#abcdef' + ) + }) + + it('formats a date-only value with no time portion', () => { + expect( + formatCellText('2024-01-15T10:30:00.000', { + renderer: RENDERER_DATE, + type: TYPE_DATE, + }) + ).toBe('2024-01-15') + }) + + it('formats a datetime value including the time portion', () => { + expect( + formatCellText('2024-01-15T10:30:00.000', { + renderer: RENDERER_DATE, + type: TYPE_DATETIME, + }) + ).toBe('2024-01-15 10:30') + }) + + it('formats an org-unit-hierarchy value as a breadcrumb', () => { + const orgUnitIdToName = new Map([ + ['country1', 'Country'], + ['ou1', 'Facility'], + ]) + expect( + formatCellText('/country1/ou1', { + renderer: RENDERER_ORG_UNIT, + orgUnitIdToName, + }) + ).toBe('Country / Facility') + }) + + it('falls back to the raw id for an org-unit segment missing from the id-to-name map', () => { + const orgUnitIdToName = new Map([['country1', 'Country']]) + expect( + formatCellText('/country1/ou1', { + renderer: RENDERER_ORG_UNIT, + orgUnitIdToName, + }) + ).toBe('Country / ou1') + }) + + it("formats an org-unit-name value as just the feature's own name", () => { + const orgUnitIdToName = new Map([['ou1', 'Facility']]) + expect( + formatCellText('/country1/ou1', { + renderer: RENDERER_ORG_UNIT_NAME, + orgUnitIdToName, + }) + ).toBe('Facility') + }) + + it('formats a boolean-renderer value as Yes/No', () => { + expect(formatCellText('1', { renderer: RENDERER_BOOLEAN })).toBe('Yes') + expect(formatCellText('0', { renderer: RENDERER_BOOLEAN })).toBe('No') + }) + + it('formats a plain number with the digit-group separator', () => { + expect( + formatCellText(1234567, { keyAnalysisDigitGroupSeparator: 'COMMA' }) + ).toBe('1,234,567') + }) + + it('formats a plain number with no separator when none is given', () => { + expect(formatCellText(1234567)).toBe('1234567') + }) + + it('leaves a plain string untouched', () => { + expect(formatCellText('Bo')).toBe('Bo') + }) +}) diff --git a/src/util/__tests__/dataTable.spec.js b/src/util/__tests__/dataTable.spec.js index 27484a9981..91cbb9fd19 100644 --- a/src/util/__tests__/dataTable.spec.js +++ b/src/util/__tests__/dataTable.spec.js @@ -1,10 +1,14 @@ +import { EXTERNAL_LAYER, THEMATIC_LAYER } from '../../constants/layers.js' import { buildFeatureIndex, + getEligibleDataTableLayers, + getLayerSelectedIds, getNextSorting, getPanelHeights, getRowClickAction, getRowId, hasActiveDataTableFilters, + isDataTableOpen, isFilterable, shouldClearFeatureHighlight, } from '../dataTable.js' @@ -109,10 +113,10 @@ describe('getNextSorting', () => { ).toEqual({ sortField: 'name', sortDirection: 'desc' }) }) - test('clicking the descending-sorted column clears back to natural order', () => { + test('clicking the descending-sorted default column resets to itself ascending - a 2-state toggle since it already is the default', () => { expect( getNextSorting('name', { sortField: 'name', sortDirection: 'desc' }) - ).toEqual({ sortField: null, sortDirection: 'asc' }) + ).toEqual({ sortField: 'name', sortDirection: 'asc' }) }) test('clicking a different column restarts the cycle at ascending', () => { @@ -120,6 +124,22 @@ describe('getNextSorting', () => { getNextSorting('type', { sortField: 'name', sortDirection: 'desc' }) ).toEqual({ sortField: 'type', sortDirection: 'asc' }) }) + + test("clicking a non-default column's third time (descending) resets to the table's actual default sort, matching what it shows on initial load - not an unsorted/natural-order state", () => { + expect( + getNextSorting('type', { sortField: 'type', sortDirection: 'desc' }) + ).toEqual({ sortField: 'name', sortDirection: 'asc' }) + }) + + test('honors a custom defaultSortField/defaultSortDirection when resetting', () => { + expect( + getNextSorting( + 'type', + { sortField: 'type', sortDirection: 'desc' }, + { defaultSortField: 'level', defaultSortDirection: 'desc' } + ) + ).toEqual({ sortField: 'level', sortDirection: 'desc' }) + }) }) describe('isFilterable', () => { @@ -206,6 +226,100 @@ describe('buildFeatureIndex', () => { }) }) +describe('getEligibleDataTableLayers', () => { + test('includes data-table-capable layer types that have finished loading', () => { + const mapViews = [ + { id: 'a', layer: THEMATIC_LAYER, isLoaded: true, data: [{}] }, + { + id: 'b', + layer: THEMATIC_LAYER, + isLoaded: true, + data: [{}, {}], + }, + ] + expect(getEligibleDataTableLayers(mapViews).map((l) => l.id)).toEqual([ + 'a', + 'b', + ]) + }) + + test('excludes layer types with no data table support', () => { + const mapViews = [ + { id: 'a', layer: EXTERNAL_LAYER, isLoaded: true, data: [{}] }, + ] + expect(getEligibleDataTableLayers(mapViews)).toEqual([]) + }) + + test('excludes a data-table-capable layer that has not finished loading yet', () => { + const mapViews = [ + { id: 'a', layer: THEMATIC_LAYER, isLoaded: false, data: [{}] }, + ] + expect(getEligibleDataTableLayers(mapViews)).toEqual([]) + }) + + test('includes a loaded, data-table-capable layer with no valid data - the caller shows an explanatory message instead of hiding it', () => { + const mapViews = [ + { id: 'a', layer: THEMATIC_LAYER, isLoaded: true, data: [] }, + ] + expect(getEligibleDataTableLayers(mapViews).map((l) => l.id)).toEqual([ + 'a', + ]) + }) +}) + +describe('isDataTableOpen', () => { + test('is open when at least one tab is open and the panel is visible', () => { + expect( + isDataTableOpen({ + openIds: ['layer1'], + isPanelVisible: true, + }) + ).toBe(true) + }) + + test('is closed when there are no open tabs', () => { + expect( + isDataTableOpen({ + openIds: [], + isPanelVisible: true, + }) + ).toBe(false) + }) + + test('is closed when the panel is hidden, even with open tabs', () => { + expect( + isDataTableOpen({ + openIds: ['layer1'], + isPanelVisible: false, + }) + ).toBe(false) + }) +}) + +describe('getLayerSelectedIds', () => { + test('returns an empty array when there is no selection', () => { + expect(getLayerSelectedIds(null, 'layer1')).toEqual([]) + }) + + test("returns this layer's own selected ids when selection.layerId matches", () => { + expect( + getLayerSelectedIds( + { layerId: 'layer1', ids: ['a', 'b'] }, + 'layer1' + ) + ).toEqual(['a', 'b']) + }) + + test('returns an empty array when selection belongs to another layer', () => { + expect( + getLayerSelectedIds( + { layerId: 'other-layer', ids: ['a'] }, + 'layer1' + ) + ).toEqual([]) + }) +}) + describe('getPanelHeights', () => { test('clamps the table height to the window, minus header/toolbar', () => { const result = getPanelHeights({ diff --git a/src/util/__tests__/filter.spec.js b/src/util/__tests__/filter.spec.js index edcd9f3fd8..603821b459 100644 --- a/src/util/__tests__/filter.spec.js +++ b/src/util/__tests__/filter.spec.js @@ -3,6 +3,13 @@ import { SENTINEL_NO_VALUE, DATE_GROUPS_GRANULARITY, ORG_UNIT_GROUPS_GRANULARITY, + TYPE_STRING, + TYPE_NUMBER, + TYPE_DATE, + TYPE_DATETIME, + TYPE_ORG_UNIT, + RENDERER_DATE, + RENDERER_ORG_UNIT, } from '../../constants/dataTable.js' import { filterByGlobalSearch, filterData } from '../filter.js' @@ -247,6 +254,25 @@ describe('filterData', () => { } expect(filterData(data, filters)).toEqual([{ a: null }]) }) + + it('an empty prefixes array matches everything when not search-derived (an inactive checkbox tree)', () => { + const filters = { + a: { granularity: ORG_UNIT_GROUPS_GRANULARITY, prefixes: [] }, + } + expect(filterData(data, filters)).toEqual(data) + }) + + it('an empty prefixes array matches nothing when search-derived (a free-text search that matched no branch)', () => { + const filters = { + a: { + granularity: ORG_UNIT_GROUPS_GRANULARITY, + prefixes: [], + searchDerived: true, + searchText: 'Nairobi', + }, + } + expect(filterData(data, filters)).toEqual([]) + }) }) describe('org-unit value filter ({ values, searchDerived, searchText }) - a committed free-text search on an org-unit-flavored plain-text column, resolved to matching raw values up front (see FilterInput.jsx)', () => { @@ -272,38 +298,35 @@ describe('filterByGlobalSearch', () => { { name: 'Entebbe Clinic', type: 'Clinic' }, { name: 'Jinja Hospital', type: 'Hospital' }, ] - const stringDataKeys = ['name', 'type'] + const headers = [ + { dataKey: 'name', type: TYPE_STRING }, + { dataKey: 'type', type: TYPE_STRING }, + ] it('returns the original data when the search string is empty', () => { - expect(filterByGlobalSearch(data, '', { stringDataKeys })).toEqual(data) - expect(filterByGlobalSearch(data, ' ', { stringDataKeys })).toEqual( - data - ) + expect(filterByGlobalSearch(data, '', { headers })).toEqual(data) + expect(filterByGlobalSearch(data, ' ', { headers })).toEqual(data) }) - it('returns the original data when there are no string or org-unit data keys', () => { + it('returns the original data when there are no headers to search', () => { expect(filterByGlobalSearch(data, 'Kampala', {})).toEqual(data) }) it('matches case-insensitively across any of the given fields', () => { - expect( - filterByGlobalSearch(data, 'kampala', { stringDataKeys }) - ).toEqual([{ name: 'Kampala Hospital', type: 'Hospital' }]) + expect(filterByGlobalSearch(data, 'kampala', { headers })).toEqual([ + { name: 'Kampala Hospital', type: 'Hospital' }, + ]) }) it('matches rows where any field contains the search string', () => { - expect( - filterByGlobalSearch(data, 'hospital', { stringDataKeys }) - ).toEqual([ + expect(filterByGlobalSearch(data, 'hospital', { headers })).toEqual([ { name: 'Kampala Hospital', type: 'Hospital' }, { name: 'Jinja Hospital', type: 'Hospital' }, ]) }) it('returns no rows when nothing matches', () => { - expect( - filterByGlobalSearch(data, 'nairobi', { stringDataKeys }) - ).toEqual([]) + expect(filterByGlobalSearch(data, 'nairobi', { headers })).toEqual([]) }) it('also matches org-unit-typed columns by their resolved name, since the raw stored value is an id/path', () => { @@ -316,11 +339,90 @@ describe('filterByGlobalSearch', () => { ['region1', 'Bo'], ['facility1', 'Bo Hospital'], ]) + const orgUnitHeaders = [ + { + dataKey: 'orgUnitPath', + type: TYPE_ORG_UNIT, + renderer: RENDERER_ORG_UNIT, + }, + ] expect( filterByGlobalSearch(orgUnitData, 'bo hospital', { - orgUnitDataKeys: ['orgUnitPath'], - idToName, + headers: orgUnitHeaders, + orgUnitIdToName: idToName, }) ).toEqual([{ id: 'a', orgUnitPath: '/country1/region1/facility1' }]) }) + + it('matches a numeric column by its raw value even when a digit-group separator would otherwise hide it', () => { + const numericData = [{ population: 1234567 }, { population: 42 }] + const numericHeaders = [{ dataKey: 'population', type: TYPE_NUMBER }] + + expect( + filterByGlobalSearch(numericData, '1234567', { + headers: numericHeaders, + keyAnalysisDigitGroupSeparator: 'COMMA', + }) + ).toEqual([{ population: 1234567 }]) + }) + + it('matches a numeric column by its formatted (digit-group-separated) value too', () => { + const numericData = [{ population: 1234567 }, { population: 42 }] + const numericHeaders = [{ dataKey: 'population', type: TYPE_NUMBER }] + + expect( + filterByGlobalSearch(numericData, '1,234,567', { + headers: numericHeaders, + keyAnalysisDigitGroupSeparator: 'COMMA', + }) + ).toEqual([{ population: 1234567 }]) + }) + + it('matches a date column by its formatted display value', () => { + const dateData = [ + { createdAt: '2024-01-15T10:30:00.000' }, + { createdAt: '2023-06-01T08:00:00.000' }, + ] + const dateHeaders = [ + { + dataKey: 'createdAt', + type: TYPE_DATE, + renderer: RENDERER_DATE, + }, + ] + + expect( + filterByGlobalSearch(dateData, '2024-01-15', { + headers: dateHeaders, + }) + ).toEqual([{ createdAt: '2024-01-15T10:30:00.000' }]) + }) + + it('matches a datetime column, including the time portion of its formatted value', () => { + const datetimeData = [{ updatedAt: '2024-01-15T10:30:00.000' }] + const datetimeHeaders = [ + { + dataKey: 'updatedAt', + type: TYPE_DATETIME, + renderer: RENDERER_DATE, + }, + ] + + expect( + filterByGlobalSearch(datetimeData, '10:30', { + headers: datetimeHeaders, + }) + ).toEqual(datetimeData) + }) + + it('ignores a header whose value is null or undefined rather than matching against "null"/"undefined"', () => { + const sparseData = [{ population: null }, { population: 42 }] + const numericHeaders = [{ dataKey: 'population', type: TYPE_NUMBER }] + + expect( + filterByGlobalSearch(sparseData, 'null', { + headers: numericHeaders, + }) + ).toEqual([]) + }) }) diff --git a/src/util/__tests__/styleByDataItem.spec.js b/src/util/__tests__/styleByDataItem.spec.js index 5baf91e5c1..133fd69947 100644 --- a/src/util/__tests__/styleByDataItem.spec.js +++ b/src/util/__tests__/styleByDataItem.spec.js @@ -119,6 +119,7 @@ describe('styleByDataItem', () => { }), ]) ) + expect(result.styleDataItem.name).toEqual(STYLE_DATA_ITEM_NAME) }) it('should include no-data events when noDataLegend is configured (default)', async () => { @@ -231,6 +232,7 @@ describe('styleByDataItem', () => { ]) ) expect(result.legend.unit).toEqual(LEGEND_SET_NAME) + expect(result.styleDataItem.name).toEqual(STYLE_DATA_ITEM_NAME) }) it('should include outside and no-data features when unclassifiedLegend and noDataLegend are configured (predefined)', async () => { @@ -353,6 +355,7 @@ describe('styleByDataItem', () => { ]) ) expect(result.legend.unit).toEqual(STYLE_DATA_ITEM_NAME) + expect(result.styleDataItem.name).toEqual(STYLE_DATA_ITEM_NAME) }) it('should include no-data features when noDataLegend is configured (auto)', async () => { @@ -446,6 +449,7 @@ describe('styleByDataItem', () => { ]) ) expect(result.legend.unit).toEqual(STYLE_DATA_ITEM_NAME) + expect(result.styleDataItem.name).toEqual(STYLE_DATA_ITEM_NAME) }) it('should include unclassified and no-data events when configured (boolean)', async () => { diff --git a/src/util/__tests__/tableColumns.spec.js b/src/util/__tests__/tableColumns.spec.js index 39efc86bd9..fb3139011a 100644 --- a/src/util/__tests__/tableColumns.spec.js +++ b/src/util/__tests__/tableColumns.spec.js @@ -12,6 +12,7 @@ import { isPinnedGroupEnd, reorderHeaderKeys, reverseVisibleKeys, + sortColumnOptions, togglePinnedKey, toggleVisibleKey, } from '../tableColumns.js' @@ -438,6 +439,38 @@ describe('getColumnDistinctValues', () => { }) }) +describe('sortColumnOptions', () => { + it('returns null when there are no distinct values', () => { + expect(sortColumnOptions(null)).toBe(null) + }) + + it('sorts each column ascending by default', () => { + const distinctValues = { + rawValue: { values: ['30', '10', '20'], type: TYPE_NUMBER }, + } + expect(sortColumnOptions(distinctValues)).toEqual({ + rawValue: [{ value: '10' }, { value: '20' }, { value: '30' }], + }) + }) + + it('sorts the active sort field descending, leaving other columns ascending', () => { + const distinctValues = { + rawValue: { values: ['30', '10', '20'], type: TYPE_NUMBER }, + name: { values: ['B', 'A'], type: 'string' }, + } + const result = sortColumnOptions(distinctValues, { + sortField: 'rawValue', + sortDirection: 'desc', + }) + expect(result.rawValue).toEqual([ + { value: '30' }, + { value: '20' }, + { value: '10' }, + ]) + expect(result.name).toEqual([{ value: 'A' }, { value: 'B' }]) + }) +}) + describe('buildRowCells', () => { const rowHeaders = [ { dataKey: 'name', type: 'string' }, @@ -480,6 +513,19 @@ describe('filterHeadersByName', () => { it('returns every header when the search text is empty', () => { expect(filterHeadersByName(headers, '')).toEqual(headers) }) + + it('matches against configName instead of name when present', () => { + const withConfigName = [ + ...headers, + { + dataKey: 'rawValue2', + name: 'Value (Jan 2023)', + configName: 'Value (Current period)', + }, + ] + const result = filterHeadersByName(withConfigName, 'current period') + expect(result.map((h) => h.dataKey)).toEqual(['rawValue2']) + }) }) describe('reorderHeaderKeys', () => { diff --git a/src/util/__tests__/tableHeaders.spec.js b/src/util/__tests__/tableHeaders.spec.js index a8d175a452..e631e642a6 100644 --- a/src/util/__tests__/tableHeaders.spec.js +++ b/src/util/__tests__/tableHeaders.spec.js @@ -27,6 +27,22 @@ jest.mock('../../components/map/MapApi.js', () => ({ })) const dataKeys = (result) => result.headers.map((h) => h.dataKey) +const defaultHiddenKeys = (result) => + result.headers.filter((h) => h.defaultHidden).map((h) => h.dataKey) + +describe('getHeadersForLayer - defaultHidden', () => { + test('Id, Org unit id, Org unit level, and Geometry type are hidden by default; Org unit and Org unit hierarchy are not', () => { + const result = getHeadersForLayer(THEMATIC_LAYER, { + isMultiPeriodThematic: false, + }) + expect(defaultHiddenKeys(result)).toEqual( + expect.arrayContaining(['id', 'level', 'type']) + ) + expect(defaultHiddenKeys(result)).not.toEqual( + expect.arrayContaining(['orgUnitOwn', 'orgUnitPath']) + ) + }) +}) describe('getHeadersForLayer - thematic', () => { test('single-period: fixed fields plus legend/range/color', () => { @@ -69,7 +85,7 @@ describe('getHeadersForLayer - thematic', () => { ) }) - test('multi-period timeline: excludes the external period from the extra columns and labels value/legend/range/color with it', () => { + test('multi-period timeline: keeps a fixed column for the external period alongside the current-period columns, and labels value/legend/range/color with it', () => { const periods = [ { id: 'p1', name: 'Jan' }, { id: 'p2', name: 'Feb' }, @@ -81,10 +97,11 @@ describe('getHeadersForLayer - thematic', () => { periods, externalPeriod, }) - expect(dataKeys(result)).not.toContain('period_p1_rawValue') + expect(dataKeys(result)).toContain('period_p1_rawValue') expect(dataKeys(result)).toContain('period_p2_rawValue') const valueHeader = result.headers.find((h) => h.dataKey === 'rawValue') expect(valueHeader.name).toContain('Jan') + expect(valueHeader.configName).toBe('Value (Current period)') }) }) @@ -387,6 +404,96 @@ describe('getHeadersForLayer - earth engine', () => { expect(meanHeader.name).toBe('Mean Rainfall') expect(meanHeader.type).toBe(TYPE_NUMBER) }) + + // Real band ids/names from src/constants/earthEngineLayers/population_age_sex_Worldpop-Global2.js + const populationBands = { + multiple: true, + list: [ + { id: 'm_00', name: 'Male 0 - 1 years' }, + { id: 'f_00', name: 'Female 0 - 1 years' }, + ], + } + + test('only 1 band selected: no per-band columns, even with a multi-stat bands.multiple layer', () => { + const result = getHeadersForLayer(EARTH_ENGINE_LAYER, { + aggregationType: ['sum', 'mean'], + legend: { title: 'Population', items: [] }, + bands: populationBands, + band: ['m_00'], + data: [{ sum: 100, mean: 10 }], + }) + expect(dataKeys(result)).toEqual( + expect.arrayContaining(['sum', 'mean']) + ) + expect(dataKeys(result)).not.toEqual( + expect.arrayContaining(['m_00', 'm_00_sum']) + ) + }) + + test('2+ bands, exactly 1 stat: one bare-band-id column per band, hidden by default', () => { + const result = getHeadersForLayer(EARTH_ENGINE_LAYER, { + aggregationType: ['sum'], + legend: { title: 'Population', items: [] }, + bands: populationBands, + band: ['m_00', 'f_00'], + data: [{ sum: 100, m_00: 60, f_00: 40 }], + }) + expect(dataKeys(result)).toEqual( + expect.arrayContaining(['sum', 'm_00', 'f_00']) + ) + const maleHeader = result.headers.find((h) => h.dataKey === 'm_00') + expect(maleHeader.name).toBe('Male 0 - 1 years') + expect(maleHeader.defaultHidden).toBe(true) + expect(maleHeader.type).toBe(TYPE_NUMBER) + }) + + test('band columns get a null roundFn (not a rounds-to-whole-numbers function) before any data has loaded', () => { + const result = getHeadersForLayer(EARTH_ENGINE_LAYER, { + aggregationType: ['sum'], + legend: { title: 'Population', items: [] }, + bands: populationBands, + band: ['m_00', 'f_00'], + data: undefined, + }) + const maleHeader = result.headers.find((h) => h.dataKey === 'm_00') + expect(maleHeader.roundFn).toBe(null) + }) + + test('2+ bands, 2+ stats: one title-cased ${band}_${type} column per band per stat, hidden by default', () => { + const result = getHeadersForLayer(EARTH_ENGINE_LAYER, { + aggregationType: ['sum', 'mean'], + legend: { title: 'Population', items: [] }, + bands: populationBands, + band: ['m_00', 'f_00'], + data: [{ sum: 100, mean: 10, m_00_sum: 60, m_00_mean: 6 }], + }) + expect(dataKeys(result)).toEqual( + expect.arrayContaining([ + 'sum', + 'mean', + 'm_00_sum', + 'm_00_mean', + 'f_00_sum', + 'f_00_mean', + ]) + ) + const header = result.headers.find((h) => h.dataKey === 'm_00_sum') + expect(header.name).toBe('Sum Male 0 - 1 Years') + expect(header.defaultHidden).toBe(true) + expect(header.roundFn(6.7891234)).toBe(6.789) + }) + + test('no bands config at all: unaffected, same as an ordinary non-multi-band EE layer', () => { + const result = getHeadersForLayer(EARTH_ENGINE_LAYER, { + aggregationType: ['sum', 'mean'], + legend: { title: 'NDVI', items: [] }, + data: [{ sum: 100, mean: 10 }], + }) + expect(dataKeys(result)).toEqual( + expect.arrayContaining(['sum', 'mean']) + ) + expect(result.headers).toHaveLength(7) + }) }) describe('getHeadersForLayer - geoJsonUrl', () => { diff --git a/src/util/__tests__/tableRows.spec.js b/src/util/__tests__/tableRows.spec.js index c66d39a5dd..829517c9ed 100644 --- a/src/util/__tests__/tableRows.spec.js +++ b/src/util/__tests__/tableRows.spec.js @@ -170,7 +170,7 @@ describe('buildTableData - multi-period thematic layer', () => { p2: { a: { value: 2 } }, } - test('timeline: overlays the external period’s value/color/legend/range and adds one column per other period', () => { + test('timeline: overlays the external period’s value/color/legend/range and adds one fixed column per period, including the external one', () => { const data = [feature('a')] const result = buildTableData(THEMATIC_LAYER, { data, @@ -187,9 +187,9 @@ describe('buildTableData - multi-period thematic layer', () => { color: '#f00', legend: 'Low', range: '0-1', + period_p1_rawValue: 1, period_p2_rawValue: 2, }) - expect(result.data[0].period_p1_rawValue).toBeUndefined() }) test('split (non-timeline): adds one column per period, with no current-period overlay', () => { diff --git a/src/util/analytics.js b/src/util/analytics.js index c403222a0d..b894bcaec9 100644 --- a/src/util/analytics.js +++ b/src/util/analytics.js @@ -47,6 +47,7 @@ export const setDataItemInColumns = (dataItem, dimension) => { expression: dataItem.expression, dimensionItemType: dim.itemType, legendSet: dataItem.legendSet, // TODO: Keep outside of columns? + aggregationType: dataItem.aggregationType, }, ], { objectName: dim.objectName } diff --git a/src/util/cellValue.js b/src/util/cellValue.js new file mode 100644 index 0000000000..253a380642 --- /dev/null +++ b/src/util/cellValue.js @@ -0,0 +1,41 @@ +import { + RENDERER_COLOR, + RENDERER_DATE, + RENDERER_ORG_UNIT, + RENDERER_ORG_UNIT_NAME, + RENDERER_BOOLEAN, + TYPE_DATE, +} from '../constants/dataTable.js' +import { formatBoolean, formatDate, formatDatetime } from './helpers.js' +import { formatWithSeparator } from './numbers.js' +import { + formatOrgUnitOwnName, + formatOrgUnitPathBreadcrumb, +} from './orgUnitGroups.js' + +export const NO_VALUE_TEXT = '—' + +export const formatCellText = ( + value, + { renderer, type, orgUnitIdToName, keyAnalysisDigitGroupSeparator } = {} +) => { + if (value == null) { + return NO_VALUE_TEXT + } + if (renderer === RENDERER_COLOR) { + return value.toLowerCase() + } + if (renderer === RENDERER_DATE) { + return type === TYPE_DATE ? formatDate(value) : formatDatetime(value) + } + if (renderer === RENDERER_ORG_UNIT) { + return formatOrgUnitPathBreadcrumb(value, orgUnitIdToName) + } + if (renderer === RENDERER_ORG_UNIT_NAME) { + return formatOrgUnitOwnName(value, orgUnitIdToName) + } + if (renderer === RENDERER_BOOLEAN) { + return formatBoolean(value) + } + return formatWithSeparator(value, keyAnalysisDigitGroupSeparator) +} diff --git a/src/util/dataTable.js b/src/util/dataTable.js index 1eba91fa02..bb891f90b8 100644 --- a/src/util/dataTable.js +++ b/src/util/dataTable.js @@ -1,18 +1,23 @@ import { SORT_ASCENDING, SORT_DESCENDING } from '../constants/dataTable.js' +import { DATA_TABLE_LAYER_TYPES } from '../constants/layers.js' export const isFilterable = (dataKey, type) => !!type export const shouldClearFeatureHighlight = (event) => event.relatedTarget?.tagName !== 'TD' -export const getNextSorting = (name, { sortField, sortDirection }) => { +export const getNextSorting = ( + name, + { sortField, sortDirection }, + { defaultSortField = 'name', defaultSortDirection = SORT_ASCENDING } = {} +) => { if (name !== sortField) { return { sortField: name, sortDirection: SORT_ASCENDING } } if (sortDirection === SORT_ASCENDING) { return { sortField: name, sortDirection: SORT_DESCENDING } } - return { sortField: null, sortDirection: SORT_ASCENDING } + return { sortField: defaultSortField, sortDirection: defaultSortDirection } } export const getRowId = (row) => @@ -54,6 +59,17 @@ export const hasActiveDataTableFilters = ({ selectionFilter?.length > 0 || !!showOnlyFeaturesInView +export const isDataTableOpen = ({ openIds, isPanelVisible }) => + isPanelVisible && openIds.length > 0 + +export const getEligibleDataTableLayers = (mapViews) => + mapViews.filter( + (l) => DATA_TABLE_LAYER_TYPES.includes(l.layer) && l.isLoaded + ) + +export const getLayerSelectedIds = (selection, layerId) => + selection?.layerId === layerId ? selection.ids ?? [] : [] + export const buildFeatureIndex = (data) => { const index = new Map() data?.forEach((f) => { diff --git a/src/util/filter.js b/src/util/filter.js index 9b8fc30f60..2a40d04ffc 100644 --- a/src/util/filter.js +++ b/src/util/filter.js @@ -3,8 +3,9 @@ import { SENTINEL_NO_VALUE, DATE_GROUPS_GRANULARITY, ORG_UNIT_GROUPS_GRANULARITY, + TYPE_NUMBER, } from '../constants/dataTable.js' -import { formatOrgUnitPathBreadcrumb } from './orgUnitGroups.js' +import { formatCellText } from './cellValue.js' // Distinguishes a prefix-group filter (date-groups, org-unit-groups, ...) export const isPrefixGroupFilter = (filter, granularity) => @@ -13,9 +14,9 @@ export const isPrefixGroupFilter = (filter, granularity) => !Array.isArray(filter) && filter.granularity === granularity -export const prefixGroupFilter = (value, { prefixes }) => { +export const prefixGroupFilter = (value, { prefixes, searchDerived }) => { if (!prefixes?.length) { - return true + return !searchDerived } const stringValue = value == null ? SENTINEL_NO_VALUE : String(value) return prefixes.some((prefix) => { @@ -104,36 +105,37 @@ export const numericFilter = (value, filter) => { }) } +const getSearchableTexts = (value, header, formatArgs) => { + if (value == null) { + return [] + } + const formatted = formatCellText(value, { + renderer: header.renderer, + type: header.type, + ...formatArgs, + }) + return header.type === TYPE_NUMBER + ? [String(value), formatted] + : [formatted] +} + export const filterByGlobalSearch = ( data, searchString, - { stringDataKeys = [], orgUnitDataKeys = [], idToName } = {} + { headers = [], orgUnitIdToName, keyAnalysisDigitGroupSeparator } = {} ) => { - if ( - !searchString?.trim() || - (!stringDataKeys.length && !orgUnitDataKeys.length) - ) { + if (!searchString?.trim() || !headers.length) { return data } const lower = searchString.toLowerCase() return data.filter((item) => { const props = item.properties || item - const stringMatch = stringDataKeys.some((key) => { - const val = props[key] - return val != null && String(val).toLowerCase().includes(lower) - }) - if (stringMatch) { - return true - } - return orgUnitDataKeys.some((key) => { - const val = props[key] - return ( - val != null && - formatOrgUnitPathBreadcrumb(val, idToName) - .toLowerCase() - .includes(lower) - ) - }) + return headers.some((header) => + getSearchableTexts(props[header.dataKey], header, { + orgUnitIdToName, + keyAnalysisDigitGroupSeparator, + }).some((text) => text.toLowerCase().includes(lower)) + ) }) } diff --git a/src/util/styleByDataItem.js b/src/util/styleByDataItem.js index 0d5913f2b3..bf6c04d3ba 100644 --- a/src/util/styleByDataItem.js +++ b/src/util/styleByDataItem.js @@ -93,6 +93,7 @@ const styleByDefault = async (config, engine) => { const { id } = styleDataItem legend.unit = await getLegendUnit(engine, styleDataItem) + config.styleDataItem = { ...styleDataItem, name: legend.unit } const eventItem = { name: i18n.t('Event'), @@ -133,6 +134,7 @@ const styleByBoolean = async (config, engine) => { const { id, values } = styleDataItem legend.unit = await getLegendUnit(engine, styleDataItem) + config.styleDataItem = { ...styleDataItem, name: legend.unit } const yesItem = { name: i18n.t('Yes'), color: values.true } const noItem = values.false @@ -204,6 +206,9 @@ const styleByNumeric = async (config, engine) => { } = config let valueFormat + const itemName = await getLegendUnit(engine, styleDataItem) + config.styleDataItem = { ...styleDataItem, name: itemName } + // If legend set if (method === CLASSIFICATION_PREDEFINED) { // Load legend set from server @@ -230,8 +235,8 @@ const styleByNumeric = async (config, engine) => { } sortedValues.sort((a, b) => a - b) - // Use data item name as legend unit (load from server if needed) - legend.unit = await getLegendUnit(engine, styleDataItem) + // Use data item name as legend unit + legend.unit = itemName // Generate legend items based on layer config const classification = getAutomaticLegendItems({ diff --git a/src/util/tableColumns.js b/src/util/tableColumns.js index 5579d28d3b..41b7346ca4 100644 --- a/src/util/tableColumns.js +++ b/src/util/tableColumns.js @@ -1,5 +1,10 @@ import { arrayMoveImmutable } from 'array-move' -import { SENTINEL_NO_VALUE, TYPE_NUMBER } from '../constants/dataTable.js' +import { + SENTINEL_NO_VALUE, + SORT_ASCENDING, + TYPE_NUMBER, +} from '../constants/dataTable.js' +import { compareColumnOptionValues } from './tableSort.js' const CHECKBOX_COLUMN_WIDTH = 76 @@ -141,6 +146,35 @@ export const getColumnDistinctValues = (headers, data) => { return result } +// Cheap: just re-orders each column's already-known distinct-value list +export const sortColumnOptions = ( + columnDistinctValues, + { sortField, sortDirection } = {} +) => { + if (!columnDistinctValues) { + return null + } + + const result = {} + Object.entries(columnDistinctValues).forEach( + ([dataKey, { values, type }]) => { + const direction = + dataKey === sortField ? sortDirection : SORT_ASCENDING + result[dataKey] = [...values] + .sort((a, b) => + compareColumnOptionValues(a, b, { + dataKey, + type, + direction, + }) + ) + .map((value) => ({ value })) + } + ) + + return Object.keys(result).length ? result : null +} + export const buildRowCells = (item, headers) => headers.map(({ dataKey, roundFn, type }) => { const value = roundFn ? roundFn(item[dataKey]) : item[dataKey] @@ -158,7 +192,7 @@ export const buildRowCells = (item, headers) => export const filterHeadersByName = (headers, search) => { const normalizedSearch = search.trim().toLowerCase() return headers.filter((h) => - h.name.toLowerCase().includes(normalizedSearch) + (h.configName ?? h.name).toLowerCase().includes(normalizedSearch) ) } diff --git a/src/util/tableHeaders.js b/src/util/tableHeaders.js index de376360b4..4a86b64bec 100644 --- a/src/util/tableHeaders.js +++ b/src/util/tableHeaders.js @@ -96,11 +96,17 @@ const ORG_UNIT_ID = ORG_UNIT_ID_DATA_KEY export const ERROR_NON_HOMOGENOUS_FEATURES = 'NON_HOMOGENOUS_FEATURES' const defaultFieldsMap = () => ({ - [ID]: { name: i18n.t('Id'), dataKey: ID, type: TYPE_STRING }, + [ID]: { + name: i18n.t('Id'), + dataKey: ID, + type: TYPE_STRING, + defaultHidden: true, + }, [ORG_UNIT_ID]: { - name: i18n.t('Org unit Id'), + name: i18n.t('Org unit id'), dataKey: ORG_UNIT_ID, type: TYPE_STRING, + defaultHidden: true, }, [ORG_UNIT]: { name: i18n.t('Org unit'), @@ -112,8 +118,14 @@ const defaultFieldsMap = () => ({ name: i18n.t('Org unit level'), dataKey: LEVEL, type: TYPE_NUMBER, + defaultHidden: true, + }, + [TYPE]: { + name: i18n.t('Geometry type'), + dataKey: TYPE, + type: TYPE_STRING, + defaultHidden: true, }, - [TYPE]: { name: i18n.t('Geometry type'), dataKey: TYPE, type: TYPE_STRING }, [VALUE]: { name: i18n.t('Value'), dataKey: VALUE, type: TYPE_NUMBER }, [LEGEND]: { name: i18n.t('Legend'), dataKey: LEGEND, type: TYPE_STRING }, [RANGE]: { name: i18n.t('Range'), dataKey: RANGE, type: TYPE_STRING }, @@ -204,7 +216,7 @@ const getStyleHeaders = ({ } const getThematicHeaders = () => - getOrgUnitCoreFields(i18n.t('Org unit Id')) + getOrgUnitCoreFields(i18n.t('Org unit id')) .concat(defaultFieldsMap()[VALUE]) .concat( getStyleHeaders({ hasLegend: true, hasRange: true, hasColor: true }) @@ -224,16 +236,15 @@ const getMultiPeriodThematicHeaders = ({ name: `${header.name} (${ externalPeriod?.name ?? i18n.t('Current period') })`, + configName: `${header.name} (${i18n.t( + 'Current period' + )})`, } : header ) : getOrgUnitHeaders() - const otherPeriods = isTimelineThematic - ? (periods ?? []).filter((p) => p.id !== externalPeriod?.id) - : periods ?? [] - - otherPeriods.forEach((period) => { + ;(periods ?? []).forEach((period) => { headers.push({ name: i18n.t('Value ({{period}})', { period: period.name }), dataKey: `period_${period.id}_rawValue`, @@ -307,7 +318,7 @@ const getOrgUnitStyleHeaders = (data) => { } const getFixedFieldsWithOrgUnitStyle = (data) => - getOrgUnitCoreFields(i18n.t('Org unit Id')) + getOrgUnitCoreFields(i18n.t('Org unit id')) .concat(getOrgUnitStyleHeaders(data)) .concat(defaultFieldsMap()[TYPE]) @@ -346,7 +357,49 @@ const toTitleCase = (str) => (txt) => txt.charAt(0).toUpperCase() + txt.substr(1).toLowerCase() ) -const getEarthEngineHeaders = ({ aggregationType, legend, data }) => { +const getFieldRoundFn = (data, dataKey) => { + if (!data?.length) { + return null + } + return getRoundToPrecisionFn(getPrecision(data.map((d) => d[dataKey]))) +} + +const getBandFields = ({ bands, band, aggregationType, data }) => { + if (!bands?.multiple || !Array.isArray(band) || band.length < 2) { + return [] + } + const selectedBands = bands.list?.filter((b) => band.includes(b.id)) ?? [] + return selectedBands.flatMap(({ id: bandId, name: bandName }) => + aggregationType.length === 1 + ? [ + { + name: bandName, + dataKey: bandId, + roundFn: getFieldRoundFn(data, bandId), + type: TYPE_NUMBER, + defaultHidden: true, + }, + ] + : aggregationType.map((type) => { + const dataKey = `${bandId}_${type}` + return { + name: toTitleCase(`${type} ${bandName}`), + dataKey, + roundFn: getFieldRoundFn(data, dataKey), + type: TYPE_NUMBER, + defaultHidden: true, + } + }) + ) +} + +const getEarthEngineHeaders = ({ + aggregationType, + legend, + data, + bands, + band, +}) => { const { title, items } = legend let customFields = [] @@ -359,22 +412,24 @@ const getEarthEngineHeaders = ({ aggregationType, legend, data }) => { type: TYPE_NUMBER, })) } else if (Array.isArray(aggregationType) && aggregationType.length) { - customFields = aggregationType.map((type) => { - let roundFn = null - if (data?.length) { - const precision = getPrecision(data.map((d) => d[type])) - roundFn = getRoundToPrecisionFn(precision) - } - return { - name: toTitleCase(`${type} ${title}`), - dataKey: type, - roundFn, - type: TYPE_NUMBER, - } - }) + customFields = aggregationType + .map((type) => { + let roundFn = null + if (data?.length) { + const precision = getPrecision(data.map((d) => d[type])) + roundFn = getRoundToPrecisionFn(precision) + } + return { + name: toTitleCase(`${type} ${title}`), + dataKey: type, + roundFn, + type: TYPE_NUMBER, + } + }) + .concat(getBandFields({ bands, band, aggregationType, data })) } - return getOrgUnitCoreFields(i18n.t('Org unit Id')) + return getOrgUnitCoreFields(i18n.t('Org unit id')) .concat(customFields) .concat(defaultFieldsMap()[TYPE]) } @@ -418,6 +473,8 @@ export const getHeadersForLayer = (layerType, ctx) => { aggregationType: ctx.aggregationType, legend: ctx.legend, data: ctx.data, + bands: ctx.bands, + band: ctx.band, }), } case FACILITY_LAYER: diff --git a/src/util/tableRows.js b/src/util/tableRows.js index 60719159d3..41e704ca1f 100644 --- a/src/util/tableRows.js +++ b/src/util/tableRows.js @@ -88,9 +88,6 @@ export const buildTableData = ( : null const otherPeriodValues = {} ;(periods ?? []).forEach((period) => { - if (isTimelineThematic && period.id === externalPeriod?.id) { - return - } otherPeriodValues[`period_${period.id}_rawValue`] = valuesByPeriod?.[period.id]?.[orgUnitId]?.value ?? null }) From 6ad20792bad501e8fc0d2fff8f8a0e0f3d42f396 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Mon, 7 Sep 2026 17:28:57 +0200 Subject: [PATCH 2/8] fix: crash opening the data table on a layer with no valid data buildKnownOrgUnitNames(rows = []) only falls back to [] for undefined - useTableData passes it dataWithAggregations, which is null whenever a layer has no valid rows (e.g. an org-unit-based layer whose selected units have no coordinates), so rows.forEach crashed instead of showing the "no valid data" message. Found by smoke-testing the data table with a facility layer that had 0 matching rows. Co-Authored-By: Claude Sonnet 5 --- .../datatable/controls/styles/PopoverPanel.module.css | 6 ++++++ src/util/__tests__/orgUnits.spec.js | 1 + src/util/orgUnits.js | 4 ++-- 3 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 src/components/datatable/controls/styles/PopoverPanel.module.css diff --git a/src/components/datatable/controls/styles/PopoverPanel.module.css b/src/components/datatable/controls/styles/PopoverPanel.module.css new file mode 100644 index 0000000000..6751005ee0 --- /dev/null +++ b/src/components/datatable/controls/styles/PopoverPanel.module.css @@ -0,0 +1,6 @@ +.popoverPanel { + padding: var(--spacers-dp8); + background-color: var(--colors-white); + border-radius: 4px; + box-shadow: var(--elevations-popover); +} diff --git a/src/util/__tests__/orgUnits.spec.js b/src/util/__tests__/orgUnits.spec.js index 3673d49297..8abd17312c 100644 --- a/src/util/__tests__/orgUnits.spec.js +++ b/src/util/__tests__/orgUnits.spec.js @@ -256,6 +256,7 @@ describe('buildKnownOrgUnitNames', () => { it('returns an empty map for no rows', () => { expect(buildKnownOrgUnitNames([])).toEqual(new Map()) expect(buildKnownOrgUnitNames()).toEqual(new Map()) + expect(buildKnownOrgUnitNames(null)).toEqual(new Map()) }) }) diff --git a/src/util/orgUnits.js b/src/util/orgUnits.js index 421b3e261c..4f0c50444a 100644 --- a/src/util/orgUnits.js +++ b/src/util/orgUnits.js @@ -347,9 +347,9 @@ export const fetchOrgUnitPaths = async (engine, ids) => { return results.flatMap((r) => r.organisationUnits.organisationUnits ?? []) } -export const buildKnownOrgUnitNames = (rows = []) => { +export const buildKnownOrgUnitNames = (rows) => { const map = new Map() - rows.forEach((row) => { + ;(rows ?? []).forEach((row) => { if (row?.id != null && row?.name != null) { map.set(row.id, row.name) } From 5703297f5ddd4ddb5e622c2b2bc2d4c988fe8f91 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Mon, 7 Sep 2026 21:00:13 +0200 Subject: [PATCH 3/8] chore: remove dead code left behind by the PR7 split ActiveLayerControl and RowCells were superseded by LayerSelectorControl and the inline CellValue-based row rendering respectively, but the files themselves were never deleted since this branch started fresh from PR6 rather than from the already-cleaned-up squash. Co-Authored-By: Claude Sonnet 5 --- src/components/datatable/RowCells.jsx | 175 ------------------ .../datatable/__tests__/RowCells.spec.jsx | 122 ------------ .../datatable/controls/ActiveLayerControl.jsx | 71 ------- .../styles/ActiveLayerControl.module.css | 38 ---- .../styles/ClearFiltersControl.module.css | 40 ---- 5 files changed, 446 deletions(-) delete mode 100644 src/components/datatable/RowCells.jsx delete mode 100644 src/components/datatable/__tests__/RowCells.spec.jsx delete mode 100644 src/components/datatable/controls/ActiveLayerControl.jsx delete mode 100644 src/components/datatable/controls/styles/ActiveLayerControl.module.css delete mode 100644 src/components/datatable/controls/styles/ClearFiltersControl.module.css diff --git a/src/components/datatable/RowCells.jsx b/src/components/datatable/RowCells.jsx deleted file mode 100644 index b0ef0b2fb4..0000000000 --- a/src/components/datatable/RowCells.jsx +++ /dev/null @@ -1,175 +0,0 @@ -import { DataTableCell } from '@dhis2/ui' -import cx from 'classnames' -import PropTypes from 'prop-types' -import React from 'react' -import { - RENDERER_COLOR, - RENDERER_ICON, - RENDERER_DATE, - RENDERER_ORG_UNIT, - RENDERER_ORG_UNIT_NAME, - RENDERER_BOOLEAN, - TYPE_DATE, - ORG_UNIT_ID_DATA_KEY, -} from '../../constants/dataTable.js' -import { isDarkColor } from '../../util/colors.js' -import { getRowId } from '../../util/dataTable.js' -import { - formatBoolean, - formatDate, - formatDatetime, -} from '../../util/helpers.js' -import { formatWithSeparator } from '../../util/numbers.js' -import { - formatOrgUnitOwnName, - formatOrgUnitPathBreadcrumb, -} from '../../util/orgUnitGroups.js' -import { getPinnedCellProps } from '../../util/tableColumns.js' -import styles from './styles/DataTable.module.css' - -const RowCells = ({ - row, - visibleHeaders, - selectedIdSet, - hoveredFeature, - layerId, - isCheckboxColumnPinned, - pinnedLeftOffsets, - pinnedColumnCount, - columnWidths, - rendererByDataKey, - typeByDataKey, - keyAnalysisDigitGroupSeparator, - orgUnitIdToName, - onToggleSelection, -}) => { - const rowId = getRowId(row) - const isSelected = !!rowId && selectedIdSet.has(rowId) - const isHovered = - !!rowId && - hoveredFeature?.id === rowId && - hoveredFeature?.layerId === layerId - - const cellsByDataKey = new Map(row.map((cell) => [cell.dataKey, cell])) - - return ( - <> - - rowId && onToggleSelection(rowId)} - onClick={(e) => e.stopPropagation()} - /> - - {visibleHeaders.map(({ dataKey }, index) => { - const cell = cellsByDataKey.get(dataKey) - if (!cell) { - return null - } - const { value, align } = cell - const { fixed, left, width, isLastPinned } = getPinnedCellProps( - dataKey, - index, - { - pinnedLeftOffsets, - pinnedColumnCount, - columnWidths, - } - ) - const renderer = rendererByDataKey.get(dataKey) - const isColorCell = renderer === RENDERER_COLOR - const isIconCell = renderer === RENDERER_ICON - const isDateCell = renderer === RENDERER_DATE - const isDateOnlyCell = typeByDataKey.get(dataKey) === TYPE_DATE - const isOrgUnitHierarchyCell = renderer === RENDERER_ORG_UNIT - const isOrgUnitNameCell = renderer === RENDERER_ORG_UNIT_NAME - const isBooleanCell = renderer === RENDERER_BOOLEAN - return ( - - {isColorCell && value?.toLowerCase()} - {isIconCell && value && ( - { - e.target.style.visibility = 'hidden' - }} - /> - )} - {isDateCell && - value && - (isDateOnlyCell - ? formatDate(value) - : formatDatetime(value))} - {isOrgUnitHierarchyCell && - value && - formatOrgUnitPathBreadcrumb(value, orgUnitIdToName)} - {isOrgUnitNameCell && - value && - formatOrgUnitOwnName(value, orgUnitIdToName)} - {isBooleanCell && value != null && formatBoolean(value)} - {!isColorCell && - !isIconCell && - !isDateCell && - !isOrgUnitHierarchyCell && - !isOrgUnitNameCell && - !isBooleanCell && - formatWithSeparator( - value, - keyAnalysisDigitGroupSeparator - )} - - ) - })} - - ) -} - -RowCells.propTypes = { - columnWidths: PropTypes.array.isRequired, - isCheckboxColumnPinned: PropTypes.bool.isRequired, - pinnedColumnCount: PropTypes.number.isRequired, - pinnedLeftOffsets: PropTypes.object.isRequired, - rendererByDataKey: PropTypes.instanceOf(Map).isRequired, - row: PropTypes.array.isRequired, - selectedIdSet: PropTypes.instanceOf(Set).isRequired, - typeByDataKey: PropTypes.instanceOf(Map).isRequired, - visibleHeaders: PropTypes.array.isRequired, - onToggleSelection: PropTypes.func.isRequired, - hoveredFeature: PropTypes.object, - keyAnalysisDigitGroupSeparator: PropTypes.string, - layerId: PropTypes.string, - orgUnitIdToName: PropTypes.object, -} - -export default RowCells diff --git a/src/components/datatable/__tests__/RowCells.spec.jsx b/src/components/datatable/__tests__/RowCells.spec.jsx deleted file mode 100644 index 7206646534..0000000000 --- a/src/components/datatable/__tests__/RowCells.spec.jsx +++ /dev/null @@ -1,122 +0,0 @@ -import { render, screen } from '@testing-library/react' -import React from 'react' -import { - RENDERER_COLOR, - RENDERER_ICON, - RENDERER_DATE, - TYPE_DATE, -} from '../../../constants/dataTable.js' -import RowCells from '../RowCells.jsx' - -const NAME_HEADER = { dataKey: 'name' } - -const defaultProps = { - visibleHeaders: [NAME_HEADER], - selectedIdSet: new Set(), - hoveredFeature: null, - layerId: 'layer1', - isCheckboxColumnPinned: false, - pinnedLeftOffsets: {}, - pinnedColumnCount: 0, - columnWidths: [], - rendererByDataKey: new Map(), - typeByDataKey: new Map(), - keyAnalysisDigitGroupSeparator: undefined, - onToggleSelection: jest.fn(), -} - -const renderRow = (row, overrides = {}) => - render( - - - - - - -
- ) - -describe('RowCells', () => { - it('renders a plain formatted value cell by default', () => { - renderRow([ - { dataKey: 'id', value: 'row1' }, - { dataKey: 'name', value: 'Bo' }, - ]) - expect(screen.getByText('Bo')).toBeInTheDocument() - }) - - it('renders a color cell as a lowercased swatch value with a background color', () => { - renderRow( - [ - { dataKey: 'id', value: 'row1' }, - { dataKey: 'name', value: '#FF0000' }, - ], - { rendererByDataKey: new Map([['name', RENDERER_COLOR]]) } - ) - const cell = screen.getByText('#ff0000') - expect(cell).toBeInTheDocument() - expect(cell.closest('td')).toHaveStyle({ - backgroundColor: '#FF0000', - }) - }) - - it('renders an icon cell as an image', () => { - const { container } = renderRow( - [ - { dataKey: 'id', value: 'row1' }, - { dataKey: 'name', value: 'https://example.com/icon.png' }, - ], - { rendererByDataKey: new Map([['name', RENDERER_ICON]]) } - ) - expect(container.querySelector('img')).toHaveAttribute( - 'src', - 'https://example.com/icon.png' - ) - }) - - it('renders a date-only cell formatted as just the date', () => { - renderRow( - [ - { dataKey: 'id', value: 'row1' }, - { dataKey: 'name', value: '2024-03-15T10:30:00' }, - ], - { - rendererByDataKey: new Map([['name', RENDERER_DATE]]), - typeByDataKey: new Map([['name', TYPE_DATE]]), - } - ) - expect(screen.getByText('2024-03-15')).toBeInTheDocument() - }) - - it('renders a datetime cell formatted with the time', () => { - renderRow( - [ - { dataKey: 'id', value: 'row1' }, - { dataKey: 'name', value: '2024-03-15T10:30:00' }, - ], - { rendererByDataKey: new Map([['name', RENDERER_DATE]]) } - ) - expect(screen.getByText('2024-03-15 10:30')).toBeInTheDocument() - }) - - it('checks the checkbox when the row id is in selectedIdSet', () => { - renderRow([{ dataKey: 'id', value: 'row1' }], { - selectedIdSet: new Set(['row1']), - }) - expect(screen.getByRole('checkbox')).toBeChecked() - }) - - it('calls onToggleSelection with the row id when the checkbox is clicked', () => { - const onToggleSelection = jest.fn() - renderRow([{ dataKey: 'id', value: 'row1' }], { onToggleSelection }) - - screen.getByRole('checkbox').click() - - expect(onToggleSelection).toHaveBeenCalledWith('row1') - }) - - it('skips headers with no matching cell in the row', () => { - renderRow([{ dataKey: 'id', value: 'row1' }]) - expect(screen.queryByText('Bo')).not.toBeInTheDocument() - }) -}) diff --git a/src/components/datatable/controls/ActiveLayerControl.jsx b/src/components/datatable/controls/ActiveLayerControl.jsx deleted file mode 100644 index 1d62b35e88..0000000000 --- a/src/components/datatable/controls/ActiveLayerControl.jsx +++ /dev/null @@ -1,71 +0,0 @@ -import PropTypes from 'prop-types' -import React, { useCallback, useRef, useState } from 'react' -import { createPortal } from 'react-dom' -import { getCssVar } from '../../../util/helpers.js' -import styles from './styles/ActiveLayerControl.module.css' - -const ActiveLayerControl = ({ name }) => { - const nameRef = useRef(null) - const [nameTooltipProps, setNameTooltipProps] = useState(null) - - const onMouseEnter = useCallback(() => { - const el = nameRef.current - if (!el || el.scrollWidth <= el.offsetWidth) { - return - } - const rect = el.getBoundingClientRect() - const computed = getComputedStyle(el) - const lineHeight = Number.parseFloat(computed.lineHeight) - const verticalPadding = getCssVar( - '--data-table-name-tooltip-vertical-padding' - ) - setNameTooltipProps({ - top: rect.top + (rect.height - lineHeight) / 2 - verticalPadding, - left: rect.left, - color: computed.color, - fontSize: computed.fontSize, - fontWeight: computed.fontWeight, - lineHeight: `${lineHeight}px`, - paddingLeft: computed.paddingLeft, - }) - }, []) - - const onMouseLeave = useCallback(() => setNameTooltipProps(null), []) - - return ( - <> - - {name} - - {nameTooltipProps && - createPortal( -
- {name} -
, - document.body - )} - - ) -} - -ActiveLayerControl.propTypes = { - name: PropTypes.string, -} - -export default ActiveLayerControl diff --git a/src/components/datatable/controls/styles/ActiveLayerControl.module.css b/src/components/datatable/controls/styles/ActiveLayerControl.module.css deleted file mode 100644 index 75d05c3372..0000000000 --- a/src/components/datatable/controls/styles/ActiveLayerControl.module.css +++ /dev/null @@ -1,38 +0,0 @@ -:root { - --data-table-name-tooltip-vertical-padding: 3px; -} - -.layerName { - font-weight: 500; - font-size: 12px; - color: var(--colors-grey800); - flex: 0 1 auto; - overflow: hidden; - white-space: nowrap; - text-overflow: ellipsis; - min-width: 0; -} - -@keyframes tooltipExpandRight { - from { - clip-path: inset(0 100% 0 0); - } - - to { - clip-path: inset(0 0% 0 0); - } -} - -.nameTooltip { - animation: tooltipExpandRight 160ms ease-out; - background: var(--colors-grey100); - border-radius: 3px; - -webkit-mask-image: linear-gradient(to left, transparent, black 2em); - mask-image: linear-gradient(to left, transparent, black 2em); - padding: var(--data-table-name-tooltip-vertical-padding) 2em - var(--data-table-name-tooltip-vertical-padding) 0; - pointer-events: none; - position: fixed; - white-space: nowrap; - z-index: 2000; -} diff --git a/src/components/datatable/controls/styles/ClearFiltersControl.module.css b/src/components/datatable/controls/styles/ClearFiltersControl.module.css deleted file mode 100644 index 6742e21179..0000000000 --- a/src/components/datatable/controls/styles/ClearFiltersControl.module.css +++ /dev/null @@ -1,40 +0,0 @@ -.filteredIcon { - position: relative; - display: flex; - align-items: center; - justify-content: center; - width: 16px; - height: 16px; -} - -.clearBadge { - position: absolute; - bottom: 0; - right: 0; - width: 8px; - height: 8px; - background: var(--colors-grey100); -} - -:global(button):hover .clearBadge { - background: var(--colors-grey300); -} - -.clearBadge::before, -.clearBadge::after { - content: ''; - position: absolute; - width: 5px; - height: 1px; - background: currentColor; - top: 50%; - left: 50%; -} - -.clearBadge::before { - transform: translate(-50%, -50%) rotate(45deg); -} - -.clearBadge::after { - transform: translate(-50%, -50%) rotate(-45deg); -} From 4dd25eab67b2183a705cf1f1e1edda590861b490 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Mon, 7 Sep 2026 21:31:42 +0200 Subject: [PATCH 4/8] test: cover remaining dataTable action creators and trackedEntityLoader Adds coverage for setMapBounds, toggleShowOnlyFeaturesInView, setSelectionFilter, setHighlightColor, and setDataTableColumnConfig, plus the trackedEntityLoader default export's orchestration and applyParsedConfig's dataTableColumnConfig branch. Co-Authored-By: Claude Sonnet 5 --- src/actions/__tests__/dataTable.spec.js | 55 +++++++++++++++++++ .../__tests__/trackedEntityLoader.spec.js | 46 +++++++++++++++- 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/src/actions/__tests__/dataTable.spec.js b/src/actions/__tests__/dataTable.spec.js index 07954d4e2d..b4c12b8bda 100644 --- a/src/actions/__tests__/dataTable.spec.js +++ b/src/actions/__tests__/dataTable.spec.js @@ -5,6 +5,11 @@ import { toggleDataTable, setActiveDataTableLayer, resizeDataTable, + setMapBounds, + toggleShowOnlyFeaturesInView, + setSelectionFilter, + setHighlightColor, + setDataTableColumnConfig, setActiveTimelinePeriod, } from '../dataTable.js' @@ -51,6 +56,56 @@ describe('resizeDataTable', () => { }) }) +describe('setMapBounds', () => { + it('creates a MAP_BOUNDS_CHANGED action', () => { + const bounds = [ + [0, 0], + [1, 1], + ] + expect(setMapBounds(bounds)).toEqual({ + type: types.MAP_BOUNDS_CHANGED, + bounds, + }) + }) +}) + +describe('toggleShowOnlyFeaturesInView', () => { + it('creates a TOGGLE_SHOW_ONLY_IN_VIEW action', () => { + expect(toggleShowOnlyFeaturesInView()).toEqual({ + type: types.TOGGLE_SHOW_ONLY_IN_VIEW, + }) + }) +}) + +describe('setSelectionFilter', () => { + it('creates a SELECTION_FILTER_SET action', () => { + expect(setSelectionFilter(['selected'])).toEqual({ + type: types.SELECTION_FILTER_SET, + value: ['selected'], + }) + }) +}) + +describe('setHighlightColor', () => { + it('creates a HIGHLIGHT_COLOR_SET action', () => { + expect(setHighlightColor('#ff0000')).toEqual({ + type: types.HIGHLIGHT_COLOR_SET, + color: '#ff0000', + }) + }) +}) + +describe('setDataTableColumnConfig', () => { + it('creates a DATA_TABLE_COLUMN_CONFIG_SET action', () => { + const config = { pinnedKeys: ['name'] } + expect(setDataTableColumnConfig('layer1', config)).toEqual({ + type: types.DATA_TABLE_COLUMN_CONFIG_SET, + layerId: 'layer1', + config, + }) + }) +}) + describe('setActiveTimelinePeriod', () => { it('creates an ACTIVE_TIMELINE_PERIOD_SET action', () => { const period = { id: '202301', name: 'January 2023' } diff --git a/src/loaders/__tests__/trackedEntityLoader.spec.js b/src/loaders/__tests__/trackedEntityLoader.spec.js index cf137be6e0..9283bca7ab 100644 --- a/src/loaders/__tests__/trackedEntityLoader.spec.js +++ b/src/loaders/__tests__/trackedEntityLoader.spec.js @@ -1,4 +1,5 @@ -import { +import { WARNING_NO_DATA } from '../../constants/alerts.js' +import trackedEntityLoader, { getAttributeHeaders, getAttributeProperties, applyParsedConfig, @@ -194,6 +195,16 @@ describe('applyParsedConfig', () => { expect(config.config).toBeUndefined() }) + it('extracts dataTableColumnConfig when set', () => { + const config = { + config: JSON.stringify({ + dataTableColumnConfig: { pinnedKeys: ['name'] }, + }), + } + applyParsedConfig(config) + expect(config.dataTableColumnConfig).toEqual({ pinnedKeys: ['name'] }) + }) + it('does nothing when config.config is absent', () => { const config = { layer: 'trackedEntity' } applyParsedConfig(config) @@ -313,3 +324,36 @@ describe('toOptionSetOptionsByCode', () => { expect(toOptionSetOptionsByCode(new Map())).toEqual({}) }) }) + +describe('trackedEntityLoader', () => { + it('returns a loaded, alerted layer when the query has no instances with valid geometry', async () => { + const config = { + trackedEntityType: { id: 'tet1', name: 'Person' }, + program: null, + rows: [], + organisationUnitSelectionMode: 'SELECTED', + startDate: '2023-01-01', + endDate: '2023-01-31', + } + const engine = { + query: jest.fn().mockResolvedValue({ + trackedEntities: { trackedEntities: [] }, + }), + } + + const result = await trackedEntityLoader({ + config, + engine, + keyAnalysisDigitGroupSeparator: ',', + serverVersion: { minor: 41 }, + }) + + expect(result.isLoaded).toBe(true) + expect(result.isLoading).toBe(false) + expect(result.data).toEqual([]) + expect(result.headers).toEqual([]) + expect(result.alerts).toEqual([ + { code: WARNING_NO_DATA, message: 'Person' }, + ]) + }) +}) From afe2a1bf48b835c603700264ee96d660d8e10e30 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Mon, 7 Sep 2026 21:42:25 +0200 Subject: [PATCH 5/8] chore: remove unreachable ERROR_NO_HEADERS check in useTableData The rows memo's own !headers.length guard could never run: getting past the earlier errorCode.current check already guarantees headers is non-empty, since the headers memo sets that same error code and returns null whenever it isn't. Co-Authored-By: Claude Sonnet 5 --- src/components/datatable/useTableData.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/components/datatable/useTableData.js b/src/components/datatable/useTableData.js index 91a12fc49b..f384e884fd 100644 --- a/src/components/datatable/useTableData.js +++ b/src/components/datatable/useTableData.js @@ -257,11 +257,6 @@ export const useTableData = ({ return null } - if (!headers.length) { - errorCode.current = ERROR_NO_HEADERS - return null - } - let filteredData = filterData(dataWithAggregations, dataFilters) if (globalSearch?.trim()) { From 86e19bfd628f3c7ea8375240e3d1e6bf56105682 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Tue, 8 Sep 2026 16:40:21 +0200 Subject: [PATCH 6/8] fix: parse numeric event data-item values consistently in dataWithoutCoords too --- src/loaders/__tests__/eventLoader.spec.js | 50 +++++++++++++++++ src/loaders/eventLoader.js | 68 +++++++++++------------ 2 files changed, 83 insertions(+), 35 deletions(-) diff --git a/src/loaders/__tests__/eventLoader.spec.js b/src/loaders/__tests__/eventLoader.spec.js index 62bc98f52e..4a49f6c994 100644 --- a/src/loaders/__tests__/eventLoader.spec.js +++ b/src/loaders/__tests__/eventLoader.spec.js @@ -13,6 +13,7 @@ import { import eventLoader, { attachOrgUnitPaths, excludeEventsOutsideOrgUnits, + parseNumericHeaders, shouldUseServerCluster, } from '../eventLoader.js' @@ -1163,3 +1164,52 @@ describe('eventLoader - extended column top-up', () => { expect(args.analyticsEngine.events.getQuery).toHaveBeenCalledTimes(1) }) }) + +describe('parseNumericHeaders', () => { + const numericHeader = { + name: 'a3kGcGDCuk6', + valueType: 'NUMBER', + } + + it('parses a numeric header value on every feature', () => { + const data = [ + { properties: { a3kGcGDCuk6: '1.0' } }, + { properties: { a3kGcGDCuk6: '2.0' } }, + ] + + const result = parseNumericHeaders(data, [numericHeader]) + + expect(result.map((d) => d.properties.a3kGcGDCuk6)).toEqual([1, 2]) + }) + + it('returns the data unchanged when there are no numeric headers', () => { + const data = [{ properties: { a3kGcGDCuk6: '1.0' } }] + + expect(parseNumericHeaders(data, [])).toBe(data) + }) + + it('skips a header with an option set', () => { + const data = [{ properties: { a3kGcGDCuk6: '1.0' } }] + + const result = parseNumericHeaders(data, [ + { ...numericHeader, optionSet: { id: 'os1' } }, + ]) + + expect(result[0].properties.a3kGcGDCuk6).toBe('1.0') + }) + + it('parses both the data and dataWithoutCoords arrays consistently, so a value is never a Number in one and a String in the other once the table merges them', () => { + const data = parseNumericHeaders( + [{ properties: { a3kGcGDCuk6: '2.0' } }], + [numericHeader] + ) + const dataWithoutCoords = parseNumericHeaders( + [{ properties: { a3kGcGDCuk6: '2.0' } }], + [numericHeader] + ) + + expect(data[0].properties.a3kGcGDCuk6).toBe( + dataWithoutCoords[0].properties.a3kGcGDCuk6 + ) + }) +}) diff --git a/src/loaders/eventLoader.js b/src/loaders/eventLoader.js index 8cc7527222..945bde4d1c 100644 --- a/src/loaders/eventLoader.js +++ b/src/loaders/eventLoader.js @@ -183,6 +183,27 @@ const eventLoader = async ({ return config } +export const parseNumericHeaders = (data, headers) => { + const numericHeaders = headers.filter( + (header) => + isValidUid(header.name) && + numberValueTypes.includes(header.valueType) && + !header.optionSet + ) + if (!numericHeaders.length) { + return data + } + return data.map((d) => { + const newD = { ...d } + numericHeaders.forEach((header) => { + newD.properties[header.name] = parseWithSeparator( + d.properties[header.name] + ) + }) + return newD + }) +} + // Merges only the new "display in reports" columns const loadExtendedEventColumns = async ({ config, @@ -249,22 +270,12 @@ const loadExtendedEventColumns = async ({ ) config.headers = [...config.headers, ...newHeaders] - const numericNewHeaders = newHeaders.filter( - (header) => - isValidUid(header.name) && - numberValueTypes.includes(header.valueType) && - !header.optionSet - ) - if (numericNewHeaders.length) { - config.data = config.data.map((d) => { - const newD = { ...d } - numericNewHeaders.forEach((header) => { - newD.properties[header.name] = parseWithSeparator( - d.properties[header.name] - ) - }) - return newD - }) + config.data = parseNumericHeaders(config.data, newHeaders) + if (config.dataWithoutCoords?.length) { + config.dataWithoutCoords = parseNumericHeaders( + config.dataWithoutCoords, + newHeaders + ) } config.isExtended = true @@ -523,25 +534,12 @@ const loadEventLayer = async ({ ) } - const numericDataItemHeaders = config.headers.filter( - (header) => - isValidUid(header.name) && - numberValueTypes.includes(header.valueType) && - !header.optionSet - ) - - if (numericDataItemHeaders.length) { - config.data = config.data.map((d) => { - const newD = { ...d } - - numericDataItemHeaders.forEach((header) => { - newD.properties[header.name] = parseWithSeparator( - d.properties[header.name] - ) - }) - - return newD - }) + config.data = parseNumericHeaders(config.data, config.headers) + if (config.dataWithoutCoords?.length) { + config.dataWithoutCoords = parseNumericHeaders( + config.dataWithoutCoords, + config.headers + ) } } From c7af004e082131b31186df1e1dcea1ba92b1d571 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Tue, 8 Sep 2026 18:00:57 +0200 Subject: [PATCH 7/8] fix: prevent native text selection and stale anchors during row multi-select --- src/components/datatable/DataTable.jsx | 34 +++- .../datatable/SelectionCheckboxColumn.jsx | 7 +- .../datatable/TableVirtuosoComponents.jsx | 7 + .../SelectionCheckboxColumn.spec.jsx | 49 +++++ .../TableVirtuosoComponents.spec.jsx | 58 +++++- .../__tests__/useRowClickSelection.spec.js | 174 +++++++++++++++++- .../datatable/useRowClickSelection.js | 67 ++++++- src/util/__tests__/dataTable.spec.js | 4 +- src/util/dataTable.js | 2 +- 9 files changed, 376 insertions(+), 26 deletions(-) create mode 100644 src/components/datatable/__tests__/SelectionCheckboxColumn.spec.jsx diff --git a/src/components/datatable/DataTable.jsx b/src/components/datatable/DataTable.jsx index e88295e101..b259ee6edc 100644 --- a/src/components/datatable/DataTable.jsx +++ b/src/components/datatable/DataTable.jsx @@ -230,10 +230,11 @@ const Table = ({ (ids) => dispatch(selectFeatureRange(ids, layer.id)), [dispatch, layer.id] ) - const onRowClick = useRowClickSelection({ + const { onRowClick, onCheckboxToggle, resetAnchor } = useRowClickSelection({ rows, onToggle: onToggleRow, onSelectRange: onSelectRowRange, + selectedIdSet, }) const onRowDoubleClick = useCallback( @@ -341,13 +342,26 @@ const Table = ({ [dispatch, layer.id] ) - const { isAllSelected, onToggleSelectAll, onReverseSelection } = - useRowSelection({ - selectedIds, - selectedIdSet, - allRowIds, - onChange: onSelectionChange, - }) + const { + isAllSelected, + onToggleSelectAll: onToggleSelectAllRows, + onReverseSelection: onReverseSelectionRows, + } = useRowSelection({ + selectedIds, + selectedIdSet, + allRowIds, + onChange: onSelectionChange, + }) + + const onToggleSelectAll = useCallback(() => { + resetAnchor() + onToggleSelectAllRows() + }, [resetAnchor, onToggleSelectAllRows]) + + const onReverseSelection = useCallback(() => { + resetAnchor() + onReverseSelectionRows() + }, [resetAnchor, onReverseSelectionRows]) const computeItemKey = useCallback( (index, row) => getRowId(row) ?? index, @@ -533,7 +547,9 @@ const Table = ({ } isSelected={isSelected} isHovered={isHovered} - onToggle={() => rowId && onToggleRow(rowId)} + onToggle={(e) => + rowId && onCheckboxToggle(rowId, e) + } /> {visibleHeaders.map(({ dataKey }, index) => { const cell = cellsByDataKey.get(dataKey) diff --git a/src/components/datatable/SelectionCheckboxColumn.jsx b/src/components/datatable/SelectionCheckboxColumn.jsx index 22a9171d12..b1ee0a1ea1 100644 --- a/src/components/datatable/SelectionCheckboxColumn.jsx +++ b/src/components/datatable/SelectionCheckboxColumn.jsx @@ -111,8 +111,11 @@ export const SelectionCheckboxCell = ({ e.stopPropagation()} + onChange={Function.prototype} + onClick={(e) => { + e.stopPropagation() + onToggle(e) + }} /> ) diff --git a/src/components/datatable/TableVirtuosoComponents.jsx b/src/components/datatable/TableVirtuosoComponents.jsx index 0acdaa1e88..23ea220e87 100644 --- a/src/components/datatable/TableVirtuosoComponents.jsx +++ b/src/components/datatable/TableVirtuosoComponents.jsx @@ -23,10 +23,17 @@ DataTableWithVirtuosoContext.propTypes = { }), } +const onRowMouseDown = (e) => { + if (e.shiftKey || e.ctrlKey || e.metaKey) { + e.preventDefault() + } +} + const DataTableRowWithVirtuosoContext = React.memo( function DataTableRowWithVirtuosoContext({ context, item, ...props }) { return ( context.onMouseEnter(item)} onMouseLeave={context.onMouseLeave} onContextMenu={(e) => context.onContextMenu(e, item)} diff --git a/src/components/datatable/__tests__/SelectionCheckboxColumn.spec.jsx b/src/components/datatable/__tests__/SelectionCheckboxColumn.spec.jsx new file mode 100644 index 0000000000..15c702006c --- /dev/null +++ b/src/components/datatable/__tests__/SelectionCheckboxColumn.spec.jsx @@ -0,0 +1,49 @@ +import { render, fireEvent, screen } from '@testing-library/react' +import React from 'react' +import { SelectionCheckboxCell } from '../SelectionCheckboxColumn.jsx' + +const renderCell = (isSelected, onToggle = jest.fn()) => { + render( + + + + + + +
+ ) + return screen.getByRole('checkbox') +} + +describe('SelectionCheckboxCell', () => { + test('calls onToggle with the click event, carrying shiftKey, on a shift-click', () => { + const onToggle = jest.fn() + const checkbox = renderCell(false, onToggle) + + fireEvent.click(checkbox, { shiftKey: true }) + + expect(onToggle).toHaveBeenCalledTimes(1) + expect(onToggle.mock.calls[0][0]).toMatchObject({ shiftKey: true }) + }) + + test('calls onToggle on a plain click, with shiftKey false', () => { + const onToggle = jest.fn() + const checkbox = renderCell(false, onToggle) + + fireEvent.click(checkbox) + + expect(onToggle).toHaveBeenCalledTimes(1) + expect(onToggle.mock.calls[0][0]).toMatchObject({ shiftKey: false }) + }) + + test('lets the native click default through, so the browser keeps its own checked state in sync', () => { + const checkbox = renderCell(false) + + const notCancelled = fireEvent.click(checkbox) + + expect(notCancelled).toBe(true) + }) +}) diff --git a/src/components/datatable/__tests__/TableVirtuosoComponents.spec.jsx b/src/components/datatable/__tests__/TableVirtuosoComponents.spec.jsx index 0134dc4510..8c8f1dfeeb 100644 --- a/src/components/datatable/__tests__/TableVirtuosoComponents.spec.jsx +++ b/src/components/datatable/__tests__/TableVirtuosoComponents.spec.jsx @@ -1,6 +1,8 @@ import { render, fireEvent, screen } from '@testing-library/react' import React from 'react' -import { EmptyPlaceholder } from '../TableVirtuosoComponents.jsx' +import TableComponents, { + EmptyPlaceholder, +} from '../TableVirtuosoComponents.jsx' const renderPlaceholder = (context) => render( @@ -59,3 +61,57 @@ describe('EmptyPlaceholder', () => { expect(screen.queryByText('No features match your filters')).toBeNull() }) }) + +describe('TableRow', () => { + const TableRow = TableComponents.TableRow + + const renderRow = () => + render( + + + + + + +
Bombali Sebora
+ ) + + test('prevents the default mousedown action for a ctrl-click, to avoid triggering native text selection', () => { + renderRow() + const notCancelled = fireEvent.mouseDown( + screen.getByTestId('dhis2-uicore-datatablerow'), + { + ctrlKey: true, + } + ) + expect(notCancelled).toBe(false) + }) + + test('prevents the default mousedown action for a shift-click', () => { + renderRow() + const notCancelled = fireEvent.mouseDown( + screen.getByTestId('dhis2-uicore-datatablerow'), + { + shiftKey: true, + } + ) + expect(notCancelled).toBe(false) + }) + + test('leaves a plain mousedown alone', () => { + renderRow() + const notCancelled = fireEvent.mouseDown( + screen.getByTestId('dhis2-uicore-datatablerow') + ) + expect(notCancelled).toBe(true) + }) +}) diff --git a/src/components/datatable/__tests__/useRowClickSelection.spec.js b/src/components/datatable/__tests__/useRowClickSelection.spec.js index 0b7be75f09..5846a720e3 100644 --- a/src/components/datatable/__tests__/useRowClickSelection.spec.js +++ b/src/components/datatable/__tests__/useRowClickSelection.spec.js @@ -12,7 +12,7 @@ describe('useRowClickSelection', () => { useRowClickSelection({ rows, onToggle, onSelectRange }) ) - result.current(row('a'), { ctrlKey: false, shiftKey: false }) + result.current.onRowClick(row('a'), { ctrlKey: false, shiftKey: false }) expect(onToggle).not.toHaveBeenCalled() expect(onSelectRange).not.toHaveBeenCalled() @@ -26,7 +26,7 @@ describe('useRowClickSelection', () => { useRowClickSelection({ rows, onToggle, onSelectRange }) ) - result.current(row('b'), { ctrlKey: true }) + result.current.onRowClick(row('b'), { ctrlKey: true }) expect(onToggle).toHaveBeenCalledWith('b') expect(onSelectRange).not.toHaveBeenCalled() @@ -40,12 +40,42 @@ describe('useRowClickSelection', () => { useRowClickSelection({ rows, onToggle, onSelectRange }) ) - result.current(row('a'), { ctrlKey: true }) - result.current(row('c'), { shiftKey: true }) + result.current.onRowClick(row('a'), { ctrlKey: true }) + result.current.onRowClick(row('c'), { shiftKey: true }) expect(onSelectRange).toHaveBeenCalledWith(['a', 'b', 'c']) }) + test('shift-click with no prior anchor selects (never toggles) just that row', () => { + const onToggle = jest.fn() + const onSelectRange = jest.fn() + const rows = [row('a'), row('b')] + const { result } = renderHook(() => + useRowClickSelection({ rows, onToggle, onSelectRange }) + ) + + result.current.onRowClick(row('b'), { shiftKey: true }) + + expect(onToggle).not.toHaveBeenCalled() + expect(onSelectRange).toHaveBeenCalledWith(['b']) + }) + + test('a shift-click range keeps the anchor fixed, so a following shift-click recomputes from the same anchor', () => { + const onToggle = jest.fn() + const onSelectRange = jest.fn() + const rows = [row('a'), row('b'), row('c')] + const { result } = renderHook(() => + useRowClickSelection({ rows, onToggle, onSelectRange }) + ) + + result.current.onRowClick(row('a'), { ctrlKey: true }) // anchor = a + result.current.onRowClick(row('c'), { shiftKey: true }) // range a-c, anchor stays a + result.current.onRowClick(row('b'), { shiftKey: true }) // range a-b, not c-b + + expect(onSelectRange).toHaveBeenNthCalledWith(1, ['a', 'b', 'c']) + expect(onSelectRange).toHaveBeenNthCalledWith(2, ['a', 'b']) + }) + test('does nothing when the row has no id', () => { const onToggle = jest.fn() const onSelectRange = jest.fn() @@ -54,8 +84,142 @@ describe('useRowClickSelection', () => { useRowClickSelection({ rows, onToggle, onSelectRange }) ) - result.current(row(null), { ctrlKey: true }) + result.current.onRowClick(row(null), { ctrlKey: true }) + + expect(onToggle).not.toHaveBeenCalled() + }) + + test('a ctrl-click that selects a row anchors it for a following shift-click', () => { + const onToggle = jest.fn() + const onSelectRange = jest.fn() + const rows = [row('a'), row('b'), row('c')] + const selectedIdSet = new Set() + const { result } = renderHook(() => + useRowClickSelection({ + rows, + onToggle, + onSelectRange, + selectedIdSet, + }) + ) + + result.current.onRowClick(row('a'), { ctrlKey: true }) // selects a, anchor = a + result.current.onRowClick(row('c'), { shiftKey: true }) + + expect(onSelectRange).toHaveBeenCalledWith(['a', 'b', 'c']) + }) + + test('a ctrl-click that deselects a row clears the anchor, so a following shift-click selects just that row', () => { + const onToggle = jest.fn() + const onSelectRange = jest.fn() + const rows = [row('a'), row('b'), row('c')] + const selectedIdSet = new Set(['a']) + const { result } = renderHook(() => + useRowClickSelection({ + rows, + onToggle, + onSelectRange, + selectedIdSet, + }) + ) + + result.current.onRowClick(row('a'), { ctrlKey: true }) // deselects a, anchor cleared + result.current.onRowClick(row('c'), { shiftKey: true }) + + expect(onToggle).toHaveBeenCalledWith('a') + expect(onSelectRange).toHaveBeenCalledWith(['c']) + }) + + test('shift-clicking a checkbox selects the range from the anchor, same as shift-clicking the row', () => { + const onToggle = jest.fn() + const onSelectRange = jest.fn() + const rows = [row('a'), row('b'), row('c'), row('d')] + const selectedIdSet = new Set() + const { result } = renderHook(() => + useRowClickSelection({ + rows, + onToggle, + onSelectRange, + selectedIdSet, + }) + ) + + result.current.onCheckboxToggle('a') // anchor = a + result.current.onCheckboxToggle('c', { shiftKey: true }) + + expect(onSelectRange).toHaveBeenCalledWith(['a', 'b', 'c']) + expect(onToggle).toHaveBeenCalledTimes(1) + expect(onToggle).toHaveBeenCalledWith('a') + }) + + test('shift-clicking a checkbox with no prior anchor selects just that row', () => { + const onToggle = jest.fn() + const onSelectRange = jest.fn() + const rows = [row('a'), row('b')] + const { result } = renderHook(() => + useRowClickSelection({ rows, onToggle, onSelectRange }) + ) + + result.current.onCheckboxToggle('b', { shiftKey: true }) expect(onToggle).not.toHaveBeenCalled() + expect(onSelectRange).toHaveBeenCalledWith(['b']) + }) + + test('checking a row via the checkbox anchors it for a following shift-click, same as ctrl-click', () => { + const onToggle = jest.fn() + const onSelectRange = jest.fn() + const rows = [row('a'), row('b'), row('c')] + const selectedIdSet = new Set() + const { result } = renderHook(() => + useRowClickSelection({ + rows, + onToggle, + onSelectRange, + selectedIdSet, + }) + ) + + result.current.onCheckboxToggle('a') + result.current.onRowClick(row('c'), { shiftKey: true }) + + expect(onToggle).toHaveBeenCalledWith('a') + expect(onSelectRange).toHaveBeenCalledWith(['a', 'b', 'c']) + }) + + test('unchecking a row via the checkbox clears the anchor, same as ctrl-click', () => { + const onToggle = jest.fn() + const onSelectRange = jest.fn() + const rows = [row('a'), row('b'), row('c')] + const selectedIdSet = new Set(['a']) + const { result } = renderHook(() => + useRowClickSelection({ + rows, + onToggle, + onSelectRange, + selectedIdSet, + }) + ) + + result.current.onCheckboxToggle('a') + result.current.onRowClick(row('c'), { shiftKey: true }) + + expect(onToggle).toHaveBeenCalledWith('a') + expect(onSelectRange).toHaveBeenCalledWith(['c']) + }) + + test('resetAnchor clears the anchor, so a following shift-click selects just that row', () => { + const onToggle = jest.fn() + const onSelectRange = jest.fn() + const rows = [row('a'), row('b'), row('c')] + const { result } = renderHook(() => + useRowClickSelection({ rows, onToggle, onSelectRange }) + ) + + result.current.onRowClick(row('a'), { ctrlKey: true }) // anchor = a + result.current.resetAnchor() + result.current.onRowClick(row('c'), { shiftKey: true }) + + expect(onSelectRange).toHaveBeenLastCalledWith(['c']) }) }) diff --git a/src/components/datatable/useRowClickSelection.js b/src/components/datatable/useRowClickSelection.js index 713c7908cb..376ca30033 100644 --- a/src/components/datatable/useRowClickSelection.js +++ b/src/components/datatable/useRowClickSelection.js @@ -1,10 +1,38 @@ import { useCallback, useRef } from 'react' import { getRowClickAction, getRowId } from '../../util/dataTable.js' -export const useRowClickSelection = ({ rows, onToggle, onSelectRange }) => { +export const useRowClickSelection = ({ + rows, + onToggle, + onSelectRange, + selectedIdSet, +}) => { const lastClickedRowIndexRef = useRef(null) - return useCallback( + const resetAnchor = useCallback(() => { + lastClickedRowIndexRef.current = null + }, []) + + const toggleWithAnchor = useCallback( + (id, rowIndex) => { + const isDeselecting = selectedIdSet?.has(id) + onToggle(id) + lastClickedRowIndexRef.current = isDeselecting ? null : rowIndex + }, + [onToggle, selectedIdSet] + ) + + const selectRangeWithAnchor = useCallback( + (ids, rowIndex, hadAnchor) => { + onSelectRange(ids) + if (!hadAnchor) { + lastClickedRowIndexRef.current = rowIndex + } + }, + [onSelectRange] + ) + + const onRowClick = useCallback( (row, event) => { const id = getRowId(row) @@ -13,6 +41,7 @@ export const useRowClickSelection = ({ rows, onToggle, onSelectRange }) => { } const rowIndex = rows.findIndex((r) => getRowId(r) === id) + const hadAnchor = lastClickedRowIndexRef.current !== null const action = getRowClickAction(event, { id, rowIndex, @@ -25,12 +54,38 @@ export const useRowClickSelection = ({ rows, onToggle, onSelectRange }) => { } if (action.type === 'range') { - onSelectRange(action.ids) + selectRangeWithAnchor(action.ids, rowIndex, hadAnchor) } else { - onToggle(action.id) + toggleWithAnchor(action.id, rowIndex) } - lastClickedRowIndexRef.current = rowIndex }, - [rows, onToggle, onSelectRange] + [rows, selectRangeWithAnchor, toggleWithAnchor] ) + + const onCheckboxToggle = useCallback( + (id, event) => { + if (!id || !rows) { + return + } + + const rowIndex = rows.findIndex((r) => getRowId(r) === id) + + if (event?.shiftKey) { + const hadAnchor = lastClickedRowIndexRef.current !== null + const action = getRowClickAction(event, { + id, + rowIndex, + rows, + lastClickedRowIndex: lastClickedRowIndexRef.current, + }) + selectRangeWithAnchor(action.ids, rowIndex, hadAnchor) + return + } + + toggleWithAnchor(id, rowIndex) + }, + [rows, selectRangeWithAnchor, toggleWithAnchor] + ) + + return { onRowClick, onCheckboxToggle, resetAnchor } } diff --git a/src/util/__tests__/dataTable.spec.js b/src/util/__tests__/dataTable.spec.js index 91cbb9fd19..8459978a7b 100644 --- a/src/util/__tests__/dataTable.spec.js +++ b/src/util/__tests__/dataTable.spec.js @@ -72,13 +72,13 @@ describe('getRowClickAction', () => { ).toEqual({ type: 'toggle', id: 'b' }) }) - test('shift-click with no prior anchor falls back to a single-row toggle', () => { + test('shift-click with no prior anchor selects just that row, never deselects it', () => { expect( getRowClickAction( { shiftKey: true }, { id: 'c', rowIndex: 2, rows, lastClickedRowIndex: null } ) - ).toEqual({ type: 'toggle', id: 'c' }) + ).toEqual({ type: 'range', ids: ['c'] }) }) test('shift-click with a prior anchor selects the range between them', () => { diff --git a/src/util/dataTable.js b/src/util/dataTable.js index bb891f90b8..b6cf507a09 100644 --- a/src/util/dataTable.js +++ b/src/util/dataTable.js @@ -29,7 +29,7 @@ export const getRowClickAction = ( ) => { if (event.shiftKey) { if (lastClickedRowIndex === null) { - return { type: 'toggle', id } + return { type: 'range', ids: [id] } } const [start, end] = [lastClickedRowIndex, rowIndex].sort( (a, b) => a - b From 7e387fd67f312eca67e4e8fff2403e06a3292251 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Tue, 8 Sep 2026 18:06:21 +0200 Subject: [PATCH 8/8] fix: maps-gl version bump --- package.json | 2 +- yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 42f07ecd43..9806b83325 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,7 @@ "@dhis2/analytics": "^29.5.5", "@dhis2/app-runtime": "^3.17.3", "@dhis2/app-service-datastore": "^1.0.0-beta.3", - "@dhis2/maps-gl": "git+https://github.com/d2-ci/maps-gl.git#6758ac621ff7ed582ad458bb4c9f90900358cedc", + "@dhis2/maps-gl": "git+https://github.com/d2-ci/maps-gl.git#7c0b414689f82daaeb8bcb55b4c6fcedabb7257e", "@dhis2/ui": "^10.17.0", "@dnd-kit/core": "^6.0.8", "@dnd-kit/modifiers": "^9.0.0", diff --git a/yarn.lock b/yarn.lock index c15648905d..2457fcf91b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2371,9 +2371,9 @@ resolved "https://registry.yarnpkg.com/@dhis2/data-engine/-/data-engine-3.17.3.tgz#0347416e9919efbf4d9739c4141fa543f89669ad" integrity sha512-hLXt7LFrFitR7QgKfGQ3ComTLrY5IAdtERonhdo/SIrsRYWoeVaMiCOkUUzC48pEaeo1/BL5qwA7Tw7jZgROQw== -"@dhis2/maps-gl@git+https://github.com/d2-ci/maps-gl.git#6758ac621ff7ed582ad458bb4c9f90900358cedc": +"@dhis2/maps-gl@git+https://github.com/d2-ci/maps-gl.git#7c0b414689f82daaeb8bcb55b4c6fcedabb7257e": version "4.4.3" - resolved "git+https://github.com/d2-ci/maps-gl.git#6758ac621ff7ed582ad458bb4c9f90900358cedc" + resolved "git+https://github.com/d2-ci/maps-gl.git#7c0b414689f82daaeb8bcb55b4c6fcedabb7257e" dependencies: "@mapbox/sphericalmercator" "^1.2.0" "@turf/area" "^7.3.5"