From 4084a6fc9c03455c0b3655214bab08f70b85d378 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Mon, 13 Jul 2026 22:23:21 +0200 Subject: [PATCH 01/27] feat: preserve optionSet on event layer table headers Needed so the multi-select column filter can distinguish option-set-backed columns (which need code->name resolution) from plain categorical columns. --- src/components/datatable/useTableData.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/datatable/useTableData.js b/src/components/datatable/useTableData.js index 55389a70a..5abb4a47f 100644 --- a/src/components/datatable/useTableData.js +++ b/src/components/datatable/useTableData.js @@ -133,6 +133,7 @@ const getEventHeaders = ({ !optionSet && numberValueTypes.includes(valueType) ? TYPE_NUMBER : TYPE_STRING, + optionSet: optionSet || null, })) customFields.push(defaultFieldsMap()[TYPE]) From 451557ba2e059bf84e68e88f9efe2c600577712e Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Mon, 13 Jul 2026 22:29:47 +0200 Subject: [PATCH 02/27] feat: compute columnOptions for categorical data table columns Adds a columnOptions memo to useTableData, allowlisted to the legend/type built-in columns plus any optionSet-backed event column, capped at 30 distinct values before falling back to free text. A blanket "any string column with few distinct values" check would wrongly turn name/id/parentName into dropdowns on small datasets. --- .../datatable/__tests__/useTableData.spec.jsx | 122 ++++++++++++++++++ src/components/datatable/useTableData.js | 40 ++++++ 2 files changed, 162 insertions(+) diff --git a/src/components/datatable/__tests__/useTableData.spec.jsx b/src/components/datatable/__tests__/useTableData.spec.jsx index c883914a5..7a4824437 100644 --- a/src/components/datatable/__tests__/useTableData.spec.jsx +++ b/src/components/datatable/__tests__/useTableData.spec.jsx @@ -976,3 +976,125 @@ describe('useTableData showOnlySelected', () => { expect(current.rows).toHaveLength(0) }) }) + +describe('useTableData columnOptions', () => { + const store = { aggregations: {} } + + const renderTableData = (layer) => + renderHook( + () => + useTableData({ + layer, + sortField: 'name', + sortDirection: 'asc', + }), + { + wrapper: ({ children }) => ( + {children} + ), + } + ).result + + test('includes legend and type for a thematic layer, but not name/id', () => { + const layer = { + layer: 'thematic', + dataFilters: null, + data: [ + { + properties: { + id: 'ou1', + name: 'Org unit 1', + rawValue: 10, + legend: 'High', + range: '5 - 15', + level: 1, + parentName: 'Country', + type: 'Point', + color: '#ff0000', + }, + }, + { + properties: { + id: 'ou2', + name: 'Org unit 2', + rawValue: 20, + legend: 'Low', + range: '15 - 25', + level: 1, + parentName: 'Country', + type: 'Point', + color: '#00ff00', + }, + }, + ], + } + + const { current } = renderTableData(layer) + + expect(current.columnOptions.legend).toEqual([ + { value: 'High' }, + { value: 'Low' }, + ]) + expect(current.columnOptions.type).toEqual([{ value: 'Point' }]) + expect(current.columnOptions.name).toBeUndefined() + expect(current.columnOptions.id).toBeUndefined() + expect(current.columnOptions.parentName).toBeUndefined() + }) + + test('falls back to free text when a column has more than 30 distinct values', () => { + const layer = { + layer: 'orgUnit', + dataFilters: null, + data: Array.from({ length: 31 }, (_, i) => ({ + properties: { + id: `ou${i}`, + name: `Org unit ${i}`, + level: 1, + parentName: 'Country', + type: `Type${i}`, + }, + })), + } + + const { current } = renderTableData(layer) + + expect(current.columnOptions.type).toBeUndefined() + }) + + test('exposes optionSet on event columns for later resolution by FilterInput', () => { + const layer = { + layer: 'event', + dataFilters: null, + isExtended: true, + headers: [ + { + name: 'AbCdEfGhIjK', + column: 'Case classification', + valueType: 'TEXT', + optionSet: { id: 'xyz123' }, + }, + ], + data: [ + { + properties: { + id: 'evt1', + type: 'Point', + ouname: 'Test OU', + eventdate: '2023-01-01', + AbCdEfGhIjK: 'CONFIRMED', + }, + }, + ], + } + + const { current } = renderTableData(layer) + + const header = current.headers.find( + (h) => h.dataKey === 'AbCdEfGhIjK' + ) + expect(header.optionSet).toEqual({ id: 'xyz123' }) + expect(current.columnOptions.AbCdEfGhIjK).toEqual([ + { value: 'CONFIRMED' }, + ]) + }) +}) diff --git a/src/components/datatable/useTableData.js b/src/components/datatable/useTableData.js index 5abb4a47f..1a207ce42 100644 --- a/src/components/datatable/useTableData.js +++ b/src/components/datatable/useTableData.js @@ -197,6 +197,10 @@ const getGeoJsonUrlHeaders = (firstDataItem) => const EMPTY_AGGREGATIONS = {} const EMPTY_LAYER = {} +const EMPTY_COLUMN_OPTIONS = {} + +const CATEGORICAL_DATA_KEYS = new Set([LEGEND, TYPE]) +const MAX_CATEGORICAL_OPTIONS = 30 export const useTableData = ({ layer, @@ -336,6 +340,41 @@ export const useTableData = ({ layerHeaders, ]) + const columnOptions = useMemo(() => { + if (!headers?.length || !dataWithAggregations?.length) { + return EMPTY_COLUMN_OPTIONS + } + + const result = {} + headers.forEach(({ dataKey, type, optionSet }) => { + if (type !== TYPE_STRING) { + return + } + if (!CATEGORICAL_DATA_KEYS.has(dataKey) && !optionSet) { + return + } + + const seen = new Set() + for (const item of dataWithAggregations) { + const val = item[dataKey] + if (val !== undefined && val !== null && val !== '') { + seen.add(String(val)) + } + if (seen.size > MAX_CATEGORICAL_OPTIONS) { + break + } + } + + if (seen.size > 0 && seen.size <= MAX_CATEGORICAL_OPTIONS) { + result[dataKey] = Array.from(seen) + .sort() + .map((value) => ({ value })) + } + }) + + return Object.keys(result).length ? result : EMPTY_COLUMN_OPTIONS + }, [headers, dataWithAggregations]) + const rows = useMemo(() => { if (errorCode.current) { return null @@ -434,5 +473,6 @@ export const useTableData = ({ error: getErrorCodeText(errorCode.current), totalCount, filteredCount, + columnOptions, } } From 4ce4b3f324e28a2ebb66fee57f24e381c889a5cb Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Mon, 13 Jul 2026 22:37:42 +0200 Subject: [PATCH 03/27] feat: support array-valued (multi-select) filters in filterData Adds an Array.isArray short-circuit before the existing numeric/string dispatch so a multi-select column filter (built from user checkbox selections) OR-matches exactly against the raw stored value, without touching stringFilter/numericFilter's existing behavior. --- src/util/__tests__/filter.spec.js | 28 ++++++++++++++++++++++++++++ src/util/filter.js | 8 ++++++++ 2 files changed, 36 insertions(+) diff --git a/src/util/__tests__/filter.spec.js b/src/util/__tests__/filter.spec.js index c23cda725..e17e51b00 100644 --- a/src/util/__tests__/filter.spec.js +++ b/src/util/__tests__/filter.spec.js @@ -68,4 +68,32 @@ describe('filterData', () => { const filters = { a: 'a', b: 'r' } expect(filterData(data, filters)).toEqual([{ a: 'banana', b: 'horse' }]) }) + + it('should OR-match an array filter against the raw stored value', () => { + const data = [{ a: 'High' }, { a: 'Medium' }, { a: 'Low' }] + const filters = { a: ['High', 'Low'] } + expect(filterData(data, filters)).toEqual([{ a: 'High' }, { a: 'Low' }]) + }) + + it('should not filter any rows when the array filter is empty', () => { + const data = [{ a: 'High' }, { a: 'Low' }] + const filters = { a: [] } + expect(filterData(data, filters)).toEqual([{ a: 'High' }, { a: 'Low' }]) + }) + + it('should match array filters against non-string values by exact string coercion', () => { + const data = [{ a: 1 }, { a: 2 }, { a: 3 }] + const filters = { a: ['1', '3'] } + expect(filterData(data, filters)).toEqual([{ a: 1 }, { a: 3 }]) + }) + + it('should combine an array filter on one field with a string filter on another', () => { + const data = [ + { a: 'High', b: 'cow' }, + { a: 'High', b: 'horse' }, + { a: 'Low', b: 'horse' }, + ] + const filters = { a: ['High'], b: 'horse' } + expect(filterData(data, filters)).toEqual([{ a: 'High', b: 'horse' }]) + }) }) diff --git a/src/util/filter.js b/src/util/filter.js index ae5d72296..6ce619eb9 100644 --- a/src/util/filter.js +++ b/src/util/filter.js @@ -15,6 +15,14 @@ export const filterData = (data, filters) => { const props = d.properties || d // GeoJSON or plain object const value = props[field] + if (Array.isArray(filter)) { + // Multi-select: OR match against the raw stored value + return ( + filter.length === 0 || + filter.includes(value == null ? '' : String(value)) + ) + } + return typeof value === 'number' ? numericFilter(value, filter) : stringFilter(value, filter) From 2d263feed8a213772e25a3cb8de5e021386cb329 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Mon, 13 Jul 2026 22:39:18 +0200 Subject: [PATCH 04/27] feat: add filterByGlobalSearch utility and thread globalSearch through useTableData Applies the global search after column filters and before selection filtering/sorting in the rows memo, matching case-insensitively across all string-typed columns. --- .../datatable/__tests__/useTableData.spec.jsx | 47 +++++++++++++++++++ src/components/datatable/useTableData.js | 15 +++++- src/util/__tests__/filter.spec.js | 42 ++++++++++++++++- src/util/filter.js | 16 +++++++ 4 files changed, 118 insertions(+), 2 deletions(-) diff --git a/src/components/datatable/__tests__/useTableData.spec.jsx b/src/components/datatable/__tests__/useTableData.spec.jsx index 7a4824437..6501dc91d 100644 --- a/src/components/datatable/__tests__/useTableData.spec.jsx +++ b/src/components/datatable/__tests__/useTableData.spec.jsx @@ -1098,3 +1098,50 @@ describe('useTableData columnOptions', () => { ]) }) }) + +describe('useTableData globalSearch', () => { + const store = { aggregations: {} } + + const layer = { + layer: 'orgUnit', + dataFilters: null, + data: [ + { properties: { id: 'a', name: 'Kampala', parentName: 'Uganda' } }, + { properties: { id: 'b', name: 'Nairobi', parentName: 'Kenya' } }, + ], + } + + const renderTableData = (globalSearch) => + renderHook( + () => + useTableData({ + layer, + sortField: 'name', + sortDirection: 'asc', + globalSearch, + }), + { + wrapper: ({ children }) => ( + {children} + ), + } + ).result + + test('includes all rows when the search string is empty', () => { + const { current } = renderTableData('') + expect(current.rows).toHaveLength(2) + }) + + test('matches case-insensitively across any string column', () => { + const { current } = renderTableData('uganda') + expect(current.rows).toHaveLength(1) + expect(current.rows[0].find((c) => c.dataKey === 'name').value).toBe( + 'Kampala' + ) + }) + + test('shows no rows when nothing matches', () => { + const { current } = renderTableData('addis ababa') + expect(current.rows).toHaveLength(0) + }) +}) diff --git a/src/components/datatable/useTableData.js b/src/components/datatable/useTableData.js index 1a207ce42..f41150a44 100644 --- a/src/components/datatable/useTableData.js +++ b/src/components/datatable/useTableData.js @@ -11,7 +11,7 @@ import { } from '../../constants/layers.js' import { numberValueTypes } from '../../constants/valueTypes.js' import { hasClasses } from '../../util/earthEngine.js' -import { filterData } from '../../util/filter.js' +import { filterByGlobalSearch, filterData } from '../../util/filter.js' import { getGeojsonDisplayData, isFeatureInBounds } from '../../util/geojson.js' import { parseRange } from '../../util/legend.js' import { getRoundToPrecisionFn, getPrecision } from '../../util/numbers.js' @@ -210,6 +210,7 @@ export const useTableData = ({ mapBounds, showOnlySelected, selectedIdSet, + globalSearch, }) => { const allAggregations = useSelector((state) => state.aggregations) const aggregations = allAggregations[layer.id] || EMPTY_AGGREGATIONS @@ -387,6 +388,17 @@ export const useTableData = ({ let filteredData = filterData(dataWithAggregations, dataFilters) + if (globalSearch?.trim()) { + const stringDataKeys = headers + .filter((h) => h.type === TYPE_STRING) + .map((h) => h.dataKey) + filteredData = filterByGlobalSearch( + filteredData, + globalSearch, + stringDataKeys + ) + } + if (showOnlySelected) { filteredData = filteredData.filter((item) => selectedIdSet?.has(item.id) @@ -450,6 +462,7 @@ export const useTableData = ({ headers, dataWithAggregations, dataFilters, + globalSearch, sortField, sortDirection, showOnlySelected, diff --git a/src/util/__tests__/filter.spec.js b/src/util/__tests__/filter.spec.js index e17e51b00..d2c4315b3 100644 --- a/src/util/__tests__/filter.spec.js +++ b/src/util/__tests__/filter.spec.js @@ -1,4 +1,4 @@ -import { filterData } from '../filter.js' +import { filterByGlobalSearch, filterData } from '../filter.js' describe('filterData', () => { it('should return the original data if no filters are provided', () => { @@ -97,3 +97,43 @@ describe('filterData', () => { expect(filterData(data, filters)).toEqual([{ a: 'High', b: 'horse' }]) }) }) + +describe('filterByGlobalSearch', () => { + const data = [ + { name: 'Kampala Hospital', type: 'Hospital' }, + { name: 'Entebbe Clinic', type: 'Clinic' }, + { name: 'Jinja Hospital', type: 'Hospital' }, + ] + + it('returns the original data when the search string is empty', () => { + expect(filterByGlobalSearch(data, '', ['name', 'type'])).toEqual(data) + expect(filterByGlobalSearch(data, ' ', ['name', 'type'])).toEqual( + data + ) + }) + + it('returns the original data when there are no string data keys', () => { + expect(filterByGlobalSearch(data, 'Kampala', [])).toEqual(data) + }) + + it('matches case-insensitively across any of the given fields', () => { + expect(filterByGlobalSearch(data, 'kampala', ['name', 'type'])).toEqual( + [{ name: 'Kampala Hospital', type: 'Hospital' }] + ) + }) + + it('matches rows where any field contains the search string', () => { + expect( + filterByGlobalSearch(data, 'hospital', ['name', 'type']) + ).toEqual([ + { name: 'Kampala Hospital', type: 'Hospital' }, + { name: 'Jinja Hospital', type: 'Hospital' }, + ]) + }) + + it('returns no rows when nothing matches', () => { + expect(filterByGlobalSearch(data, 'nairobi', ['name', 'type'])).toEqual( + [] + ) + }) +}) diff --git a/src/util/filter.js b/src/util/filter.js index 6ce619eb9..4faa3c85a 100644 --- a/src/util/filter.js +++ b/src/util/filter.js @@ -48,6 +48,22 @@ export const numericFilter = (value, filter) => { }) } +// Matches rows where any of the given string-typed fields contains +// the search string (case-insensitive) +export const filterByGlobalSearch = (data, searchString, stringDataKeys) => { + if (!searchString?.trim() || !stringDataKeys?.length) { + return data + } + const lower = searchString.toLowerCase() + return data.filter((item) => { + const props = item.properties || item + return stringDataKeys.some((key) => { + const val = props[key] + return val != null && String(val).toLowerCase().includes(lower) + }) + }) +} + // Returns true if the filter is true const isTrueFilter = (value, filter) => { if (filter.includes('>=')) { From d040a5874b4ecad19adf43df0dc1e7ce1ba585bf Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Mon, 13 Jul 2026 22:43:10 +0200 Subject: [PATCH 05/27] feat: add multi-select dropdown filter to FilterInput Adds a Popover + checkbox-list path (reusing the pattern already established by TableContextMenu.jsx) rendered when an `options` prop is passed in. Option-set-backed columns get a separate wrapper component (OptionSetMultiSelectFilter) so useOptionSet/useDataQuery is only ever mounted when an optionSetId actually exists - legend/type columns never have one, and no test in the repo mocks useDataQuery. --- src/components/datatable/FilterInput.jsx | 134 +++++++++++++++++- .../datatable/__tests__/FilterInput.spec.jsx | 112 +++++++++++++++ .../datatable/styles/FilterInput.module.css | 18 +++ 3 files changed, 257 insertions(+), 7 deletions(-) create mode 100644 src/components/datatable/__tests__/FilterInput.spec.jsx create mode 100644 src/components/datatable/styles/FilterInput.module.css diff --git a/src/components/datatable/FilterInput.jsx b/src/components/datatable/FilterInput.jsx index 16bab4a9c..31a7bd91e 100644 --- a/src/components/datatable/FilterInput.jsx +++ b/src/components/datatable/FilterInput.jsx @@ -1,17 +1,113 @@ import i18n from '@dhis2/d2-i18n' -import { Input } from '@dhis2/ui' +import { Input, Popover } from '@dhis2/ui' import PropTypes from 'prop-types' -import React from 'react' +import React, { useRef, useState } from 'react' import { useDispatch, useSelector } from 'react-redux' import { setDataFilter, clearDataFilter } from '../../actions/dataFilters.js' +import useOptionSet from '../../hooks/useOptionSet.js' +import Checkbox from '../core/Checkbox.jsx' +import styles from './styles/FilterInput.module.css' -const FilterInput = ({ type, dataKey, name }) => { +// Shared popover UI — label resolution is injected so it never needs to +// know whether it's an option-set column or a plain categorical one. +const MultiSelectPopover = ({ + dataKey, + layerId, + filterValue, + options, + resolveLabel, +}) => { + const dispatch = useDispatch() + const anchorRef = useRef(null) + const [isOpen, setIsOpen] = useState(false) + const selected = Array.isArray(filterValue) ? filterValue : [] + + const toggleValue = (value) => { + const next = selected.includes(value) + ? selected.filter((v) => v !== value) + : [...selected, value] + next.length + ? dispatch(setDataFilter(layerId, dataKey, next)) + : dispatch(clearDataFilter(layerId, dataKey)) + } + + const buttonLabel = + selected.length === 0 + ? i18n.t('All') + : i18n.t('{{count}} selected', { count: selected.length }) + + return ( + <> + + {isOpen && ( + setIsOpen(false)} + > +
+ {options.map(({ value }) => ( + toggleValue(value)} + /> + ))} +
+ + )} + + ) +} + +MultiSelectPopover.propTypes = { + dataKey: PropTypes.string.isRequired, + options: PropTypes.arrayOf(PropTypes.shape({ value: PropTypes.string })) + .isRequired, + resolveLabel: PropTypes.func.isRequired, + filterValue: PropTypes.oneOfType([ + PropTypes.string, + PropTypes.arrayOf(PropTypes.string), + ]), + layerId: PropTypes.string, +} + +// Plain categorical columns (legend, type): raw value IS the display label. +const MultiSelectFilter = (props) => ( + value} /> +) + +// Option-set-backed event columns: translate stored code -> display name. +// useOptionSet/useDataQuery is only ever mounted here, never for legend/type, +// since those columns never have an optionSetId. +const OptionSetMultiSelectFilter = ({ optionSetId, ...props }) => { + const { optionSet } = useOptionSet(optionSetId) + const resolveLabel = (value) => + optionSet?.options.find((o) => o.code === value)?.name ?? value + return +} + +OptionSetMultiSelectFilter.propTypes = { + optionSetId: PropTypes.string.isRequired, +} + +const FilterInput = ({ type, dataKey, name, options, optionSetId }) => { const dispatch = useDispatch() const dataTable = useSelector((state) => state.dataTable) const map = useSelector((state) => state.map) const overlay = - dataTable && map.mapViews.filter((layer) => layer.id === dataTable)[0] + dataTable && map.mapViews.find((layer) => layer.id === dataTable) let layerId let filters @@ -20,19 +116,41 @@ const FilterInput = ({ type, dataKey, name }) => { filters = overlay.dataFilters || {} } - const filterValue = filters[dataKey] || '' + const filterValue = filters?.[dataKey] + + if (options?.length) { + return optionSetId ? ( + + ) : ( + + ) + } + + const stringFilterValue = + typeof filterValue === 'string' ? filterValue : '' const onChange = ({ value }) => value !== '' ? dispatch(setDataFilter(layerId, dataKey, value)) - : dispatch(clearDataFilter(layerId, dataKey, value)) + : dispatch(clearDataFilter(layerId, dataKey)) return ( 3&<8' : i18n.t('Search')} - value={filterValue} + value={stringFilterValue} onChange={onChange} /> ) @@ -42,6 +160,8 @@ FilterInput.propTypes = { dataKey: PropTypes.string.isRequired, name: PropTypes.string.isRequired, type: PropTypes.string.isRequired, + options: PropTypes.arrayOf(PropTypes.shape({ value: PropTypes.string })), + optionSetId: PropTypes.string, } export default FilterInput diff --git a/src/components/datatable/__tests__/FilterInput.spec.jsx b/src/components/datatable/__tests__/FilterInput.spec.jsx new file mode 100644 index 000000000..380b9c48f --- /dev/null +++ b/src/components/datatable/__tests__/FilterInput.spec.jsx @@ -0,0 +1,112 @@ +import { render, fireEvent, screen } from '@testing-library/react' +import React from 'react' +import { Provider } from 'react-redux' +import configureMockStore from 'redux-mock-store' +import FilterInput from '../FilterInput.jsx' + +jest.mock('../../../hooks/useOptionSet.js', () => ({ + __esModule: true, + default: jest.fn(), +})) + +// eslint-disable-next-line import/first +import useOptionSet from '../../../hooks/useOptionSet.js' + +const mockStore = configureMockStore() + +const renderFilterInput = (props, dataFilters) => { + const store = mockStore({ + dataTable: 'layer1', + map: { + mapViews: [{ id: 'layer1', dataFilters: dataFilters || {} }], + }, + }) + return render( + + + + ) +} + +describe('FilterInput text/numeric path', () => { + test('renders a free-text input when no options are provided', () => { + renderFilterInput({}) + expect( + screen + .getByTestId('data-table-column-filter-input-Name') + .querySelector('input') + ).toBeInTheDocument() + }) + + test('shows the current filter value', () => { + renderFilterInput({}, { name: 'hospital' }) + expect( + screen + .getByTestId('data-table-column-filter-input-Name') + .querySelector('input') + ).toHaveValue('hospital') + }) +}) + +describe('FilterInput multi-select path (no optionSetId)', () => { + const options = [{ value: 'High' }, { value: 'Low' }] + + test('shows "All" when nothing is selected', () => { + renderFilterInput({ dataKey: 'legend', name: 'Legend', options }) + expect(screen.getByText('All')).toBeInTheDocument() + }) + + test('shows the selected count when a filter is active', () => { + renderFilterInput( + { dataKey: 'legend', name: 'Legend', options }, + { legend: ['High'] } + ) + expect(screen.getByText('1 selected')).toBeInTheDocument() + }) + + test('opens a popover with a checkbox per option, using the raw value as the label', () => { + renderFilterInput({ dataKey: 'legend', name: 'Legend', options }) + fireEvent.click(screen.getByText('All')) + expect(screen.getByLabelText('High')).toBeInTheDocument() + expect(screen.getByLabelText('Low')).toBeInTheDocument() + }) +}) + +describe('FilterInput multi-select path (optionSetId)', () => { + const options = [{ value: 'CONFIRMED' }, { value: 'PROBABLE' }] + + beforeEach(() => { + useOptionSet.mockReturnValue({ + optionSet: { + options: [ + { code: 'CONFIRMED', name: 'Confirmed case' }, + { code: 'PROBABLE', name: 'Probable case' }, + ], + }, + }) + }) + + test('resolves stored codes to display names in the popover', () => { + renderFilterInput({ + dataKey: 'caseType', + name: 'Case classification', + options, + optionSetId: 'optionSet1', + }) + fireEvent.click(screen.getByText('All')) + expect(screen.getByLabelText('Confirmed case')).toBeInTheDocument() + expect(screen.getByLabelText('Probable case')).toBeInTheDocument() + }) + + test('falls back to the raw code when the option set has not loaded yet', () => { + useOptionSet.mockReturnValue({ optionSet: null }) + renderFilterInput({ + dataKey: 'caseType', + name: 'Case classification', + options, + optionSetId: 'optionSet1', + }) + fireEvent.click(screen.getByText('All')) + expect(screen.getByLabelText('CONFIRMED')).toBeInTheDocument() + }) +}) diff --git a/src/components/datatable/styles/FilterInput.module.css b/src/components/datatable/styles/FilterInput.module.css new file mode 100644 index 000000000..e309ad01b --- /dev/null +++ b/src/components/datatable/styles/FilterInput.module.css @@ -0,0 +1,18 @@ +.multiSelectButton { + width: 100%; + height: 24px; + font-size: 11px; + padding: 4px 6px; + border: 1px solid var(--colors-grey400); + border-radius: 3px; + background: var(--colors-white); + cursor: pointer; + text-align: left; +} + +.multiSelectPopover { + padding: var(--spacers-dp8); + max-height: 260px; + overflow-y: auto; + min-width: 180px; +} From 8c4a62127ee5bdbbe665dae2f788239356992759 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Mon, 13 Jul 2026 22:45:21 +0200 Subject: [PATCH 06/27] feat: add numeric filter syntax help tooltip Replaces the cryptic '2,>3&<8' placeholder with a simpler '> 5, < 8' and adds an info icon + Tooltip explaining the AND/OR/comparison syntax, consistent with Tooltip's existing use throughout DataTable.jsx/BottomPanel.jsx. --- src/components/datatable/FilterInput.jsx | 42 +++++++++++++++---- .../datatable/__tests__/FilterInput.spec.jsx | 14 +++++++ .../datatable/styles/FilterInput.module.css | 18 ++++++++ 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/src/components/datatable/FilterInput.jsx b/src/components/datatable/FilterInput.jsx index 31a7bd91e..95045cc2d 100644 --- a/src/components/datatable/FilterInput.jsx +++ b/src/components/datatable/FilterInput.jsx @@ -1,5 +1,5 @@ import i18n from '@dhis2/d2-i18n' -import { Input, Popover } from '@dhis2/ui' +import { Input, Popover, Tooltip, IconInfo16 } from '@dhis2/ui' import PropTypes from 'prop-types' import React, { useRef, useState } from 'react' import { useDispatch, useSelector } from 'react-redux' @@ -8,6 +8,16 @@ import useOptionSet from '../../hooks/useOptionSet.js' import Checkbox from '../core/Checkbox.jsx' import styles from './styles/FilterInput.module.css' +const NUMERIC_FILTER_HELP = ( +
+
{'> 5 — ' + i18n.t('greater than 5')}
+
{'>= 5 — ' + i18n.t('greater than or equal to 5')}
+
{'< 5, <= 5 — ' + i18n.t('less than (or equal to) 5')}
+
{'2, > 8 — ' + i18n.t('equal to 2 OR greater than 8')}
+
{'> 3 & < 8 — ' + i18n.t('greater than 3 AND less than 8')}
+
+) + // Shared popover UI — label resolution is injected so it never needs to // know whether it's an option-set column or a plain categorical one. const MultiSelectPopover = ({ @@ -146,13 +156,29 @@ const FilterInput = ({ type, dataKey, name, options, optionSetId }) => { : dispatch(clearDataFilter(layerId, dataKey)) return ( - 3&<8' : i18n.t('Search')} - value={stringFilterValue} - onChange={onChange} - /> + + 5, < 8' : i18n.t('Search')} + value={stringFilterValue} + onChange={onChange} + /> + {type === 'number' && ( + + + + + + )} + ) } diff --git a/src/components/datatable/__tests__/FilterInput.spec.jsx b/src/components/datatable/__tests__/FilterInput.spec.jsx index 380b9c48f..92c54a113 100644 --- a/src/components/datatable/__tests__/FilterInput.spec.jsx +++ b/src/components/datatable/__tests__/FilterInput.spec.jsx @@ -46,6 +46,20 @@ describe('FilterInput text/numeric path', () => { .querySelector('input') ).toHaveValue('hospital') }) + + test('shows a numeric filter syntax help icon for number columns', () => { + renderFilterInput({ type: 'number' }) + expect( + screen.getByTestId('data-table-numeric-filter-help') + ).toBeInTheDocument() + }) + + test('does not show the help icon for string columns', () => { + renderFilterInput({ type: 'string' }) + expect( + screen.queryByTestId('data-table-numeric-filter-help') + ).not.toBeInTheDocument() + }) }) describe('FilterInput multi-select path (no optionSetId)', () => { diff --git a/src/components/datatable/styles/FilterInput.module.css b/src/components/datatable/styles/FilterInput.module.css index e309ad01b..c5d585d03 100644 --- a/src/components/datatable/styles/FilterInput.module.css +++ b/src/components/datatable/styles/FilterInput.module.css @@ -16,3 +16,21 @@ overflow-y: auto; min-width: 180px; } + +.numericFilterWrapper { + display: flex; + align-items: center; + gap: 2px; +} + +.numericFilterWrapper > :global(div) { + flex: 1 1 auto; + min-width: 0; +} + +.helpIcon { + display: inline-flex; + flex-shrink: 0; + color: var(--colors-grey600); + cursor: help; +} From 0cbfac0186d00f26b0abf7f44712c0ab77a1f16c Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Mon, 13 Jul 2026 23:18:46 +0200 Subject: [PATCH 07/27] feat: pass columnOptions and optionSet through DataTable to FilterInput Table now forwards globalSearch into useTableData and threads columnOptions[dataKey]/optionSet.id from the header objects into FilterInput so it can pick between free-text and multi-select rendering. --- src/components/datatable/DataTable.jsx | 141 +++++++++++++---------- src/components/datatable/FilterInput.jsx | 2 +- 2 files changed, 84 insertions(+), 59 deletions(-) diff --git a/src/components/datatable/DataTable.jsx b/src/components/datatable/DataTable.jsx index 7fd1f0ead..41caa6e38 100644 --- a/src/components/datatable/DataTable.jsx +++ b/src/components/datatable/DataTable.jsx @@ -131,7 +131,12 @@ const TableComponents = { ), } -const Table = ({ availableWidth, onCountChange, showOnlySelected }) => { +const Table = ({ + availableWidth, + onCountChange, + showOnlySelected, + globalSearch, +}) => { const { systemSettings: { keyAnalysisDigitGroupSeparator }, } = useCachedData() @@ -235,16 +240,24 @@ const Table = ({ availableWidth, onCountChange, showOnlySelected }) => { ) const selectedIdSet = useMemo(() => new Set(selectedIds), [selectedIds]) - const { headers, rows, isLoading, error, totalCount, filteredCount } = - useTableData({ - layer, - sortField, - sortDirection, - showOnlyFeaturesInView, - mapBounds, - showOnlySelected, - selectedIdSet, - }) + const { + headers, + rows, + isLoading, + error, + totalCount, + filteredCount, + columnOptions, + } = useTableData({ + layer, + sortField, + sortDirection, + showOnlyFeaturesInView, + mapBounds, + showOnlySelected, + selectedIdSet, + globalSearch, + }) useEffect(() => { onCountChange?.(totalCount, filteredCount) @@ -457,55 +470,66 @@ const Table = ({ availableWidth, onCountChange, showOnlySelected }) => { onChange={onToggleSelectAll} /> - {headers.map(({ name, dataKey, type }, index) => ( - - ) - } - width={ - columnWidths.length > 0 - ? `${columnWidths[index]}px` - : 'auto' - } - > - - {name} - - - - - - ))} + ) + } + width={ + columnWidths.length > 0 + ? `${columnWidths[index]}px` + : 'auto' + } + > + + {name} + + + + + + ) + )} )} itemContent={(_, row) => { @@ -587,6 +611,7 @@ const Table = ({ availableWidth, onCountChange, showOnlySelected }) => { Table.propTypes = { availableWidth: PropTypes.number, + globalSearch: PropTypes.string, showOnlySelected: PropTypes.bool, onCountChange: PropTypes.func, } diff --git a/src/components/datatable/FilterInput.jsx b/src/components/datatable/FilterInput.jsx index 95045cc2d..d600da690 100644 --- a/src/components/datatable/FilterInput.jsx +++ b/src/components/datatable/FilterInput.jsx @@ -186,8 +186,8 @@ FilterInput.propTypes = { dataKey: PropTypes.string.isRequired, name: PropTypes.string.isRequired, type: PropTypes.string.isRequired, - options: PropTypes.arrayOf(PropTypes.shape({ value: PropTypes.string })), optionSetId: PropTypes.string, + options: PropTypes.arrayOf(PropTypes.shape({ value: PropTypes.string })), } export default FilterInput From 7edbeb0d095ca024b9420510bc7ac8c7f0bb2524 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Mon, 13 Jul 2026 23:34:31 +0200 Subject: [PATCH 08/27] feat: add global search box to data table toolbar Adds a dense Input between the "Clear filters" button and the show-only toggles, sized to shrink before the layer name has to truncate further. "Clear filters" now also resets the search box, and hasActiveFilters accounts for both column filters and the search string. --- src/components/datatable/BottomPanel.jsx | 20 ++++++++++++++++--- .../datatable/styles/BottomPanel.module.css | 6 ++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/components/datatable/BottomPanel.jsx b/src/components/datatable/BottomPanel.jsx index 8353d0e2d..9fa3084f0 100644 --- a/src/components/datatable/BottomPanel.jsx +++ b/src/components/datatable/BottomPanel.jsx @@ -4,6 +4,7 @@ import { IconFilter16, IconEmptyFrame16, IconCheckmarkCircle16, + Input, Tooltip, } from '@dhis2/ui' import cx from 'classnames' @@ -49,7 +50,6 @@ const BottomPanel = () => { state.map.mapViews.find((l) => l.id === activeLayerId) ) const dataFilters = activeLayer?.dataFilters ?? {} - const hasActiveFilters = Object.keys(dataFilters).length > 0 const showOnlyFeaturesInView = useSelector( (state) => state.ui.showOnlyFeaturesInView ) @@ -69,6 +69,10 @@ const BottomPanel = () => { const [filteredCount, setFilteredCount] = useState(null) const [nameTooltipPos, setNameTooltipPos] = useState(null) const [isCollapsed, setIsCollapsed] = useState(false) + const [globalSearch, setGlobalSearch] = useState('') + + const hasActiveFilters = + Object.keys(dataFilters).length > 0 || globalSearch.trim() !== '' const maxHeight = height - getCssVar('--header-height') - getCssVar('--toolbar-height') @@ -246,9 +250,10 @@ const BottomPanel = () => { )} + setGlobalSearch(value)} + className={styles.globalSearch} + /> @@ -270,6 +268,7 @@ const BottomPanel = () => { value={globalSearch} onChange={({ value }) => setGlobalSearch(value)} className={styles.globalSearch} + onDoubleClick={(e) => e.stopPropagation()} /> - @@ -310,8 +301,10 @@ const BottomPanel = () => { className={styles.closeIcon} onClick={() => dispatch(closeDataTable())} > - - + + + + @@ -321,8 +314,8 @@ const BottomPanel = () => { diff --git a/src/components/datatable/DataTable.jsx b/src/components/datatable/DataTable.jsx index 3635cb274..356b2d7b2 100644 --- a/src/components/datatable/DataTable.jsx +++ b/src/components/datatable/DataTable.jsx @@ -9,7 +9,10 @@ import { ComponentCover, CenteredContent, CircularLoader, - Tooltip, + Popover, + Popper, + Portal, + IconSync16, } from '@dhis2/ui' import cx from 'classnames' import PropTypes from 'prop-types' @@ -23,6 +26,7 @@ import React, { } from 'react' import { useSelector, useDispatch } from 'react-redux' import { TableVirtuoso } from 'react-virtuoso' +import { setSelectionFilter } from '../../actions/dataTable.js' import { highlightFeature } from '../../actions/feature.js' import { toggleFeatureSelection, @@ -30,14 +34,164 @@ import { selectFeatureRange, clearSelection, } from '../../actions/selection.js' +import { + SELECTION_FILTER_SELECTED, + SELECTION_FILTER_NOT_SELECTED, +} from '../../constants/selection.js' import { isDarkColor } from '../../util/colors.js' import { formatWithSeparator } from '../../util/numbers.js' import { useCachedData } from '../cachedDataProvider/CachedDataProvider.jsx' +import Checkbox from '../core/Checkbox.jsx' import { SortIcon } from '../core/icons.jsx' import FilterInput from './FilterInput.jsx' import styles from './styles/DataTable.module.css' import TableContextMenu from './TableContextMenu.jsx' -import { useTableData } from './useTableData.js' +import { useTableData, SELECTED_SORT_KEY } from './useTableData.js' + +const SELECTION_FILTER_OPTIONS = [ + { value: SELECTION_FILTER_SELECTED, label: i18n.t('Selected') }, + { value: SELECTION_FILTER_NOT_SELECTED, label: i18n.t('Not selected') }, +] + +// Every filterable column dispatches its dataFilters value straight through +// to filterData against each layer's real feature properties (see +// ThematicLayer.jsx/EventLayer.jsx/etc.). Index is a synthetic row number +// computed only for table display (see useTableData.js), never present on +// the underlying feature data - filtering by it narrows the table but +// can't affect the map. Still shown: it's a useful table-only tool (e.g. +// narrowing to a row-number range) even though it doesn't reach the map. +export const isFilterable = (dataKey, type) => !!type + +// Inverts selection scoped to the currently-filtered/visible rows only, +// mirroring how the "select all" checkbox already treats them (see +// onToggleSelectAll) - ids selected before a filter narrowed the rows stay +// selected (offViewSelected), only the visible portion actually flips. +export const getReversedSelection = (selectedIds, allRowIds) => { + const selectedIdSet = new Set(selectedIds) + const allRowIdSet = new Set(allRowIds) + const offViewSelected = selectedIds.filter((id) => !allRowIdSet.has(id)) + const invertedVisible = allRowIds.filter((id) => !selectedIdSet.has(id)) + return [...offViewSelected, ...invertedVisible] +} + +const SelectionFilterButton = ({ value, onChange }) => { + const anchorRef = useRef(null) + const [isOpen, setIsOpen] = useState(false) + + const toggleValue = (optionValue) => { + const next = value.includes(optionValue) + ? value.filter((v) => v !== optionValue) + : [...value, optionValue] + onChange(next) + } + + const buttonLabel = + value.length === 0 + ? i18n.t('All') + : i18n.t('{{count}} selected', { count: value.length }) + + return ( + <> + + {isOpen && ( + setIsOpen(false)} + > +
+ {SELECTION_FILTER_OPTIONS.map((option) => ( + toggleValue(option.value)} + style={{ margin: '4px 0' }} + /> + ))} +
+ + )} + + ) +} + +SelectionFilterButton.propTypes = { + value: PropTypes.arrayOf(PropTypes.string).isRequired, + onChange: PropTypes.func.isRequired, +} + +const topTooltipModifiers = [{ name: 'offset', options: { offset: [0, 4] } }] + +// @dhis2/ui's Tooltip always includes a flip modifier that checks the +// nearest scrolling ancestor's clip box for room. The table header is +// position:sticky, pinned to the top of that scrolling container, so the +// flip modifier always reports "no room above" and flips the tooltip below +// the icon - even though there's plenty of room on screen. This variant +// skips the flip modifier so sort-icon tooltips stay pinned above the icon. +const TopTooltip = ({ content, children }) => { + const [open, setOpen] = useState(false) + const referenceRef = useRef(null) + const openTimerRef = useRef(null) + const closeTimerRef = useRef(null) + + const onOpen = () => { + clearTimeout(closeTimerRef.current) + openTimerRef.current = setTimeout(() => setOpen(true), 200) + } + + const onClose = () => { + clearTimeout(openTimerRef.current) + closeTimerRef.current = setTimeout(() => setOpen(false), 200) + } + + useEffect( + () => () => { + clearTimeout(openTimerRef.current) + clearTimeout(closeTimerRef.current) + }, + [] + ) + + return ( + + {children} + {open && ( + + +
+ {content} +
+ + + )} + + ) +} + +TopTooltip.propTypes = { + children: PropTypes.node.isRequired, + content: PropTypes.node.isRequired, +} const ASCENDING = 'asc' const DESCENDING = 'desc' @@ -45,6 +199,20 @@ const DESCENDING = 'desc' export const shouldClearFeatureHighlight = (event) => event.relatedTarget?.tagName !== 'TD' +// Cycles a column through ascending -> descending -> none (natural order) -> +// ascending... Once sortField is null, every column looks "unsorted" again, +// so clicking any of them (including the one that was just cleared) +// naturally restarts the cycle at ascending. +export const getNextSorting = (name, { sortField, sortDirection }) => { + if (name !== sortField) { + return { sortField: name, sortDirection: ASCENDING } + } + if (sortDirection === ASCENDING) { + return { sortField: name, sortDirection: DESCENDING } + } + return { sortField: null, sortDirection: ASCENDING } +} + const getRowId = (row) => row.find((r) => r.dataKey === 'id')?.value || row[0]?.itemId @@ -115,27 +283,52 @@ DataTableRowWithVirtuosoContext.propTypes = { ), } +const EmptyPlaceholder = ({ context }) => ( + + +
+ {context.totalCount > 0 ? ( + <> + {i18n.t('No features match your filters')} + {context.hasActiveFilters && ( + + )} + + ) : ( + i18n.t('No results found') + )} +
+ + +) + +EmptyPlaceholder.propTypes = { + context: PropTypes.shape({ + hasActiveFilters: PropTypes.bool, + totalCount: PropTypes.number, + onClearFilters: PropTypes.func, + }), +} + const TableComponents = { Table: DataTableWithVirtuosoContext, TableBody: DataTableBody, TableHead: DataTableHead, TableRow: DataTableRowWithVirtuosoContext, - EmptyPlaceholder: () => ( - - -
- {i18n.t('No results found')} -
- - - ), + EmptyPlaceholder, } const Table = ({ availableWidth, onCountChange, - showOnlySelected, globalSearch, + onClearFilters, }) => { const { systemSettings: { keyAnalysisDigitGroupSeparator }, @@ -155,6 +348,7 @@ const Table = ({ (state) => state.ui.showOnlyFeaturesInView ) const mapBounds = useSelector((state) => state.ui.mapBounds) + const selectionFilter = useSelector((state) => state.ui.selectionFilter) const [{ sortField, sortDirection }, setSorting] = useReducer( (sorting, newSorting) => ({ ...sorting, ...newSorting }), { @@ -167,13 +361,7 @@ const Table = ({ const sortData = useCallback( ({ name }) => { - setSorting({ - sortField: name, - sortDirection: - name === sortField && sortDirection === ASCENDING - ? DESCENDING - : ASCENDING, - }) + setSorting(getNextSorting(name, { sortField, sortDirection })) }, [sortField, sortDirection] ) @@ -244,6 +432,7 @@ const Table = ({ headers, rows, isLoading, + loadingReason, error, totalCount, filteredCount, @@ -254,7 +443,7 @@ const Table = ({ sortDirection, showOnlyFeaturesInView, mapBounds, - showOnlySelected, + selectionFilter, selectedIdSet, globalSearch, }) @@ -315,6 +504,11 @@ const Table = ({ [dispatch, layer.id] ) + const hasActiveFilters = + Object.keys(layer.dataFilters ?? {}).length > 0 || + !!globalSearch?.trim() || + selectionFilter?.length > 0 + const tableContext = useMemo( () => ({ onMouseEnter: setFeatureHighlight, @@ -323,6 +517,9 @@ const Table = ({ onRowClick, onRowDoubleClick, layout: columnWidths.length > 0 ? 'fixed' : 'auto', + totalCount, + hasActiveFilters, + onClearFilters, }), [ setFeatureHighlight, @@ -331,6 +528,9 @@ const Table = ({ onRowClick, onRowDoubleClick, columnWidths, + totalCount, + hasActiveFilters, + onClearFilters, ] ) @@ -384,6 +584,16 @@ const Table = ({ } }, [dispatch, isAllSelected, allRowIds, allRowIdSet, selectedIds, layer.id]) + const onReverseSelection = useCallback(() => { + const nextIds = getReversedSelection(selectedIds, allRowIds) + + if (nextIds.length) { + dispatch(selectAllFeatures(nextIds, layer.id)) + } else { + dispatch(clearSelection()) + } + }, [dispatch, selectedIds, allRowIds, layer.id]) + useEffect(() => { // Measure column widths in auto layout, then switch to fixed to prevent content shift during virtual scrolling if (columnWidths.length === 0 && headerRowRef.current) { @@ -445,6 +655,15 @@ const Table = ({ return

{error}

} + // Thematic layers carry both a `legend` (name) and `color` (hex) column; + // merge them into one swatch+name cell instead of two separate columns. + const hasLegendColorPair = + headers.some((h) => h.dataKey === 'legend') && + headers.some((h) => h.dataKey === 'color') + const visibleHeaders = hasLegendColorPair + ? headers.filter((h) => h.dataKey !== 'color') + : headers + return ( <> getRowId(row) ?? index} + increaseViewportBy={{ top: 400, bottom: 400 }} fixedHeaderContent={() => ( + dispatch(setSelectionFilter(next)) + } + /> + } > - +
+ + + + + + + +
- {headers.map( + {visibleHeaders.map( ({ name, dataKey, type, optionSet }, index) => ( {name} - - + ) @@ -562,30 +830,73 @@ const Table = ({ onClick={(e) => e.stopPropagation()} /> - {row.map(({ dataKey, value, align }) => ( - - {dataKey === 'color' - ? value?.toLowerCase() - : formatWithSeparator( - value, - keyAnalysisDigitGroupSeparator - )} - - ))} + {row + .filter( + ({ dataKey }) => + !hasLegendColorPair || + dataKey !== 'color' + ) + .map(({ dataKey, value, align }) => { + const isLegendCell = + hasLegendColorPair && + dataKey === 'legend' + const swatchColor = isLegendCell + ? row.find((c) => c.dataKey === 'color') + ?.value + : null + + return ( + + {isLegendCell ? ( + + {swatchColor && ( + + )} + {value} + + ) : dataKey === 'color' ? ( + value?.toLowerCase() + ) : ( + formatWithSeparator( + value, + keyAnalysisDigitGroupSeparator + ) + )} + + ) + })} ) }} @@ -593,7 +904,14 @@ const Table = ({ {(isLoading || layer?.isLoaded === false || layer?.isLoading) && ( - +
+ + {loadingReason && ( + + {loadingReason} + + )} +
)} @@ -601,6 +919,7 @@ const Table = ({ contextMenu={tableContextMenu} layer={layer} selectedIds={selectedIds} + filteredIds={hasActiveFilters ? allRowIds : null} onClose={() => setTableContextMenu(null)} /> @@ -610,7 +929,7 @@ const Table = ({ Table.propTypes = { availableWidth: PropTypes.number, globalSearch: PropTypes.string, - showOnlySelected: PropTypes.bool, + onClearFilters: PropTypes.func, onCountChange: PropTypes.func, } diff --git a/src/components/datatable/FilterInput.jsx b/src/components/datatable/FilterInput.jsx index f20d30342..b20408c8f 100644 --- a/src/components/datatable/FilterInput.jsx +++ b/src/components/datatable/FilterInput.jsx @@ -1,15 +1,54 @@ import i18n from '@dhis2/d2-i18n' -import { Input, Popover, Tooltip, IconInfo16 } from '@dhis2/ui' +import { + Input, + Layer, + Popper, + Portal, + IconFilter16, + IconSync16, +} from '@dhis2/ui' +import cx from 'classnames' import PropTypes from 'prop-types' -import React, { useRef, useState } from 'react' +import React, { useEffect, useRef, useState } from 'react' import { useDispatch, useSelector } from 'react-redux' +import { Virtuoso } from 'react-virtuoso' import { setDataFilter, clearDataFilter } from '../../actions/dataFilters.js' import useOptionSet from '../../hooks/useOptionSet.js' +import { numericFilter, ANY_VALUE_KEY } from '../../util/filter.js' import Checkbox from '../core/Checkbox.jsx' import styles from './styles/FilterInput.module.css' +// Must match useTableData.js's NOT_SET_VALUE - not imported directly from +// there to avoid pulling that module's heavy transitive dependencies +// (map/earth-engine loaders) into this component. +const NOT_SET_VALUE = '' + +// Checkbox rows are a fixed height (dense label + the 4px/4px margin set +// below), so the list can be virtualized with a known row size instead of +// measuring each one - this is what lets a column's full value list (no +// cap - see useTableData.js) stay cheap to render regardless of size. +const OPTION_ROW_HEIGHT = 28 +const MAX_LIST_HEIGHT = 260 + +// Rough upper bound (in px) on the dropdown's own rendered height (the +// checkbox list capped at MAX_LIST_HEIGHT, plus its padding/gap and the +// pinned custom-filter row) - used to decide, once per render, whether +// every column's dropdown should open below or above (see +// SearchableFilterPopover's dropdownSide). All columns share the same +// header row, so this is computed the same way for each of them and they +// always agree - there's no per-column flip, just a single below/above +// choice that applies to the whole row. +const ESTIMATED_POPOVER_HEIGHT = MAX_LIST_HEIGHT + 80 + +// Rough content heights (in px) for the two tooltip variants, used only to +// decide whether there's enough room to show the tooltip at all - see +// FilterHelpTooltip's hasRoom check. +const NUMERIC_HELP_HEIGHT = 140 +const TEXT_HELP_HEIGHT = 40 + const NUMERIC_FILTER_HELP = (
+
{i18n.t('Select values, or type a numerical filter:')}
{'> 5 — ' + i18n.t('greater than 5')}
{'>= 5 — ' + i18n.t('greater than or equal to 5')}
{'< 5, <= 5 — ' + i18n.t('less than (or equal to) 5')}
@@ -18,73 +57,683 @@ const NUMERIC_FILTER_HELP = (
) +const TEXT_FILTER_HELP = ( +
+ {i18n.t('Select values, or type text to match rows that contain it.')} +
+) + +// Everything a numeric filter expression can legally contain (see +// isTrueFilter/numericFilter in util/filter.js): digits, decimals, +// negative signs, comparison operators, and the AND/OR separators. +const NUMERIC_INPUT_DISALLOWED = /[^0-9.\-<>=,&\s]/g + +// @dhis2-ui/popper's Popper component always merges in its own base flip +// modifier unless a modifier of the same name is passed in to override it - +// omitting flip from this list does NOT turn it off, it just leaves the +// base one active. Disabling it by name (rather than simply not mentioning +// it) is what actually stops it from silently overriding `placement`. +const helpTooltipModifiers = [ + { name: 'offset', options: { offset: [0, 4] } }, + { name: 'flip', enabled: false }, +] + +// @dhis2/ui's Tooltip always includes a flip modifier that checks the +// nearest scrolling ancestor's clip box for room (see the identical +// TopTooltip in DataTable.jsx) - the column header is position:sticky, +// pinned to the top of that scrolling container, so the flip modifier +// always reports "no room above" and flips to the bottom regardless of +// the requested placement. This variant skips that modifier so it always +// opens on whichever side the caller passes (SearchableFilterPopover always +// passes the opposite of the dropdown's own side) - and, since it can no +// longer flip out of the way, it checks for itself whether there's +// actually room on that side before showing anything at all, rather than +// risk covering other UI or getting clipped. +const FilterHelpTooltip = ({ + content, + placement, + estimatedHeight, + dataTest, + children, +}) => { + const [open, setOpen] = useState(false) + const referenceRef = useRef(null) + const openTimerRef = useRef(null) + const closeTimerRef = useRef(null) + + const onOpen = () => { + clearTimeout(closeTimerRef.current) + openTimerRef.current = setTimeout(() => setOpen(true), 200) + } + + const onClose = () => { + clearTimeout(openTimerRef.current) + closeTimerRef.current = setTimeout(() => setOpen(false), 200) + } + + useEffect( + () => () => { + clearTimeout(openTimerRef.current) + clearTimeout(closeTimerRef.current) + }, + [] + ) + + const referenceRect = referenceRef.current?.getBoundingClientRect() + const spaceAvailable = referenceRect + ? placement === 'top' + ? referenceRect.top + : window.innerHeight - referenceRect.bottom + : Infinity + const hasRoom = spaceAvailable >= estimatedHeight + + return ( + + {children} + {open && hasRoom && ( + + +
+ {content} +
+ + + )} + + ) +} + +FilterHelpTooltip.propTypes = { + children: PropTypes.node.isRequired, + content: PropTypes.node.isRequired, + dataTest: PropTypes.string.isRequired, + estimatedHeight: PropTypes.number.isRequired, + placement: PropTypes.oneOf(['top', 'bottom']).isRequired, +} + +// See helpTooltipModifiers above - the flip modifier has to be disabled by +// name, not just left out, or the Popper component's own base flip modifier +// stays active underneath and keeps overriding `placement` per-column. +const dropdownModifiers = [ + { name: 'offset', options: { offset: [0, 0] } }, + { name: 'flip', enabled: false }, +] + +// @dhis2/ui's Popover always includes its own flip modifier with no way to +// opt out, which lets each column decide independently based on its own +// available space - a column near the bottom of the table could open +// upward while every other column opens downward. This reimplements just +// enough of Popover - Layer for the backdrop/click-outside behavior, Popper +// for positioning, flip disabled - so `placement` is always honored exactly; +// the caller (SearchableFilterPopover) computes one placement per render +// from the shared header row's position, so every column's dropdown agrees. +const FilterDropdownPopover = ({ + reference, + placement, + onClickOutside, + className, + children, +}) => ( + + + {children} + + +) + +FilterDropdownPopover.propTypes = { + children: PropTypes.node.isRequired, + placement: PropTypes.oneOf(['top-start', 'bottom-start']).isRequired, + reference: PropTypes.object.isRequired, + onClickOutside: PropTypes.func.isRequired, + className: PropTypes.string, +} + // Shared popover UI — label resolution is injected so it never needs to // know whether it's an option-set column or a plain categorical one. -const MultiSelectPopover = ({ +// State is derived straight from the applied `filterValue` (never tracked +// in parallel), so picking a value and applying a custom filter stay +// mutually exclusive for free: whichever one is dispatched last is what +// `filterValue` holds, and both branches read from it the same way. +const SearchableFilterPopover = ({ dataKey, + name, layerId, filterValue, options, resolveLabel, + type, + allowCustomFilter = true, }) => { const dispatch = useDispatch() const anchorRef = useRef(null) + const listRef = useRef(null) const [isOpen, setIsOpen] = useState(false) + const [searchText, setSearchText] = useState('') + const [highlightedIndex, setHighlightedIndex] = useState(-1) + const selected = Array.isArray(filterValue) ? filterValue : [] + const appliedString = typeof filterValue === 'string' ? filterValue : '' + + const openPopover = () => { + setSearchText(appliedString) + setHighlightedIndex(-1) + setIsOpen(true) + } + + const closePopover = () => setIsOpen(false) + + // Read directly from the DOM rather than a resize observer: the trigger + // is already mounted (this only matters once isOpen is true, by which + // point it's had at least one paint) and column widths only change on + // table resize, when the popover is closed anyway. + const anchorRect = anchorRef.current?.getBoundingClientRect() + const anchorWidth = anchorRect?.width + + // Every column's filter trigger sits in the same header row, so this + // resolves to the same answer for all of them - below by default, or + // above if the row doesn't have room to open the dropdown downward + // (e.g. the table is short, or the page is scrolled so the row sits + // near the bottom of the viewport). That single choice is what keeps + // every column's dropdown opening on the same side. The help tooltip + // always takes the opposite side, so the two never compete for space. + const dropdownSide = + anchorRect != null && + window.innerHeight - anchorRect.bottom < ESTIMATED_POPOVER_HEIGHT + ? 'top' + : 'bottom' + const dropdownPlacement = `${dropdownSide}-start` + const tooltipPlacement = dropdownSide === 'top' ? 'bottom' : 'top' + + const applyValues = (next) => + next.length + ? dispatch(setDataFilter(layerId, dataKey, next)) + : dispatch(clearDataFilter(layerId, dataKey)) const toggleValue = (value) => { const next = selected.includes(value) ? selected.filter((v) => v !== value) : [...selected, value] - next.length - ? dispatch(setDataFilter(layerId, dataKey, next)) + applyValues(next) + } + + const applyCustomFilter = (text) => + text + ? dispatch(setDataFilter(layerId, dataKey, text)) : dispatch(clearDataFilter(layerId, dataKey)) + + // "No value" is pinned above the list with "Any value" (see the render + // below) rather than mixed in among the column's real distinct values, + // so it's excluded here and never part of the virtualized/searchable + // list. + const hasNotSetOption = options.some(({ value }) => value === NOT_SET_VALUE) + const realOptions = options.filter(({ value }) => value !== NOT_SET_VALUE) + const anyValueActive = selected.includes(ANY_VALUE_KEY) + + // Toggling "Any value" always rebuilds the selection from scratch + // rather than adding/removing just the one key - turning it on + // collapses any individually-picked real values into the wildcard + // (they're now redundant), and turning it off unticks every real + // value along with it (there's nothing meaningful to "fall back" to). + // "No value" is independent either way and carries over untouched. + const onToggleAnyValue = () => { + const keepNotSet = selected.includes(NOT_SET_VALUE) + applyValues( + anyValueActive + ? keepNotSet + ? [NOT_SET_VALUE] + : [] + : keepNotSet + ? [ANY_VALUE_KEY, NOT_SET_VALUE] + : [ANY_VALUE_KEY] + ) } - const buttonLabel = - selected.length === 0 - ? i18n.t('All') - : i18n.t('{{count}} selected', { count: selected.length }) + // Every value "Reverse selection" can flip - the column's full value + // domain, not just whatever the current search happens to narrow the + // list down to (search is for finding/toggling individual values, not + // for scoping a bulk action). + const invertibleValues = hasNotSetOption + ? [NOT_SET_VALUE, ...realOptions.map((o) => o.value)] + : realOptions.map((o) => o.value) + + // A real value's checkbox is ticked either because it's individually + // selected, or because "Any value" is active (which stands for "every + // real value" - "No value" is the one exception, handled on its own + // below). Clicking one while "Any value" is active means "everything + // except this one" - not "add this one on top of Any value" - so it + // has to expand Any value into its concrete equivalent (every real + // value but this one) rather than going through the plain toggle, + // which would otherwise just add the clicked value to an array that + // still has ANY_VALUE_KEY in it and leave every other checkbox ticked + // for the wrong reason. + const onToggleRealValue = (value) => { + if (anyValueActive) { + const next = realOptions + .map((o) => o.value) + .filter((v) => v !== value) + applyValues( + selected.includes(NOT_SET_VALUE) + ? [...next, NOT_SET_VALUE] + : next + ) + return + } + + const next = selected.includes(value) + ? selected.filter((v) => v !== value) + : [...selected, value] + + // Checking every real value one by one ends up in the same place + // as checking "Any value" directly - collapse to that instead of + // leaving a literal array that happens to list them all, so the + // two are always the same underlying state. + const allRealValuesChecked = + realOptions.length > 0 && + realOptions.every((o) => next.includes(o.value)) + applyValues( + allRealValuesChecked + ? next.includes(NOT_SET_VALUE) + ? [ANY_VALUE_KEY, NOT_SET_VALUE] + : [ANY_VALUE_KEY] + : next + ) + } + + // Reverses every checkbox's *effective* ticked state, not just literal + // array membership - while "Any value" is active every real value + // reads as ticked (see onToggleRealValue above), so reversing has to + // untick all of them (and "Any value" along with them, since there's + // no way to represent "every real value unticked" while it's still + // set) rather than leaving them all ticked and only toggling values + // that were never literally in the array to begin with. "No value" is + // unaffected by "Any value" and simply flips on its own. + const onReverseSelection = () => { + const invertedRealValues = anyValueActive + ? [] + : realOptions + .map((o) => o.value) + .filter((v) => !selected.includes(v)) + const invertedNotSet = + hasNotSetOption && !selected.includes(NOT_SET_VALUE) + + // Same rule as onToggleRealValue: ending up with every real value + // ticked (e.g. reversing a selection of just "No value") is the + // same state as "Any value" being active, so it collapses into + // that rather than a literal array that happens to list them all. + const allRealValuesInverted = + !anyValueActive && + realOptions.length > 0 && + invertedRealValues.length === realOptions.length + + if (allRealValuesInverted) { + applyValues( + invertedNotSet + ? [ANY_VALUE_KEY, NOT_SET_VALUE] + : [ANY_VALUE_KEY] + ) + return + } + + applyValues( + invertedNotSet + ? [NOT_SET_VALUE, ...invertedRealValues] + : invertedRealValues + ) + } + + const trimmedSearch = searchText.trim() + const normalizedSearch = trimmedSearch.toLowerCase() + // Numeric columns narrow the list using the same comparison the typed + // text would apply to the table's rows (>, <, ranges, ...), so what's + // checked here always matches what "Use filter" would actually select - + // a plain substring match wouldn't understand "> 100" against "150". + const filteredOptions = !trimmedSearch + ? realOptions + : type === 'number' + ? realOptions.filter(({ value }) => + numericFilter(Number(value), trimmedSearch) + ) + : realOptions.filter(({ value }) => + resolveLabel(value).toLowerCase().includes(normalizedSearch) + ) + const hasExactMatch = filteredOptions.some( + ({ value }) => resolveLabel(value).toLowerCase() === normalizedSearch + ) + const showCustomFilterRow = + allowCustomFilter && normalizedSearch !== '' && !hasExactMatch + const totalCount = filteredOptions.length + (showCustomFilterRow ? 1 : 0) + + const customFilterTag = + type === 'number' ? i18n.t('Use filter') : i18n.t('Contains') + + const hasActiveFilter = selected.length > 0 || appliedString !== '' + + // Applies (or clears) live as the user types - typing a value that + // doesn't match an existing option filters the table immediately, + // exactly like picking a checkbox already does, rather than waiting + // for an explicit commit step. Clearing the text (including via the + // input's own built-in clear button) clears whatever filter is active, + // whether it's picked values or a typed one. + const onSearchChange = ({ value }) => { + // Numeric columns only ever match a numericFilter expression + // (digits, comparison operators, & / ,) - letters could never + // apply to a number column, so strip them as they're typed rather + // than accepting them and silently matching nothing. + const sanitized = + type === 'number' + ? value.replace(NUMERIC_INPUT_DISALLOWED, '') + : value + setSearchText(sanitized) + setHighlightedIndex(-1) + + const trimmed = sanitized.trim() + if (trimmed === '') { + if (hasActiveFilter) { + dispatch(clearDataFilter(layerId, dataKey)) + } + return + } + + if (!allowCustomFilter) { + return + } + + const normalized = trimmed.toLowerCase() + const exactMatch = options.some( + ({ value: optionValue }) => + resolveLabel(optionValue).toLowerCase() === normalized + ) + if (!exactMatch) { + applyCustomFilter(trimmed) + } + } + + // The checkbox list is virtualized, so scrolling the highlighted row + // into view has to be requested explicitly rather than relying on the + // browser's native scrollIntoView over an already-rendered DOM node. + const scrollHighlightedIntoView = (index) => { + const optionIndex = showCustomFilterRow ? index - 1 : index + if (optionIndex >= 0 && optionIndex < filteredOptions.length) { + listRef.current?.scrollToIndex({ + index: optionIndex, + align: 'center', + }) + } + } + + const onSearchKeyDown = (_, event) => { + switch (event.key) { + case 'ArrowDown': + event.preventDefault() + setHighlightedIndex((i) => { + const next = totalCount ? (i + 1) % totalCount : -1 + scrollHighlightedIntoView(next) + return next + }) + break + case 'ArrowUp': + event.preventDefault() + setHighlightedIndex((i) => { + const next = totalCount + ? (i - 1 + totalCount) % totalCount + : -1 + scrollHighlightedIntoView(next) + return next + }) + break + case 'Enter': { + event.preventDefault() + // The custom-filter row (when shown) sits first, matching + // its visual position above the checkbox list. It's + // usually already applied live by this point (see + // onSearchChange) - toggling it again here is a no-op. + if (highlightedIndex === -1) { + if (showCustomFilterRow) { + applyCustomFilter(searchText.trim()) + } + } else if (showCustomFilterRow && highlightedIndex === 0) { + applyCustomFilter(searchText.trim()) + } else { + const optionIndex = showCustomFilterRow + ? highlightedIndex - 1 + : highlightedIndex + if ( + optionIndex >= 0 && + optionIndex < filteredOptions.length + ) { + toggleValue(filteredOptions[optionIndex].value) + } + } + closePopover() + break + } + case 'Escape': + event.preventDefault() + closePopover() + break + default: + break + } + } + + // Closed, this reads like the old trigger button ("3 selected", the + // applied filter text, or empty so the "Search" placeholder shows). + // Open, it's a live, editable search/filter field - the same input + // serves both roles instead of a button revealing a separate one. + const displayValue = isOpen + ? searchText + : selected.length + ? i18n.t('{{count}} selected', { count: selected.length }) + : appliedString + + const mainInput = ( + 5, < 8…') + : i18n.t('Search') + } + value={displayValue} + onFocus={() => { + if (!isOpen) { + openPopover() + } + }} + onChange={onSearchChange} + onKeyDown={onSearchKeyDown} + /> + ) return ( - <> - + {mainInput} + {isOpen && ( - setIsOpen(false)} + placement={dropdownPlacement} + onClickOutside={closePopover} + className={cx( + styles.dropdownPopper, + dropdownSide === 'top' && styles.dropdownPopperAbove + )} > -
- {options.map(({ value }) => ( +
+ {showCustomFilterRow && ( + + )} +
+ toggleValue(value)} + label={i18n.t('Any value')} + checked={anyValueActive} + onChange={onToggleAnyValue} + className={styles.specialOption} + style={{ margin: '4px 0' }} + dataTest={`data-table-column-filter-any-${name}`} /> - ))} + {hasNotSetOption && ( + toggleValue(NOT_SET_VALUE)} + className={styles.specialOption} + style={{ margin: '4px 0' }} + dataTest={`data-table-column-filter-novalue-${name}`} + /> + )} +
+
+ {!showCustomFilterRow && + (options.length === 0 ? ( +
+ {i18n.t( + 'Too many values to list - type to filter this column' + )} +
+ ) : ( + filteredOptions.length === 0 && ( +
+ {i18n.t('No matches')} +
+ ) + ))} + {filteredOptions.length > 0 && ( + option.value} + itemContent={(index, option) => ( + + onToggleRealValue(option.value) + } + className={cx( + dataKey === 'id' && + styles.monoOption, + highlightedIndex === + (showCustomFilterRow + ? index + 1 + : index) && + styles.highlighted + )} + style={{ margin: '4px 0' }} + /> + )} + /> + )} +
- + )} - +
) } -MultiSelectPopover.propTypes = { +SearchableFilterPopover.propTypes = { dataKey: PropTypes.string.isRequired, + name: PropTypes.string.isRequired, options: PropTypes.arrayOf(PropTypes.shape({ value: PropTypes.string })) .isRequired, resolveLabel: PropTypes.func.isRequired, + type: PropTypes.string.isRequired, + allowCustomFilter: PropTypes.bool, filterValue: PropTypes.oneOfType([ PropTypes.string, PropTypes.arrayOf(PropTypes.string), @@ -92,27 +741,51 @@ MultiSelectPopover.propTypes = { layerId: PropTypes.string, } -// Plain categorical columns (legend, type): raw value IS the display label. -const MultiSelectFilter = (props) => ( - value} /> +// Plain categorical columns (legend, type, and every other column discovered +// generically by useTableData): raw value IS the display label, except for +// the NOT_SET_VALUE sentinel representing blank/missing cells. +const PlainSearchableFilter = (props) => ( + + value === NOT_SET_VALUE ? i18n.t('No value') : value + } + /> ) // Option-set-backed event columns: translate stored code -> display name. -// useOptionSet/useDataQuery is only ever mounted here, never for legend/type, -// since those columns never have an optionSetId. -const OptionSetMultiSelectFilter = ({ optionSetId, ...props }) => { +// useOptionSet/useDataQuery is only ever mounted here, never for other +// columns, since those never have an optionSetId. The custom-filter row is +// disabled here: filterData matches the raw stored code, not the resolved +// name the user sees, so free text typed against the visible label couldn't +// be applied correctly - and since option sets are a closed, fully +// enumerable set already covered by the checkbox list, there's no real gap +// left for free text to fill. +const OptionSetSearchableFilter = ({ optionSetId, ...props }) => { const { optionSet } = useOptionSet(optionSetId) const resolveLabel = (value) => - optionSet?.options.find((o) => o.code === value)?.name ?? value - return + value === NOT_SET_VALUE + ? i18n.t('No value') + : optionSet?.options.find((o) => o.code === value)?.name ?? value + return ( + + ) } -OptionSetMultiSelectFilter.propTypes = { +OptionSetSearchableFilter.propTypes = { optionSetId: PropTypes.string.isRequired, } +// Every column (aside from the checkbox/selection column, which has its own +// SelectionFilterButton in DataTable.jsx) gets the same searchable popover, +// even ones with no known distinct values (over the cap, or not yet loaded) +// - they just render with an empty options list, which still lets the +// custom-filter row work exactly as it always has. const FilterInput = ({ type, dataKey, name, options, optionSetId }) => { - const dispatch = useDispatch() const dataTable = useSelector((state) => state.dataTable) const map = useSelector((state) => state.map) @@ -128,56 +801,25 @@ const FilterInput = ({ type, dataKey, name, options, optionSetId }) => { const filterValue = filters?.[dataKey] - if (options?.length) { - return optionSetId ? ( - - ) : ( - - ) - } - - const stringFilterValue = typeof filterValue === 'string' ? filterValue : '' - - const onChange = ({ value }) => - value !== '' - ? dispatch(setDataFilter(layerId, dataKey, value)) - : dispatch(clearDataFilter(layerId, dataKey)) - - return ( - - 5, < 8' : i18n.t('Search')} - value={stringFilterValue} - onChange={onChange} - /> - {type === 'number' && ( - - - - - - )} - + return optionSetId ? ( + + ) : ( + ) } diff --git a/src/components/datatable/TableContextMenu.jsx b/src/components/datatable/TableContextMenu.jsx index 21f582693..add98bcb6 100644 --- a/src/components/datatable/TableContextMenu.jsx +++ b/src/components/datatable/TableContextMenu.jsx @@ -24,7 +24,13 @@ import { drillUpDown } from '../../util/map.js' import { useCachedData } from '../cachedDataProvider/CachedDataProvider.jsx' import { IconZoomIn16 } from '../core/icons.jsx' -const TableContextMenu = ({ contextMenu, layer, selectedIds, onClose }) => { +const TableContextMenu = ({ + contextMenu, + layer, + selectedIds, + filteredIds, + onClose, +}) => { const anchorRef = useRef() const dispatch = useDispatch() const { @@ -191,6 +197,23 @@ const TableContextMenu = ({ contextMenu, layer, selectedIds, onClose }) => { onClose() }} /> + } + disabled={!filteredIds?.length} + onClick={() => { + dispatch( + highlightFeature({ + ids: filteredIds, + layerId: layer.id, + origin: 'table', + zoom: true, + }) + ) + onClose() + }} + /> @@ -205,6 +228,7 @@ TableContextMenu.propTypes = { x: PropTypes.number, y: PropTypes.number, }), + filteredIds: PropTypes.array, selectedIds: PropTypes.array, } diff --git a/src/components/datatable/__tests__/DataTable.spec.jsx b/src/components/datatable/__tests__/DataTable.spec.jsx index e236e5183..347a57d60 100644 --- a/src/components/datatable/__tests__/DataTable.spec.jsx +++ b/src/components/datatable/__tests__/DataTable.spec.jsx @@ -1,6 +1,9 @@ import { shouldClearFeatureHighlight, getRowClickAction, + getNextSorting, + isFilterable, + getReversedSelection, } from '../DataTable.jsx' // DataTable.jsx transitively imports MapApi.js (maplibre-gl), which is not @@ -82,3 +85,74 @@ describe('getRowClickAction', () => { ).toEqual({ type: 'range', ids: ['a', 'b', 'c'] }) }) }) + +describe('getNextSorting', () => { + test('clicking an unsorted column starts at ascending', () => { + expect( + getNextSorting('name', { sortField: null, sortDirection: 'asc' }) + ).toEqual({ sortField: 'name', sortDirection: 'asc' }) + }) + + test('clicking the ascending-sorted column moves to descending', () => { + expect( + getNextSorting('name', { sortField: 'name', sortDirection: 'asc' }) + ).toEqual({ sortField: 'name', sortDirection: 'desc' }) + }) + + test('clicking the descending-sorted column clears back to natural order', () => { + expect( + getNextSorting('name', { sortField: 'name', sortDirection: 'desc' }) + ).toEqual({ sortField: null, sortDirection: 'asc' }) + }) + + test('clicking a different column restarts the cycle at ascending', () => { + expect( + getNextSorting('type', { sortField: 'name', sortDirection: 'desc' }) + ).toEqual({ sortField: 'type', sortDirection: 'asc' }) + }) +}) + +describe('isFilterable', () => { + test('allows the Index column - it filters the table by row-number range even though it cannot narrow the map', () => { + expect(isFilterable('index', 'number')).toBe(true) + }) + + test('allows other numeric and string columns, which are real feature properties', () => { + expect(isFilterable('rawValue', 'number')).toBe(true) + expect(isFilterable('name', 'string')).toBe(true) + }) + + test('excludes columns with no type (no known filter UI for them)', () => { + expect(isFilterable('someKey', undefined)).toBe(false) + }) +}) + +describe('getReversedSelection', () => { + test('selects every visible row when nothing is currently selected', () => { + expect(getReversedSelection([], ['a', 'b', 'c'])).toEqual([ + 'a', + 'b', + 'c', + ]) + }) + + test('deselects every visible row when all are currently selected', () => { + expect(getReversedSelection(['a', 'b', 'c'], ['a', 'b', 'c'])).toEqual( + [] + ) + }) + + test('flips only the visible rows, keeping the rest of the selection as-is', () => { + expect(getReversedSelection(['a'], ['a', 'b', 'c'])).toEqual(['b', 'c']) + }) + + test('preserves ids selected outside the current filtered view untouched', () => { + // "z" was selected before a column filter narrowed the visible rows + // down to just a/b/c - reversing must not drop it. + expect(getReversedSelection(['a', 'z'], ['a', 'b', 'c'])).toEqual([ + 'z', + 'b', + 'c', + ]) + }) +}) diff --git a/src/components/datatable/__tests__/FilterInput.spec.jsx b/src/components/datatable/__tests__/FilterInput.spec.jsx index 5651f4565..d373541ca 100644 --- a/src/components/datatable/__tests__/FilterInput.spec.jsx +++ b/src/components/datatable/__tests__/FilterInput.spec.jsx @@ -1,8 +1,14 @@ 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 useOptionSet from '../../../hooks/useOptionSet.js' +import { ANY_VALUE_KEY } from '../../../util/filter.js' import FilterInput from '../FilterInput.jsx' jest.mock('../../../hooks/useOptionSet.js', () => ({ @@ -19,69 +25,180 @@ const renderFilterInput = (props, dataFilters) => { mapViews: [{ id: 'layer1', dataFilters: dataFilters || {} }], }, }) - return render( + // The checkbox list is virtualized (react-virtuoso) - jsdom doesn't do + // real layout, so without this fixed-size mock context Virtuoso thinks + // the viewport is 0px tall and renders no rows at all. + const result = render( - + + + ) + return { ...result, store } } -describe('FilterInput text/numeric path', () => { - test('renders a free-text input when no options are provided', () => { +// The trigger and the dropdown's search field are the same (see +// FilterInput.jsx) - test-ids are keyed by the column's display `name`, +// not its `dataKey`, since dataKey can be an opaque uid for event custom +// fields but name is always the human-readable label cypress/users see. +const getInput = (name) => + screen + .getByTestId(`data-table-column-filter-search-${name}`) + .querySelector('input') + +const openPopover = (name) => fireEvent.focus(getInput(name)) + +describe('FilterInput with no known options (over the cap, or not yet loaded)', () => { + test('shows an empty input with a "Search" placeholder by default', () => { + renderFilterInput({}) + const input = getInput('Name') + expect(input).toHaveValue('') + expect(input).toHaveAttribute('placeholder', 'Search') + }) + + test('explains there is no picklist instead of showing an empty popover', () => { renderFilterInput({}) + openPopover('Name') expect( - screen - .getByTestId('data-table-column-filter-input-Name') - .querySelector('input') + screen.getByText( + 'Too many values to list - type to filter this column' + ) ).toBeInTheDocument() }) - test('shows the current filter value', () => { - renderFilterInput({}, { name: 'hospital' }) + test('hides that hint once the user starts typing', () => { + renderFilterInput({}) + openPopover('Name') + fireEvent.change(getInput('Name'), { + target: { value: 'hospital' }, + }) expect( - screen - .getByTestId('data-table-column-filter-input-Name') - .querySelector('input') - ).toHaveValue('hospital') + screen.queryByText( + 'Too many values to list - type to filter this column' + ) + ).not.toBeInTheDocument() }) - test('shows a numeric filter syntax help icon for number columns', () => { + test('shows the current filter value on the (closed) trigger', () => { + renderFilterInput({}, { name: 'hospital' }) + expect(getInput('Name')).toHaveValue('hospital') + }) + + test('applies a custom filter live, as soon as it no longer matches an option', () => { + const { store } = renderFilterInput({}) + openPopover('Name') + fireEvent.change(getInput('Name'), { + target: { value: 'hospital' }, + }) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_SET, + layerId: 'layer1', + fieldId: 'name', + filter: 'hospital', + }) + }) + + test('wraps the search input in a numeric syntax help tooltip for number columns', () => { renderFilterInput({ type: 'number' }) expect( - screen.getByTestId('data-table-numeric-filter-help') + screen.getByTestId('data-table-filter-help-reference') ).toBeInTheDocument() }) - test('does not show the help icon for string columns', () => { + test('also wraps the search input in a help tooltip for string columns', () => { renderFilterInput({ type: 'string' }) expect( - screen.queryByTestId('data-table-numeric-filter-help') - ).not.toBeInTheDocument() + screen.getByTestId('data-table-filter-help-reference') + ).toBeInTheDocument() }) }) describe('FilterInput multi-select path (no optionSetId)', () => { const options = [{ value: 'High' }, { value: 'Low' }] - test('shows "All" when nothing is selected', () => { + test('shows an empty input when nothing is selected', () => { renderFilterInput({ dataKey: 'legend', name: 'Legend', options }) - expect(screen.getByText('All')).toBeInTheDocument() + expect(getInput('Legend')).toHaveValue('') }) - test('shows the selected count when a filter is active', () => { + test('shows the selected count on the closed trigger', () => { renderFilterInput( { dataKey: 'legend', name: 'Legend', options }, { legend: ['High'] } ) - expect(screen.getByText('1 selected')).toBeInTheDocument() + expect(getInput('Legend')).toHaveValue('1 selected') }) test('opens a popover with a checkbox per option, using the raw value as the label', () => { renderFilterInput({ dataKey: 'legend', name: 'Legend', options }) - fireEvent.click(screen.getByText('All')) + openPopover('Legend') expect(screen.getByLabelText('High')).toBeInTheDocument() expect(screen.getByLabelText('Low')).toBeInTheDocument() }) + + test('shows a "No value" label for the blank-value sentinel option', () => { + renderFilterInput({ + dataKey: 'parentName', + name: 'Parent', + options: [{ value: '' }, { value: 'Country' }], + }) + openPopover('Parent') + expect(screen.getByLabelText('No value')).toBeInTheDocument() + expect(screen.getByLabelText('Country')).toBeInTheDocument() + }) + + test('"No value" is grouped with "Any value" above the divider, not mixed into the searchable list', () => { + renderFilterInput({ + dataKey: 'parentName', + name: 'Parent', + options: [{ value: '' }, { value: 'Country' }], + }) + openPopover('Parent') + expect( + screen.getByLabelText('No value').closest('.pinnedOptions') + ).toBeInTheDocument() + expect( + screen.getByLabelText('Any value').closest('.pinnedOptions') + ).toBeInTheDocument() + }) + + test('"No value" stays visible while searching, unlike the narrowed checkbox list', () => { + renderFilterInput({ + dataKey: 'parentName', + name: 'Parent', + options: [{ value: '' }, { value: 'Country' }], + }) + openPopover('Parent') + fireEvent.change(getInput('Parent'), { target: { value: 'zzz' } }) + expect(screen.getByLabelText('No value')).toBeInTheDocument() + expect(screen.queryByLabelText('Country')).not.toBeInTheDocument() + }) + + test('omits "No value" entirely when the column has no blank cells', () => { + renderFilterInput({ dataKey: 'legend', name: 'Legend', options }) + openPopover('Legend') + expect(screen.queryByLabelText('No value')).not.toBeInTheDocument() + }) + + test('renders Id column options in monospace', () => { + renderFilterInput({ + dataKey: 'id', + name: 'Id', + options: [{ value: 'abc123' }], + }) + openPopover('Id') + expect( + screen.getByLabelText('abc123').closest('.monoOption') + ).toBeInTheDocument() + }) }) describe('FilterInput multi-select path (optionSetId)', () => { @@ -105,7 +222,7 @@ describe('FilterInput multi-select path (optionSetId)', () => { options, optionSetId: 'optionSet1', }) - fireEvent.click(screen.getByText('All')) + openPopover('Case classification') expect(screen.getByLabelText('Confirmed case')).toBeInTheDocument() expect(screen.getByLabelText('Probable case')).toBeInTheDocument() }) @@ -118,7 +235,640 @@ describe('FilterInput multi-select path (optionSetId)', () => { options, optionSetId: 'optionSet1', }) - fireEvent.click(screen.getByText('All')) + openPopover('Case classification') expect(screen.getByLabelText('CONFIRMED')).toBeInTheDocument() }) + + test('search narrows the list by the resolved label, not the raw code', () => { + renderFilterInput({ + dataKey: 'caseType', + name: 'Case classification', + options, + optionSetId: 'optionSet1', + }) + openPopover('Case classification') + fireEvent.change(getInput('Case classification'), { + target: { value: 'confirmed' }, + }) + expect(screen.getByLabelText('Confirmed case')).toBeInTheDocument() + expect(screen.queryByLabelText('Probable case')).not.toBeInTheDocument() + }) + + test('never shows a custom-filter row, even for non-matching text', () => { + renderFilterInput({ + dataKey: 'caseType', + name: 'Case classification', + options, + optionSetId: 'optionSet1', + }) + openPopover('Case classification') + fireEvent.change(getInput('Case classification'), { + target: { value: 'no such option anywhere' }, + }) + expect( + screen.queryByTestId( + 'data-table-column-filter-custom-Case classification' + ) + ).not.toBeInTheDocument() + }) + + test('typing never dispatches a filter (custom filter disabled)', () => { + const { store } = renderFilterInput({ + dataKey: 'caseType', + name: 'Case classification', + options, + optionSetId: 'optionSet1', + }) + openPopover('Case classification') + fireEvent.change(getInput('Case classification'), { + target: { value: 'no such option anywhere' }, + }) + expect(store.getActions()).toEqual([]) + }) +}) + +describe('FilterInput searchable popover — search', () => { + const options = [{ value: 'High' }, { value: 'Low' }] + + test('narrows the checkbox list to matching labels', () => { + renderFilterInput({ dataKey: 'legend', name: 'Legend', options }) + openPopover('Legend') + fireEvent.change(getInput('Legend'), { + target: { value: 'hi' }, + }) + expect(screen.getByLabelText('High')).toBeInTheDocument() + expect(screen.queryByLabelText('Low')).not.toBeInTheDocument() + }) + + test("narrows a numeric column's checkbox list using the typed filter expression, not substring match", () => { + renderFilterInput({ + dataKey: 'value', + name: 'Value', + type: 'number', + options: [{ value: '10' }, { value: '150' }, { value: '200' }], + }) + openPopover('Value') + fireEvent.change(getInput('Value'), { + target: { value: '< 100' }, + }) + expect(screen.getByLabelText('10')).toBeInTheDocument() + expect(screen.queryByLabelText('150')).not.toBeInTheDocument() + expect(screen.queryByLabelText('200')).not.toBeInTheDocument() + }) + + test('shows a "no matches" message when nothing matches and no custom filter applies', () => { + useOptionSet.mockReturnValue({ + optionSet: { options: [{ code: 'CONFIRMED', name: 'Confirmed' }] }, + }) + renderFilterInput({ + dataKey: 'caseType', + name: 'Case classification', + options: [{ value: 'CONFIRMED' }], + optionSetId: 'optionSet1', + }) + openPopover('Case classification') + fireEvent.change(getInput('Case classification'), { + target: { value: 'zzz' }, + }) + expect(screen.getByText('No matches')).toBeInTheDocument() + }) +}) + +describe('FilterInput searchable popover — custom filter row', () => { + test('offers "Use filter" wording and dispatches the typed text live for number columns', () => { + const { store } = renderFilterInput({ + dataKey: 'value', + name: 'Value', + type: 'number', + options: [{ value: '10' }, { value: '20' }], + }) + openPopover('Value') + fireEvent.change(getInput('Value'), { + target: { value: '> 15' }, + }) + const row = screen.getByTestId('data-table-column-filter-custom-Value') + expect(row).toHaveTextContent('Use filter') + expect(row).toHaveTextContent('> 15') + + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_SET, + layerId: 'layer1', + fieldId: 'value', + filter: '> 15', + }) + }) + + test('strips letters typed into a numeric column, keeping the filter syntax characters', () => { + renderFilterInput({ + dataKey: 'value', + name: 'Value', + type: 'number', + options: [{ value: '10' }, { value: '20' }], + }) + openPopover('Value') + fireEvent.change(getInput('Value'), { + target: { value: 'abc> 1x5xyz' }, + }) + expect(getInput('Value')).toHaveValue('> 15') + }) + + test('offers "Contains" wording for string columns', () => { + const options = [{ value: 'High' }, { value: 'Low' }] + renderFilterInput({ dataKey: 'legend', name: 'Legend', options }) + openPopover('Legend') + fireEvent.change(getInput('Legend'), { + target: { value: 'medium' }, + }) + const row = screen.getByTestId('data-table-column-filter-custom-Legend') + expect(row).toHaveTextContent('Contains') + expect(row).toHaveTextContent('medium') + }) + + test('is hidden when the typed text exactly matches an existing option', () => { + const options = [{ value: 'High' }, { value: 'Low' }] + renderFilterInput({ dataKey: 'legend', name: 'Legend', options }) + openPopover('Legend') + fireEvent.change(getInput('Legend'), { + target: { value: 'High' }, + }) + expect( + screen.queryByTestId('data-table-column-filter-custom-Legend') + ).not.toBeInTheDocument() + }) + + test('clearing the typed text clears an already-applied custom filter live', () => { + const options = [{ value: 'High' }, { value: 'Low' }] + const { store } = renderFilterInput( + { dataKey: 'legend', name: 'Legend', options }, + { legend: 'medium' } + ) + openPopover('Legend') + fireEvent.change(getInput('Legend'), { target: { value: '' } }) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_CLEAR, + layerId: 'layer1', + fieldId: 'legend', + }) + }) +}) + +describe('FilterInput searchable popover — keyboard behavior', () => { + const options = [{ value: 'High' }, { value: 'Low' }] + + test('Enter applies the highlighted checkbox and closes the popover', () => { + const { store } = renderFilterInput({ + dataKey: 'legend', + name: 'Legend', + options, + }) + openPopover('Legend') + const input = getInput('Legend') + fireEvent.keyDown(input, { key: 'ArrowDown' }) + fireEvent.keyDown(input, { key: 'Enter' }) + + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_SET, + layerId: 'layer1', + fieldId: 'legend', + filter: ['High'], + }) + expect(screen.queryByLabelText('High')).not.toBeInTheDocument() + }) + + test('Enter applies the custom filter directly with no prior arrow-navigation, and closes', () => { + const { store } = renderFilterInput({ + dataKey: 'legend', + name: 'Legend', + options, + }) + openPopover('Legend') + const input = getInput('Legend') + fireEvent.change(input, { target: { value: 'medium' } }) + fireEvent.keyDown(input, { key: 'Enter' }) + + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_SET, + layerId: 'layer1', + fieldId: 'legend', + filter: 'medium', + }) + expect( + screen.queryByTestId('data-table-column-filter-custom-Legend') + ).not.toBeInTheDocument() + }) + + test('Escape closes the popover without dispatching anything further', () => { + const { store } = renderFilterInput({ + dataKey: 'legend', + name: 'Legend', + options, + }) + openPopover('Legend') + const input = getInput('Legend') + fireEvent.keyDown(input, { key: 'Escape' }) + + expect(store.getActions()).toEqual([]) + expect( + screen.queryByTestId('data-table-column-filter-custom-Legend') + ).not.toBeInTheDocument() + }) +}) + +describe('FilterInput searchable popover — clear filter', () => { + const options = [{ value: 'High' }, { value: 'Low' }] + + // The clear-x is @dhis2/ui's own Input `clearable` button now (not a + // custom-positioned element) - it fires the same onChange({value:''}) + // as manually clearing the text, which is exactly what's exercised here. + + test('clearing an active array (checkbox) filter closes it out', () => { + const { store } = renderFilterInput( + { dataKey: 'legend', name: 'Legend', options }, + { legend: ['High'] } + ) + // Cleared from the closed state, where the input shows "1 selected" - + // opening first would reset the search text to '' (array filters + // have no string to prefill), leaving nothing to actually clear. + fireEvent.change(getInput('Legend'), { target: { value: '' } }) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_CLEAR, + layerId: 'layer1', + fieldId: 'legend', + }) + }) + + test('clearing an active custom-string filter closes it out', () => { + const { store } = renderFilterInput( + { dataKey: 'legend', name: 'Legend', options }, + { legend: 'medium' } + ) + openPopover('Legend') + fireEvent.change(getInput('Legend'), { target: { value: '' } }) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_CLEAR, + layerId: 'layer1', + fieldId: 'legend', + }) + }) + + test('clearing empty text with no active filter dispatches nothing', () => { + const { store } = renderFilterInput({ + dataKey: 'legend', + name: 'Legend', + options, + }) + openPopover('Legend') + fireEvent.change(getInput('Legend'), { target: { value: '' } }) + expect(store.getActions()).toEqual([]) + }) + + test('checking the pinned "Any value" option collapses any existing selection into just ANY_VALUE_KEY', () => { + const { store } = renderFilterInput( + { dataKey: 'legend', name: 'Legend', options }, + { legend: ['High'] } + ) + openPopover('Legend') + fireEvent.click(screen.getByLabelText('Any value')) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_SET, + layerId: 'layer1', + fieldId: 'legend', + filter: [ANY_VALUE_KEY], + }) + }) + + test('"Any value" is only checked when it is itself part of the selection - it does not track whether any filter is active', () => { + renderFilterInput( + { dataKey: 'legend', name: 'Legend', options }, + { legend: ['High'] } + ) + openPopover('Legend') + expect(screen.getByLabelText('Any value')).not.toBeChecked() + }) + + test('"Any value" stays checked when it is explicitly selected', () => { + renderFilterInput( + { dataKey: 'legend', name: 'Legend', options }, + { legend: [ANY_VALUE_KEY] } + ) + openPopover('Legend') + expect(screen.getByLabelText('Any value')).toBeChecked() + }) + + test('"Any value" stays visible while searching, unlike the narrowed checkbox list', () => { + renderFilterInput({ dataKey: 'legend', name: 'Legend', options }) + openPopover('Legend') + fireEvent.change(getInput('Legend'), { target: { value: 'hi' } }) + expect(screen.getByLabelText('Any value')).toBeInTheDocument() + }) +}) + +describe('FilterInput searchable popover — reopening state', () => { + test('pre-fills the search box with an already-applied custom filter', () => { + const options = [{ value: '10' }, { value: '20' }] + renderFilterInput( + { dataKey: 'value', name: 'Value', type: 'number', options }, + { value: '> 5' } + ) + openPopover('Value') + expect(getInput('Value')).toHaveValue('> 5') + }) +}) + +describe('FilterInput searchable popover — dropdown placement', () => { + test('opens below the trigger by default', () => { + const options = [{ value: 'High' }, { value: 'Low' }] + renderFilterInput({ dataKey: 'legend', name: 'Legend', options }) + openPopover('Legend') + expect( + screen.getByLabelText('High').closest('.dropdownPopperAbove') + ).not.toBeInTheDocument() + }) + + test('flips above when the shared header row has no room to open below', () => { + const options = [{ value: 'High' }, { value: 'Low' }] + renderFilterInput({ dataKey: 'legend', name: 'Legend', options }) + const trigger = getInput('Legend').closest('.filterTrigger') + jest.spyOn(trigger, 'getBoundingClientRect').mockReturnValue({ + bottom: window.innerHeight - 50, + top: window.innerHeight - 78, + width: 100, + }) + openPopover('Legend') + expect( + screen.getByLabelText('High').closest('.dropdownPopperAbove') + ).toBeInTheDocument() + }) +}) + +describe('FilterInput searchable popover — reverse selection', () => { + const getReverseButton = (name) => + screen.getByTestId(`data-table-column-filter-reverse-${name}`) + + test('selects "Any value" when nothing is currently selected - every real value ends up ticked, which collapses to the wildcard', () => { + const options = [ + { value: 'High' }, + { value: 'Medium' }, + { value: 'Low' }, + ] + const { store } = renderFilterInput({ + dataKey: 'legend', + name: 'Legend', + options, + }) + openPopover('Legend') + fireEvent.click(getReverseButton('Legend')) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_SET, + layerId: 'layer1', + fieldId: 'legend', + filter: [ANY_VALUE_KEY], + }) + }) + + test("flips each value's checked state relative to the current selection", () => { + const options = [ + { value: 'High' }, + { value: 'Medium' }, + { value: 'Low' }, + ] + const { store } = renderFilterInput( + { dataKey: 'legend', name: 'Legend', options }, + { legend: ['High'] } + ) + openPopover('Legend') + fireEvent.click(getReverseButton('Legend')) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_SET, + layerId: 'layer1', + fieldId: 'legend', + filter: ['Medium', 'Low'], + }) + }) + + test('includes "No value" in the values it flips', () => { + const options = [ + { value: '' }, + { value: 'Country' }, + { value: 'District' }, + ] + const { store } = renderFilterInput( + { dataKey: 'parentName', name: 'Parent', options }, + { parentName: ['Country'] } + ) + openPopover('Parent') + fireEvent.click(getReverseButton('Parent')) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_SET, + layerId: 'layer1', + fieldId: 'parentName', + filter: ['', 'District'], + }) + }) + + test('collapses to "Any value" (plus "No value" if that was unset) when reversing ends up ticking every real value', () => { + const options = [{ value: '' }, { value: 'Country' }] + const { store } = renderFilterInput({ + dataKey: 'parentName', + name: 'Parent', + options, + }) + openPopover('Parent') + fireEvent.click(getReverseButton('Parent')) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_SET, + layerId: 'layer1', + fieldId: 'parentName', + filter: [ANY_VALUE_KEY, ''], + }) + }) + + test('turns off "Any value" (and every real value with it) when it was active - there is no way to represent "all unticked" while it stays on', () => { + const options = [{ value: 'High' }, { value: 'Low' }] + const { store } = renderFilterInput( + { dataKey: 'legend', name: 'Legend', options }, + { legend: ['High', ANY_VALUE_KEY] } + ) + openPopover('Legend') + fireEvent.click(getReverseButton('Legend')) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_CLEAR, + layerId: 'layer1', + fieldId: 'legend', + }) + }) + + test('reversing while "Any value" is active flips "No value" independently, since it is unaffected by "Any value"', () => { + const options = [{ value: '' }, { value: 'Country' }] + const { store } = renderFilterInput( + { dataKey: 'parentName', name: 'Parent', options }, + { parentName: [ANY_VALUE_KEY] } + ) + openPopover('Parent') + fireEvent.click(getReverseButton('Parent')) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_SET, + layerId: 'layer1', + fieldId: 'parentName', + filter: [''], + }) + }) + + test('ignores the current search text - inverts the full value list, not just what is visible', () => { + const options = [ + { value: 'High' }, + { value: 'Medium' }, + { value: 'Low' }, + ] + const { store } = renderFilterInput( + { dataKey: 'legend', name: 'Legend', options }, + { legend: ['High'] } + ) + openPopover('Legend') + // Narrows the visible checkbox list down to just "Low" - if reverse + // scoped itself to that, the result would only ever mention "Low". + fireEvent.change(getInput('Legend'), { target: { value: 'lo' } }) + fireEvent.click(getReverseButton('Legend')) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_SET, + layerId: 'layer1', + fieldId: 'legend', + filter: ['Medium', 'Low'], + }) + }) + + test('is disabled when the column has no known values to invert', () => { + renderFilterInput({ dataKey: 'name', name: 'Name' }) + openPopover('Name') + expect(getReverseButton('Name')).toBeDisabled() + }) +}) + +describe('FilterInput searchable popover — "Any value" / real value interaction', () => { + const options = [{ value: 'High' }, { value: 'Medium' }, { value: 'Low' }] + + test('every real value reads as checked while "Any value" is active', () => { + renderFilterInput( + { dataKey: 'legend', name: 'Legend', options }, + { legend: [ANY_VALUE_KEY] } + ) + openPopover('Legend') + expect(screen.getByLabelText('High')).toBeChecked() + expect(screen.getByLabelText('Medium')).toBeChecked() + expect(screen.getByLabelText('Low')).toBeChecked() + }) + + test('"No value" does not read as checked just because "Any value" is active', () => { + renderFilterInput( + { + dataKey: 'parentName', + name: 'Parent', + options: [{ value: '' }, { value: 'Country' }], + }, + { parentName: [ANY_VALUE_KEY] } + ) + openPopover('Parent') + expect(screen.getByLabelText('No value')).not.toBeChecked() + }) + + test('unticking one real value while "Any value" is active unticks "Any value" too, but keeps every other value ticked', () => { + const { store } = renderFilterInput( + { dataKey: 'legend', name: 'Legend', options }, + { legend: [ANY_VALUE_KEY] } + ) + openPopover('Legend') + fireEvent.click(screen.getByLabelText('Medium')) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_SET, + layerId: 'layer1', + fieldId: 'legend', + filter: ['High', 'Low'], + }) + }) + + test('unticking a real value while "Any value" is active preserves "No value" if it was set', () => { + const { store } = renderFilterInput( + { + dataKey: 'parentName', + name: 'Parent', + options: [{ value: '' }, { value: 'A' }, { value: 'B' }], + }, + { parentName: [ANY_VALUE_KEY, ''] } + ) + openPopover('Parent') + fireEvent.click(screen.getByLabelText('A')) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_SET, + layerId: 'layer1', + fieldId: 'parentName', + filter: ['B', ''], + }) + }) + + test('manually ticking every real value collapses the selection into "Any value"', () => { + const { store } = renderFilterInput( + { dataKey: 'legend', name: 'Legend', options }, + { legend: ['High', 'Medium'] } + ) + openPopover('Legend') + fireEvent.click(screen.getByLabelText('Low')) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_SET, + layerId: 'layer1', + fieldId: 'legend', + filter: [ANY_VALUE_KEY], + }) + }) + + test('manually ticking every real value preserves "No value" while collapsing to "Any value"', () => { + const { store } = renderFilterInput( + { + dataKey: 'parentName', + name: 'Parent', + options: [{ value: '' }, { value: 'A' }, { value: 'B' }], + }, + { parentName: ['A', ''] } + ) + openPopover('Parent') + fireEvent.click(screen.getByLabelText('B')) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_SET, + layerId: 'layer1', + fieldId: 'parentName', + filter: [ANY_VALUE_KEY, ''], + }) + }) + + test('unchecking "Any value" unticks every real value too, not just the wildcard', () => { + const { store } = renderFilterInput( + { dataKey: 'legend', name: 'Legend', options }, + { legend: [ANY_VALUE_KEY] } + ) + openPopover('Legend') + fireEvent.click(screen.getByLabelText('Any value')) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_CLEAR, + layerId: 'layer1', + fieldId: 'legend', + }) + }) + + test('unchecking "Any value" preserves "No value" independently', () => { + const { store } = renderFilterInput( + { + dataKey: 'parentName', + name: 'Parent', + options: [{ value: '' }, { value: 'Country' }], + }, + { parentName: [ANY_VALUE_KEY, ''] } + ) + openPopover('Parent') + fireEvent.click(screen.getByLabelText('Any value')) + expect(store.getActions()).toContainEqual({ + type: DATA_FILTER_SET, + layerId: 'layer1', + fieldId: 'parentName', + filter: [''], + }) + }) }) diff --git a/src/components/datatable/__tests__/TableContextMenu.spec.jsx b/src/components/datatable/__tests__/TableContextMenu.spec.jsx new file mode 100644 index 000000000..112b6f7a3 --- /dev/null +++ b/src/components/datatable/__tests__/TableContextMenu.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 { FEATURE_HIGHLIGHT } from '../../../constants/actionTypes.js' +import { FACILITY_LAYER } from '../../../constants/layers.js' +import TableContextMenu from '../TableContextMenu.jsx' + +jest.mock('../../cachedDataProvider/CachedDataProvider.jsx', () => ({ + useCachedData: () => ({ + systemSettings: { keyAnalysisDigitGroupSeparator: ',' }, + }), +})) + +const mockStore = configureMockStore() + +const layer = { id: 'layer1', layer: FACILITY_LAYER, name: 'Test layer' } +const contextMenu = { x: 10, y: 10, featureProps: {} } + +// @dhis2/ui's MenuItem puts `data-test` on the outer
  • , but `aria-disabled` +// and the click handler both live on the inner . +const getZoomToFilteredLink = () => + screen + .getByTestId('data-table-context-menu-zoom-to-filtered') + .querySelector('a') + +const renderMenu = (props) => { + const store = mockStore({}) + const result = render( + + + + ) + return { ...result, store } +} + +describe('TableContextMenu — zoom to filtered features', () => { + test('is disabled when no filter is active (filteredIds is null)', () => { + renderMenu({ filteredIds: null }) + expect(getZoomToFilteredLink()).toHaveAttribute('aria-disabled', 'true') + }) + + test('is disabled when the filtered id list is empty', () => { + renderMenu({ filteredIds: [] }) + expect(getZoomToFilteredLink()).toHaveAttribute('aria-disabled', 'true') + }) + + test('dispatches highlightFeature with the filtered ids and closes the menu when clicked', () => { + const onClose = jest.fn() + const { store } = renderMenu({ + filteredIds: ['a', 'b', 'c'], + onClose, + }) + fireEvent.click(getZoomToFilteredLink()) + expect(store.getActions()).toContainEqual({ + type: FEATURE_HIGHLIGHT, + payload: { + ids: ['a', 'b', 'c'], + layerId: 'layer1', + origin: 'table', + zoom: true, + }, + }) + expect(onClose).toHaveBeenCalled() + }) +}) diff --git a/src/components/datatable/__tests__/useTableData.spec.jsx b/src/components/datatable/__tests__/useTableData.spec.jsx index 55edee96a..881e591cf 100644 --- a/src/components/datatable/__tests__/useTableData.spec.jsx +++ b/src/components/datatable/__tests__/useTableData.spec.jsx @@ -2,7 +2,11 @@ import { renderHook } from '@testing-library/react' import React from 'react' import { Provider } from 'react-redux' import configureMockStore from 'redux-mock-store' -import { useTableData } from '../useTableData.js' +import { + useTableData, + SELECTED_SORT_KEY, + NOT_SET_VALUE, +} from '../useTableData.js' jest.mock('../../map/MapApi.js', () => ({ loadEarthEngineWorker: jest.fn(), @@ -837,6 +841,93 @@ describe('useTableData sorting', () => { const valueColumn = result.current.rows.map((row) => row[3]?.value) // Value column expect(valueColumn).toEqual([null, null, null]) }) + + test('falls back to natural (index) order when sortField is null', () => { + // Deliberately not alphabetical/numerical, so this only passes if the + // natural input order is preserved rather than some other sort. + const layerInInputOrder = { + id: 'test-layer', + layer: 'thematic', + dataFilters: null, + data: [ + { properties: { id: '1', name: 'Item C', rawValue: 3 } }, + { properties: { id: '2', name: 'Item A', rawValue: 1 } }, + { properties: { id: '3', name: 'Item B', rawValue: 2 } }, + ], + } + const store = { aggregations: {} } + const { result } = renderHook( + () => + useTableData({ + layer: layerInInputOrder, + sortField: null, + sortDirection: 'asc', + }), + { + wrapper: ({ children }) => ( + {children} + ), + } + ) + + const names = result.current.rows.map( + (row) => row.find((c) => c.dataKey === 'name')?.value + ) + expect(names).toEqual(['Item C', 'Item A', 'Item B']) + }) + + describe('sorting by selection state', () => { + // `id` must live inside `properties` - useTableData flattens rows via + // `{...d.properties}`, so a top-level `id` (as used by the rest of + // this describe block's `mockLayer`) would not survive flattening. + const layerWithIds = { + id: 'test-layer', + layer: 'thematic', + dataFilters: null, + data: [ + { properties: { id: '1', name: 'Item A' } }, + { properties: { id: '2', name: 'Item B' } }, + { properties: { id: '3', name: 'Item C' } }, + { properties: { id: '4', name: 'Item D' } }, + { properties: { id: '5', name: 'Item E' } }, + ], + } + const store = { aggregations: {} } + + const renderSorted = (sortDirection) => + renderHook( + () => + useTableData({ + layer: layerWithIds, + sortField: SELECTED_SORT_KEY, + sortDirection, + selectedIdSet: new Set(['2', '4']), + }), + { + wrapper: ({ children }) => ( + {children} + ), + } + ).result + + test('ascending (the default on first click) puts selected rows first', () => { + const { current } = renderSorted('asc') + const ids = current.rows.map( + (row) => row.find((c) => c.dataKey === 'id')?.value + ) + expect(ids.slice(0, 2).sort()).toEqual(['2', '4']) + expect(ids.slice(2).sort()).toEqual(['1', '3', '5']) + }) + + test('descending puts selected rows last', () => { + const { current } = renderSorted('desc') + const ids = current.rows.map( + (row) => row.find((c) => c.dataKey === 'id')?.value + ) + expect(ids.slice(0, 3).sort()).toEqual(['1', '3', '5']) + expect(ids.slice(3).sort()).toEqual(['2', '4']) + }) + }) }) describe('useTableData showOnlyFeaturesInView', () => { @@ -920,7 +1011,7 @@ describe('useTableData showOnlyFeaturesInView', () => { }) }) -describe('useTableData showOnlySelected', () => { +describe('useTableData selectionFilter', () => { const store = { aggregations: {} } const layer = { @@ -940,23 +1031,23 @@ describe('useTableData showOnlySelected', () => { ), }).result - test('includes all rows when the toggle is off', () => { + test('includes all rows when no filter is applied', () => { const { current } = renderTableData({ layer, sortField: 'name', sortDirection: 'asc', - showOnlySelected: false, + selectionFilter: [], selectedIdSet: new Set(['a']), }) expect(current.rows).toHaveLength(2) }) - test('includes only selected rows when the toggle is on', () => { + test('includes only selected rows when filtered to "selected"', () => { const { current } = renderTableData({ layer, sortField: 'name', sortDirection: 'asc', - showOnlySelected: true, + selectionFilter: ['selected'], selectedIdSet: new Set(['a']), }) expect(current.rows).toHaveLength(1) @@ -965,12 +1056,37 @@ describe('useTableData showOnlySelected', () => { ) }) - test('shows no rows when the toggle is on and nothing is selected', () => { + test('includes only non-selected rows when filtered to "not-selected"', () => { + const { current } = renderTableData({ + layer, + sortField: 'name', + sortDirection: 'asc', + selectionFilter: ['not-selected'], + selectedIdSet: new Set(['a']), + }) + expect(current.rows).toHaveLength(1) + expect(current.rows[0].find((c) => c.dataKey === 'name').value).toBe( + 'Item B' + ) + }) + + test('includes all rows when both options are checked', () => { + const { current } = renderTableData({ + layer, + sortField: 'name', + sortDirection: 'asc', + selectionFilter: ['selected', 'not-selected'], + selectedIdSet: new Set(['a']), + }) + expect(current.rows).toHaveLength(2) + }) + + test('shows no rows when filtered to "selected" and nothing is selected', () => { const { current } = renderTableData({ layer, sortField: 'name', sortDirection: 'asc', - showOnlySelected: true, + selectionFilter: ['selected'], selectedIdSet: new Set(), }) expect(current.rows).toHaveLength(0) @@ -995,7 +1111,7 @@ describe('useTableData columnOptions', () => { } ).result - test('includes legend and type for a thematic layer, but not name/id', () => { + test('gives options to every column type once distinct values are within the cap', () => { const layer = { layer: 'thematic', dataFilters: null, @@ -1036,12 +1152,24 @@ describe('useTableData columnOptions', () => { { value: 'Low' }, ]) expect(current.columnOptions.type).toEqual([{ value: 'Point' }]) - expect(current.columnOptions.name).toBeUndefined() - expect(current.columnOptions.id).toBeUndefined() - expect(current.columnOptions.parentName).toBeUndefined() + expect(current.columnOptions.name).toEqual([ + { value: 'Org unit 1' }, + { value: 'Org unit 2' }, + ]) + expect(current.columnOptions.id).toEqual([ + { value: 'ou1' }, + { value: 'ou2' }, + ]) + expect(current.columnOptions.parentName).toEqual([{ value: 'Country' }]) + // Numeric columns (previously excluded outright) qualify too now. + expect(current.columnOptions.rawValue).toEqual([ + { value: '10' }, + { value: '20' }, + ]) + expect(current.columnOptions.level).toEqual([{ value: '1' }]) }) - test('falls back to free text when a column has more than 30 distinct values', () => { + test('gives options to a column even with many distinct values - no cap', () => { const layer = { layer: 'orgUnit', dataFilters: null, @@ -1058,7 +1186,78 @@ describe('useTableData columnOptions', () => { const { current } = renderTableData(layer) - expect(current.columnOptions.type).toBeUndefined() + expect(current.columnOptions.type).toHaveLength(31) + }) + + test('sorts numeric column options numerically, not lexically', () => { + const layer = { + layer: 'thematic', + dataFilters: null, + data: [10, 2, 33].map((rawValue, i) => ({ + properties: { + id: `ou${i}`, + name: `Org unit ${i}`, + rawValue, + legend: 'High', + range: '0 - 100', + level: 1, + parentName: 'Country', + type: 'Point', + color: '#ff0000', + }, + })), + } + + const { current } = renderTableData(layer) + + expect(current.columnOptions.rawValue).toEqual([ + { value: '2' }, + { value: '10' }, + { value: '33' }, + ]) + }) + + test('includes a NOT_SET_VALUE option when some rows have a blank value', () => { + const layer = { + layer: 'orgUnit', + dataFilters: null, + data: [ + { + properties: { + id: 'ou1', + name: 'Org unit 1', + level: 1, + parentName: 'Country', + type: 'Point', + }, + }, + { + properties: { + id: 'ou2', + name: 'Org unit 2', + level: 1, + parentName: '', + type: 'Point', + }, + }, + { + properties: { + id: 'ou3', + name: 'Org unit 3', + level: 1, + // parentName omitted entirely (undefined) + type: 'Point', + }, + }, + ], + } + + const { current } = renderTableData(layer) + + expect(current.columnOptions.parentName).toEqual([ + { value: NOT_SET_VALUE }, + { value: 'Country' }, + ]) }) test('exposes optionSet on event columns for later resolution by FilterInput', () => { diff --git a/src/components/datatable/styles/BottomPanel.module.css b/src/components/datatable/styles/BottomPanel.module.css index 5058846a5..e95268f60 100644 --- a/src/components/datatable/styles/BottomPanel.module.css +++ b/src/components/datatable/styles/BottomPanel.module.css @@ -128,6 +128,16 @@ padding: 0; } +.alignIcon1 { + display: flex; + margin-top: 1px; +} + +.alignIcon2 { + display: flex; + margin-top: 2px; +} + .clearFiltersButton:hover, .closeIcon:hover, .toggleButton:hover { diff --git a/src/components/datatable/styles/DataTable.module.css b/src/components/datatable/styles/DataTable.module.css index 8f09b2881..39d96c664 100644 --- a/src/components/datatable/styles/DataTable.module.css +++ b/src/components/datatable/styles/DataTable.module.css @@ -2,6 +2,22 @@ height: 1px; } +/* @dhis2/ui's Table draws a 1px border on all sides via its own scoped + styled-jsx rule, which our .dataTable class alone can't reliably + out-specificity. Pairing it with the table's stable data-test attribute + (with !important) guarantees this wins, while staying scoped to just this + table via .dataTable (a bare data-test selector would match every + DataTable instance in the app). That border (not just the top side) is + what makes the browser compute the sticky header's normal-flow and stuck + positions on very slightly different subpixel grids, causing it to + visibly snap by ~1-2px the instant scrolling engages position:sticky. + Removing the table's own border entirely (confirmed via devtools) removes + the snap; row/column delineation still comes from each cell's own + border-bottom/border-inline-end. */ +.dataTable[data-test='dhis2-uicore-datatable'] { + border: none !important; +} + .dataTable > :global(thead) { user-select: none; } @@ -21,15 +37,65 @@ td.lightText { color: var(--colors-white); } +td.monoCell { + font-family: ui-monospace, 'SF Mono', 'Cascadia Mono', 'Consolas', monospace; +} + +.legendCell { + display: flex; + align-items: center; + gap: var(--spacers-dp8); +} + +.legendSwatch { + flex-shrink: 0; + width: 10px; + height: 10px; + border-radius: 2px; + border: 1px solid var(--colors-grey400); +} + th.checkboxCell, td.checkboxCell { - width: 32px; - min-width: 32px; - max-width: 32px; + width: 76px; + min-width: 76px; + max-width: 76px; text-align: center; padding: 0; } +.checkboxHeaderContent { + display: flex; + align-items: center; + justify-content: center; + gap: 2px; +} + +.selectionFilterButton { + width: 100%; + height: 24px; + font-size: 11px; + padding: 4px 6px; + border: 1px solid var(--colors-grey500); + border-radius: 3px; + box-shadow: inset 0 0 1px 0 rgba(48, 54, 60, 0.1); + background: var(--colors-white); + cursor: pointer; + text-align: left; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.selectionFilterPopover { + padding: var(--spacers-dp8); + min-width: 140px; +} + +.selectionFilterPopover :global(label) { + font-size: 11px !important; +} + td.selected { background-color: var(--colors-blue050); } @@ -50,6 +116,21 @@ td.hovered { gap: 2px; } +.reverseButton { + display: inline-flex; + align-items: center; + justify-content: center; + flex-shrink: 0; + width: 26px; + height: 26px; + padding-top: 1px; + margin-right: -8px; + border: none; + border-radius: 4px; + background: transparent; + cursor: pointer; +} + .sortButton { display: inline-flex; align-items: center; @@ -64,8 +145,8 @@ td.hovered { cursor: pointer; } -.sortButton:hover, -.sortButton:focus-visible { +.sortButton:hover:not(:disabled), +.sortButton:focus-visible:not(:disabled) { background: var(--colors-grey400); } @@ -73,6 +154,24 @@ td.hovered { outline: none; } +.sortButton:disabled { + opacity: 0.4; + cursor: not-allowed; +} + +.topTooltipContent { + z-index: 2000; + max-width: 300px; + padding: 4px 6px; + background-color: var(--colors-grey900); + border-radius: 3px; + color: var(--colors-white); + font-size: 13px; + line-height: 17px; + word-break: normal; + overflow-wrap: break-word; +} + .columnHeader :global(input.dense) { padding: 4px 6px; font-size: 11px; @@ -82,12 +181,21 @@ td.hovered { color: var(--colors-grey400); } -/* Hide the filter icon */ +/* The filter icon button is a required prop of DataTableColumnHeader + whenever a filter is passed, but the filter itself is always shown (see + the "Filtering: Inline" pattern), so the icon has no real toggle behavior. + Remove it from layout entirely instead of just hiding it, to reclaim the + header's horizontal space. Applies to the checkbox column too, since it + also has its own filter (the selection filter). */ .columnHeader + > :global(span.container) + > :global(span.top) + > button:last-of-type, +.checkboxCell > :global(span.container) > :global(span.top) > button:last-of-type { - visibility: hidden; + display: none; } .noResults { @@ -95,10 +203,39 @@ td.hovered { color: var(--colors-grey600); align-items: center; justify-content: center; + gap: var(--spacers-dp8); + font-size: 12px; font-style: italic; min-height: 40px; } +.clearFiltersLink { + font-size: 12px; + font-style: normal; + color: var(--colors-blue600); + background: transparent; + border: none; + padding: 0; + cursor: pointer; + text-decoration: underline; +} + +.clearFiltersLink:hover { + color: var(--colors-blue700); +} + +.loadingContent { + display: flex; + flex-direction: column; + align-items: center; + gap: var(--spacers-dp8); +} + +.loadingReason { + font-size: 12px; + color: var(--colors-grey700); +} + .noSupport { position: absolute; top: 50%; diff --git a/src/components/datatable/styles/FilterInput.module.css b/src/components/datatable/styles/FilterInput.module.css index c5d585d03..28f2e3e54 100644 --- a/src/components/datatable/styles/FilterInput.module.css +++ b/src/components/datatable/styles/FilterInput.module.css @@ -1,36 +1,227 @@ -.multiSelectButton { +.filterTrigger { + position: relative; + display: flex; + align-items: center; width: 100%; - height: 24px; - font-size: 11px; +} + +/* The trigger and the dropdown's search field are the same now + (see FilterInput.jsx) - style it to match the old compact trigger + button's size regardless of which role it's playing at the moment. + Padding-right for the built-in clear button is handled by @dhis2/ui's + own `.input-clearable input` rule - don't compete with it here. */ +.filterTrigger :global(input.dense) { padding: 4px 6px; - border: 1px solid var(--colors-grey400); - border-radius: 3px; - background: var(--colors-white); + font-size: 11px; +} + +.filterTrigger :global(input::placeholder) { + color: var(--colors-grey400); +} + +/* Every column's dropdown opens on the same side (see dropdownSide in + FilterInput.jsx - below by default, or above if the shared header row + doesn't have room to open downward) - this supplies the same look + @dhis2/ui's Popover would (white background, elevation shadow, rounded + corners), minus whichever corner touches the trigger, which is squared + off so it reads as a continuation of the input rather than a separate + floating box. */ +.dropdownPopper { + background-color: var(--colors-white); + border-radius: 4px; + border-top-left-radius: 0; + border-top-right-radius: 0; + box-shadow: 0 4px 12px rgba(12, 14, 16, 0.15), + 0 0 0 1px rgba(12, 14, 16, 0.05); +} + +.dropdownPopperAbove { + border-top-left-radius: 4px; + border-top-right-radius: 4px; + border-bottom-left-radius: 0; + border-bottom-right-radius: 0; +} + +.searchableFilterPopover { + display: flex; + flex-direction: column; + gap: var(--spacers-dp2); + padding: var(--spacers-dp8); + min-width: 140px; + box-sizing: border-box; +} + +/* When the dropdown opens above the trigger, flip the whole stack so the + real value list ends up farthest from the input and the custom-filter + row ends up right next to it, same as when it opens below - dropdownSide + is computed once in JS (see FilterInput.jsx), so this is a plain class + toggle rather than a Popper-attribute selector. Ties in flex `order` + fall back to DOM order, which would put "Any value"/"No value" closest + to the input instead of the custom-filter row - giving each group its + own order keeps the same relative arrangement as the non-flipped case, + just mirrored. */ +.reversedOrder .multiSelectPopover { + order: 0; +} + +.reversedOrder .pinnedOptions { + order: 1; +} + +.reversedOrder .customFilterRow { + order: 2; +} + +/* "Any value" and "No value" are grouped together above a divider, + separate from the column's real distinct values below - no horizontal + padding of its own (the 8px inset already comes from + .searchableFilterPopover's padding, same as .multiSelectPopover below - + adding more here would indent these two checkboxes further than the + rest) and the same font-size as the list underneath, so they read as + part of the same control rather than a visually distinct element bolted + on top. `min-width: 0` overrides the flex item default of `auto` (which + floors a flex item's size at its content's min-content width) - without + it, this row's own content could force `.searchableFilterPopover` wider + than the trigger input on narrow columns even though "Any value"/"No + value" are short, fixed strings that never need the extra room the + real-value list below is allowed to take. `width: 100%` makes it match + the popover's own (input-matching) width explicitly, rather than + relying only on the flex stretch default - belt-and-braces against this + row silently narrowing the popover below the trigger input's width. */ +.pinnedOptions { + position: relative; + width: 100%; + min-width: 0; + box-sizing: border-box; + border-bottom: 1px solid var(--colors-grey300); +} + +.pinnedOptions :global(label) { + font-size: 11px !important; +} + +/* "Reverse selection" - a ghost icon button, absolutely positioned over + the "Any value" row so it never contributes to that row's own width. */ +.reverseSelectionButton { + position: absolute; + top: 0; + right: 0; + display: flex; + align-items: center; + justify-content: center; + width: 24px; + height: 24px; + padding: 0; + border: none; + border-radius: 4px; + background: transparent; + color: var(--colors-grey700); cursor: pointer; - text-align: left; +} + +.reverseSelectionButton:hover:not(:disabled) { + background: var(--colors-grey100); +} + +.reverseSelectionButton:disabled { + color: var(--colors-grey400); + cursor: not-allowed; +} + +/* When the dropdown opens above the trigger, this group sits at the + bottom of the stack (closest to the input - see the order swap above), + so the divider needs to move to its top edge instead: it should always + separate the pinned group from the real value list, regardless of + which one is visually on top. */ +.reversedOrder .pinnedOptions { + border-bottom: none; + border-top: 1px solid var(--colors-grey300); } .multiSelectPopover { - padding: var(--spacers-dp8); max-height: 260px; overflow-y: auto; - min-width: 180px; } -.numericFilterWrapper { +.multiSelectPopover :global(label) { + font-size: 11px !important; +} + +.monoOption :global(label) { + font-family: ui-monospace, 'SF Mono', 'Cascadia Mono', 'Consolas', monospace; +} + +/* "Any value" (matches any non-blank value) and "No value" (the blank-cell + sentinel) are opposite ends of the same predicate, not real data values - + italicize and mute them so they read as special/meta options rather than + something that could appear in the underlying data. */ +.specialOption :global(label) { + color: var(--colors-grey700) !important; + font-style: italic; +} + +.noResults { + padding: 4px 2px; + font-size: 11px; + font-style: italic; + color: var(--colors-grey600); +} + +/* Styled like an active/selectable row (blue, the app's convention for + "actionable/selected" - see td.selected in DataTable.module.css) rather + than a warning, since applying a filter isn't an exceptional action. */ +.customFilterRow { display: flex; align-items: center; - gap: 2px; + gap: 6px; + width: 100%; + text-align: left; + font-size: 11px; + padding: 6px 8px; + border: none; + border-radius: 3px; + background: var(--colors-blue050); + color: var(--colors-blue700); + cursor: pointer; } -.numericFilterWrapper > :global(div) { - flex: 1 1 auto; - min-width: 0; +.customFilterRow svg { + flex-shrink: 0; } -.helpIcon { - display: inline-flex; - flex-shrink: 0; - color: var(--colors-grey600); - cursor: help; +.customFilterRow:hover, +.customFilterRow.highlighted { + background: var(--colors-blue100); +} + +.customFilterTag { + color: var(--colors-blue700); + white-space: nowrap; +} + +.customFilterExpr { + font-family: ui-monospace, 'SF Mono', 'Cascadia Mono', 'Consolas', monospace; + font-weight: 600; + word-break: break-word; +} + +.multiSelectPopover .highlighted { + background: var(--colors-grey100); +} + +/* Matches DataTable.module.css's .topTooltipContent - this is the same + no-flip tooltip pattern (see FilterHelpTooltip in FilterInput.jsx), just + rendering its own dark tooltip box instead of relying on @dhis2/ui's + Tooltip (which supplies that styling itself). */ +.filterHelpTooltip { + z-index: 2000; + max-width: 300px; + padding: 4px 6px; + background-color: var(--colors-grey900); + border-radius: 3px; + color: var(--colors-white); + font-size: 11px; + line-height: 17px; + word-break: normal; + overflow-wrap: break-word; } diff --git a/src/components/datatable/useTableData.js b/src/components/datatable/useTableData.js index f41150a44..c65aaad2b 100644 --- a/src/components/datatable/useTableData.js +++ b/src/components/datatable/useTableData.js @@ -9,6 +9,10 @@ import { FACILITY_LAYER, GEOJSON_URL_LAYER, } from '../../constants/layers.js' +import { + SELECTION_FILTER_SELECTED, + SELECTION_FILTER_NOT_SELECTED, +} from '../../constants/selection.js' import { numberValueTypes } from '../../constants/valueTypes.js' import { hasClasses } from '../../util/earthEngine.js' import { filterByGlobalSearch, filterData } from '../../util/filter.js' @@ -19,11 +23,25 @@ import { isValidUid } from '../../util/uid.js' const ASCENDING = 'asc' +// Sentinel sortField value for the checkbox column - not a real dataKey +export const SELECTED_SORT_KEY = '__selected' + +// Sentinel option value representing missing/blank cells in a column's +// distinct-value list - matches filterData's existing null/undefined -> +// empty-string coercion (src/util/filter.js), so no filtering logic changes +// are needed to support it. +export const NOT_SET_VALUE = '' + const TYPE_NUMBER = 'number' const TYPE_STRING = 'string' const TYPE_DATE = 'date' -const INDEX = 'index' +// The Index column is a synthetic row number computed only for table +// display (see the `index` assigned from array position in +// dataWithAggregations below) - it's never written onto the underlying +// layer's actual feature data, so it can't be used as a map filter (see +// DataTable.jsx, which excludes it from getting a FilterInput at all). +export const INDEX = 'index' const NAME = 'name' const ID = 'id' const VALUE = 'rawValue' @@ -199,16 +217,13 @@ const EMPTY_AGGREGATIONS = {} const EMPTY_LAYER = {} const EMPTY_COLUMN_OPTIONS = {} -const CATEGORICAL_DATA_KEYS = new Set([LEGEND, TYPE]) -const MAX_CATEGORICAL_OPTIONS = 30 - export const useTableData = ({ layer, sortField, sortDirection, showOnlyFeaturesInView, mapBounds, - showOnlySelected, + selectionFilter, selectedIdSet, globalSearch, }) => { @@ -341,34 +356,49 @@ export const useTableData = ({ layerHeaders, ]) + // The full distinct-value list for every column, regardless of size - + // the filter popover virtualizes its checkbox list (renders only what's + // near the viewport) and narrows live as the user searches, so there's + // no longer a rendering-cost reason to cap or omit this. const columnOptions = useMemo(() => { if (!headers?.length || !dataWithAggregations?.length) { return EMPTY_COLUMN_OPTIONS } const result = {} - headers.forEach(({ dataKey, type, optionSet }) => { - if (type !== TYPE_STRING) { - return - } - if (!CATEGORICAL_DATA_KEYS.has(dataKey) && !optionSet) { - return - } - + headers.forEach(({ dataKey, type }) => { const seen = new Set() for (const item of dataWithAggregations) { const val = item[dataKey] - if (val !== undefined && val !== null && val !== '') { - seen.add(String(val)) - } - if (seen.size > MAX_CATEGORICAL_OPTIONS) { - break - } + seen.add( + val === undefined || val === null || val === '' + ? NOT_SET_VALUE + : String(val) + ) } - if (seen.size > 0 && seen.size <= MAX_CATEGORICAL_OPTIONS) { + if (seen.size > 0) { result[dataKey] = Array.from(seen) - .sort() + .sort((a, b) => { + if (a === NOT_SET_VALUE) { + return -1 + } + if (b === NOT_SET_VALUE) { + return 1 + } + // Distinct values are stored as strings (they're + // sourced alongside string columns) - sort numeric + // columns by numeric value rather than lexically + // (so 2 sorts before 10), everything else keeps the + // default string ordering. + return type === TYPE_NUMBER + ? Number(a) - Number(b) + : a < b + ? -1 + : a > b + ? 1 + : 0 + }) .map((value) => ({ value })) } }) @@ -399,14 +429,38 @@ export const useTableData = ({ ) } - if (showOnlySelected) { - filteredData = filteredData.filter((item) => - selectedIdSet?.has(item.id) + if (selectionFilter?.length) { + const wantSelected = selectionFilter.includes( + SELECTION_FILTER_SELECTED + ) + const wantNotSelected = selectionFilter.includes( + SELECTION_FILTER_NOT_SELECTED ) + // Both (or neither) checked means "show everything" + if (wantSelected !== wantNotSelected) { + filteredData = filteredData.filter( + (item) => !!selectedIdSet?.has(item.id) === wantSelected + ) + } } //sort filteredData.sort((a, b) => { + // "None" (third click of the cycle) - fall back to natural order + if (!sortField) { + return a.index - b.index + } + + if (sortField === SELECTED_SORT_KEY) { + const aSelected = selectedIdSet?.has(a.id) ? 1 : 0 + const bSelected = selectedIdSet?.has(b.id) ? 1 : 0 + // Ascending (the default on first click) puts selected rows + // first, since that's what a user clicking this is after. + return sortDirection === ASCENDING + ? bSelected - aSelected + : aSelected - bSelected + } + const aVal = a[sortField] const bVal = b[sortField] @@ -465,16 +519,23 @@ export const useTableData = ({ globalSearch, sortField, sortDirection, - showOnlySelected, + selectionFilter, selectedIdSet, ]) // EE layers and event layers may be loading additional data - const isLoading = - (layerType === EARTH_ENGINE_LAYER && - aggregationType?.length && - (!aggregations || aggregations === EMPTY_AGGREGATIONS)) || - (layerType === EVENT_LAYER && !layer.isExtended && !serverCluster) + const isLoadingAggregations = + layerType === EARTH_ENGINE_LAYER && + aggregationType?.length && + (!aggregations || aggregations === EMPTY_AGGREGATIONS) + const isExtendingEvents = + layerType === EVENT_LAYER && !layer.isExtended && !serverCluster + const isLoading = isLoadingAggregations || isExtendingEvents + const loadingReason = isLoadingAggregations + ? i18n.t('Loading Earth Engine data…') + : isExtendingEvents + ? i18n.t('Loading additional events…') + : null const totalCount = dataWithAggregations?.length ?? 0 const filteredCount = rows?.length ?? 0 @@ -483,6 +544,7 @@ export const useTableData = ({ headers, rows, isLoading, + loadingReason, error: getErrorCodeText(errorCode.current), totalCount, filteredCount, diff --git a/src/components/map/Map.jsx b/src/components/map/Map.jsx index 334080146..3358c2f28 100644 --- a/src/components/map/Map.jsx +++ b/src/components/map/Map.jsx @@ -57,10 +57,10 @@ class Map extends Component { nameProperty: PropTypes.string, resizeCount: PropTypes.number, selection: PropTypes.object, + selectionFilter: PropTypes.array, setAggregations: PropTypes.func, setFeatureProfile: PropTypes.func, setMapObject: PropTypes.func, - showOnlySelected: PropTypes.bool, toggleFeatureSelection: PropTypes.func, zoom: PropTypes.number, } @@ -185,7 +185,7 @@ class Map extends Component { selection, highlightFeature, highlightColor, - showOnlySelected, + selectionFilter, clickFeature, toggleFeatureSelection, coordinatePopup: coordinates, @@ -237,7 +237,7 @@ class Map extends Component { selection={selection} highlightFeature={highlightFeature} highlightColor={highlightColor} - showOnlySelected={showOnlySelected} + selectionFilter={selectionFilter} clickFeature={clickFeature} toggleFeatureSelection={ toggleFeatureSelection diff --git a/src/components/map/MapContainer.jsx b/src/components/map/MapContainer.jsx index 97a5acda7..6db6795fe 100644 --- a/src/components/map/MapContainer.jsx +++ b/src/components/map/MapContainer.jsx @@ -24,7 +24,7 @@ const MapContainer = ({ resizeCount, setMap }) => { ) const feature = useSelector((state) => state.feature) const selection = useSelector((state) => state.selection) - const { layersSorting, highlightColor, showOnlySelected } = useSelector( + const { layersSorting, highlightColor, selectionFilter } = useSelector( (state) => state.ui ) const basemapConfig = useBasemapConfig(basemap) @@ -52,7 +52,7 @@ const MapContainer = ({ resizeCount, setMap }) => { feature={feature} selection={selection} highlightColor={highlightColor} - showOnlySelected={showOnlySelected} + selectionFilter={selectionFilter} highlightFeature={debouncedHighlightFeature} clickFeature={(payload) => dispatch(clickFeature(payload))} toggleFeatureSelection={(id, layerId) => diff --git a/src/components/map/MapView.jsx b/src/components/map/MapView.jsx index 92a562417..53e354b3d 100644 --- a/src/components/map/MapView.jsx +++ b/src/components/map/MapView.jsx @@ -20,7 +20,7 @@ const MapView = (props) => { selection, highlightFeature, highlightColor, - showOnlySelected, + selectionFilter, clickFeature, toggleFeatureSelection, bounds, @@ -66,7 +66,7 @@ const MapView = (props) => { selection={selection} highlightFeature={highlightFeature} highlightColor={highlightColor} - showOnlySelected={showOnlySelected} + selectionFilter={selectionFilter} clickFeature={clickFeature} toggleFeatureSelection={toggleFeatureSelection} interpretationModalOpen={interpretationModalOpen} @@ -87,7 +87,7 @@ const MapView = (props) => { selection={selection} highlightFeature={highlightFeature} highlightColor={highlightColor} - showOnlySelected={showOnlySelected} + selectionFilter={selectionFilter} clickFeature={clickFeature} toggleFeatureSelection={toggleFeatureSelection} coordinatePopup={coordinatePopup} @@ -124,8 +124,8 @@ MapView.propTypes = { openContextMenu: PropTypes.func, resizeCount: PropTypes.number, selection: PropTypes.object, + selectionFilter: PropTypes.array, setMapObject: PropTypes.func, - showOnlySelected: PropTypes.bool, toggleFeatureSelection: PropTypes.func, } diff --git a/src/components/map/SplitView.jsx b/src/components/map/SplitView.jsx index c6cfab5e7..0a1f13bcd 100644 --- a/src/components/map/SplitView.jsx +++ b/src/components/map/SplitView.jsx @@ -16,7 +16,7 @@ const SplitView = ({ selection, highlightFeature, highlightColor, - showOnlySelected, + selectionFilter, clickFeature, toggleFeatureSelection, controls, @@ -101,7 +101,7 @@ const SplitView = ({ selection={selection} highlightFeature={highlightFeature} highlightColor={highlightColor} - showOnlySelected={showOnlySelected} + selectionFilter={selectionFilter} clickFeature={clickFeature} toggleFeatureSelection={toggleFeatureSelection} openContextMenu={openContextMenu} @@ -136,8 +136,8 @@ SplitView.propTypes = { layersSorting: PropTypes.bool, resizeCount: PropTypes.number, selection: PropTypes.object, + selectionFilter: PropTypes.array, setMapObject: PropTypes.func, - showOnlySelected: PropTypes.bool, toggleFeatureSelection: PropTypes.func, } diff --git a/src/components/map/layers/Layer.js b/src/components/map/layers/Layer.js index ab8c8a13f..a274a071b 100644 --- a/src/components/map/layers/Layer.js +++ b/src/components/map/layers/Layer.js @@ -7,6 +7,10 @@ import { PADDING_DEFAULT, DURATION_DEFAULT, } from '../../../constants/layers.js' +import { + SELECTION_FILTER_SELECTED, + SELECTION_FILTER_NOT_SELECTED, +} from '../../../constants/selection.js' export const idsEqual = (a, b) => a.length === b.length && a.every((id, i) => id === b[i]) @@ -33,7 +37,7 @@ class Layer extends PureComponent { opacity: PropTypes.number, openContextMenu: PropTypes.func, selection: PropTypes.object, - showOnlySelected: PropTypes.bool, + selectionFilter: PropTypes.array, toggleFeatureSelection: PropTypes.func, } @@ -145,14 +149,14 @@ class Layer extends PureComponent { } handleVisibleIdsChange(prevProps) { - const { selection, showOnlySelected } = this.props + const { selection, selectionFilter } = this.props if ( !idsEqual( this.getVisibleIds( prevProps.selection, - prevProps.showOnlySelected + prevProps.selectionFilter ) ?? [], - this.getVisibleIds(selection, showOnlySelected) ?? [] + this.getVisibleIds(selection, selectionFilter) ?? [] ) ) { this.updateVisibleIds() @@ -286,12 +290,33 @@ class Layer extends PureComponent { getVisibleIds( selection = this.props.selection, - showOnlySelected = this.props.showOnlySelected + selectionFilter = this.props.selectionFilter ) { - if (!showOnlySelected || selection?.layerId !== this.props.id) { + if (!selectionFilter?.length || selection?.layerId !== this.props.id) { + return null + } + + const wantSelected = selectionFilter.includes(SELECTION_FILTER_SELECTED) + const wantNotSelected = selectionFilter.includes( + SELECTION_FILTER_NOT_SELECTED + ) + + // Both (or neither, though that's already handled above) checked + // means "show everything" - same as no filter at all. + if (wantSelected === wantNotSelected) { return null } - return this.getSelectedIds(selection) + + const selectedIds = this.getSelectedIds(selection) + + if (wantSelected) { + return selectedIds + } + + const selectedIdSet = new Set(selectedIds) + return (this.props.data ?? []) + .map((feature) => feature.properties?.id ?? feature.id) + .filter((id) => id != null && !selectedIdSet.has(id)) } updateVisibleIds() { diff --git a/src/components/map/layers/__tests__/Layer.spec.js b/src/components/map/layers/__tests__/Layer.spec.js new file mode 100644 index 000000000..bb5d18d1f --- /dev/null +++ b/src/components/map/layers/__tests__/Layer.spec.js @@ -0,0 +1,69 @@ +import Layer from '../Layer.js' + +// Layer's constructor touches this.context.map (only available once mounted +// by React), but getVisibleIds/getSelectedIds only read this.props, so an +// un-constructed instance (no context, no maps-gl layer) is enough to test +// this method in isolation. +const createLayer = (props) => { + const instance = Object.create(Layer.prototype) + instance.props = props + return instance +} + +describe('Layer#getVisibleIds', () => { + const data = [ + { properties: { id: 'a' } }, + { properties: { id: 'b' } }, + { properties: { id: 'c' } }, + ] + + test('returns null (show everything) when selectionFilter is empty', () => { + const layer = createLayer({ + id: 'layer1', + data, + selection: { layerId: 'layer1', ids: ['a'] }, + selectionFilter: [], + }) + expect(layer.getVisibleIds()).toBe(null) + }) + + test('returns null when the selection belongs to a different layer', () => { + const layer = createLayer({ + id: 'layer1', + data, + selection: { layerId: 'other-layer', ids: ['a'] }, + selectionFilter: ['selected'], + }) + expect(layer.getVisibleIds()).toBe(null) + }) + + test('returns only the selected ids when filtered to "selected"', () => { + const layer = createLayer({ + id: 'layer1', + data, + selection: { layerId: 'layer1', ids: ['a', 'c'] }, + selectionFilter: ['selected'], + }) + expect(layer.getVisibleIds()).toEqual(['a', 'c']) + }) + + test('returns only the non-selected ids when filtered to "not-selected"', () => { + const layer = createLayer({ + id: 'layer1', + data, + selection: { layerId: 'layer1', ids: ['a'] }, + selectionFilter: ['not-selected'], + }) + expect(layer.getVisibleIds()).toEqual(['b', 'c']) + }) + + test('returns null (show everything) when both options are checked', () => { + const layer = createLayer({ + id: 'layer1', + data, + selection: { layerId: 'layer1', ids: ['a'] }, + selectionFilter: ['selected', 'not-selected'], + }) + expect(layer.getVisibleIds()).toBe(null) + }) +}) diff --git a/src/constants/actionTypes.js b/src/constants/actionTypes.js index 7b43834cd..0fbf8c5ce 100644 --- a/src/constants/actionTypes.js +++ b/src/constants/actionTypes.js @@ -42,8 +42,7 @@ export const DATA_TABLE_TOGGLE = 'DATA_TABLE_TOGGLE' 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' -export const TOGGLE_SHOW_ONLY_SELECTED = 'TOGGLE_SHOW_ONLY_SELECTED' -export const SHOW_ONLY_SELECTED_SET = 'SHOW_ONLY_SELECTED_SET' +export const SELECTION_FILTER_SET = 'SELECTION_FILTER_SET' export const HIGHLIGHT_COLOR_SET = 'HIGHLIGHT_COLOR_SET' export const MAP_FEATURE_CLICKED = 'MAP_FEATURE_CLICKED' diff --git a/src/constants/selection.js b/src/constants/selection.js new file mode 100644 index 000000000..1be35537b --- /dev/null +++ b/src/constants/selection.js @@ -0,0 +1,4 @@ +// Values for state.ui.selectionFilter - controls which features are shown +// (in both the data table and on the map) based on their selection state. +export const SELECTION_FILTER_SELECTED = 'selected' +export const SELECTION_FILTER_NOT_SELECTED = 'not-selected' diff --git a/src/reducers/__tests__/ui.spec.js b/src/reducers/__tests__/ui.spec.js index 70adf262a..e3e53fb76 100644 --- a/src/reducers/__tests__/ui.spec.js +++ b/src/reducers/__tests__/ui.spec.js @@ -26,29 +26,18 @@ describe('ui reducer — highlightColor', () => { }) }) -describe('ui reducer — showOnlySelected', () => { - it('defaults to false', () => { - expect(ui(undefined, {}).showOnlySelected).toBe(false) +describe('ui reducer — selectionFilter', () => { + it('defaults to an empty array', () => { + expect(ui(undefined, {}).selectionFilter).toEqual([]) }) - it('toggles on TOGGLE_SHOW_ONLY_SELECTED', () => { - const state = ui(undefined, { type: types.TOGGLE_SHOW_ONLY_SELECTED }) - expect(state.showOnlySelected).toBe(true) - - const toggledBack = ui(state, { - type: types.TOGGLE_SHOW_ONLY_SELECTED, - }) - expect(toggledBack.showOnlySelected).toBe(false) - }) - - it('sets an explicit value on SHOW_ONLY_SELECTED_SET', () => { - const prevState = { ...ui(undefined, {}), showOnlySelected: true } - const state = ui(prevState, { - type: types.SHOW_ONLY_SELECTED_SET, - value: false, + it('sets an explicit value on SELECTION_FILTER_SET', () => { + const state = ui(undefined, { + type: types.SELECTION_FILTER_SET, + value: ['selected'], }) - expect(state.showOnlySelected).toBe(false) + expect(state.selectionFilter).toEqual(['selected']) }) it.each([ @@ -56,11 +45,14 @@ describe('ui reducer — showOnlySelected', () => { types.MAP_SET, types.DATA_TABLE_CLOSE, types.DATA_TABLE_TOGGLE, - ])('resets to false on %s', (type) => { - const prevState = { ...ui(undefined, {}), showOnlySelected: true } + ])('resets to an empty array on %s', (type) => { + const prevState = { + ...ui(undefined, {}), + selectionFilter: ['selected'], + } const state = ui(prevState, { type }) - expect(state.showOnlySelected).toBe(false) + expect(state.selectionFilter).toEqual([]) }) }) diff --git a/src/reducers/ui.js b/src/reducers/ui.js index b5d4ca63d..45fb37be3 100644 --- a/src/reducers/ui.js +++ b/src/reducers/ui.js @@ -11,7 +11,7 @@ const defaultState = { layersSorting: false, mapBounds: null, showOnlyFeaturesInView: false, - showOnlySelected: false, + selectionFilter: [], highlightColor: null, lastClickedFeature: null, } @@ -51,7 +51,7 @@ const ui = (state = defaultState, action) => { return { ...state, rightPanelOpen: false, - showOnlySelected: false, + selectionFilter: [], lastClickedFeature: null, } @@ -59,7 +59,7 @@ const ui = (state = defaultState, action) => { case types.DATA_TABLE_TOGGLE: return { ...state, - showOnlySelected: false, + selectionFilter: [], } case types.DOWNLOAD_MODE_OPEN: @@ -103,16 +103,10 @@ const ui = (state = defaultState, action) => { showOnlyFeaturesInView: !state.showOnlyFeaturesInView, } - case types.TOGGLE_SHOW_ONLY_SELECTED: + case types.SELECTION_FILTER_SET: return { ...state, - showOnlySelected: !state.showOnlySelected, - } - - case types.SHOW_ONLY_SELECTED_SET: - return { - ...state, - showOnlySelected: action.value, + selectionFilter: action.value, } case types.HIGHLIGHT_COLOR_SET: diff --git a/src/util/__tests__/filter.spec.js b/src/util/__tests__/filter.spec.js index d2c4315b3..c2490ea94 100644 --- a/src/util/__tests__/filter.spec.js +++ b/src/util/__tests__/filter.spec.js @@ -1,4 +1,4 @@ -import { filterByGlobalSearch, filterData } from '../filter.js' +import { filterByGlobalSearch, filterData, ANY_VALUE_KEY } from '../filter.js' describe('filterData', () => { it('should return the original data if no filters are provided', () => { @@ -96,6 +96,18 @@ describe('filterData', () => { const filters = { a: ['High'], b: 'horse' } expect(filterData(data, filters)).toEqual([{ a: 'High', b: 'horse' }]) }) + + it('ANY_VALUE_KEY matches every row with a non-blank value, the opposite of the blank sentinel', () => { + const data = [{ a: 'High' }, { a: '' }, { a: null }, { a: 'Low' }] + const filters = { a: [ANY_VALUE_KEY] } + expect(filterData(data, filters)).toEqual([{ a: 'High' }, { a: 'Low' }]) + }) + + it('combining ANY_VALUE_KEY with the blank sentinel ("") matches every row', () => { + const data = [{ a: 'High' }, { a: '' }, { a: null }] + const filters = { a: [ANY_VALUE_KEY, ''] } + expect(filterData(data, filters)).toEqual(data) + }) }) describe('filterByGlobalSearch', () => { diff --git a/src/util/filter.js b/src/util/filter.js index 4faa3c85a..1c1d82342 100644 --- a/src/util/filter.js +++ b/src/util/filter.js @@ -1,3 +1,10 @@ +// Pseudo-value for multi-select filters meaning "the field has any +// non-blank value" - the logical opposite of selecting the blank-cell +// sentinel (''). Works generically even for columns with too many distinct +// values to list individually, since it's a predicate ("is it blank or +// not?"), not a membership check against a known list of values. +export const ANY_VALUE_KEY = '__any_value__' + // Filters an array of object with a set of filters export const filterData = (data, filters) => { if (!filters) { @@ -17,9 +24,11 @@ export const filterData = (data, filters) => { if (Array.isArray(filter)) { // Multi-select: OR match against the raw stored value + const stringValue = value == null ? '' : String(value) return ( filter.length === 0 || - filter.includes(value == null ? '' : String(value)) + filter.includes(stringValue) || + (stringValue !== '' && filter.includes(ANY_VALUE_KEY)) ) } From 51feb4fa09b844925624c44cc9384f89d4874a20 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Tue, 14 Jul 2026 22:35:47 +0200 Subject: [PATCH 13/27] revert: remove legend-click-to-filter, deferred to a future PR Reverts c2b5aac0. The thematic-only implementation is being pulled out of this PR - the feature should work across bubble, event, facility, and org-unit layers too, which needs its own investigation and belongs in a separate PR rather than this one. --- .../layers/overlays/OverlayCard.jsx | 54 +-------- .../overlays/__tests__/OverlayCard.spec.jsx | 111 ------------------ src/components/legend/Legend.jsx | 10 -- src/components/legend/LegendItem.jsx | 26 +--- .../legend/styles/LegendItem.module.css | 21 ---- 5 files changed, 3 insertions(+), 219 deletions(-) diff --git a/src/components/layers/overlays/OverlayCard.jsx b/src/components/layers/overlays/OverlayCard.jsx index e8940790a..a490f28cd 100644 --- a/src/components/layers/overlays/OverlayCard.jsx +++ b/src/components/layers/overlays/OverlayCard.jsx @@ -5,7 +5,6 @@ import i18n from '@dhis2/d2-i18n' import PropTypes from 'prop-types' import React, { useState } from 'react' import { connect } from 'react-redux' -import { setDataFilter, clearDataFilter } from '../../../actions/dataFilters.js' import { toggleDataTable } from '../../../actions/dataTable.js' import { editLayer, @@ -25,7 +24,6 @@ import { DATA_TABLE_LAYER_TYPES, OPEN_AS_LAYER_TYPES, EXTERNAL_LAYER, - THEMATIC_LAYER, } from '../../../constants/layers.js' import { getAnalyticalObjectFromThematicLayer, @@ -47,9 +45,6 @@ const OverlayCard = ({ toggleLayerExpand, toggleLayerVisibility, toggleDataTable, - setDataFilter, - clearDataFilter, - activeDataTableLayerId, }) => { const [showDataDownloadDialog, setShowDataDownloadDialog] = useState(false) const { baseUrl } = useConfig() @@ -66,37 +61,12 @@ 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 canFilterByLegend = layerType === THEMATIC_LAYER - - const onLegendItemClick = (item) => { - if (!item?.name) { - return - } - const currentLegendFilter = Array.isArray(dataFilters?.legend) - ? dataFilters.legend - : [] - const isActive = currentLegendFilter.includes(item.name) - const nextLegendFilter = isActive - ? currentLegendFilter.filter((n) => n !== item.name) - : [...currentLegendFilter, item.name] - - if (nextLegendFilter.length) { - setDataFilter(id, 'legend', nextLegendFilter) - } else { - clearDataFilter(id, 'legend') - } - - if (activeDataTableLayerId !== id) { - toggleDataTable(id) - } - } const getCardContent = () => { if (loadError) { @@ -114,18 +84,7 @@ const OverlayCard = ({ return ( legend && (
    - +
    ) ) @@ -197,23 +156,16 @@ const OverlayCard = ({ OverlayCard.propTypes = { changeLayerOpacity: PropTypes.func.isRequired, - clearDataFilter: PropTypes.func.isRequired, duplicateLayer: PropTypes.func.isRequired, editLayer: PropTypes.func.isRequired, layer: PropTypes.object.isRequired, removeLayer: PropTypes.func.isRequired, - setDataFilter: PropTypes.func.isRequired, toggleDataTable: PropTypes.func.isRequired, toggleLayerExpand: PropTypes.func.isRequired, toggleLayerVisibility: PropTypes.func.isRequired, - activeDataTableLayerId: PropTypes.string, } -const mapStateToProps = (state) => ({ - activeDataTableLayerId: state.dataTable, -}) - -export default connect(mapStateToProps, { +export default connect(null, { editLayer, removeLayer, duplicateLayer, @@ -221,6 +173,4 @@ export default connect(mapStateToProps, { toggleLayerExpand, toggleLayerVisibility, toggleDataTable, - setDataFilter, - clearDataFilter, })(OverlayCard) diff --git a/src/components/layers/overlays/__tests__/OverlayCard.spec.jsx b/src/components/layers/overlays/__tests__/OverlayCard.spec.jsx index c70cce431..4da6e6ae8 100644 --- a/src/components/layers/overlays/__tests__/OverlayCard.spec.jsx +++ b/src/components/layers/overlays/__tests__/OverlayCard.spec.jsx @@ -25,19 +25,6 @@ jest.mock('@dhis2/app-service-alerts', () => ({ useAlert: () => ({ show: mockShow }), })) -jest.mock('../../../cachedDataProvider/CachedDataProvider.jsx', () => ({ - useCachedData: jest.fn(() => ({ - systemSettings: { keyAnalysisDigitGroupSeparator: 'NONE' }, - })), -})) - -// jsdom has no ResizeObserver; Legend.jsx uses one to measure overflow, which -// is irrelevant to the legend-click wiring under test here. -global.ResizeObserver = class { - observe() {} - disconnect() {} -} - const mockStore = configureMockStore() describe('OverlayCard', () => { @@ -72,101 +59,3 @@ describe('OverlayCard', () => { }) }) }) - -describe('OverlayCard legend-driven filter', () => { - const layer = { - id: 'layer1', - name: 'Test layer', - layer: 'thematic', - isLoaded: true, - isExpanded: true, - isVisible: true, - opacity: 1, - dataFilters: {}, - legend: { - items: [ - { name: 'High', color: '#ff0000' }, - { name: 'Low', color: '#00ff00' }, - ], - }, - } - - const renderCard = (store) => - render( - - - - ) - - test('opens the table and sets the legend filter on click when the table is closed', () => { - const store = mockStore({ dataTable: null, aggregations: {} }) - renderCard(store) - - fireEvent.click(screen.getByText('High')) - - const actions = store.getActions() - expect(actions).toContainEqual({ - type: 'DATA_FILTER_SET', - layerId: 'layer1', - fieldId: 'legend', - filter: ['High'], - }) - expect(actions).toContainEqual({ - type: 'DATA_TABLE_TOGGLE', - id: 'layer1', - }) - }) - - test('adds to the filter without re-toggling the table when it is already open for this layer', () => { - const store = mockStore({ dataTable: 'layer1', aggregations: {} }) - renderCard(store) - - fireEvent.click(screen.getByText('Low')) - - const actions = store.getActions() - expect(actions).toContainEqual({ - type: 'DATA_FILTER_SET', - layerId: 'layer1', - fieldId: 'legend', - filter: ['Low'], - }) - expect(actions).not.toContainEqual( - expect.objectContaining({ type: 'DATA_TABLE_TOGGLE' }) - ) - }) - - test('clears the filter when clicking an already-active legend class', () => { - const activeLayer = { - ...layer, - dataFilters: { legend: ['High'] }, - } - const store = mockStore({ dataTable: 'layer1', aggregations: {} }) - render( - - - - ) - - fireEvent.click(screen.getByText('High')) - - expect(store.getActions()).toContainEqual({ - type: 'DATA_FILTER_CLEAR', - layerId: 'layer1', - fieldId: 'legend', - }) - }) - - test('does not wire legend clicks for non-thematic layers', () => { - const facilityLayer = { ...layer, layer: 'facility' } - const store = mockStore({ dataTable: null, aggregations: {} }) - render( - - - - ) - - fireEvent.click(screen.getByText('High')) - - expect(store.getActions()).toEqual([]) - }) -}) diff --git a/src/components/legend/Legend.jsx b/src/components/legend/Legend.jsx index 9bd4793a3..ae7603787 100644 --- a/src/components/legend/Legend.jsx +++ b/src/components/legend/Legend.jsx @@ -98,8 +98,6 @@ const Legend = ({ orgUnitsWithoutCoordinatesCount, orgUnitsPointOnly = false, isPlugin = false, - onItemClick, - activeLegendNames, }) => { const { systemSettings: { keyAnalysisDigitGroupSeparator }, @@ -298,12 +296,6 @@ const Legend = ({ isPlugin={isPlugin} suppressRange={suppressAllRanges} forceScientific={forceScientific} - onClick={onItemClick ? () => onItemClick(item) : undefined} - isActive={ - !!activeLegendNames && - !!item.name && - activeLegendNames.includes(item.name) - } key={`${item.name ?? ''}-${item.startValue ?? ''}-${ item.endValue ?? '' }-${index}`} @@ -398,7 +390,6 @@ const Legend = ({ } Legend.propTypes = { - activeLegendNames: PropTypes.array, bubbles: PropTypes.shape({ radiusHigh: PropTypes.number.isRequired, radiusLow: PropTypes.number.isRequired, @@ -423,7 +414,6 @@ Legend.propTypes = { sourceUrl: PropTypes.string, unit: PropTypes.string, url: PropTypes.string, - onItemClick: PropTypes.func, } export default Legend diff --git a/src/components/legend/LegendItem.jsx b/src/components/legend/LegendItem.jsx index b2b9bb866..fc66495d8 100644 --- a/src/components/legend/LegendItem.jsx +++ b/src/components/legend/LegendItem.jsx @@ -1,4 +1,3 @@ -import cx from 'classnames' import PropTypes from 'prop-types' import React from 'react' import LegendItemRange from './LegendItemRange.jsx' @@ -27,8 +26,6 @@ const LegendItem = ({ isPlugin, suppressRange, forceScientific, - onClick, - isActive, }) => { if (!name && startValue === undefined && endValue === undefined) { return null @@ -54,26 +51,7 @@ const LegendItem = ({ const lineWeight = weight ? Math.min(weight, maxLineWeight) : null return ( - { - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault() - onClick() - } - } - : undefined - } - > + {weight ? ( type === 'LineString' ? ( @@ -113,7 +91,6 @@ LegendItem.propTypes = { fillColor: PropTypes.string, forceScientific: PropTypes.bool, image: PropTypes.string, - isActive: PropTypes.bool, isPlugin: PropTypes.bool, name: PropTypes.string, radius: PropTypes.number, @@ -124,7 +101,6 @@ LegendItem.propTypes = { type: PropTypes.string, useCompact: PropTypes.bool, weight: PropTypes.number, - onClick: PropTypes.func, } export default LegendItem diff --git a/src/components/legend/styles/LegendItem.module.css b/src/components/legend/styles/LegendItem.module.css index ffc6ff384..fdc4d8aa7 100644 --- a/src/components/legend/styles/LegendItem.module.css +++ b/src/components/legend/styles/LegendItem.module.css @@ -23,24 +23,3 @@ print-color-adjust: exact; /* Firefox */ } - -.clickable { - cursor: pointer; -} - -.clickable:hover { - background-color: var(--colors-grey100); -} - -.clickable:focus-visible { - outline: 2px solid var(--colors-blue600); - outline-offset: -2px; -} - -.active { - background-color: var(--colors-blue050); -} - -.active:hover { - background-color: var(--colors-blue100); -} From c0716a638c787e02c1282c85c23775910fcd9116 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Tue, 14 Jul 2026 23:01:52 +0200 Subject: [PATCH 14/27] chore: sonarqube issues --- i18n/en.pot | 10 +- src/components/datatable/DataTable.jsx | 63 ++++--- src/components/datatable/FilterInput.jsx | 165 +++++++++++------ .../datatable/styles/DataTable.module.css | 8 +- .../datatable/styles/FilterInput.module.css | 18 +- src/components/datatable/useTableData.js | 168 ++++++++++-------- 6 files changed, 247 insertions(+), 185 deletions(-) diff --git a/i18n/en.pot b/i18n/en.pot index a0954b833..eb9354f37 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-07-14T19:48:56.770Z\n" -"PO-Revision-Date: 2026-07-14T19:48:56.771Z\n" +"POT-Creation-Date: 2026-07-14T20:24:31.895Z\n" +"PO-Revision-Date: 2026-07-14T20:24:31.895Z\n" msgid "2020" msgstr "2020" @@ -205,12 +205,12 @@ msgstr "No results found" msgid "Select all" msgstr "Select all" -msgid "Sort by Selected" -msgstr "Sort by Selected" - msgid "Reverse selection" msgstr "Reverse selection" +msgid "Sort by Selected" +msgstr "Sort by Selected" + msgid "Sort by {{column}}" msgstr "Sort by {{column}}" diff --git a/src/components/datatable/DataTable.jsx b/src/components/datatable/DataTable.jsx index 356b2d7b2..724370111 100644 --- a/src/components/datatable/DataTable.jsx +++ b/src/components/datatable/DataTable.jsx @@ -241,6 +241,35 @@ export const getRowClickAction = ( return null } +// Thematic layers merge their legend name + color into one swatch+name +// cell (see hasLegendColorPair); every other column is either the raw +// color's own cell (lowercased hex) or a plain formatted value. +const getCellContent = ({ + isLegendCell, + swatchColor, + value, + dataKey, + keyAnalysisDigitGroupSeparator, +}) => { + if (isLegendCell) { + return ( + + {swatchColor && ( + + )} + {value} + + ) + } + if (dataKey === 'color') { + return value?.toLowerCase() + } + return formatWithSeparator(value, keyAnalysisDigitGroupSeparator) +} + const DataTableWithVirtuosoContext = ({ context, ...props }) => ( - {isLegendCell ? ( - - {swatchColor && ( - - )} - {value} - - ) : dataKey === 'color' ? ( - value?.toLowerCase() - ) : ( - formatWithSeparator( - value, - keyAnalysisDigitGroupSeparator - ) - )} + {getCellContent({ + isLegendCell, + swatchColor, + value, + dataKey, + keyAnalysisDigitGroupSeparator, + })} ) })} diff --git a/src/components/datatable/FilterInput.jsx b/src/components/datatable/FilterInput.jsx index b20408c8f..2556c2964 100644 --- a/src/components/datatable/FilterInput.jsx +++ b/src/components/datatable/FilterInput.jsx @@ -120,11 +120,13 @@ const FilterHelpTooltip = ({ ) const referenceRect = referenceRef.current?.getBoundingClientRect() - const spaceAvailable = referenceRect - ? placement === 'top' - ? referenceRect.top - : window.innerHeight - referenceRect.bottom - : Infinity + let spaceAvailable = Infinity + if (referenceRect) { + spaceAvailable = + placement === 'top' + ? referenceRect.top + : window.innerHeight - referenceRect.bottom + } const hasRoom = spaceAvailable >= estimatedHeight return ( @@ -208,6 +210,66 @@ FilterDropdownPopover.propTypes = { className: PropTypes.string, } +// See onToggleAnyValue - turning "Any value" on collapses any individually +// -picked real values into just the wildcard, off unticks every real value +// along with it. "No value" carries through untouched either way. +const getAnyValueToggleResult = (anyValueActive, keepNotSet) => { + if (anyValueActive) { + return keepNotSet ? [NOT_SET_VALUE] : [] + } + return keepNotSet ? [ANY_VALUE_KEY, NOT_SET_VALUE] : [ANY_VALUE_KEY] +} + +// See onToggleRealValue - once every real value ends up ticked, that's the +// same state as "Any value" being active, so it collapses into the wildcard +// rather than a literal array that happens to list them all. +const getRealValueToggleResult = (next, allRealValuesChecked) => { + if (!allRealValuesChecked) { + return next + } + return next.includes(NOT_SET_VALUE) + ? [ANY_VALUE_KEY, NOT_SET_VALUE] + : [ANY_VALUE_KEY] +} + +// See "Numeric columns narrow..." comment at the filteredOptions call site - +// numeric columns match the typed text as a numericFilter expression, +// string columns match it as a case-insensitive substring of the resolved +// label. +const getFilteredOptions = ({ + realOptions, + trimmedSearch, + normalizedSearch, + type, + resolveLabel, +}) => { + if (!trimmedSearch) { + return realOptions + } + if (type === 'number') { + return realOptions.filter(({ value }) => + numericFilter(Number(value), trimmedSearch) + ) + } + return realOptions.filter(({ value }) => + resolveLabel(value).toLowerCase().includes(normalizedSearch) + ) +} + +// Closed, this reads like the old trigger button ("3 selected", the applied +// filter text, or empty so the "Search" placeholder shows). Open, it's a +// live, editable search/filter field - the same input serves both roles +// instead of a button revealing a separate one. +const getDisplayValue = ({ isOpen, searchText, selected, appliedString }) => { + if (isOpen) { + return searchText + } + if (selected.length) { + return i18n.t('{{count}} selected', { count: selected.length }) + } + return appliedString +} + // Shared popover UI — label resolution is injected so it never needs to // know whether it's an option-set column or a plain categorical one. // State is derived straight from the applied `filterValue` (never tracked @@ -297,15 +359,7 @@ const SearchableFilterPopover = ({ // "No value" is independent either way and carries over untouched. const onToggleAnyValue = () => { const keepNotSet = selected.includes(NOT_SET_VALUE) - applyValues( - anyValueActive - ? keepNotSet - ? [NOT_SET_VALUE] - : [] - : keepNotSet - ? [ANY_VALUE_KEY, NOT_SET_VALUE] - : [ANY_VALUE_KEY] - ) + applyValues(getAnyValueToggleResult(anyValueActive, keepNotSet)) } // Every value "Reverse selection" can flip - the column's full value @@ -350,13 +404,7 @@ const SearchableFilterPopover = ({ const allRealValuesChecked = realOptions.length > 0 && realOptions.every((o) => next.includes(o.value)) - applyValues( - allRealValuesChecked - ? next.includes(NOT_SET_VALUE) - ? [ANY_VALUE_KEY, NOT_SET_VALUE] - : [ANY_VALUE_KEY] - : next - ) + applyValues(getRealValueToggleResult(next, allRealValuesChecked)) } // Reverses every checkbox's *effective* ticked state, not just literal @@ -407,15 +455,13 @@ const SearchableFilterPopover = ({ // text would apply to the table's rows (>, <, ranges, ...), so what's // checked here always matches what "Use filter" would actually select - // a plain substring match wouldn't understand "> 100" against "150". - const filteredOptions = !trimmedSearch - ? realOptions - : type === 'number' - ? realOptions.filter(({ value }) => - numericFilter(Number(value), trimmedSearch) - ) - : realOptions.filter(({ value }) => - resolveLabel(value).toLowerCase().includes(normalizedSearch) - ) + const filteredOptions = getFilteredOptions({ + realOptions, + trimmedSearch, + normalizedSearch, + type, + resolveLabel, + }) const hasExactMatch = filteredOptions.some( ({ value }) => resolveLabel(value).toLowerCase() === normalizedSearch ) @@ -481,6 +527,29 @@ const SearchableFilterPopover = ({ } } + // The custom-filter row (when shown) sits first, matching its visual + // position above the checkbox list. It's usually already applied live + // by this point (see onSearchChange) - toggling it again here is a + // no-op. + const onEnterKey = () => { + if (highlightedIndex === -1) { + if (showCustomFilterRow) { + applyCustomFilter(searchText.trim()) + } + return + } + if (showCustomFilterRow && highlightedIndex === 0) { + applyCustomFilter(searchText.trim()) + return + } + const optionIndex = showCustomFilterRow + ? highlightedIndex - 1 + : highlightedIndex + if (optionIndex >= 0 && optionIndex < filteredOptions.length) { + toggleValue(filteredOptions[optionIndex].value) + } + } + const onSearchKeyDown = (_, event) => { switch (event.key) { case 'ArrowDown': @@ -501,32 +570,11 @@ const SearchableFilterPopover = ({ return next }) break - case 'Enter': { + case 'Enter': event.preventDefault() - // The custom-filter row (when shown) sits first, matching - // its visual position above the checkbox list. It's - // usually already applied live by this point (see - // onSearchChange) - toggling it again here is a no-op. - if (highlightedIndex === -1) { - if (showCustomFilterRow) { - applyCustomFilter(searchText.trim()) - } - } else if (showCustomFilterRow && highlightedIndex === 0) { - applyCustomFilter(searchText.trim()) - } else { - const optionIndex = showCustomFilterRow - ? highlightedIndex - 1 - : highlightedIndex - if ( - optionIndex >= 0 && - optionIndex < filteredOptions.length - ) { - toggleValue(filteredOptions[optionIndex].value) - } - } + onEnterKey() closePopover() break - } case 'Escape': event.preventDefault() closePopover() @@ -540,11 +588,12 @@ const SearchableFilterPopover = ({ // applied filter text, or empty so the "Search" placeholder shows). // Open, it's a live, editable search/filter field - the same input // serves both roles instead of a button revealing a separate one. - const displayValue = isOpen - ? searchText - : selected.length - ? i18n.t('{{count}} selected', { count: selected.length }) - : appliedString + const displayValue = getDisplayValue({ + isOpen, + searchText, + selected, + appliedString, + }) const mainInput = ( { + if (a === NOT_SET_VALUE) { + return -1 + } + if (b === NOT_SET_VALUE) { + return 1 + } + if (type === TYPE_NUMBER) { + return Number(a) - Number(b) + } + if (a < b) { + return -1 + } + if (a > b) { + return 1 + } + return 0 +} + +// Ascending (the default on first click) puts selected rows first, since +// that's what a user clicking this is after. +const compareBySelected = (a, b, { selectedIdSet, sortDirection }) => { + const aSelected = selectedIdSet?.has(a.id) ? 1 : 0 + const bSelected = selectedIdSet?.has(b.id) ? 1 : 0 + return sortDirection === ASCENDING + ? bSelected - aSelected + : aSelected - bSelected +} + +const compareRangeValues = (aVal, bVal, sortDirection) => { + const [aStart, aEnd] = parseRange(aVal) + const [bStart, bEnd] = parseRange(bVal) + const startDiff = + sortDirection === ASCENDING ? aStart - bStart : bStart - aStart + if (startDiff !== 0) { + return startDiff + } + return sortDirection === ASCENDING ? aEnd - bEnd : bEnd - aEnd +} + +const compareFieldValues = (aVal, bVal, { sortField, sortDirection }) => { + // All undefined values should be sorted to the end + if (aVal === undefined && bVal === undefined) { + return 0 + } + if (aVal === undefined) { + return 1 + } + if (bVal === undefined) { + return -1 + } + if (typeof aVal === TYPE_NUMBER) { + return sortDirection === ASCENDING ? aVal - bVal : bVal - aVal + } + if (sortField === RANGE) { + return compareRangeValues(aVal, bVal, sortDirection) + } + // TODO: Make sure sorting works across different locales + return sortDirection === ASCENDING + ? aVal.localeCompare(bVal) + : bVal.localeCompare(aVal) +} + +const compareRows = (a, b, options) => { + const { sortField } = options + // "None" (third click of the cycle) - fall back to natural order + if (!sortField) { + return a.index - b.index + } + if (sortField === SELECTED_SORT_KEY) { + return compareBySelected(a, b, options) + } + return compareFieldValues(a[sortField], b[sortField], options) +} + export const useTableData = ({ layer, sortField, @@ -379,26 +458,7 @@ export const useTableData = ({ if (seen.size > 0) { result[dataKey] = Array.from(seen) - .sort((a, b) => { - if (a === NOT_SET_VALUE) { - return -1 - } - if (b === NOT_SET_VALUE) { - return 1 - } - // Distinct values are stored as strings (they're - // sourced alongside string columns) - sort numeric - // columns by numeric value rather than lexically - // (so 2 sorts before 10), everything else keeps the - // default string ordering. - return type === TYPE_NUMBER - ? Number(a) - Number(b) - : a < b - ? -1 - : a > b - ? 1 - : 0 - }) + .sort((a, b) => compareColumnOptionValues(a, b, type)) .map((value) => ({ value })) } }) @@ -445,60 +505,9 @@ export const useTableData = ({ } //sort - filteredData.sort((a, b) => { - // "None" (third click of the cycle) - fall back to natural order - if (!sortField) { - return a.index - b.index - } - - if (sortField === SELECTED_SORT_KEY) { - const aSelected = selectedIdSet?.has(a.id) ? 1 : 0 - const bSelected = selectedIdSet?.has(b.id) ? 1 : 0 - // Ascending (the default on first click) puts selected rows - // first, since that's what a user clicking this is after. - return sortDirection === ASCENDING - ? bSelected - aSelected - : aSelected - bSelected - } - - const aVal = a[sortField] - const bVal = b[sortField] - - // All undefined values should be sorted to the end - if (aVal === undefined && bVal === undefined) { - return 0 - } - - if (aVal === undefined) { - return 1 // aVal goes to end - } - - if (bVal === undefined) { - return -1 // bVal goes to end - } - - if (typeof aVal === TYPE_NUMBER) { - return sortDirection === ASCENDING ? aVal - bVal : bVal - aVal - } - - if (sortField === RANGE) { - const [aStart, aEnd] = parseRange(aVal) - const [bStart, bEnd] = parseRange(bVal) - const startDiff = - sortDirection === ASCENDING - ? aStart - bStart - : bStart - aStart - if (startDiff !== 0) { - return startDiff - } - return sortDirection === ASCENDING ? aEnd - bEnd : bEnd - aEnd - } - - // TODO: Make sure sorting works across different locales - return sortDirection === ASCENDING - ? aVal.localeCompare(bVal) - : bVal.localeCompare(aVal) - }) + filteredData.sort((a, b) => + compareRows(a, b, { sortField, sortDirection, selectedIdSet }) + ) return filteredData.map((item) => headers.map(({ dataKey, roundFn, type }) => { @@ -531,11 +540,12 @@ export const useTableData = ({ const isExtendingEvents = layerType === EVENT_LAYER && !layer.isExtended && !serverCluster const isLoading = isLoadingAggregations || isExtendingEvents - const loadingReason = isLoadingAggregations - ? i18n.t('Loading Earth Engine data…') - : isExtendingEvents - ? i18n.t('Loading additional events…') - : null + let loadingReason = null + if (isLoadingAggregations) { + loadingReason = i18n.t('Loading Earth Engine data…') + } else if (isExtendingEvents) { + loadingReason = i18n.t('Loading additional events…') + } const totalCount = dataWithAggregations?.length ?? 0 const filteredCount = rows?.length ?? 0 From 26ded6147a703b9c8e4287de43abdf07800b651f Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Tue, 14 Jul 2026 23:07:39 +0200 Subject: [PATCH 15/27] chore: sonarqube issues --- src/components/datatable/FilterInput.jsx | 65 ++++++++++++------- .../datatable/styles/DataTable.module.css | 23 +++---- 2 files changed, 53 insertions(+), 35 deletions(-) diff --git a/src/components/datatable/FilterInput.jsx b/src/components/datatable/FilterInput.jsx index 2556c2964..f49d77d75 100644 --- a/src/components/datatable/FilterInput.jsx +++ b/src/components/datatable/FilterInput.jsx @@ -270,6 +270,43 @@ const getDisplayValue = ({ isOpen, searchText, selected, appliedString }) => { return appliedString } +// Widget state is derived straight from the applied `filterValue` (never +// tracked in parallel) - an array means a multi-select filter, a string +// means a custom typed one, anything else (no filter applied) is neither. +const getSelectedAndAppliedString = (filterValue) => ({ + selected: Array.isArray(filterValue) ? filterValue : [], + appliedString: typeof filterValue === 'string' ? filterValue : '', +}) + +// Every column's filter trigger sits in the same header row, so this +// resolves to the same answer for all of them - below by default, or above +// if the row doesn't have room to open the dropdown downward (e.g. the +// table is short, or the page is scrolled so the row sits near the bottom +// of the viewport). That single choice is what keeps every column's +// dropdown opening on the same side. The help tooltip always takes the +// opposite side, so the two never compete for space. +const getDropdownPlacement = (anchorRect) => { + const dropdownSide = + anchorRect != null && + window.innerHeight - anchorRect.bottom < ESTIMATED_POPOVER_HEIGHT + ? 'top' + : 'bottom' + return { + dropdownSide, + dropdownPlacement: `${dropdownSide}-start`, + tooltipPlacement: dropdownSide === 'top' ? 'bottom' : 'top', + } +} + +// Every value "Reverse selection" can flip - the column's full value +// domain, not just whatever the current search happens to narrow the list +// down to (search is for finding/toggling individual values, not for +// scoping a bulk action). +const getInvertibleValues = (hasNotSetOption, realOptions) => + hasNotSetOption + ? [NOT_SET_VALUE, ...realOptions.map((o) => o.value)] + : realOptions.map((o) => o.value) + // Shared popover UI — label resolution is injected so it never needs to // know whether it's an option-set column or a plain categorical one. // State is derived straight from the applied `filterValue` (never tracked @@ -293,8 +330,7 @@ const SearchableFilterPopover = ({ const [searchText, setSearchText] = useState('') const [highlightedIndex, setHighlightedIndex] = useState(-1) - const selected = Array.isArray(filterValue) ? filterValue : [] - const appliedString = typeof filterValue === 'string' ? filterValue : '' + const { selected, appliedString } = getSelectedAndAppliedString(filterValue) const openPopover = () => { setSearchText(appliedString) @@ -310,21 +346,8 @@ const SearchableFilterPopover = ({ // table resize, when the popover is closed anyway. const anchorRect = anchorRef.current?.getBoundingClientRect() const anchorWidth = anchorRect?.width - - // Every column's filter trigger sits in the same header row, so this - // resolves to the same answer for all of them - below by default, or - // above if the row doesn't have room to open the dropdown downward - // (e.g. the table is short, or the page is scrolled so the row sits - // near the bottom of the viewport). That single choice is what keeps - // every column's dropdown opening on the same side. The help tooltip - // always takes the opposite side, so the two never compete for space. - const dropdownSide = - anchorRect != null && - window.innerHeight - anchorRect.bottom < ESTIMATED_POPOVER_HEIGHT - ? 'top' - : 'bottom' - const dropdownPlacement = `${dropdownSide}-start` - const tooltipPlacement = dropdownSide === 'top' ? 'bottom' : 'top' + const { dropdownPlacement, dropdownSide, tooltipPlacement } = + getDropdownPlacement(anchorRect) const applyValues = (next) => next.length @@ -362,13 +385,7 @@ const SearchableFilterPopover = ({ applyValues(getAnyValueToggleResult(anyValueActive, keepNotSet)) } - // Every value "Reverse selection" can flip - the column's full value - // domain, not just whatever the current search happens to narrow the - // list down to (search is for finding/toggling individual values, not - // for scoping a bulk action). - const invertibleValues = hasNotSetOption - ? [NOT_SET_VALUE, ...realOptions.map((o) => o.value)] - : realOptions.map((o) => o.value) + const invertibleValues = getInvertibleValues(hasNotSetOption, realOptions) // A real value's checkbox is ticked either because it's individually // selected, or because "Any value" is active (which stands for "every diff --git a/src/components/datatable/styles/DataTable.module.css b/src/components/datatable/styles/DataTable.module.css index e23fd7795..551233a27 100644 --- a/src/components/datatable/styles/DataTable.module.css +++ b/src/components/datatable/styles/DataTable.module.css @@ -3,17 +3,18 @@ } /* @dhis2/ui's Table draws a 1px border on all sides via its own scoped - styled-jsx rule, which our .dataTable class alone can't reliably - out-specificity. Pairing it with the table's stable data-test attribute - (with !important) guarantees this wins, while staying scoped to just this - table via .dataTable (a bare data-test selector would match every - DataTable instance in the app). That border (not just the top side) is - what makes the browser compute the sticky header's normal-flow and stuck - positions on very slightly different subpixel grids, causing it to - visibly snap by roughly 1-2 pixels the instant scrolling engages the - sticky positioning. Removing the table's own border entirely (confirmed - via devtools) removes the snap; row and column delineation still comes - from each cell's own bottom and inline-end borders. */ + styled-jsx rule, which the local dataTable class name alone can't + reliably out-specificity. Pairing that class name with the table's + stable data-test attribute (marked important) guarantees this wins, + while staying scoped to just this table (a bare attribute selector on + its own would match every DataTable instance in the app). That full + border (not just the top side) is what makes the browser compute the + sticky header's normal-flow and stuck positions on very slightly + different subpixel grids, causing a visible snap of roughly 1-2 pixels + the instant scrolling engages sticky positioning. Removing the table's + own border entirely (confirmed via devtools) removes the snap; row and + column delineation still comes from each cell's own bottom and + inline-end borders. */ .dataTable[data-test='dhis2-uicore-datatable'] { border: none !important; } From b672eed77872578ba9ab0485418976157eeeffb3 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Tue, 14 Jul 2026 23:10:29 +0200 Subject: [PATCH 16/27] chore: sonarqube issue --- .../datatable/styles/DataTable.module.css | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/components/datatable/styles/DataTable.module.css b/src/components/datatable/styles/DataTable.module.css index 551233a27..75ab48f58 100644 --- a/src/components/datatable/styles/DataTable.module.css +++ b/src/components/datatable/styles/DataTable.module.css @@ -2,19 +2,6 @@ height: 1px; } -/* @dhis2/ui's Table draws a 1px border on all sides via its own scoped - styled-jsx rule, which the local dataTable class name alone can't - reliably out-specificity. Pairing that class name with the table's - stable data-test attribute (marked important) guarantees this wins, - while staying scoped to just this table (a bare attribute selector on - its own would match every DataTable instance in the app). That full - border (not just the top side) is what makes the browser compute the - sticky header's normal-flow and stuck positions on very slightly - different subpixel grids, causing a visible snap of roughly 1-2 pixels - the instant scrolling engages sticky positioning. Removing the table's - own border entirely (confirmed via devtools) removes the snap; row and - column delineation still comes from each cell's own bottom and - inline-end borders. */ .dataTable[data-test='dhis2-uicore-datatable'] { border: none !important; } From f7cc6ee779c3677359480ef494db5e696dfd65b0 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Wed, 15 Jul 2026 00:03:42 +0200 Subject: [PATCH 17/27] fix: handle narrow columns and hide index column --- i18n/en.pot | 10 ++-- src/components/datatable/DataTable.jsx | 26 ++++++---- src/components/datatable/FilterInput.jsx | 14 +++-- .../datatable/__tests__/DataTable.spec.jsx | 6 +-- .../datatable/__tests__/useTableData.spec.jsx | 52 +++++++------------ .../datatable/styles/DataTable.module.css | 10 +++- src/components/datatable/useTableData.js | 32 ++++-------- 7 files changed, 70 insertions(+), 80 deletions(-) diff --git a/i18n/en.pot b/i18n/en.pot index eb9354f37..8440e8cdc 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-07-14T20:24:31.895Z\n" -"PO-Revision-Date: 2026-07-14T20:24:31.895Z\n" +"POT-Creation-Date: 2026-07-14T21:42:31.767Z\n" +"PO-Revision-Date: 2026-07-14T21:42:31.767Z\n" msgid "2020" msgstr "2020" @@ -296,9 +296,6 @@ msgstr "" msgid "No valid data fields were found for this layer." msgstr "No valid data fields were found for this layer." -msgid "Index" -msgstr "Index" - msgid "Id" msgstr "Id" @@ -1797,6 +1794,9 @@ msgstr "16-day" msgid "Since February 2000" msgstr "Since February 2000" +msgid "Index" +msgstr "Index" + msgid "NDVI" msgstr "NDVI" diff --git a/src/components/datatable/DataTable.jsx b/src/components/datatable/DataTable.jsx index 724370111..30c550141 100644 --- a/src/components/datatable/DataTable.jsx +++ b/src/components/datatable/DataTable.jsx @@ -9,7 +9,6 @@ import { ComponentCover, CenteredContent, CircularLoader, - Popover, Popper, Portal, IconSync16, @@ -43,7 +42,10 @@ import { formatWithSeparator } from '../../util/numbers.js' import { useCachedData } from '../cachedDataProvider/CachedDataProvider.jsx' import Checkbox from '../core/Checkbox.jsx' import { SortIcon } from '../core/icons.jsx' -import FilterInput from './FilterInput.jsx' +import FilterInput, { + FilterDropdownPopover, + getDropdownPlacement, +} from './FilterInput.jsx' import styles from './styles/DataTable.module.css' import TableContextMenu from './TableContextMenu.jsx' import { useTableData, SELECTED_SORT_KEY } from './useTableData.js' @@ -55,11 +57,7 @@ const SELECTION_FILTER_OPTIONS = [ // Every filterable column dispatches its dataFilters value straight through // to filterData against each layer's real feature properties (see -// ThematicLayer.jsx/EventLayer.jsx/etc.). Index is a synthetic row number -// computed only for table display (see useTableData.js), never present on -// the underlying feature data - filtering by it narrows the table but -// can't affect the map. Still shown: it's a useful table-only tool (e.g. -// narrowing to a row-number range) even though it doesn't reach the map. +// ThematicLayer.jsx/EventLayer.jsx/etc.). export const isFilterable = (dataKey, type) => !!type // Inverts selection scoped to the currently-filtered/visible rows only, @@ -90,6 +88,13 @@ const SelectionFilterButton = ({ value, onChange }) => { ? i18n.t('All') : i18n.t('{{count}} selected', { count: value.length }) + // Sits in the same sticky header row as every other column's FilterInput + // dropdown, so computing its placement the same way (rather than using + // @dhis2/ui's Popover, which flips independently based on its own + // available space) keeps it opening on the same side as the rest. + const anchorRect = anchorRef.current?.getBoundingClientRect() + const { dropdownPlacement } = getDropdownPlacement(anchorRect) + return ( <> {isOpen && ( - setIsOpen(false)} >
    @@ -119,7 +123,7 @@ const SelectionFilterButton = ({ value, onChange }) => { /> ))}
    -     + )} ) diff --git a/src/components/datatable/FilterInput.jsx b/src/components/datatable/FilterInput.jsx index f49d77d75..f8150cca4 100644 --- a/src/components/datatable/FilterInput.jsx +++ b/src/components/datatable/FilterInput.jsx @@ -40,6 +40,11 @@ const MAX_LIST_HEIGHT = 260 // choice that applies to the whole row. const ESTIMATED_POPOVER_HEIGHT = MAX_LIST_HEIGHT + 80 +// Floor for the dropdown's width - narrow columns still need enough room +// for the checkbox list/search text to be usable, so the popover shouldn't +// shrink down to match a very narrow trigger's own width. +const MIN_POPOVER_WIDTH = 140 + // Rough content heights (in px) for the two tooltip variants, used only to // decide whether there's enough room to show the tooltip at all - see // FilterHelpTooltip's hasRoom check. @@ -183,7 +188,7 @@ const dropdownModifiers = [ // for positioning, flip disabled - so `placement` is always honored exactly; // the caller (SearchableFilterPopover) computes one placement per render // from the shared header row's position, so every column's dropdown agrees. -const FilterDropdownPopover = ({ +export const FilterDropdownPopover = ({ reference, placement, onClickOutside, @@ -285,7 +290,7 @@ const getSelectedAndAppliedString = (filterValue) => ({ // of the viewport). That single choice is what keeps every column's // dropdown opening on the same side. The help tooltip always takes the // opposite side, so the two never compete for space. -const getDropdownPlacement = (anchorRect) => { +export const getDropdownPlacement = (anchorRect) => { const dropdownSide = anchorRect != null && window.innerHeight - anchorRect.bottom < ESTIMATED_POPOVER_HEIGHT @@ -663,7 +668,10 @@ const SearchableFilterPopover = ({ })} style={{ minWidth: anchorWidth - ? `${anchorWidth}px` + ? `${Math.max( + anchorWidth, + MIN_POPOVER_WIDTH + )}px` : undefined, }} > diff --git a/src/components/datatable/__tests__/DataTable.spec.jsx b/src/components/datatable/__tests__/DataTable.spec.jsx index 347a57d60..30629f109 100644 --- a/src/components/datatable/__tests__/DataTable.spec.jsx +++ b/src/components/datatable/__tests__/DataTable.spec.jsx @@ -113,11 +113,7 @@ describe('getNextSorting', () => { }) describe('isFilterable', () => { - test('allows the Index column - it filters the table by row-number range even though it cannot narrow the map', () => { - expect(isFilterable('index', 'number')).toBe(true) - }) - - test('allows other numeric and string columns, which are real feature properties', () => { + test('allows numeric and string columns', () => { expect(isFilterable('rawValue', 'number')).toBe(true) expect(isFilterable('name', 'string')).toBe(true) }) diff --git a/src/components/datatable/__tests__/useTableData.spec.jsx b/src/components/datatable/__tests__/useTableData.spec.jsx index 881e591cf..d0f3bf08c 100644 --- a/src/components/datatable/__tests__/useTableData.spec.jsx +++ b/src/components/datatable/__tests__/useTableData.spec.jsx @@ -48,17 +48,15 @@ describe('useTableData headers', () => { ) const { headers, rows, isLoading } = result.current - expect(headers).toHaveLength(4) + expect(headers).toHaveLength(3) expect(headers).toMatchObject([ - { name: 'Index', dataKey: 'index', type: 'number' }, { name: 'Name', dataKey: 'name', type: 'string' }, { name: 'Id', dataKey: 'id', type: 'string' }, { name: 'Type', dataKey: 'type', type: 'string' }, ]) expect(rows).toHaveLength(1) - expect(rows[0]).toHaveLength(4) + expect(rows[0]).toHaveLength(3) expect(rows[0]).toMatchObject([ - { value: 0, dataKey: 'index' }, { value: 'Facility 1', dataKey: 'name' }, { value: 'facility-1', dataKey: 'id' }, { value: 'Point', dataKey: 'type' }, @@ -100,9 +98,8 @@ describe('useTableData headers', () => { } ) const { headers, rows, isLoading } = result.current - expect(headers).toHaveLength(6) + expect(headers).toHaveLength(5) expect(headers).toMatchObject([ - { name: 'Index', dataKey: 'index', type: 'number' }, { name: 'Name', dataKey: 'name', type: 'string' }, { name: 'Id', dataKey: 'id', type: 'string' }, { name: 'Level', dataKey: 'level', type: 'number' }, @@ -110,9 +107,8 @@ describe('useTableData headers', () => { { name: 'Type', dataKey: 'type', type: 'string' }, ]) expect(rows).toHaveLength(1) - expect(rows[0]).toHaveLength(6) + expect(rows[0]).toHaveLength(5) expect(rows[0]).toMatchObject([ - { value: 0, dataKey: 'index' }, { value: 'OrgUnitName 1', dataKey: 'name' }, { value: 'orgunit-id-1', dataKey: 'id' }, { value: 3, dataKey: 'level' }, @@ -160,9 +156,8 @@ describe('useTableData headers', () => { } ) const { headers, rows, isLoading } = result.current - expect(headers).toHaveLength(10) + expect(headers).toHaveLength(9) expect(headers).toMatchObject([ - { name: 'Index', dataKey: 'index', type: 'number' }, { name: 'Name', dataKey: 'name', type: 'string' }, { name: 'Id', dataKey: 'id', type: 'string' }, { name: 'Value', dataKey: 'rawValue', type: 'number' }, @@ -179,9 +174,8 @@ describe('useTableData headers', () => { }, ]) expect(rows).toHaveLength(1) - expect(rows[0]).toHaveLength(10) + expect(rows[0]).toHaveLength(9) expect(rows[0]).toMatchObject([ - { value: 0, dataKey: 'index' }, { value: 'Ngelehun CHC', dataKey: 'name' }, { value: 'thematicId-1', dataKey: 'id' }, { value: 106.3, dataKey: 'rawValue' }, @@ -262,9 +256,8 @@ describe('useTableData headers', () => { } ) const { headers, rows, isLoading } = result.current - expect(headers).toHaveLength(8) + expect(headers).toHaveLength(7) expect(headers).toMatchObject([ - { name: 'Index', dataKey: 'index', type: 'number' }, { name: 'Org unit', dataKey: 'ouname', type: 'string' }, { name: 'Id', dataKey: 'id', type: 'string' }, { @@ -279,9 +272,8 @@ describe('useTableData headers', () => { { name: 'Type', dataKey: 'type', type: 'string' }, ]) expect(rows).toHaveLength(1) - expect(rows[0]).toHaveLength(8) + expect(rows[0]).toHaveLength(7) expect(rows[0]).toMatchObject([ - { value: 0, dataKey: 'index' }, { value: 'Lumley Hospital', dataKey: 'ouname' }, { value: 'a9712323629', dataKey: 'id' }, { value: '2023-05-15 00:00:00.0', dataKey: 'eventdate' }, @@ -442,9 +434,8 @@ describe('useTableData headers', () => { ) const { headers, rows, isLoading } = result.current - expect(headers).toHaveLength(6) + expect(headers).toHaveLength(5) expect(headers).toMatchObject([ - { name: 'Index', dataKey: 'index', type: 'number' }, { name: 'Name', dataKey: 'name', type: 'string' }, { name: 'Id', dataKey: 'id', type: 'string' }, { name: 'Type', dataKey: 'type', type: 'string' }, @@ -461,12 +452,11 @@ describe('useTableData headers', () => { type: 'number', }, ]) + expect(headers[3].roundFn).toBeInstanceOf(Function) expect(headers[4].roundFn).toBeInstanceOf(Function) - expect(headers[5].roundFn).toBeInstanceOf(Function) expect(rows).toHaveLength(2) - expect(rows[0]).toHaveLength(6) + expect(rows[0]).toHaveLength(5) expect(rows[0]).toMatchObject([ - { value: 0, dataKey: 'index' }, { value: 'Bo', dataKey: 'name' }, { value: 'boOu', dataKey: 'id' }, { value: 'Polygon', dataKey: 'type' }, @@ -599,9 +589,8 @@ describe('useTableData headers', () => { ) const { headers, rows, isLoading } = result.current - expect(headers).toHaveLength(6) + expect(headers).toHaveLength(5) expect(headers).toMatchObject([ - { name: 'Index', dataKey: 'index', type: 'number' }, { name: 'Name', dataKey: 'name', type: 'string' }, { name: 'Id', dataKey: 'id', type: 'string' }, { name: 'Type', dataKey: 'type', type: 'string' }, @@ -618,12 +607,11 @@ describe('useTableData headers', () => { type: 'number', }, ]) + expect(headers[3].roundFn).toBeInstanceOf(Function) expect(headers[4].roundFn).toBeInstanceOf(Function) - expect(headers[5].roundFn).toBeInstanceOf(Function) expect(rows).toHaveLength(2) - expect(rows[0]).toHaveLength(6) + expect(rows[0]).toHaveLength(5) expect(rows[0]).toMatchObject([ - { value: 0, dataKey: 'index' }, { value: 'Badija', dataKey: 'name' }, { value: 'boOU', dataKey: 'id' }, { value: 'Polygon', dataKey: 'type' }, @@ -666,7 +654,7 @@ describe('useTableData sorting', () => { } ) - const valueColumn = result.current.rows.map((row) => row[3]?.value) // Value column + const valueColumn = result.current.rows.map((row) => row[2]?.value) // Value column expect(valueColumn).toEqual([5, 10, 15, null, null]) }) @@ -688,7 +676,7 @@ describe('useTableData sorting', () => { } ) - const valueColumn = result.current.rows.map((row) => row[3]?.value) // Value column + const valueColumn = result.current.rows.map((row) => row[2]?.value) // Value column expect(valueColumn).toEqual([15, 10, 5, null, null]) }) @@ -722,7 +710,7 @@ describe('useTableData sorting', () => { } ) - const nameColumn = result.current.rows.map((row) => row[1]?.value) // Name column + const nameColumn = result.current.rows.map((row) => row[0]?.value) // Name column expect(nameColumn).toEqual(['Apple', 'Banana', 'Zebra', undefined]) }) @@ -756,7 +744,7 @@ describe('useTableData sorting', () => { } ) - const nameColumn = result.current.rows.map((row) => row[1]?.value) // Name column + const nameColumn = result.current.rows.map((row) => row[0]?.value) // Name column expect(nameColumn).toEqual(['Zebra', 'Banana', 'Apple', undefined]) }) @@ -796,7 +784,7 @@ describe('useTableData sorting', () => { } ) - const valueColumn = result.current.rows.map((row) => row[3]?.value) // Value column + const valueColumn = result.current.rows.map((row) => row[2]?.value) // Value column expect(valueColumn).toEqual([5, 10, null, null]) }) @@ -838,7 +826,7 @@ describe('useTableData sorting', () => { } ) - const valueColumn = result.current.rows.map((row) => row[3]?.value) // Value column + const valueColumn = result.current.rows.map((row) => row[2]?.value) // Value column expect(valueColumn).toEqual([null, null, null]) }) diff --git a/src/components/datatable/styles/DataTable.module.css b/src/components/datatable/styles/DataTable.module.css index 75ab48f58..38b6a32cf 100644 --- a/src/components/datatable/styles/DataTable.module.css +++ b/src/components/datatable/styles/DataTable.module.css @@ -75,9 +75,16 @@ td.checkboxCell { white-space: nowrap; } +/* FilterDropdownPopover (Layer + Popper, see FilterInput.jsx) renders no + background/elevation of its own - unlike @dhis2/ui's Popover, which this + replaced, styling that is left entirely to the caller. */ .selectionFilterPopover { padding: var(--spacers-dp8); min-width: 140px; + background-color: var(--colors-white); + border-radius: 4px; + box-shadow: 0 4px 12px rgba(12, 14, 16, 0.15), + 0 0 0 1px rgba(12, 14, 16, 0.05); } .selectionFilterPopover :global(label) { @@ -93,7 +100,8 @@ td.hovered { background-color: var(--colors-blue100); } -.columnHeader > :global(span.container) { +.columnHeader > :global(span.container), +.checkboxCell > :global(span.container) { justify-content: space-between; } diff --git a/src/components/datatable/useTableData.js b/src/components/datatable/useTableData.js index d7f135579..19ffa1878 100644 --- a/src/components/datatable/useTableData.js +++ b/src/components/datatable/useTableData.js @@ -36,12 +36,6 @@ const TYPE_NUMBER = 'number' const TYPE_STRING = 'string' const TYPE_DATE = 'date' -// The Index column is a synthetic row number computed only for table -// display (see the `index` assigned from array position in -// dataWithAggregations below) - it's never written onto the underlying -// layer's actual feature data, so it can't be used as a map filter (see -// DataTable.jsx, which excludes it from getting a FilterInput at all). -export const INDEX = 'index' const NAME = 'name' const ID = 'id' const VALUE = 'rawValue' @@ -82,7 +76,6 @@ const getErrorCodeText = (code) => { } const defaultFieldsMap = () => ({ - [INDEX]: { name: i18n.t('Index'), dataKey: INDEX, type: TYPE_NUMBER }, [NAME]: { name: i18n.t('Name'), dataKey: NAME, type: TYPE_STRING }, [ID]: { name: i18n.t('Id'), dataKey: ID, type: TYPE_STRING }, [LEVEL]: { name: i18n.t('Level'), dataKey: LEVEL, type: TYPE_NUMBER }, @@ -116,25 +109,16 @@ const defaultFieldsMap = () => ({ }) const getThematicHeaders = () => - [ - INDEX, - NAME, - ID, - VALUE, - LEGEND, - RANGE, - LEVEL, - PARENT_NAME, - TYPE, - COLOR, - ].map((field) => defaultFieldsMap()[field]) + [NAME, ID, VALUE, LEGEND, RANGE, LEVEL, PARENT_NAME, TYPE, COLOR].map( + (field) => defaultFieldsMap()[field] + ) const getEventHeaders = ({ layerHeaders = [], styleDataItem, countEventsOutsideOrgUnits, }) => { - const fields = [INDEX, OUNAME, ID, EVENTDATE].map( + const fields = [OUNAME, ID, EVENTDATE].map( (field) => defaultFieldsMap()[field] ) @@ -164,12 +148,12 @@ const getEventHeaders = ({ } const getOrgUnitHeaders = () => - [INDEX, NAME, ID, LEVEL, PARENT_NAME, TYPE].map( + [NAME, ID, LEVEL, PARENT_NAME, TYPE].map( (field) => defaultFieldsMap()[field] ) const getFacilityHeaders = () => - [INDEX, NAME, ID, TYPE].map((field) => defaultFieldsMap()[field]) + [NAME, ID, TYPE].map((field) => defaultFieldsMap()[field]) const toTitleCase = (str) => str.replace( @@ -205,7 +189,7 @@ const getEarthEngineHeaders = ({ aggregationType, legend, data }) => { }) } - return [INDEX, NAME, ID, TYPE] + return [NAME, ID, TYPE] .map((field) => defaultFieldsMap()[field]) .concat(customFields) } @@ -357,6 +341,8 @@ export const useTableData = ({ .map((d, index) => ({ ...(d.properties || d), ...aggregations[d.id], + // Row-order tie-breaker for compareRows when no sortField is + // set - not a real column, not shown or filterable in the table. index, })) // boundsDependency intentionally proxies mapBounds only while the toggle is on From e5ff457d5f20e586d4bbbf4352120cf6028a5e52 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Thu, 16 Jul 2026 10:47:34 +0200 Subject: [PATCH 18/27] fix: rollback merging range and color columns --- i18n/en.pot | 17 ++- src/components/datatable/BottomPanel.jsx | 26 +++- src/components/datatable/DataTable.jsx | 120 +++++------------- src/components/datatable/FilterInput.jsx | 8 +- src/components/datatable/ResizeHandle.jsx | 106 +++++++++------- .../datatable/styles/BottomPanel.module.css | 7 + .../datatable/styles/DataTable.module.css | 14 -- .../datatable/styles/ResizeHandle.module.css | 18 ++- 8 files changed, 149 insertions(+), 167 deletions(-) diff --git a/i18n/en.pot b/i18n/en.pot index 8440e8cdc..a39e400c3 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-07-14T21:42:31.767Z\n" -"PO-Revision-Date: 2026-07-14T21:42:31.767Z\n" +"POT-Creation-Date: 2026-07-16T08:26:26.615Z\n" +"PO-Revision-Date: 2026-07-16T08:26:26.615Z\n" msgid "2020" msgstr "2020" @@ -167,6 +167,9 @@ msgstr "Restore" msgid "Collapse" msgstr "Collapse" +msgid "Highlight color" +msgstr "Highlight color" + msgid "Clear filters" msgstr "Clear filters" @@ -176,9 +179,6 @@ msgstr "Search all columns" msgid "Show only features in current map view" msgstr "Show only features in current map view" -msgid "Highlight color" -msgstr "Highlight color" - msgid "Close" msgstr "Close" @@ -232,8 +232,11 @@ msgstr "equal to 2 OR greater than 8" msgid "greater than 3 AND less than 8" msgstr "greater than 3 AND less than 8" -msgid "Select values, or type text to match rows that contain it." -msgstr "Select values, or type text to match rows that contain it." +msgid "Select values, or type text" +msgstr "Select values, or type text" + +msgid "to match rows that contain it" +msgstr "to match rows that contain it" msgid "Use filter" msgstr "Use filter" diff --git a/src/components/datatable/BottomPanel.jsx b/src/components/datatable/BottomPanel.jsx index a9630ab81..6e62214be 100644 --- a/src/components/datatable/BottomPanel.jsx +++ b/src/components/datatable/BottomPanel.jsx @@ -68,7 +68,8 @@ const BottomPanel = () => { const hasActiveFilters = Object.keys(dataFilters).length > 0 || globalSearch.trim() !== '' || - selectionFilter?.length > 0 + selectionFilter?.length > 0 || + showOnlyFeaturesInView const maxHeight = height - getCssVar('--header-height') - getCssVar('--toolbar-height') @@ -115,7 +116,12 @@ const BottomPanel = () => { dispatch(clearDataFilters(activeLayerId)) dispatch(setSelectionFilter([])) setGlobalSearch('') - }, [dispatch, activeLayerId]) + // toggleShowOnlyFeaturesInView flips the flag, so only dispatch it + // when the toggle is actually on - otherwise this would turn it on. + if (showOnlyFeaturesInView) { + dispatch(toggleShowOnlyFeaturesInView()) + } + }, [dispatch, activeLayerId, showOnlyFeaturesInView]) const onNameMouseEnter = useCallback(() => { const el = nameRef.current @@ -208,6 +214,7 @@ const BottomPanel = () => { )} + { , document.body )} + + + + + dispatch(setHighlightColor(color)) + } + /> + + { - if (isLegendCell) { - return ( - - {swatchColor && ( - - )} - {value} - - ) - } - if (dataKey === 'color') { - return value?.toLowerCase() - } - return formatWithSeparator(value, keyAnalysisDigitGroupSeparator) -} - const DataTableWithVirtuosoContext = ({ context, ...props }) => ( {error}

    } - // Thematic layers carry both a `legend` (name) and `color` (hex) column; - // merge them into one swatch+name cell instead of two separate columns. - const hasLegendColorPair = - headers.some((h) => h.dataKey === 'legend') && - headers.some((h) => h.dataKey === 'color') - const visibleHeaders = hasLegendColorPair - ? headers.filter((h) => h.dataKey !== 'color') - : headers - return ( <> - {visibleHeaders.map( + {headers.map( ({ name, dataKey, type, optionSet }, index) => ( e.stopPropagation()} /> - {row - .filter( - ({ dataKey }) => - !hasLegendColorPair || - dataKey !== 'color' - ) - .map(({ dataKey, value, align }) => { - const isLegendCell = - hasLegendColorPair && - dataKey === 'legend' - const swatchColor = isLegendCell - ? row.find((c) => c.dataKey === 'color') - ?.value - : null - - return ( - - {getCellContent({ - isLegendCell, - swatchColor, - value, - dataKey, - keyAnalysisDigitGroupSeparator, - })} - - ) - })} + {row.map(({ dataKey, value, align }) => ( + + {dataKey === 'color' + ? value?.toLowerCase() + : formatWithSeparator( + value, + keyAnalysisDigitGroupSeparator + )} + + ))} ) }} diff --git a/src/components/datatable/FilterInput.jsx b/src/components/datatable/FilterInput.jsx index f8150cca4..ff3b30e18 100644 --- a/src/components/datatable/FilterInput.jsx +++ b/src/components/datatable/FilterInput.jsx @@ -49,7 +49,7 @@ const MIN_POPOVER_WIDTH = 140 // decide whether there's enough room to show the tooltip at all - see // FilterHelpTooltip's hasRoom check. const NUMERIC_HELP_HEIGHT = 140 -const TEXT_HELP_HEIGHT = 40 +const TEXT_HELP_HEIGHT = 56 const NUMERIC_FILTER_HELP = (
    @@ -64,7 +64,8 @@ const NUMERIC_FILTER_HELP = ( const TEXT_FILTER_HELP = (
    - {i18n.t('Select values, or type text to match rows that contain it.')} +
    {i18n.t('Select values, or type text')}
    +
    {i18n.t('to match rows that contain it')}
    ) @@ -779,7 +780,8 @@ const SearchableFilterPopover = ({ onToggleRealValue(option.value) } className={cx( - dataKey === 'id' && + (dataKey === 'id' || + dataKey === 'color') && styles.monoOption, highlightedIndex === (showCustomFilterRow diff --git a/src/components/datatable/ResizeHandle.jsx b/src/components/datatable/ResizeHandle.jsx index 13985be16..82da4ec76 100644 --- a/src/components/datatable/ResizeHandle.jsx +++ b/src/components/datatable/ResizeHandle.jsx @@ -1,15 +1,16 @@ import PropTypes from 'prop-types' -import React from 'react' +import React, { useEffect, useRef } from 'react' import { IconDrag } from '../core/icons.jsx' import styles from './styles/ResizeHandle.module.css' -// Pre-decoded so setDragImage doesn't fall back to the macOS Chrome globe icon -// when the image isn't yet `complete` on the dragstart tick. -// https://www.sam.today/blog/html5-dnd-globe-icon -const EMPTY_DRAG_IMAGE = new Image(1, 1) -EMPTY_DRAG_IMAGE.src = - 'data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7' - +// Pointer Events + setPointerCapture, not HTML5 drag-and-drop: a native drag +// session shows the browser's own drop-target cursor (grabbing/not-allowed/ +// move) instead of any CSS cursor rule, and elements with no drop handling +// of their own (e.g. the map's WebGL canvas) count as invalid drop targets - +// showing "not-allowed" for as long as the pointer is over them. Pointer +// capture sidesteps that protocol entirely, and keeps reporting move/up +// events to this element even if the pointer leaves it (or the window) +// mid-drag - more reliable than a plain mousemove/mouseup pair for that case. const ResizeHandle = ({ onResize, onResizeStart, @@ -17,63 +18,70 @@ const ResizeHandle = ({ minHeight = 50, maxHeight = 500, }) => { - let dragHeight = 0 - - const onDragStart = (evt) => { - // https://stackoverflow.com/questions/7680285/how-do-you-turn-off-setdragimage - if (EMPTY_DRAG_IMAGE.complete) { - evt.dataTransfer.setDragImage(EMPTY_DRAG_IMAGE, 0, 0) - } + const isDraggingRef = useRef(false) + + const getHeight = (clientY) => { + const height = window.innerHeight - clientY + return height < minHeight + ? minHeight + : height > maxHeight + ? maxHeight + : height + } - evt.dataTransfer.setData('text/plain', 'node') // Required to initialize dragging in Firefox + const onPointerDown = (evt) => { + evt.preventDefault() // avoid text selection while dragging + evt.currentTarget.setPointerCapture(evt.pointerId) + isDraggingRef.current = true onResizeStart?.() - - // https://stackoverflow.com/questions/23992091/drag-and-drop-directive-no-e-clientx-or-e-clienty-on-drag-event-in-firefox - document.ondragover = onDrag + // Set on both the handle and the body: the handle's own `cursor: + // grab` CSS rule otherwise beats body's *inherited* cursor while the + // pointer is over it, so body alone never actually shows grabbing + // here - only once the pointer strays over something with no cursor + // rule of its own (e.g. the map). + evt.currentTarget.style.cursor = 'grabbing' + document.body.style.cursor = 'grabbing' } - const onDrag = (evt) => { - const height = getHeight(evt || window.event) - - if (height && onResize) { - onResize(height) - dragHeight = height + const onPointerMove = (evt) => { + if (isDraggingRef.current) { + onResize?.(getHeight(evt.clientY)) } } - const onDragEnd = (evt) => { - const height = getHeight(evt) - - if (height && onResizeEnd) { - onResizeEnd(height) + const onPointerUp = (evt) => { + if (!isDraggingRef.current) { + return } - - document.ondragover = null + isDraggingRef.current = false + evt.currentTarget.releasePointerCapture(evt.pointerId) + evt.currentTarget.style.removeProperty('cursor') + document.body.style.removeProperty('cursor') + onResizeEnd?.(getHeight(evt.clientY)) } - const getHeight = (evt) => { - if (evt.pageY) { - const height = window.innerHeight - evt.pageY - dragHeight = - height < minHeight - ? minHeight - : height > maxHeight - ? maxHeight - : height - } - - return dragHeight - } + // In case the handle/panel unmounts mid-drag + useEffect( + () => () => { + if (isDraggingRef.current) { + document.body.style.removeProperty('cursor') + } + }, + [] + ) return (
    onDragStart(evt)} - onDragEnd={(evt) => onDragEnd(evt)} + onPointerDown={onPointerDown} + onPointerMove={onPointerMove} + onPointerUp={onPointerUp} + onPointerCancel={onPointerUp} > - + + +
    ) } diff --git a/src/components/datatable/styles/BottomPanel.module.css b/src/components/datatable/styles/BottomPanel.module.css index e95268f60..d9e75af48 100644 --- a/src/components/datatable/styles/BottomPanel.module.css +++ b/src/components/datatable/styles/BottomPanel.module.css @@ -47,6 +47,13 @@ flex-shrink: 0; } +.divider { + width: 1px; + height: 20px; + background-color: var(--colors-grey300); + flex-shrink: 0; +} + @keyframes tooltipExpandRight { from { clip-path: inset(0 100% 0 0); diff --git a/src/components/datatable/styles/DataTable.module.css b/src/components/datatable/styles/DataTable.module.css index 38b6a32cf..19eaf7cfc 100644 --- a/src/components/datatable/styles/DataTable.module.css +++ b/src/components/datatable/styles/DataTable.module.css @@ -29,20 +29,6 @@ td.monoCell { font-family: ui-monospace, 'SF Mono', 'Cascadia Mono', 'Consolas', monospace; } -.legendCell { - display: flex; - align-items: center; - gap: var(--spacers-dp8); -} - -.legendSwatch { - flex-shrink: 0; - width: 10px; - height: 10px; - border-radius: 2px; - border: 1px solid var(--colors-grey400); -} - th.checkboxCell, td.checkboxCell { width: 76px; diff --git a/src/components/datatable/styles/ResizeHandle.module.css b/src/components/datatable/styles/ResizeHandle.module.css index 2bec5229a..bec35a033 100644 --- a/src/components/datatable/styles/ResizeHandle.module.css +++ b/src/components/datatable/styles/ResizeHandle.module.css @@ -9,12 +9,18 @@ cursor: grab; } -.resizeHandle:hover, -.resizeHandle:active { - background-color: var(--colors-grey300); - color: var(--colors-grey900); +.gripBox { + width: 99%; + height: 24px; + border-radius: 3px; + display: flex; + align-items: center; + justify-content: center; + color: var(--colors-grey600); } -.resizeHandle:active { - cursor: grabbing; +.resizeHandle:hover .gripBox, +.resizeHandle:active .gripBox { + background-color: var(--colors-grey200); + color: var(--colors-grey900); } From e53ed3e7b2f5570a88acc3028e0dfeea75cc045f Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Thu, 16 Jul 2026 13:35:05 +0200 Subject: [PATCH 19/27] chore: PR clean-up --- cypress/integration/dataTable.cy.js | 50 ++- i18n/en.pot | 15 +- src/components/datatable/BottomPanel.jsx | 2 - src/components/datatable/DataTable.jsx | 191 +++------ .../datatable/FilterDropdownPopover.jsx | 50 +++ src/components/datatable/FilterInput.jsx | 363 ++---------------- src/components/datatable/ResizeHandle.jsx | 13 - .../datatable/__tests__/DataTable.spec.jsx | 8 +- .../datatable/__tests__/FilterInput.spec.jsx | 50 +-- .../__tests__/TableContextMenu.spec.jsx | 2 - .../datatable/__tests__/useTableData.spec.jsx | 108 +++++- .../datatable/styles/BottomPanel.module.css | 8 +- .../datatable/styles/DataTable.module.css | 16 +- .../datatable/styles/FilterInput.module.css | 59 +-- src/components/datatable/useColumnWidths.js | 66 ++++ src/components/datatable/useRowSelection.js | 57 +++ src/components/datatable/useTableData.js | 114 +----- .../map/layers/__tests__/Layer.spec.js | 4 - src/constants/dataTable.js | 6 + src/constants/selection.js | 2 - src/util/__tests__/filter.spec.js | 11 +- src/util/__tests__/filterSelection.spec.js | 145 +++++++ src/util/__tests__/tableSort.spec.js | 190 +++++++++ src/util/filter.js | 12 +- src/util/filterSelection.js | 66 ++++ src/util/tableSort.js | 86 +++++ 26 files changed, 937 insertions(+), 757 deletions(-) create mode 100644 src/components/datatable/FilterDropdownPopover.jsx create mode 100644 src/components/datatable/useColumnWidths.js create mode 100644 src/components/datatable/useRowSelection.js create mode 100644 src/constants/dataTable.js create mode 100644 src/util/__tests__/filterSelection.spec.js create mode 100644 src/util/__tests__/tableSort.spec.js create mode 100644 src/util/filterSelection.js create mode 100644 src/util/tableSort.js diff --git a/cypress/integration/dataTable.cy.js b/cypress/integration/dataTable.cy.js index 741933dbd..0bfffeadd 100644 --- a/cypress/integration/dataTable.cy.js +++ b/cypress/integration/dataTable.cy.js @@ -36,7 +36,7 @@ const checkTableCell = ({ row = 0, column = 0, expectedContent }) => { } describe('data table', () => { - it('opens data table and filters and sorts', () => { + it('opens data table for a Thematic layer and filters and sorts', () => { const viewportHeight = Cypress.config('viewportHeight') const expectedBottoms1 = [viewportHeight] const expectedHeights1 = [ @@ -47,7 +47,7 @@ describe('data table', () => { cy.visit(`/#/${map.id}`) cy.get('canvas', EXTENDED_TIMEOUT).should('be.visible') - //check that the map resizes properly + // Check that the map resizes properly assertMapPosition(expectedBottoms1, expectedHeights1) cy.getByDataTest('moremenubutton').first().click() @@ -62,10 +62,10 @@ describe('data table', () => { .contains('Show data table') .click() - //check that the bottom panel is present + // Check that the bottom panel is present cy.getByDataTest('bottom-panel').should('be.visible') - //check that the map resizes properly + // Check that the map resizes properly cy.getByDataTest('bottom-panel') .invoke('height') .then((height) => { @@ -80,9 +80,7 @@ describe('data table', () => { // Collapse the Layers Panel to give the table more width cy.getByDataTest('layers-toggle-button').click() - // check number of columns - // (Legend + Color are merged into one swatch+name column for - // thematic layers, so this is one fewer than the number of headers) + // Check number of columns cy.getByDataTest('bottom-panel') .findByDataTest('dhis2-uicore-datatablecellhead') .should('have.length', 10) @@ -92,13 +90,13 @@ describe('data table', () => { .find('input') .type('bar{enter}') - // check that the filter returned the correct number of rows + // Check that the filter returned the correct number of rows cy.getByDataTest('bottom-panel') .findByDataTest('dhis2-uicore-tablebody') .findByDataTest('dhis2-uicore-datatablerow') .should('have.length', 7) - // confirm that the sort order is initially ascending by Name + // 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' }) @@ -110,16 +108,16 @@ describe('data table', () => { // so we reset to top before asserting on row indices below cy.get('[data-testid="virtuoso-scroller"]').scrollTo('top') - // confirm that the rows are sorted by Name descending + // Confirm that the rows are sorted by Name descending checkTableCell({ row: 0, column: 2, expectedContent: 'Upper Bambara' }) checkTableCell({ row: 6, column: 2, expectedContent: 'Bargbe' }) - // filter by Value (numeric) + // Filter by Value (numeric) cy.getByDataTest('data-table-column-filter-search-Value') .find('input') .type('>26{enter}') - // check that the (combined) filter returned the correct number of rows + // Check that the (combined) filter returned the correct number of rows cy.getByDataTest('bottom-panel') .findByDataTest('dhis2-uicore-tablebody') .findByDataTest('dhis2-uicore-datatablerow') @@ -131,11 +129,11 @@ describe('data table', () => { // Reset scroll position after sorting - see comment above cy.get('[data-testid="virtuoso-scroller"]').scrollTo('top') - // check that the rows are sorted by Value ascending + // Check that the rows are sorted by Value ascending checkTableCell({ row: 0, column: 4, expectedContent: '35' }) checkTableCell({ row: 4, column: 4, expectedContent: '76' }) - // right-click a row and select "View profile" + // Right-click a row and select "View profile" cy.getByDataTest('bottom-panel') .findByDataTest('dhis2-uicore-tablebody') .findByDataTest('dhis2-uicore-datatablerow') @@ -144,22 +142,22 @@ describe('data table', () => { cy.getByDataTest('data-table-context-menu-view-profile').click() - // check that the org unit profile drawer is opened + // Check that the org unit profile drawer is opened cy.getByDataTest('org-unit-profile').should('be.visible') cy.getByDataTest('layers-toggle-button').click() - // close the datatable + // Close the datatable cy.getByDataTest('moremenubutton').first().click() cy.getByDataTest('more-menu') .find('li') .contains('Hide data table') .click() - //check that the bottom panel is closed + // Check that the bottom panel is closed cy.getByDataTest('bottom-panel').should('not.exist') - //check that the map resizes properly + // Check that the map resizes properly assertMapPosition(expectedBottoms1, expectedHeights1) }) @@ -194,7 +192,7 @@ describe('data table', () => { // Collapse the Layers Panel to give the table more width cy.getByDataTest('layers-toggle-button').click() - // check number of columns + // Check number of columns cy.getByDataTest('bottom-panel') .findByDataTest('dhis2-uicore-datatablecellhead') .should('have.length', 11) @@ -204,13 +202,13 @@ describe('data table', () => { .contains('Age in years', { matchCase: false }) .should('be.visible') - // filter by Org unit + // Filter by Org unit const ouName = 'Moyowa' cy.getByDataTest('data-table-column-filter-search-Org unit') .find('input') .type(`${ouName}{enter}`) - // check that all the rows have Org unit Moyowa + // Check that all the rows have Org unit Moyowa checkTableCell({ row: 0, column: 2, expectedContent: ouName }) checkTableCell({ row: 2, column: 2, expectedContent: ouName }) @@ -219,7 +217,7 @@ describe('data table', () => { .findByDataTest('dhis2-uicore-datatablerow') .should('have.length', 3) - // filter by Mode of Discharge + // Filter by Mode of Discharge cy.getByDataTest('data-table-column-filter-search-Mode of Discharge') .find('input') .type('Absconded') @@ -239,12 +237,12 @@ describe('data table', () => { .findByDataTest('dhis2-uicore-datatablerow') .should('have.length', 3) - // filter by Age in years (numeric) + // Filter by Age in years (numeric) cy.getByDataTest('data-table-column-filter-search-Age in years') .find('input') .type('<51{enter}') - // check that the filter returned the correct number of rows + // Check that the filter returned the correct number of rows cy.getByDataTest('bottom-panel') .findByDataTest('dhis2-uicore-tablebody') .findByDataTest('dhis2-uicore-datatablerow') @@ -258,7 +256,7 @@ describe('data table', () => { checkTableCell({ row: 0, column: 8, expectedContent: '6' }) checkTableCell({ row: 1, column: 8, expectedContent: '32' }) - // right-click a row: Event layers have no profile to view + // Right-click a row: Event layers have no profile to view cy.getByDataTest('bottom-panel') .findByDataTest('dhis2-uicore-tablebody') .findByDataTest('dhis2-uicore-datatablerow') @@ -269,7 +267,7 @@ describe('data table', () => { 'not.exist' ) - // check that the org unit profile drawer is NOT opened + // Check that the org unit profile drawer is NOT opened cy.getByDataTest('org-unit-profile').should('not.exist') }) diff --git a/i18n/en.pot b/i18n/en.pot index a39e400c3..a28648bb6 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-07-16T08:26:26.615Z\n" -"PO-Revision-Date: 2026-07-16T08:26:26.615Z\n" +"POT-Creation-Date: 2026-07-16T11:34:34.211Z\n" +"PO-Revision-Date: 2026-07-16T11:34:34.212Z\n" msgid "2020" msgstr "2020" @@ -202,11 +202,11 @@ msgstr "No features match your filters" msgid "No results found" msgstr "No results found" -msgid "Select all" -msgstr "Select all" +msgid "Select all visible rows" +msgstr "Select all visible rows" -msgid "Reverse selection" -msgstr "Reverse selection" +msgid "Reverse selection of visible rows" +msgstr "Reverse selection of visible rows" msgid "Sort by Selected" msgstr "Sort by Selected" @@ -250,6 +250,9 @@ msgstr "Search or type > 5, < 8…" msgid "Search" msgstr "Search" +msgid "Reverse selection" +msgstr "Reverse selection" + msgid "Any value" msgstr "Any value" diff --git a/src/components/datatable/BottomPanel.jsx b/src/components/datatable/BottomPanel.jsx index 6e62214be..7f8edc200 100644 --- a/src/components/datatable/BottomPanel.jsx +++ b/src/components/datatable/BottomPanel.jsx @@ -116,8 +116,6 @@ const BottomPanel = () => { dispatch(clearDataFilters(activeLayerId)) dispatch(setSelectionFilter([])) setGlobalSearch('') - // toggleShowOnlyFeaturesInView flips the flag, so only dispatch it - // when the toggle is actually on - otherwise this would turn it on. if (showOnlyFeaturesInView) { dispatch(toggleShowOnlyFeaturesInView()) } diff --git a/src/components/datatable/DataTable.jsx b/src/components/datatable/DataTable.jsx index 20c085ec0..5b72ec12d 100644 --- a/src/components/datatable/DataTable.jsx +++ b/src/components/datatable/DataTable.jsx @@ -29,10 +29,13 @@ import { setSelectionFilter } from '../../actions/dataTable.js' import { highlightFeature } from '../../actions/feature.js' import { toggleFeatureSelection, - selectAllFeatures, selectFeatureRange, - clearSelection, } from '../../actions/selection.js' +import { + SENTINEL_SELECTED_ROW, + SORT_ASCENDING, + SORT_DESCENDING, +} from '../../constants/dataTable.js' import { SELECTION_FILTER_SELECTED, SELECTION_FILTER_NOT_SELECTED, @@ -42,36 +45,24 @@ import { formatWithSeparator } from '../../util/numbers.js' import { useCachedData } from '../cachedDataProvider/CachedDataProvider.jsx' import Checkbox from '../core/Checkbox.jsx' import { SortIcon } from '../core/icons.jsx' -import FilterInput, { +import { FilterDropdownPopover, getDropdownPlacement, -} from './FilterInput.jsx' +} from './FilterDropdownPopover.jsx' +import FilterInput from './FilterInput.jsx' import styles from './styles/DataTable.module.css' import TableContextMenu from './TableContextMenu.jsx' -import { useTableData, SELECTED_SORT_KEY } from './useTableData.js' +import { useColumnWidths } from './useColumnWidths.js' +import { useRowSelection } from './useRowSelection.js' +import { useTableData } from './useTableData.js' const SELECTION_FILTER_OPTIONS = [ { value: SELECTION_FILTER_SELECTED, label: i18n.t('Selected') }, { value: SELECTION_FILTER_NOT_SELECTED, label: i18n.t('Not selected') }, ] -// Every filterable column dispatches its dataFilters value straight through -// to filterData against each layer's real feature properties (see -// ThematicLayer.jsx/EventLayer.jsx/etc.). export const isFilterable = (dataKey, type) => !!type -// Inverts selection scoped to the currently-filtered/visible rows only, -// mirroring how the "select all" checkbox already treats them (see -// onToggleSelectAll) - ids selected before a filter narrowed the rows stay -// selected (offViewSelected), only the visible portion actually flips. -export const getReversedSelection = (selectedIds, allRowIds) => { - const selectedIdSet = new Set(selectedIds) - const allRowIdSet = new Set(allRowIds) - const offViewSelected = selectedIds.filter((id) => !allRowIdSet.has(id)) - const invertedVisible = allRowIds.filter((id) => !selectedIdSet.has(id)) - return [...offViewSelected, ...invertedVisible] -} - const SelectionFilterButton = ({ value, onChange }) => { const anchorRef = useRef(null) const [isOpen, setIsOpen] = useState(false) @@ -88,10 +79,6 @@ const SelectionFilterButton = ({ value, onChange }) => { ? i18n.t('All') : i18n.t('{{count}} selected', { count: value.length }) - // Sits in the same sticky header row as every other column's FilterInput - // dropdown, so computing its placement the same way (rather than using - // @dhis2/ui's Popover, which flips independently based on its own - // available space) keeps it opening on the same side as the rest. const anchorRect = anchorRef.current?.getBoundingClientRect() const { dropdownPlacement } = getDropdownPlacement(anchorRect) @@ -136,12 +123,6 @@ SelectionFilterButton.propTypes = { const topTooltipModifiers = [{ name: 'offset', options: { offset: [0, 4] } }] -// @dhis2/ui's Tooltip always includes a flip modifier that checks the -// nearest scrolling ancestor's clip box for room. The table header is -// position:sticky, pinned to the top of that scrolling container, so the -// flip modifier always reports "no room above" and flips the tooltip below -// the icon - even though there's plenty of room on screen. This variant -// skips the flip modifier so sort-icon tooltips stay pinned above the icon. const TopTooltip = ({ content, children }) => { const [open, setOpen] = useState(false) const referenceRef = useRef(null) @@ -197,24 +178,17 @@ TopTooltip.propTypes = { content: PropTypes.node.isRequired, } -const ASCENDING = 'asc' -const DESCENDING = 'desc' - export const shouldClearFeatureHighlight = (event) => event.relatedTarget?.tagName !== 'TD' -// Cycles a column through ascending -> descending -> none (natural order) -> -// ascending... Once sortField is null, every column looks "unsorted" again, -// so clicking any of them (including the one that was just cleared) -// naturally restarts the cycle at ascending. export const getNextSorting = (name, { sortField, sortDirection }) => { if (name !== sortField) { - return { sortField: name, sortDirection: ASCENDING } + return { sortField: name, sortDirection: SORT_ASCENDING } } - if (sortDirection === ASCENDING) { - return { sortField: name, sortDirection: DESCENDING } + if (sortDirection === SORT_ASCENDING) { + return { sortField: name, sortDirection: SORT_DESCENDING } } - return { sortField: null, sortDirection: ASCENDING } + return { sortField: null, sortDirection: SORT_ASCENDING } } const getRowId = (row) => @@ -338,10 +312,7 @@ const Table = ({ systemSettings: { keyAnalysisDigitGroupSeparator }, } = useCachedData() - const headerRowRef = useRef(null) const virtuosoRef = useRef(null) - const [columnWidths, setColumnWidths] = useState([]) - const minColumnWidthsRef = useRef([]) const { mapViews } = useSelector((state) => state.map) const activeLayerId = useSelector((state) => state.dataTable) @@ -357,7 +328,7 @@ const Table = ({ (sorting, newSorting) => ({ ...sorting, ...newSorting }), { sortField: 'name', - sortDirection: ASCENDING, + sortDirection: SORT_ASCENDING, } ) @@ -452,6 +423,12 @@ const Table = ({ globalSearch, }) + const { headerRowRef, columnWidths } = useColumnWidths({ + availableWidth, + headers, + error, + }) + useEffect(() => { onCountChange?.(totalCount, filteredCount) }, [onCountChange, totalCount, filteredCount]) @@ -567,93 +544,14 @@ const Table = ({ () => rows?.map(getRowId).filter(Boolean) ?? [], [rows] ) - const allRowIdSet = useMemo(() => new Set(allRowIds), [allRowIds]) - - const isAllSelected = useMemo( - () => - allRowIds.length > 0 && - allRowIds.every((id) => selectedIdSet.has(id)), - [allRowIds, selectedIdSet] - ) - - const onToggleSelectAll = useCallback(() => { - const nextIds = isAllSelected - ? selectedIds.filter((id) => !allRowIdSet.has(id)) - : [...new Set([...selectedIds, ...allRowIds])] - - if (nextIds.length) { - dispatch(selectAllFeatures(nextIds, layer.id)) - } else { - dispatch(clearSelection()) - } - }, [dispatch, isAllSelected, allRowIds, allRowIdSet, selectedIds, layer.id]) - - const onReverseSelection = useCallback(() => { - const nextIds = getReversedSelection(selectedIds, allRowIds) - - if (nextIds.length) { - dispatch(selectAllFeatures(nextIds, layer.id)) - } else { - dispatch(clearSelection()) - } - }, [dispatch, selectedIds, allRowIds, layer.id]) - - useEffect(() => { - // Measure column widths in auto layout, then switch to fixed to prevent content shift during virtual scrolling - if (columnWidths.length === 0 && headerRowRef.current) { - const frameId = requestAnimationFrame(() => { - if (!headerRowRef.current) { - return - } - - const measuredColumnWidths = [] - - const dataCells = Array.from(headerRowRef.current.cells).slice( - 1 - ) - - for (const cell of dataCells) { - const rect = cell.getBoundingClientRect() - measuredColumnWidths.push(Math.floor(rect.width)) - } - - minColumnWidthsRef.current = measuredColumnWidths - setColumnWidths(measuredColumnWidths) - }) - - return () => cancelAnimationFrame(frameId) - } - }, [columnWidths]) - useEffect(() => { - // Reset to auto layout for re-measurement when headers change - if (!error) { - minColumnWidthsRef.current = [] - setColumnWidths([]) - } - }, [headers, error]) - - useEffect(() => { - // Scale column widths proportionally on resize, clamped to initial measured widths - if (!error) { - setColumnWidths((prev) => { - if (prev.length === 0) { - return prev - } - const prevTotal = prev.reduce((sum, w) => sum + w, 0) - if (prevTotal === 0 || availableWidth === 0) { - return [] - } - const minWidths = minColumnWidthsRef.current - return prev.map((w, i) => - Math.max( - minWidths[i] ?? 0, - Math.round((w / prevTotal) * availableWidth) - ) - ) - }) - } - }, [availableWidth, error]) + const { isAllSelected, onToggleSelectAll, onReverseSelection } = + useRowSelection({ + selectedIds, + selectedIdSet, + allRowIds, + layerId: layer.id, + }) if (error) { return

    {error}

    @@ -689,14 +587,22 @@ const Table = ({ } >
    - + + + - )} - - ) : ( - i18n.t('No results found') - )} -
    - - + + + +
    + {context.totalCount > 0 ? ( + <> + {i18n.t('No features match your filters')} + {context.hasActiveFilters && ( + + )} + + ) : ( + i18n.t('No results found') + )} +
    + + + ) EmptyPlaceholder.propTypes = { From e8e3f668ec7b37dc70f1c3d0e97e3e314a647983 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Sun, 19 Jul 2026 15:31:12 +0200 Subject: [PATCH 26/27] chore: create and use replica accounts in CI --- cypress.config.js | 30 ---------------------------- cypress/plugins/e2eReplicaAccount.js | 3 +-- cypress/support/util.js | 3 +-- 3 files changed, 2 insertions(+), 34 deletions(-) diff --git a/cypress.config.js b/cypress.config.js index a54646481..3f4d51c1a 100644 --- a/cypress.config.js +++ b/cypress.config.js @@ -29,36 +29,6 @@ async function setupNodeEvents(on, config) { ) } - config.env.useReplicaAccount = !!process.env.CI - - if (config.env.useReplicaAccount) { - try { - const { username, password, replicaUserId } = - await createReplicaAccountForRun({ - baseUrl: config.env.dhis2BaseUrl, - username: config.env.dhis2Username, - password: config.env.dhis2Password, - }) - - config.env.replicaUsername = username - config.env.replicaPassword = password - - on('after:run', () => - deleteReplicaAccount({ - baseUrl: config.env.dhis2BaseUrl, - username: config.env.dhis2Username, - password: config.env.dhis2Password, - replicaUserId, - }) - ) - } catch (error) { - console.warn( - `WARNING: could not create e2e replica account, falling back to the standard account: ${error.message}` - ) - config.env.useReplicaAccount = false - } - } - return config } diff --git a/cypress/plugins/e2eReplicaAccount.js b/cypress/plugins/e2eReplicaAccount.js index 7fa7d2fe7..5658ee88b 100644 --- a/cypress/plugins/e2eReplicaAccount.js +++ b/cypress/plugins/e2eReplicaAccount.js @@ -59,8 +59,7 @@ const dhis2Fetch = async ( } const buildReplicaUsername = () => - `e2e_mapsapp_run${ - process.env.GITHUB_RUN_ID ?? 'local' + `e2e_mapsapp_run${process.env.GITHUB_RUN_ID ?? 'local' }_${uniqueId().replaceAll('-', '_')}` const createReplicaUser = async ({ baseUrl, adminId, auth }) => { diff --git a/cypress/support/util.js b/cypress/support/util.js index 23e725590..1093391d0 100644 --- a/cypress/support/util.js +++ b/cypress/support/util.js @@ -326,8 +326,7 @@ export const assertIntercepts = ({ normalizeErrors(errors).forEach((error) => { // Single intercept cy.log( - `[${n}] Intercepting single: ${alias}${ - error !== undefined ? ` - ${error}` : '' + `[${n}] Intercepting single: ${alias}${error !== undefined ? ` - ${error}` : '' }` ) From 09f29cc308c29abe012c8048c9cf7ec657f120f3 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Mon, 20 Jul 2026 14:01:12 +0200 Subject: [PATCH 27/27] chore: update e2eReplicaAccount.js --- cypress.config.js | 30 ++++++++++++++++++++++++++++ cypress/plugins/e2eReplicaAccount.js | 3 ++- cypress/support/util.js | 3 ++- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/cypress.config.js b/cypress.config.js index 3f4d51c1a..a54646481 100644 --- a/cypress.config.js +++ b/cypress.config.js @@ -29,6 +29,36 @@ async function setupNodeEvents(on, config) { ) } + config.env.useReplicaAccount = !!process.env.CI + + if (config.env.useReplicaAccount) { + try { + const { username, password, replicaUserId } = + await createReplicaAccountForRun({ + baseUrl: config.env.dhis2BaseUrl, + username: config.env.dhis2Username, + password: config.env.dhis2Password, + }) + + config.env.replicaUsername = username + config.env.replicaPassword = password + + on('after:run', () => + deleteReplicaAccount({ + baseUrl: config.env.dhis2BaseUrl, + username: config.env.dhis2Username, + password: config.env.dhis2Password, + replicaUserId, + }) + ) + } catch (error) { + console.warn( + `WARNING: could not create e2e replica account, falling back to the standard account: ${error.message}` + ) + config.env.useReplicaAccount = false + } + } + return config } diff --git a/cypress/plugins/e2eReplicaAccount.js b/cypress/plugins/e2eReplicaAccount.js index 5658ee88b..7fa7d2fe7 100644 --- a/cypress/plugins/e2eReplicaAccount.js +++ b/cypress/plugins/e2eReplicaAccount.js @@ -59,7 +59,8 @@ const dhis2Fetch = async ( } const buildReplicaUsername = () => - `e2e_mapsapp_run${process.env.GITHUB_RUN_ID ?? 'local' + `e2e_mapsapp_run${ + process.env.GITHUB_RUN_ID ?? 'local' }_${uniqueId().replaceAll('-', '_')}` const createReplicaUser = async ({ baseUrl, adminId, auth }) => { diff --git a/cypress/support/util.js b/cypress/support/util.js index 1093391d0..23e725590 100644 --- a/cypress/support/util.js +++ b/cypress/support/util.js @@ -326,7 +326,8 @@ export const assertIntercepts = ({ normalizeErrors(errors).forEach((error) => { // Single intercept cy.log( - `[${n}] Intercepting single: ${alias}${error !== undefined ? ` - ${error}` : '' + `[${n}] Intercepting single: ${alias}${ + error !== undefined ? ` - ${error}` : '' }` )