Skip to content

fix: consistent value-to-legend-item matching and small fixes [PR1] [DHIS2-18242]#3660

Merged
BRaimbault merged 7 commits into
feat/DHIS2-18242from
feat/DHIS2-18242-PR1
Jun 2, 2026
Merged

fix: consistent value-to-legend-item matching and small fixes [PR1] [DHIS2-18242]#3660
BRaimbault merged 7 commits into
feat/DHIS2-18242from
feat/DHIS2-18242-PR1

Conversation

@BRaimbault
Copy link
Copy Markdown
Collaborator

Parent

Note: PR re-created from: #3645

Overview

Foundational refactor and small bug fixes extracted from the parent epic, landing first as a prerequisite for the classification and legend work to follow. No user-visible feature change (except the clamp fix, see below); mostly internal plumbing.

Changes

Thread valueFormat through value-to-legend-item matching

getLegendItems and its helpers (getEqualIntervals, getQuantiles) now return { items, valueFormat } instead of a bare array. getAutomaticLegendItems propagates the valueFormat, and both callers (thematicLoader.js and styleByDataItem.js) pass it to getLegendItemForValue. Incoming values are rounded to the same precision as bin boundaries before comparison, eliminating a class of off-by-epsilon mismatches at bin edges.

Files: src/util/classify.js, src/util/legend.js, src/loaders/thematicLoader.js, src/util/styleByDataItem.js

Named args for getAutomaticLegendItems

Switched from positional to named parameters; removes the eslint-disable max-params override.

Unified clamp behavior between thematic and event layers

Event style-by-data-item with automatic classification didn't clamp out-of-range values while thematic layer did — a pre-existing asymmetry in master. Both callers now use clamp: method !== CLASSIFICATION_PREDEFINED.

Consequence: events with values slightly outside the auto-classified range (typically a rounding-gap artefact) are now pulled into the nearest bin instead of being dropped to the "Other" colour. The "Other" bucket still catches values that genuinely have no data.

Files: src/loaders/thematicLoader.js, src/util/styleByDataItem.js

Fix min/max calculation in thematicLoader.js for legend sets

When a predefined legendSet was used, minValue/maxValue were read from legend.items[0] and legend.items[legend.items.length - 1]. If a noData item was present at either end, its sentinel value was picked
up and corrupted the bubble-radius scaleSqrt domain. Fixed by filtering noData items first. The .at(-1) rewrite is cosmetic.

Files: src/loaders/thematicLoader.js

Robust legend-item sorting

sortLegendItems rewritten:

  • no longer mutates input ([...items].sort(...))
  • normalises both { from, to } and { startValue, endValue } shapes on both sides of the comparator (previously only a was inspected)
  • handles items with no range shape (sorted to the end deterministically)
  • tiebreaker on equal start values

Files: src/util/legend.js

Filter noData items by flag in styleByDataItem.js

Replaced config.legend.items.slice(0, -1) with legend.items.filter((item) => !item.noData). The "Other" legend item
now carries noData: true so the filter captures it. Mirrors the pattern already used in thematicLoader.js; forward-compatible with additional noData items that later PRs may introduce.

Files: src/util/styleByDataItem.js

Local variable renames and dead-guard removal in classify.js

Pure readability cleanup in getQuantiles / getEqualIntervals: binsitems, binSizeclassSize, binCountvaluesCount, binLastValPoslastValuePosition. Dropped the unreachable binCount === 0 ? 0 : binCount guard (always equivalent to binCount because the enclosing if (values.length > 0) is skipped when it would matter). Added ?? {} fallback in getLegendItems for unknown methods.

Files: src/util/classify.js

Drive-by modernisations

  • String.prototype.replace(/…/g, …)replaceAll(…, …) in 9 files
  • isNaN(x)Number.isNaN(x) in PeriodSelect.jsx and RadiusSelect.jsx

No behaviour change. Addresses SonarQube findings.

Tests update

  • src/util/__tests__/classify.spec.js
  • src/util/__tests__/legend.spec.js

Quality checklist

Add N/A to items that are not applicable.

  • Jest tests added/updated
  • Docs added N/A
  • d2-ci dependencies replaced N/A
  • Include plugin in testing N/A
  • Tester approved N/A

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 2, 2026

@dhis2-bot
Copy link
Copy Markdown
Contributor

🚀 Deployed on https://pr-3660.maps.netlify.dhis2.org

@dhis2-bot dhis2-bot temporarily deployed to netlify June 2, 2026 08:41 Inactive
@BRaimbault BRaimbault merged commit 732281f into feat/DHIS2-18242 Jun 2, 2026
70 of 72 checks passed
@BRaimbault BRaimbault deleted the feat/DHIS2-18242-PR1 branch June 2, 2026 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants