Emissions lineplots - #660
Conversation
Expected version change and release notes🚨 WARNING: This PR is not expected to trigger a new versionTo trigger a version bump, use at least one conventional commit message in this branch. See: https://www.conventionalcommits.org/en/v1.0.0/ |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-660.westus2.2.azurestaticapps.net |
Expected version change and release notes🚨 WARNING: This PR is not expected to trigger a new versionTo trigger a version bump, use at least one conventional commit message in this branch. See: https://www.conventionalcommits.org/en/v1.0.0/ |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-660.westus2.2.azurestaticapps.net |
|
I think the plots would have to have a fixed lower bound of the y-axis at zero, at least for absolute emissions |
Expected version change and release notes🚨 WARNING: This PR is not expected to trigger a new versionTo trigger a version bump, use at least one conventional commit message in this branch. See: https://www.conventionalcommits.org/en/v1.0.0/ |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-660.westus2.2.azurestaticapps.net |
Expected version change and release notes🚨 WARNING: This PR is not expected to trigger a new versionTo trigger a version bump, use at least one conventional commit message in this branch. See: https://www.conventionalcommits.org/en/v1.0.0/ |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-660.westus2.2.azurestaticapps.net |
Expected version change and release notes🚨 WARNING: This PR is not expected to trigger a new versionTo trigger a version bump, use at least one conventional commit message in this branch. See: https://www.conventionalcommits.org/en/v1.0.0/ |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-660.westus2.2.azurestaticapps.net |
Expected version change and release notes🚨 WARNING: This PR is not expected to trigger a new versionTo trigger a version bump, use at least one conventional commit message in this branch. See: https://www.conventionalcommits.org/en/v1.0.0/ |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-660.westus2.2.azurestaticapps.net |
There was a problem hiding this comment.
Pull request overview
Updates the emissions visualizations to use line plots by routing emissions-related plot types through the existing MultiLineChart component and adapting MultiLineChart’s series grouping for emissions metrics.
Changes:
- Switch
absoluteEmissionsandemissionsIntensityplots inPlotSelectorfromVerticalBarCharttoMultiLineChart. - Adjust
MultiLineChartdata preprocessing to support emissions metrics by remapping the series key viatechnology.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/components/PlotSelector.tsx | Routes emissions plot types to MultiLineChart and removes the unused VerticalBarChart import. |
| src/components/MultiLineChart.tsx | Updates memoized filtering/grouping preparation to support emissions metrics in a line chart. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (metric == "emissionsIntensity" || metric == "absoluteEmissions") { | ||
| filtered = filtered.map((d) => ({ ...d, technology: d.metric })); | ||
| } | ||
| return filtered; | ||
| }, [data.data, sector, metric]); |
|
@jacobvjk is this something we can either wrap up or close out? |
Expected version change and release notes🚨 WARNING: This PR is not expected to trigger a new versionTo trigger a version bump, use at least one conventional commit message in this branch. See: https://www.conventionalcommits.org/en/v1.0.0/ |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-660.westus2.2.azurestaticapps.net |
Expected version change and release notes🚨 WARNING: This PR is not expected to trigger a new versionTo trigger a version bump, use at least one conventional commit message in this branch. See: https://www.conventionalcommits.org/en/v1.0.0/ |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-660.westus2.2.azurestaticapps.net |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
src/components/MultiLineChart.tsx:65
d3datarewritestechnologyforabsoluteEmissions/emissionsIntensityto collapse into a single series, butselectRefis initialized later from the originaldata.datatechnology values. That can leave the initial highlighted series unmatched (nothing selected until the user clicks). InitializeselectReffromd3datainstead so it always matches the grouped keys.
(d) => d.sector === sector && d.metric === metric,
);
if (metric == "emissionsIntensity" || metric == "absoluteEmissions") {
filtered = filtered.map((d) => ({ ...d, technology: d.metric }));
}
src/components/MultiLineChart.tsx:63
- Use strict equality checks for
metriccomparisons to avoid unintended type coercion and to match the rest of the codebase’s===usage.
if (metric == "emissionsIntensity" || metric == "absoluteEmissions") {
Expected version change and release notes:1.16.0-dev.13 (v1.16.0-dev.12...emissions-lineplots ) (2026-08-18T14:40 UTC)Features
Other |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-660.westus2.2.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-660.westus2.2.azurestaticapps.net |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/components/ComparisonPlots.test.tsx:151
- Same issue as above: without a
VerticalBarChartmock that injectsdata-testid="vertical-bar-chart", this assertion is effectively a no-op and won’t prevent a regression back to bar charts.
expect(screen.queryByTestId("vertical-bar-chart")).not.toBeInTheDocument();
src/components/ComparisonPlots.test.tsx:147
- This assertion is no longer meaningful because
VerticalBarChartis not mocked (and is no longer imported/rendered byComparisonPlots). IfVerticalBarChartwere reintroduced, the real component is unlikely to renderdata-testid="vertical-bar-chart", so this check would still pass and wouldn’t catch regressions. Either re-add avi.mock("./VerticalBarChart")that renders a sentinel test id, or remove this assertion.
This issue also appears on line 151 of the same file.
expect(screen.queryByTestId("vertical-bar-chart")).not.toBeInTheDocument();
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-660.westus2.2.azurestaticapps.net |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/components/ComparisonPlots.tsx:258
hoveredSeriesis shared across plot types, but it isn't cleared when the user switches plot/geography. Since emissions charts now useexternalHoveredSeriestoo, switching from a technology-based plot (e.g. capacity) can leave a stale hover value (e.g. "Solar") that matches no series in emissions, causing the single emissions line to render dimmed (#ddd) until hover state resets.
selectedPlot === "emissionsIntensity" ? 0 : Math.min(...allValues);
return { yMin, yMax };
}, [entries, selectedPlot, selectedGeography]);
const [hoveredSeries, setHoveredSeries] = useState<string | null>(null);
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-660.westus2.2.azurestaticapps.net |
No description provided.