From 92aa80622c2fdf749baf75989cb40ff3980932db Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Thu, 24 Sep 2026 10:37:11 +0200 Subject: [PATCH 1/5] refactor: take PivotTableEngine constructor args as an object The engine is about to gain a fourth argument, and a fourth positional one would be both unreadable at the call site and over the lint threshold. The class is internal - it is not re-exported from `src/index.js`, and the `exports` map in `package.json` blocks deep imports of the built files - so the signature can change outright rather than growing an options argument alongside the positional ones. Co-Authored-By: Claude Opus 5 --- src/components/PivotTable/PivotTable.js | 7 ++- src/modules/pivotTable/PivotTableEngine.js | 2 +- .../pivotTableEngineHierarchy.spec.js | 56 ++++++++++--------- 3 files changed, 38 insertions(+), 27 deletions(-) diff --git a/src/components/PivotTable/PivotTable.js b/src/components/PivotTable/PivotTable.js index ab5c74ca0..206ee5308 100644 --- a/src/components/PivotTable/PivotTable.js +++ b/src/components/PivotTable/PivotTable.js @@ -20,7 +20,12 @@ const PivotTable = ({ const { width, height } = useParentSize(containerRef, renderCounter) const engine = useMemo( - () => new PivotTableEngine(visualization, data, legendSets), + () => + new PivotTableEngine({ + visualization, + data, + legendSets, + }), [visualization, data, legendSets] ) diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js index cdcc8257c..e006a05b2 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -298,7 +298,7 @@ export class PivotTableEngine { columnMap = [] accumulators = { rows: {} } - constructor(visualization, data, legendSets) { + constructor({ visualization, data, legendSets }) { this.visualization = Object.assign( {}, defaultVisualizationProps, diff --git a/src/modules/pivotTable/__tests__/pivotTableEngineHierarchy.spec.js b/src/modules/pivotTable/__tests__/pivotTableEngineHierarchy.spec.js index 56b5e8b3b..e854d2093 100644 --- a/src/modules/pivotTable/__tests__/pivotTableEngineHierarchy.spec.js +++ b/src/modules/pivotTable/__tests__/pivotTableEngineHierarchy.spec.js @@ -57,10 +57,10 @@ const cellValues = (engine) => describe('PivotTableEngine org unit hierarchy', () => { it('applies the hierarchy to a bare `ou` dimension', () => { - const engine = new PivotTableEngine( - buildVisualization('ou'), - buildData('ou') - ) + const engine = new PivotTableEngine({ + visualization: buildVisualization('ou'), + data: buildData('ou'), + }) expect(rowHierarchies(engine)).toEqual([ ['Sierra Leone', 'Northern Province', 'Bombali'], @@ -70,10 +70,10 @@ describe('PivotTableEngine org unit hierarchy', () => { it('applies the hierarchy to a stage-qualified `ou` dimension', () => { const dimension = `${STAGE}.ou` - const engine = new PivotTableEngine( - buildVisualization(dimension), - buildData(dimension) - ) + const engine = new PivotTableEngine({ + visualization: buildVisualization(dimension), + data: buildData(dimension), + }) expect(rowHierarchies(engine)).toEqual([ ['Sierra Leone', 'Northern Province', 'Bombali'], @@ -82,10 +82,10 @@ describe('PivotTableEngine org unit hierarchy', () => { }) it('applies the hierarchy to an `enrollmentou` dimension', () => { - const engine = new PivotTableEngine( - buildVisualization('enrollmentou'), - buildData('enrollmentou') - ) + const engine = new PivotTableEngine({ + visualization: buildVisualization('enrollmentou'), + data: buildData('enrollmentou'), + }) expect(rowHierarchies(engine)).toEqual([ ['Sierra Leone', 'Northern Province', 'Bombali'], @@ -94,10 +94,13 @@ describe('PivotTableEngine org unit hierarchy', () => { }) it('leaves items untouched when showHierarchy is off', () => { - const engine = new PivotTableEngine( - { ...buildVisualization(`${STAGE}.ou`), showHierarchy: false }, - buildData(`${STAGE}.ou`) - ) + const engine = new PivotTableEngine({ + visualization: { + ...buildVisualization(`${STAGE}.ou`), + showHierarchy: false, + }, + data: buildData(`${STAGE}.ou`), + }) expect(rowHierarchies(engine)).toEqual([undefined, undefined]) expect(engine.getRowHeader(0)[0].uid).toBe(OU_A) @@ -106,10 +109,10 @@ describe('PivotTableEngine org unit hierarchy', () => { /* Sorting rewrites itemIds, which the row lookup resolves data rows * through. If the two fall out of step every cell renders empty. */ it('keeps values aligned with the re-sorted rows', () => { - const engine = new PivotTableEngine( - buildVisualization(`${STAGE}.ou`), - buildData(`${STAGE}.ou`) - ) + const engine = new PivotTableEngine({ + visualization: buildVisualization(`${STAGE}.ou`), + data: buildData(`${STAGE}.ou`), + }) expect(rowNames(engine)).toEqual(['Bombali', 'Bo']) expect(cellValues(engine)).toEqual(['2', '1']) @@ -122,7 +125,10 @@ describe('PivotTableEngine org unit hierarchy', () => { const data = buildData(dimension) data.metaData.ouNameHierarchy = {} - const engine = new PivotTableEngine(buildVisualization(dimension), data) + const engine = new PivotTableEngine({ + visualization: buildVisualization(dimension), + data, + }) expect(rowNames(engine)).toEqual(['Bo', 'Bombali']) expect(rowHierarchies(engine)).toEqual([undefined, undefined]) @@ -131,10 +137,10 @@ describe('PivotTableEngine org unit hierarchy', () => { it('leaves non-org-unit dimensions untouched', () => { const dimension = `${STAGE}.de1` - const engine = new PivotTableEngine( - buildVisualization(dimension), - buildData(dimension) - ) + const engine = new PivotTableEngine({ + visualization: buildVisualization(dimension), + data: buildData(dimension), + }) expect(rowHierarchies(engine)).toEqual([undefined, undefined]) }) From 40ccb45ddbbbde7c1097ca0d6bffc75371989728 Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Thu, 24 Sep 2026 11:06:56 +0200 Subject: [PATCH 2/5] feat: let the caller supply the pivot table's filter line text `PivotTableTitleRows` derives the filter row from `visualization.filters` and `rawData.metaData` via `getFilterText`. Sometimes it is easier for the consuming app to work that line out itself - Event Visualizer filters are often value conditions such as `GE:5:LE:10`, which need option sets and value types this package has no access to - so let the caller opt out of the derived text and pass its own in. `PivotTable` takes an optional `filterText` prop, passed through to the engine and kept on `engine.options` alongside `title` and `subtitle`. It deliberately does not travel on `visualization`, which should stay a partial of the saved visualization rather than a bag of render options. The prop picks the text and nothing else. Whether there is a filter row at all remains gated on `visualization.filters?.length`, exactly as before, so a supplied string cannot conjure a row the layout has no filters for, and an empty one renders an empty row rather than suppressing it. Hence `??` rather than `||`: the empty string is a supplied text like any other, and must not fall back to the derived line. `getFilterText` itself is untouched, as are its six other consumers in the Highcharts title and subtitle adapters. Co-Authored-By: Claude Opus 5 --- src/components/PivotTable/PivotTable.js | 5 +- .../PivotTable/PivotTableTitleRows.js | 12 ++- .../__tests__/PivotTableTitleRows.spec.js | 97 +++++++++++++++++++ src/modules/pivotTable/PivotTableEngine.js | 7 +- 4 files changed, 115 insertions(+), 6 deletions(-) create mode 100644 src/components/PivotTable/__tests__/PivotTableTitleRows.spec.js diff --git a/src/components/PivotTable/PivotTable.js b/src/components/PivotTable/PivotTable.js index 206ee5308..b480bc784 100644 --- a/src/components/PivotTable/PivotTable.js +++ b/src/components/PivotTable/PivotTable.js @@ -13,6 +13,7 @@ const PivotTable = ({ visualization, data, legendSets, + filterText, renderCounter, onToggleContextualMenu, }) => { @@ -25,8 +26,9 @@ const PivotTable = ({ visualization, data, legendSets, + filterText, }), - [visualization, data, legendSets] + [visualization, data, legendSets, filterText] ) const { sortBy, onSortByColumn } = useSortableColumns(engine) @@ -63,6 +65,7 @@ const PivotTable = ({ PivotTable.propTypes = { data: PropTypes.object.isRequired, visualization: PropTypes.object.isRequired, + filterText: PropTypes.string, legendSets: PropTypes.arrayOf(PropTypes.object), renderCounter: PropTypes.number, onToggleContextualMenu: PropTypes.func, diff --git a/src/components/PivotTable/PivotTableTitleRows.js b/src/components/PivotTable/PivotTableTitleRows.js index b29439261..b4e5b5498 100644 --- a/src/components/PivotTable/PivotTableTitleRows.js +++ b/src/components/PivotTable/PivotTableTitleRows.js @@ -4,6 +4,13 @@ import getFilterText from '../../visualizations/util/getFilterText.js' import { usePivotTableEngine } from './PivotTableEngineContext.js' import { PivotTableTitleRow } from './PivotTableTitleRow.js' +/* Whether there is a filter row at all is the layout's call, as it has + * always been. `filterText` only says what goes in it: supplied by the + * caller when there is one, derived from the layout when there is not. */ +const getFilterRowTitle = (engine) => + engine.options.filterText ?? + getFilterText(engine.visualization.filters, engine.rawData.metaData) + export const PivotTableTitleRows = ({ clippingResult, width }) => { const engine = usePivotTableEngine() @@ -25,10 +32,7 @@ export const PivotTableTitleRows = ({ clippingResult, width }) => { ) : null} {engine.visualization.filters?.length ? ( diff --git a/src/components/PivotTable/__tests__/PivotTableTitleRows.spec.js b/src/components/PivotTable/__tests__/PivotTableTitleRows.spec.js new file mode 100644 index 000000000..5a03c711a --- /dev/null +++ b/src/components/PivotTable/__tests__/PivotTableTitleRows.spec.js @@ -0,0 +1,97 @@ +import { render, screen } from '@testing-library/react' +import React from 'react' +import deepWithFiltersData from '../../../__demo__/data/aggregate/deepWithFilters.data.json' +import deepWithFiltersVisualization from '../../../__demo__/data/aggregate/deepWithFilters.visualization.json' +import PivotTable from '../PivotTable.js' + +/* The engine measures every cell against a canvas 2d context, which jsdom + * does not implement. */ +jest.mock('../../../modules/pivotTable/measureText.js', () => ({ + measureTextWithWrapping: () => ({ width: 100, height: 20 }), +})) + +/* The container renders nothing at all until it has a size, and jsdom reports + * every element as 0 x 0. */ +jest.mock('../../../modules/pivotTable/useParentSize.js', () => ({ + useParentSize: () => ({ width: 800, height: 600 }), +})) + +/* `deepWithFilters` filters on an org unit group set, which `getFilterText` + * resolves through `metaData.dimensions` to this name. */ +const DERIVED_FILTER_TEXT = 'Eastern Area' + +const visualization = { + ...deepWithFiltersVisualization, + title: 'Hello', + subtitle: 'Goodbye', +} + +const renderTable = (props) => + render( + + ) + +const titleRowTexts = () => + screen + .queryAllByTestId('visualization-title') + .map((element) => element.textContent) + +describe('PivotTable title rows', () => { + it('derives the filter row from the layout when no text is supplied', () => { + renderTable() + + expect(titleRowTexts()).toEqual([ + 'Hello', + 'Goodbye', + DERIVED_FILTER_TEXT, + ]) + }) + + it('renders no filter row when the layout has no filters', () => { + renderTable({ visualization: { ...visualization, filters: [] } }) + + expect(titleRowTexts()).toEqual(['Hello', 'Goodbye']) + }) + + it('renders the supplied text in place of the derived one', () => { + renderTable({ filterText: 'Age 5 - 10, Female' }) + + expect(titleRowTexts()).toEqual([ + 'Hello', + 'Goodbye', + 'Age 5 - 10, Female', + ]) + }) + + /* Whether there is a row remains the layout's call - supplying the text + * says what goes in the row, not whether there is one. */ + it('does not render a supplied text the layout has no row for', () => { + renderTable({ + visualization: { ...visualization, filters: [] }, + filterText: 'Age 5 - 10, Female', + }) + + expect(titleRowTexts()).toEqual(['Hello', 'Goodbye']) + }) + + /* An empty string is a supplied text like any other, and must not fall + * back to the derived line. */ + it('renders an empty row for an empty supplied text', () => { + renderTable({ filterText: '' }) + + expect(titleRowTexts()).toEqual(['Hello', 'Goodbye', '']) + }) + + it('leaves the title and subtitle to the visualization', () => { + renderTable({ + visualization: { ...visualization, hideSubtitle: true }, + filterText: 'Age 5 - 10', + }) + + expect(titleRowTexts()).toEqual(['Hello', 'Age 5 - 10']) + }) +}) diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js index e006a05b2..a4d09c5cc 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -298,7 +298,7 @@ export class PivotTableEngine { columnMap = [] accumulators = { rows: {} } - constructor({ visualization, data, legendSets }) { + constructor({ visualization, data, legendSets, filterText }) { this.visualization = Object.assign( {}, defaultVisualizationProps, @@ -336,6 +336,11 @@ export class PivotTableEngine { ? visualization.fixRowHeaders : false, cumulativeValues: visualization.cumulativeValues, + // Rendering-only: the caller's filter line, used in place of + // the text derived from `visualization.filters`. It does not + // affect whether the filter row is shown - that stays gated on + // the layout. + filterText, } this.adaptiveClippingController = new AdaptiveClippingController(this) From 098bbdb80eb4cfd806aba19d7ff3606de159758e Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Thu, 24 Sep 2026 11:06:56 +0200 Subject: [PATCH 3/5] docs(storybook): add a story for the supplied filter text Toggles the `deep - filter` layout between the line the lib derives from the layout ("Eastern Area") and one supplied by the caller, with a third mode showing that an empty `filters` layout has no row for either. Co-Authored-By: Claude Opus 5 --- src/__demo__/PivotTable.aggregate.stories.js | 48 ++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/__demo__/PivotTable.aggregate.stories.js b/src/__demo__/PivotTable.aggregate.stories.js index d3b558eaf..3602c89d5 100644 --- a/src/__demo__/PivotTable.aggregate.stories.js +++ b/src/__demo__/PivotTable.aggregate.stories.js @@ -470,6 +470,54 @@ DeepFilter.story = { name: 'deep - filter', } +/* This layout has a filter the lib can read, so it derives "Eastern Area". + * A caller that would rather format the line itself passes `filterText`. + * Whether there is a row at all stays the layout's call either way, which + * the third mode shows. */ +const filterTextModes = [ + { label: 'Derived by the lib' }, + { label: 'Supplied by the caller', filterText: 'Age: 5 - 10, Female' }, + { + label: 'Supplied, but the layout has no filters', + filterText: 'Age: 5 - 10, Female', + filters: [], + }, +] + +export const DeepSuppliedFilterText = (_, { pivotTableOptions }) => { + const [modeIndex, setModeIndex] = useState(0) + const mode = filterTextModes[modeIndex] + const visualization = { + ...deepWithFiltersVisualization, + ...visualizationReset, + ...pivotTableOptions, + showDimensionLabels: false, + filters: mode.filters ?? deepWithFiltersVisualization.filters, + } + return ( +
+ {filterTextModes.map((each, index) => ( + + ))} +
+ +
+
+ ) +} + +DeepSuppliedFilterText.storyName = 'deep - filter text supplied by the caller' + export const DeepTitleSubtitleFilter = (_, { pivotTableOptions }) => { const visualization = { ...deepVisualization, From fc40a49c979f36914d73a47ea2a8280913d00529 Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Thu, 24 Sep 2026 11:41:54 +0200 Subject: [PATCH 4/5] chore: tweak comment --- src/modules/pivotTable/PivotTableEngine.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js index a4d09c5cc..d2125c7e3 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -336,10 +336,8 @@ export class PivotTableEngine { ? visualization.fixRowHeaders : false, cumulativeValues: visualization.cumulativeValues, - // Rendering-only: the caller's filter line, used in place of - // the text derived from `visualization.filters`. It does not - // affect whether the filter row is shown - that stays gated on - // the layout. + /* If visualization.filters is populated and this option is present, + * this text is used instead of the text derived from `visualization.filters` */ filterText, } From ce8358254f3646fd9a778025f1f0c0df156a2814 Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Thu, 24 Sep 2026 12:07:29 +0200 Subject: [PATCH 5/5] fix: use generated string when provided string is empty and move logic inline --- src/components/PivotTable/PivotTableTitleRows.js | 15 +++++++-------- .../__tests__/PivotTableTitleRows.spec.js | 12 ++++++++---- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/components/PivotTable/PivotTableTitleRows.js b/src/components/PivotTable/PivotTableTitleRows.js index b4e5b5498..6376a7e95 100644 --- a/src/components/PivotTable/PivotTableTitleRows.js +++ b/src/components/PivotTable/PivotTableTitleRows.js @@ -4,13 +4,6 @@ import getFilterText from '../../visualizations/util/getFilterText.js' import { usePivotTableEngine } from './PivotTableEngineContext.js' import { PivotTableTitleRow } from './PivotTableTitleRow.js' -/* Whether there is a filter row at all is the layout's call, as it has - * always been. `filterText` only says what goes in it: supplied by the - * caller when there is one, derived from the layout when there is not. */ -const getFilterRowTitle = (engine) => - engine.options.filterText ?? - getFilterText(engine.visualization.filters, engine.rawData.metaData) - export const PivotTableTitleRows = ({ clippingResult, width }) => { const engine = usePivotTableEngine() @@ -32,7 +25,13 @@ export const PivotTableTitleRows = ({ clippingResult, width }) => { ) : null} {engine.visualization.filters?.length ? ( diff --git a/src/components/PivotTable/__tests__/PivotTableTitleRows.spec.js b/src/components/PivotTable/__tests__/PivotTableTitleRows.spec.js index 5a03c711a..b045d9f0f 100644 --- a/src/components/PivotTable/__tests__/PivotTableTitleRows.spec.js +++ b/src/components/PivotTable/__tests__/PivotTableTitleRows.spec.js @@ -78,12 +78,16 @@ describe('PivotTable title rows', () => { expect(titleRowTexts()).toEqual(['Hello', 'Goodbye']) }) - /* An empty string is a supplied text like any other, and must not fall - * back to the derived line. */ - it('renders an empty row for an empty supplied text', () => { + /* An empty string is a caller with nothing to say, not a caller asking + * for an empty row. */ + it('derives the filter row for an empty supplied text', () => { renderTable({ filterText: '' }) - expect(titleRowTexts()).toEqual(['Hello', 'Goodbye', '']) + expect(titleRowTexts()).toEqual([ + 'Hello', + 'Goodbye', + DERIVED_FILTER_TEXT, + ]) }) it('leaves the title and subtitle to the visualization', () => {