Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #49 +/- ##
==========================================
- Coverage 91.55% 91.40% -0.15%
==========================================
Files 78 78
Lines 1574 1535 -39
Branches 288 282 -6
==========================================
- Hits 1441 1403 -38
+ Misses 117 116 -1
Partials 16 16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Updates the region “Compare” long-term cases visualization from a line chart to a grouped column (bar) chart, aiming to make differences between present/baseline-long-term/full-long-term scenarios easier to interpret.
Changes:
- Switched compare chart series and Highcharts config from
linetocolumn, including category x-axis + cost shown via data labels. - Updated the compare results component to pass scenario ordering into
getCasesCompareConfig. - Updated (and removed now-obsolete parts of) the cases chart config unit tests to reflect the new chart type/signature.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
client/src/lib/charts/casesConfig.ts |
Reworks compare chart config to a column chart with category x-axis and cost data labels. |
client/src/routes/projects/[project]/regions/[region]/compare/_components/CompareResults.svelte |
Passes derived scenario list into the updated compare chart config function. |
client/src/routes/projects/[project]/regions/[region]/compare/+page.server.ts |
Adds explicit typing for compareParameters. |
client/src/tests/lib/charts/casesConfig.spec.ts |
Updates tests for new column-series behavior and new getCasesCompareConfig signature; removes tests for removed helpers. |
| data: Object.entries(totalCasesAndCosts).map(([scenario, { totalCases, totalCost }]) => ({ | ||
| name: ScenarioToLabel[scenario as Scenario], | ||
| y: totalCases, | ||
| custom: { | ||
| intervention: ScenarioToLabel[scenario as Scenario] | ||
| dataLabels: { | ||
| enabled: true, |
There was a problem hiding this comment.
createCasesCompareSeries builds data via Object.entries(totalCasesAndCosts), but the chart’s x-axis categories come from the separate scenarios array in getCasesCompareConfig. If any series is missing a scenario (or objects have different key insertion order), points will be plotted under the wrong category index, breaking the grouped-bar comparison and shared tooltip.
To fix, build each series’ data by mapping over the scenarios array (and inserting null/undefined y values for missing scenarios), or explicitly set x based on the scenario index so all series align with xAxis.categories.
| import type { CompareTotals } from '$lib/types/compare'; | ||
| import type { Scenario } from '$lib/types/userState'; | ||
| import type { PointOptionsObject } from 'highcharts'; | ||
| import { describe, expect, it } from 'vitest'; |
There was a problem hiding this comment.
This spec uses vi.spyOn(...) later in the file but vi is no longer imported from vitest, so the tests will fail at runtime. Add vi back to the Vitest import (or avoid vi in the test).
| import { describe, expect, it } from 'vitest'; | |
| import { describe, expect, it, vi } from 'vitest'; |
After discussions with jack. we have decided to change the long term cases chart to be a grouped bar chart. This was because the line chart was a bit confusing to interpret. The new bar chart allows us to more easily interpret how cases change with updating baseline parameters and interventions.
Testing:
Can use mint-dev as its deployed on there. Just run a region and then go to long term. play around with changing formvalues.
OLD