Skip to content

Commit ed69a51

Browse files
authored
fix: fix percentage label consistency across all data-viz components (#5515)
1 parent fa5b4dc commit ed69a51

38 files changed

Lines changed: 2489 additions & 639 deletions

AGENTS.md

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,77 @@ create("suiteName", (data) => {
350350

351351
If you need validation state in a component, store the result with `useEffect` or call `suite.run()` inline during render (not in an effect that also resets state). See `ReviewAgreement.hooks.js` for the `useEffect`-based pattern.
352352

353+
### Data-Visualization Percentage Display Convention
354+
355+
**CRITICAL**: Data-viz components use a custom rounding scheme to avoid contradictory or broken labels. Never use `Math.round()` directly on chart percentages — always go through the shared helpers in `frontend/src/helpers/utils.js`.
356+
357+
#### Figma design rules (source of truth)
358+
359+
| Scenario | Display value |
360+
|---|---|
361+
| Value is exactly zero | `0%` |
362+
| Non-zero value that rounds to 0% | `<1%` — never show `0%` for a real non-zero value |
363+
| Dominant value that rounds to 100% while non-zero peers exist | `99%` — never show `100%` when other categories exist |
364+
| Integer labels would otherwise sum to 99% or 101% | Use largest remainder so the displayed whole-number labels sum to `100%` |
365+
| All other values | Rounded to nearest whole number |
366+
367+
**What NOT to show**: `>99%`, `0%` for non-zero values, `100%` when other non-zero items exist.
368+
369+
#### Three shared helpers (all exported from `utils.js`)
370+
371+
| Helper | Purpose |
372+
|---|---|
373+
| `computeDisplayPercent(value, total)` | Single-item display percent; returns `"<1"` instead of `0` for non-zero tiny values |
374+
| `computeDisplayPercents(items)` | Cross-item normalisation; caps a dominant item at `99` and uses largest remainder so integer labels sum to `100` when possible |
375+
| `applyMinimumArcValue(items, total)` | Floors arc slices to 1% of total **for chart geometry only** (never for legend labels) |
376+
377+
#### Why `"<1"`, `99`, and largest remainder exist
378+
379+
- `Math.round(0.4%)``0` — renders as "0%" in the legend, hiding a real slice.
380+
- `Math.round(99.6%)` with a non-zero peer → `100%` — contradicts the peer's label (e.g. "100% + <1%").
381+
- `Math.round(33.3%) + Math.round(33.3%) + Math.round(33.4%)``33 + 33 + 33 = 99` — the legend no longer adds up to the whole.
382+
383+
The helpers fix both cases:
384+
385+
```javascript
386+
// ✅ CORRECT: use shared helpers for chart labels
387+
import { computeDisplayPercents } from "../../helpers/utils";
388+
389+
const itemsWithPercent = computeDisplayPercents(rawItems);
390+
// Each item now has a `percent` field: integer (including 99 for capped dominant), or "<1"
391+
// Example: 333/333/334 becomes 33/33/34 so the legend sums to 100%
392+
```
393+
394+
```javascript
395+
// ❌ INCORRECT: raw Math.round produces "0%" for tiny non-zero values
396+
const percent = Math.round((item.value / total) * 100); // may return 0 for <0.5%
397+
```
398+
399+
#### Arc-visibility vs. legend values
400+
401+
`applyMinimumArcValue` is applied **inside `ResponsiveDonutWithInnerPercent`** automatically. Callers must **not** apply it before passing data to the component — it would corrupt the legend numbers. Supply the real `value` fields; the component floors arc geometry internally.
402+
403+
```javascript
404+
// ✅ CORRECT: pass real values; arc flooring is automatic
405+
<ResponsiveDonutWithInnerPercent data={itemsWithRealValues} />
406+
407+
// ❌ INCORRECT: pre-flooring distorts both chart AND legend
408+
const safeData = applyMinimumArcValue(items, total);
409+
<ResponsiveDonutWithInnerPercent data={safeData} />
410+
```
411+
412+
#### `HorizontalStackedBar` segment filtering
413+
414+
`HorizontalStackedBar` filters and sizes bars by `item.value` (always numeric), **not** by `item.percent` (which may be the string `"<1"`). Ensure every segment object has a numeric `value` field alongside the display `percent`.
415+
416+
```javascript
417+
// ✅ CORRECT: segment has numeric value
418+
{ id: 1, value: 500, percent: "<1", color: "...", label: "..." }
419+
420+
// ❌ INCORRECT: filtering/sizing by percent string breaks layout
421+
// Do not rely on item.percent for conditional rendering inside bar components
422+
```
423+
353424
### Fee Percentage Format Convention
354425

355426
**CRITICAL**: Fee percentages must be consistently formatted throughout the application:

docs/developers/frontend/accessibility/issue-5149-wave-3.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ Use Wave 3 to convert the existing accessibility baseline and regression-gate sc
8686
- `npx cypress run --config-file ./cypress.config.js --headless --spec "cypress/e2e/agreementDetails.cy.js,cypress/e2e/budgetLineItemsList.cy.js,cypress/e2e/createAgreementWithValidations.cy.js,cypress/e2e/uploadDocument.cy.js"` passes
8787
- no structural allowlist-backed failures remain in that targeted regression set
8888
- Completed `svg-img-alt` burn-down for the remaining tracked specs:
89-
- hid decorative portfolio legend and CAN funding icons from assistive tech in `frontend/src/components/Portfolios/PortfolioSummaryCards/PortfolioLegend.jsx` and `frontend/src/components/Portfolios/PortfolioFundingByCAN/PortfolioFundingByCAN.jsx`
89+
- hid decorative portfolio legend and CAN funding icons from assistive tech in `frontend/src/components/Portfolios/PortfolioSummaryCards/PortfolioLegend.jsx` and `frontend/src/components/Portfolios/PortfolioFundingByCAN/PortfolioFundingByCAN.jsx` (note: `PortfolioFundingByCAN` was subsequently removed in #5515)
9090
- hid decorative external-link icon in `frontend/src/components/Portfolios/PortfolioHero/HeroDescription.jsx`
9191
- hid decorative budget line summary and change-action icons in `frontend/src/components/BudgetLineItems/BLIStatusSummaryCard/BLIStatusSummaryCard.jsx` and `frontend/src/components/BudgetLineItems/ChangeIcons/ChangeIcons.jsx`
9292
- removed the final `svg-img-alt` allowlist entries from `frontend/cypress/support/a11yAllowlist.js`

frontend/AGENTS.md

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
# Frontend AGENTS.md
2+
3+
This file provides frontend-scoped guidance for agents and developers working in `frontend/`.
4+
5+
## Key Frontend Conventions
6+
7+
### Vest v6 Form Validation
8+
9+
Canonical guidance: repository root `AGENTS.md` under `Vest v6 Form Validation Patterns`.
10+
11+
- Numeric fields must not use `isNotBlank()`.
12+
- Use `suite.run(data)`, not `suite(data)`.
13+
- Do not call `only(data)` with a whole form object.
14+
- Avoid `suite.reset()` during render.
15+
16+
See root `AGENTS.md` for examples and rationale.
17+
18+
### Data-Viz Percentage Display Convention
19+
20+
Canonical guidance: repository root `AGENTS.md` under `Data-Visualization Percentage Display Convention`.
21+
22+
- Source of truth: `frontend/src/helpers/utils.js`
23+
- Use `computeDisplayPercent(value, total)` for single-item display percent logic.
24+
- Use `computeDisplayPercents(items)` for whole-part chart legends and labels.
25+
- Do not use raw `Math.round()` directly for displayed chart percentages.
26+
- Do not pre-apply `applyMinimumArcValue` to legend data.
27+
- `HorizontalStackedBar` must size and filter by numeric `value`, not display `percent`.
28+
29+
Key display rules:
30+
31+
- Exact zero displays as `0%`.
32+
- Non-zero values that would round to `0%` display as `<1%`.
33+
- A dominant value that would round to `100%` with non-zero peers displays as `99%`.
34+
- Whole-part labels use largest remainder when needed so displayed integers sum to `100%`.
35+
36+
#### Components affected by this convention
37+
38+
This rule applies to shared chart components and to feature components that prepare chart legend data, including:
39+
40+
- `src/components/Agreements/`
41+
- `src/components/BudgetLineItems/`
42+
- `src/components/Portfolios/`
43+
- `src/components/Projects/`
44+
- `src/components/UI/DataViz/`
45+
46+
### Fee Percentage Format Convention
47+
48+
Canonical guidance: repository root `AGENTS.md` under `Fee Percentage Format Convention`.
49+
50+
- Backend/storage format: `5.0` means `5%`
51+
- `calculateTotal` in `src/helpers/agreement.helpers.js` expects whole numbers
52+
- Do not pre-divide fee percentages by `100` before calling helpers
53+
- Use whole-number fee percentages in frontend tests too
54+
55+
For full examples and rationale, see the repository root `AGENTS.md`.

frontend/cypress/e2e/agreementList.cy.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,10 @@ describe("Agreement List", () => {
513513
let totalPercent = 0;
514514
$tags.each((_, tag) => {
515515
const percentText = Cypress.$(tag).text();
516-
totalPercent += parseInt(percentText, 10);
516+
// Strip display-string prefixes (">99%" → 99, "<1%" → 1, "0%" → 0)
517+
// so the sum check works correctly after cross-item normalisation.
518+
const normalized = percentText.replace(/^[><]/, "").replace(/%/g, "").trim();
519+
totalPercent += parseInt(normalized, 10) || 0;
517520
});
518521
// Due to rounding, the sum may not be exactly 100 but should be close
519522
// (within 2% due to Math.round on each individual percentage)
Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { calculatePercent } from "../../../helpers/utils";
1+
import { computeDisplayPercents } from "../../../helpers/utils";
22
import { AGREEMENT_TYPE_ORDER } from "./AgreementSpendingCards.constants";
33

44
/**
@@ -12,33 +12,37 @@ export const transformToChartData = (agreementTypes, totalSpending) => {
1212
return [];
1313
}
1414

15-
return AGREEMENT_TYPE_ORDER.flatMap((config) => {
15+
// Build all segments first without percents so we can normalise across
16+
// the full set in one pass — prevents independent-rounding sum drift and
17+
// the 100% + <1% contradiction.
18+
const segments = AGREEMENT_TYPE_ORDER.flatMap((config) => {
1619
const typeData = agreementTypes.find((at) => at.type === config.type);
1720
const newAmount = Number(typeData?.new || 0);
1821
const continuingAmount = Number(typeData?.continuing || 0);
1922

20-
const segments = [];
23+
const result = [];
2124

2225
if (newAmount > 0) {
23-
segments.push({
26+
result.push({
2427
id: config.type,
2528
label: `${config.label} (New)`,
2629
value: newAmount,
27-
color: config.color,
28-
percent: calculatePercent(newAmount, totalSpending)
30+
color: config.color
2931
});
3032
}
3133

3234
if (continuingAmount > 0) {
33-
segments.push({
35+
result.push({
3436
id: `${config.type}_CONTINUING`,
3537
label: `${config.label} (Continuing)`,
3638
value: continuingAmount,
37-
color: config.continuingColor,
38-
percent: calculatePercent(continuingAmount, totalSpending)
39+
color: config.continuingColor
3940
});
4041
}
4142

42-
return segments;
43+
return result;
4344
});
45+
46+
// Apply cross-item normalisation: 99-cap when dominant + <1% guard across all segments
47+
return computeDisplayPercents(segments);
4448
};

frontend/src/components/Agreements/AgreementSpendingCards/AgreementSpendingCards.helpers.test.js

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,4 +67,50 @@ describe("transformToChartData", () => {
6767
expect(result[6].id).toBe("DIRECT_OBLIGATION");
6868
expect(result[7].id).toBe("DIRECT_OBLIGATION_CONTINUING");
6969
});
70+
71+
it("each segment has a percent field", () => {
72+
const result = transformToChartData(mockAgreementTypes, totalSpending);
73+
result.forEach((segment) => {
74+
expect(segment).toHaveProperty("percent");
75+
});
76+
});
77+
78+
it("dominant segment shows 99 (not 100 or '>99') when non-zero peers exist", () => {
79+
const dominantTypes = [
80+
{ type: "CONTRACT", new: 996, continuing: 0 },
81+
{ type: "GRANT", new: 4, continuing: 0 }
82+
];
83+
const result = transformToChartData(dominantTypes, 1000);
84+
const contract = result.find((d) => d.id === "CONTRACT");
85+
const grant = result.find((d) => d.id === "GRANT");
86+
expect(contract.percent).toBe(99);
87+
expect(grant.percent).toBe("<1");
88+
});
89+
90+
it("3-way equal split: largest remainder assigns the extra point to the biggest segment", () => {
91+
const equalTypes = [
92+
{ type: "CONTRACT", new: 333, continuing: 0 },
93+
{ type: "GRANT", new: 333, continuing: 0 },
94+
{ type: "DIRECT_OBLIGATION", new: 334, continuing: 0 }
95+
];
96+
const result = transformToChartData(equalTypes, 1000);
97+
98+
expect(result.map((segment) => segment.percent)).toEqual([33, 33, 34]);
99+
});
100+
101+
it("sub-1% non-zero segment shows '<1' instead of 0", () => {
102+
const tinyTypes = [
103+
{ type: "CONTRACT", new: 996, continuing: 0 },
104+
{ type: "GRANT", new: 4, continuing: 0 }
105+
];
106+
const result = transformToChartData(tinyTypes, 1000);
107+
const grant = result.find((d) => d.id === "GRANT");
108+
expect(grant.percent).toBe("<1");
109+
});
110+
111+
it("single segment gets 100% (no non-zero peers — correct)", () => {
112+
const singleType = [{ type: "CONTRACT", new: 1000, continuing: 0 }];
113+
const result = transformToChartData(singleType, 1000);
114+
expect(result[0].percent).toBe(100);
115+
});
70116
});

frontend/src/components/Agreements/AgreementSpendingSummaryCard/AgreementSpendingSummaryCard.jsx

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React from "react";
2-
import { calculatePercent } from "../../../helpers/utils";
2+
import { computeDisplayPercents } from "../../../helpers/utils";
33
import ResponsiveDonutWithInnerPercent from "../../UI/DataViz/ResponsiveDonutWithInnerPercent";
44
import CustomLayerComponent from "../../UI/DataViz/ResponsiveDonutWithInnerPercent/CustomLayerComponent";
55
import LegendItem from "../../UI/Cards/LineGraphWithLegendCard/LegendItem";
@@ -28,41 +28,45 @@ const AgreementSpendingSummaryCard = ({
2828

2929
const totalAmount = contractTotal + partnerTotal + grantTotal + directObligationTotal;
3030

31-
const data = [
31+
// Build raw items then apply cross-item percent normalisation so that no
32+
// single slice can show "100%" while non-zero peers exist, and no non-zero
33+
// slice rounds down to "0%".
34+
const rawItems = [
3235
{
3336
id: 1,
3437
label: "Contract",
3538
value: contractTotal,
3639
color: "var(--data-viz-agreement-contract)",
37-
percent: calculatePercent(contractTotal, totalAmount),
3840
tagStyleActive: "whiteOnContractBlue"
3941
},
4042
{
4143
id: 2,
4244
label: "Partner",
4345
value: partnerTotal,
4446
color: "var(--data-viz-agreement-partner)",
45-
percent: calculatePercent(partnerTotal, totalAmount),
4647
tagStyleActive: "darkOnPartnerGreen"
4748
},
4849
{
4950
id: 3,
5051
label: "Grant",
5152
value: grantTotal,
5253
color: "var(--data-viz-agreement-grant)",
53-
percent: calculatePercent(grantTotal, totalAmount),
5454
tagStyleActive: "darkOnGrantOrange"
5555
},
5656
{
5757
id: 4,
5858
label: "Direct Obligation",
5959
value: directObligationTotal,
6060
color: "var(--data-viz-agreement-direct-obligation)",
61-
percent: calculatePercent(directObligationTotal, totalAmount),
6261
tagStyleActive: "whiteOnDirectObligationPink"
6362
}
6463
];
6564

65+
// Legend data: real values + cross-item-normalised display percents
66+
const legendData = computeDisplayPercents(rawItems);
67+
68+
// ResponsiveDonutWithInnerPercent applies applyMinimumArcValue internally.
69+
6670
return (
6771
<RoundedBox
6872
dataCy="agreement-spending-summary-card"
@@ -75,7 +79,7 @@ const AgreementSpendingSummaryCard = ({
7579
className="font-12px flex-fill"
7680
style={{ marginRight: "1rem" }}
7781
>
78-
{data.map((item) => (
82+
{legendData.map((item) => (
7983
<LegendItem
8084
key={item.id}
8185
id={item.id}
@@ -96,7 +100,7 @@ const AgreementSpendingSummaryCard = ({
96100
role="img"
97101
>
98102
<ResponsiveDonutWithInnerPercent
99-
data={data}
103+
data={legendData}
100104
width={150}
101105
height={150}
102106
margin={{ top: 10, right: 10, bottom: 10, left: 10 }}

0 commit comments

Comments
 (0)