feat: add Combined (multilayer) data table [PR7] - #3741
Draft
BRaimbault wants to merge 70 commits into
Draft
Conversation
Previously any DATA_TABLE_TOGGLE cleared the whole selection regardless of which layer's tab was toggled. Harmless with today's single-layer data table, but would silently wipe another layer's selection once multiple tabs can be open at once.
…edView, joinConfig} state.dataTable moves from a single layer id (string|null) to an object supporting multiple open tabs plus a Combined cross-layer join config. combinedView/joinConfig are deliberately independent of openIds - toggling an individual tab never touches them, only LAYER_REMOVE prunes dangling join references and auto-clears combinedView if that removal makes the join insufficient. Also restored from a saved favourite via MAP_SET's payload (wired up fully in a later persistence commit).
DataTable/FilterInput now receive the active layer id as a prop from BottomPanel instead of each independently reading state.dataTable, since the panel is the natural single source of truth for "which tab is active". LayerToolbarMoreMenu and useLayersLoader switch from a scalar layer-id comparison to openIds.includes(id) - the useLayersLoader change also fixes a latent imprecision where a brand-new event layer's loadExtended flag was keyed off "is any table open" rather than "is *this* layer's table open".
Renders one Tab per open layer plus a "Combined" tab whenever the map has 2+ data-table-capable layers with loaded data - the Combined tab isn't gated on how many tabs happen to be open, only on how many layers exist to combine (wired up fully in a later commit). Per-layer-only toolbar controls (highlight color, column picker, clear filters, search, show-in-view) hide while Combined is active, since they don't apply to a cross-layer view. Each tab's close icon can't be a real <button> - Tab's own root element is already a <button>, and HTML forbids nesting interactive elements - so it's a role="button" span with its own click/keydown handling instead.
Join level (org unit / parent org unit / spatial) is chosen via a select in the toolbar; the spatial option only appears once at least one point layer and one polygon layer exist on the map (classified by actual feature geometry, not layer type). Org unit/parent-org-unit modes pick which layers participate via the new JoinLayersControl, a checkbox popover mirroring ColumnPickerControl's pattern; spatial mode uses two selects for the point source and polygon target instead. The actual cross-layer join/render logic lands in later commits - this only wires up the config UI and dispatches DATA_TABLE_JOIN_CONFIG_SET.
…olygon dep Matches each point feature in one layer to the first polygon feature in another that spatially contains it, for the Combined view's spatial join mode. @turf/boolean-point-in-polygon is already a dependency (used in util/geojson.js) - no new package needed.
…al joins) Joins the selected layers' data for the Combined view: - orgUnit: rows keyed by orgUnitId, one value/legend column pair per layer, blank for layers with no feature at that org unit. - parentOrgUnit: same, but grouped one level up the org unit hierarchy (parent id/path derived, name resolved via the existing useOrgUnitAncestorNames hook from the PR6 filters work) with numeric values averaged per group and legend left blank (no single class applies to an aggregate). - spatial: point-in-polygon via spatialJoin, with a spatialWarning flag above a 10k-feature threshold on either layer. No Index column, matching the PR3 decision to drop it entirely rather than resurrect the abandoned resolved-ids filtering approach.
Renders useCombinedTableData's headers/rows via TableVirtuoso, reusing the same @dhis2/ui DataTable primitives as the single-layer table for visual consistency, with a warning banner above the 10k-feature spatial-join threshold. BottomPanel now routes to it instead of DataTable whenever combinedView is active, resolving the layer set from joinConfig. Also fixes a bug this surfaced: activeLayerId was seeded via useState(null) and only synced to openIds a render later via useEffect, so a child requiring a non-null layerId (ColumnPickerControl) saw null for one render and logged a prop-types warning. Replaced with a value derived synchronously from openIds plus the last manually-clicked tab, so there's no render where it's out of sync. Regenerated i18n/en.pot to catch up the translatable strings added across this commit and the preceding tab-bar/join-controls ones.
…View state Fresh-context review of commits 1-8 caught three issues: - useCombinedTableData read orgUnitOwn directly as a display name, but it's a raw slash-path of org unit ids at the data layer (see attachOrgUnitPaths) - every other consumer resolves it through formatOrgUnitOwnName + an idToName map first. The org unit and spatial join modes were showing raw id paths instead of names; only the parentOrgUnit mode already resolved correctly. Now all three modes share one useOrgUnitAncestorNames call (passing full org-unit paths resolves every ancestor id along the way, including parents, for free). - dataTable reducer's DATA_TABLE_TOGGLE left combinedView/joinConfig untouched even when it closed the last open tab, even though the panel itself fully unmounts at that point (App.jsx gates on openIds.length). Reopening any single layer's table afterward would silently land back in a stale Combined view. Now a toggle that empties openIds gets the same full reset as an explicit DATA_TABLE_CLOSE. - BottomPanel's combinedLayers array was rebuilt with a new identity on every render, defeating useCombinedTableData's internal memoization on unrelated re-renders (row count updates, resizes). Wrapped in useMemo. Also fixes two test fixtures (useCombinedTableData.spec.js, CombinedDataTable.spec.jsx) that stubbed orgUnitOwn with an already-resolved name string, which would have passed even with the resolution bug present.
…w joins The Combined table showed 0 rows for org unit and parent-org-unit joins whenever the participating layers were org-unit-identity types (thematic, org unit, facility) - confirmed live against real running data. orgUnitId is only populated by attachOrgUnitPaths() (util/orgUnits.js), used for layers whose features reference an org unit they aren't themselves (events, tracked entities). For layers where the feature IS the org unit, data is built by the older toGeoJson() (util/map.js), which sets orgUnitPath/orgUnitOwn/level but never orgUnitId - the org unit's id is just the feature's own id there. useCombinedTableData only read orgUnitId, so byOrgUnit came back empty for these layer types and every row got filtered out. Also fixes the test fixtures, which had set orgUnitId explicitly and so never exercised this path - same masking pattern as the earlier orgUnitOwn-as-resolved-name fixture bug.
…S2-20543] A Combined-view row can span multiple layers at once, but state.feature and state.selection are each scoped to a single layerId - there was no way to say "highlight these ids on layer A and those ids on layer B" simultaneously. Add an optional crossLayerIds map to both, consumed alongside the existing single-layer id in Layer.js, so every existing single-layer dispatch (map hover, table row selection) is unaffected.
…a [DHIS2-20543] Rows had no way to say "which feature id(s) on which layer(s) does this row correspond to", blocking cross-layer map highlight/selection for Combined rows. Add featureIdsByOrgUnit (all matching feature ids per org unit per layer, not just the one whose value is displayed) and a rowFeatureIds map from each row's key to its per-layer feature ids, covering all three join modes.
… and filtering [DHIS2-20543] CombinedDataTable had no sorting or filtering at all. Restructure useCombinedTableData to build flat row objects per join mode and finalize them through the same filterData/filterByGlobalSearch/compareRows/ buildRowCells pipeline the single-layer table already uses, instead of hand-rolling cell arrays. Add column sort buttons and a per-column filter input to CombinedDataTable, and let global search/clear-filters in BottomPanel's toolbar apply to the Combined view too (session-only, matching its existing ephemeral column-config scope).
…S2-20543] Combined rows had no map interaction at all. Add a checkbox column and row hover, dispatching the same highlightFeature/setCrossLayerSelection actions the single-layer table uses but with crossLayerIds (per-layer feature ids merged across every affected row) instead of a single layerId/id pair, so a Combined row lights up its matching feature on every participating layer at once. Selection stays local to the table (session-only); only the map-facing Redux dispatch is shared.
…and CombinedDataTable [DHIS2-20543] CombinedDataTable's selection/hover/sort logic (previous commit) hand-copied patterns already in DataTable.jsx, and its header/checkbox/cell markup and CSS were separately hand-rolled rather than shared - leaving the two tables visually and functionally inconsistent (a stray unhidden filter-icon button, missing row/header padding and font-size, and a plain text filter input instead of the real searchable/checkbox popover). Extract the callback-driven, dispatch-agnostic pieces into shared hooks/ components used by both: - useRowSelection: takes an onChange(nextIds) callback instead of dispatching Redux directly, so the caller decides where selection state lives (a single layer's Redux selection, or Combined's local state). - useRowClickSelection: shift/ctrl row-click-to-selection-action handling, extracted from DataTable.jsx. - useSortState: the three-click asc/desc/none sort useReducer. - SortableColumnHeader / SelectionCheckboxColumn: the sortable header cell and checkbox column markup, byte-for-byte identical between the tables. - FilterInput: generalized to take filterValue/onChange/onClear props instead of reading a real layer's Redux dataFilters and dispatching directly, so Combined can reuse the same searchable/checkbox popover UI against its own local, session-only filter state. DataTable.jsx's behavior and dispatched actions are unchanged. - tableColumns.js: extracted sortColumnOptions (the cheap re-sort step already used by DataTable's column filter options) so Combined's filter popover gets the same sorted distinct-value list; useCombinedTableData now computes columnOptions the same way useTableData does. - CombinedDataTable now reuses TableVirtuosoComponents' generic row/table wiring and DataTable.module.css's dataCell/columnHeader classes instead of hand-rolled duplicates. Verified live against a real DHIS2 instance: Combined's header/row/filter appearance now matches the single-layer table exactly, and a row's checkbox selection correctly sets crossLayerIds across all participating layers in both state.selection and state.feature.
…ataTable [DHIS2-20543] CombinedDataTable rendered raw cell values directly, so numeric columns never got digit-group formatting (or any of DataTable's renderer-specific formatting for color/icon/date/org-unit/boolean columns), unlike the single-layer table. Extract DataTable.jsx's inline cell-content logic into a shared CellValue component (plus a getCellRendererFlags helper for the className-side flags each caller still needs), used by both tables. Also unify on Combined's em-dash placeholder for blank cells (previously DataTable rendered these as empty) - CellValue now applies it uniformly regardless of renderer, so both tables show "—" for a missing value.
…taTable [DHIS2-20543] Reuses the same getVisibleHeaders/getPinnedCount/getPinnedLeftOffsets/ getPinnedCellProps/useColumnWidths pipeline DataTable.jsx already has, plus the same ColumnPickerControl UI, generalized to be dispatch-agnostic (onChange callback instead of an internal setDataTableColumnConfig dispatch) so Combined can drive it with local, session-only state instead of a per-layer Redux field.
….jsx [DHIS2-20543] Map.jsx only passed each Layer instance its feature prop when feature.layerId === config.id, an ownership check that a crossLayerIds-based highlight (layerId: null, set by CombinedDataTable row hover) can never satisfy - the highlight silently reached no layer at all. Layer.js's own getHoverIds already narrows crossLayerIds down per layer, so Map.jsx only needs to forward the feature to layers actually named in it.
…-20543] Zoom to feature/selected/filtered features and drill up/down (orgUnit join mode only, no "View profile" since a Combined row can span several features across layers) - mirrors TableContextMenu.jsx's design, reduced to the scope confirmed for Combined. Since a Combined row has no single owning layerId, its zoom can't reuse each Layer instance's own fitBounds - multiple matching instances would race. Map.jsx now also fits a precomputed union bbox directly for a crossLayerIds zoom (fitCrossLayerZoomBounds in util/map.js), computed by CombinedDataTable from every matching feature across every participating layer's raw data (getUnionBounds in util/dataTable.js).
… review [DHIS2-20543] A fresh-context review of the full PR7 diff found three real gaps in the crossLayerIds mechanism, all instances of the same class of bug already fixed once this session for Map.jsx's feature highlight: - BottomPanel stays open with openIds empty when combinedView is still a valid, active state (e.g. every single-layer tab was closed while Combined stayed open) - App.jsx/MapPosition.jsx were gating the whole panel on openIds.length alone, hiding it even though Combined was still legitimately showing. New isDataTableOpen() util fixes both. - DataTable.jsx's own selectedIds (checkbox column, row highlighting) ignored selection.crossLayerIds, so a Combined-originated selection never showed as selected when switching to that layer's own tab, even though the map highlight was already correct. Extracted the merge logic Layer.js#getSelectedIds already had into a shared getLayerSelectedIds(), used by both now. - Layer.js#getVisibleIds (the "show only selected/not-selected" map filter) had the same ownership-only gate, so a crossLayerIds selection was silently treated as "no selection" for this filter. Also: useCombinedTableData.js only read layer.data, never layer.dataWithoutCoords - unlike the single-layer table (util/tableRows.js), so org units/facilities missing coordinates were silently dropped from the Combined join entirely, not just hidden from the map.
…st-tab-close [DHIS2-20543] Confirmed with the user: closing the last open single-layer tab should behave the same as removing a layer does (LAYER_REMOVE already got this right) - combinedView/joinConfig survive as long as they're still valid, matching the plan's stated decoupling of Combined from openIds. Panel visibility is entirely isDataTableOpen()'s job now, not this reducer's.
…nner) [DHIS2-20543]
i18next-scanner's default nsSeparator is ':', so i18n.t('Spatial:
point inside polygon') got parsed as namespace "Spatial" + key " point
inside polygon" instead of one translation key - reproduced via
`yarn start` (fails during string extraction) and `d2-app-scripts i18n
extract`.
…enu-bar Data Table shortcut [DHIS2-20543] BottomPanel no longer shows one tab per open layer plus a Combined tab - a single dropdown (LayerSelectorControl) lists every open layer by name, with Combined as an additional option, greyed out unless the map has 2+ eligible layers to join. Per-tab closing is dropped: a layer's table can already be closed via "Show data table" in that layer's own overlay-card menu, so the panel doesn't need a second, redundant close affordance. Also adds a "Data Table" button to the main menu bar, after Download. If no data table is open yet, it opens Combined (pre-populated with every eligible layer) when 2+ are eligible, or the single eligible layer's own table otherwise; it's a no-op if a table is already open, and disabled when the map has no eligible layers at all. getEligibleDataTableLayers() and isDataTableOpen() (util/dataTable.js) are now shared between BottomPanel and the new button rather than duplicated.
…open ones [DHIS2-20543] LayerSelectorControl now lists every data-table-eligible layer on the map, whether or not its table has been opened yet - selecting one that isn't open yet opens it (dispatches toggleDataTable) as well as making it active, matching how the "Data Table" menu button already opens a layer's table on demand.
…s [DHIS2-20543] Reuses the thematic layer's own aggregation type set (minus DEFAULT, which only makes sense for a thematic layer's own data element config) for the Combined data table's upcoming per-layer aggregation choice, plus a small reducer map to actually apply one of these types to a list of raw values.
…UnitDialog [DHIS2-20543] A hidden, non-rendered org-unit layer type that will back the Combined data table's join (reference org unit set) - deliberately its own type rather than orgUnit + a flag, so it's excluded from DOWNLOADABLE_LAYER_TYPES/DATA_TABLE_LAYER_TYPES and the "Add layer" popover just by omission. Registered against the existing orgUnitLoader/OrgUnitDialog so it's fully functional without duplicating either. Excluded from LayersPanel's list (with its own drag-reorder index fix, since LAYER_SORT's reducer computes positions against the full mapViews array, not a filtered display list) and from both the standalone app's and dashboard plugin's map-rendering pipelines (including their respective "is everything loaded" bookkeeping, which would otherwise get stuck on a reference-only map).
…e reference layer [DHIS2-20543]
OrgUnitDialog can now hide its Style tab entirely (no style is needed
for a non-rendered reference layer) via a new hideStyleTab prop.
LayerEdit.jsx computes it, and gives the reference layer type a single
state-agnostic modal title ("Configure reference org units") instead
of the generic Edit/Add wording every other layer type gets.
New ReferenceOrgUnitControl (not yet wired into BottomPanel's toolbar -
that lands with the rest of the Combined join UI rework) opens the
existing reference layer for editing, or a fresh draft if none exists
yet, via the same editLayer/LayerEdit.jsx flow every other layer uses.
…543] Replaces the single global join mode (org unit / parent org unit / fixed point+polygon spatial) with a per-layer choice: each participating layer picks its own join type (org unit hierarchy, or spatial via point-in-polygon against the reference org unit's own boundary, using a centroid for non-point geometry) and aggregation type per numeric column. Rows are now always one per reference org unit, sourced directly from the combinedTableRef layer's own resolved features. Also fixes a bootstrapping bug caught during manual testing: selecting "Combined" before a reference has been configured now opens its editor directly instead of leaving the option in an unreachable disabled state, and Spatial join eligibility is decided purely by geometry rather than layer type, so layers with no org-unit identity of their own (e.g. GeoJSON URL) aren't left with no working join mechanism.
Stamps state.dataTable.joinConfig.layers onto the combinedTableRef mapView just before save (its own Redux slice, so it can't ride along automatically the way dataTableColumnConfig does), packs/restores it through the existing config JSON mechanism (favorites.js/ orgUnitLoader.js), and hydrates it back into session state once, the moment the reference layer finishes loading a saved map.
…543] - DataTableButton.jsx (the menu-bar quick-open shortcut) still built a joinConfig in the old level/layerIds/pointLayerId/polygonLayerId shape and forced Combined on directly, bypassing the reference-layer requirement entirely and crashing the table - it now only opens Combined when a reference is already configured, falling back to the first eligible layer otherwise. - The dataTable reducer wiped joinConfig back to empty on DATA_TABLE_CLOSE/DOWNLOAD_MODE_OPEN/DOWNLOAD_MODE_CLOSE, silently discarding a user's join setup on the ordinary, frequent act of closing the panel - now only openIds/combinedView reset there, joinConfig itself survives (it's real, savable configuration now, not throwaway display state). - JoinLayersControl defaulted every newly-joined layer to Org unit join, which can never match for a layer with no org-unit identity of its own (e.g. GeoJSON URL) - now defaults to whichever join type can actually produce a result.
…ide the current-period column
|
BRaimbault
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements DHIS2-20543
Description
Adds the Combined data table — a new view that joins multiple map layers into a single table, keyed off a reference org-unit layer, instead of viewing one layer's data at a time.
The rest of the branch is stack cleanup carried along from earlier PRs in this series: data table filter/sort/selection consistency fixes, hidden technical columns, a handful of Combined-table persistence and join-parity fixes found in review, and a duplicate-API-call performance pass (org unit names, option sets, and Event layer reloads were being fetched more than once).
Quality checklist
Add N/A to items that are not applicable.
Screenshots
supporting images