From 27369b43d63a02fba69ff05fdb86b5bed26108e9 Mon Sep 17 00:00:00 2001 From: Lukas Date: Mon, 20 Apr 2026 19:47:10 +0200 Subject: [PATCH] Memoize Fluent chart token resolution --- Docs/Tasks.md | 6 ++- frontend/src/components/BanTrendChart.tsx | 41 +++++++++++++++---- .../src/components/JailDistributionChart.tsx | 12 ++++-- .../src/components/TopCountriesBarChart.tsx | 14 +++++-- .../src/components/TopCountriesPieChart.tsx | 6 ++- .../__tests__/BanTrendChart.test.tsx | 19 +++++++++ .../__tests__/JailDistributionChart.test.tsx | 18 ++++++++ .../__tests__/TopCountriesBarChart.test.tsx | 21 ++++++++++ .../__tests__/TopCountriesPieChart.test.tsx | 21 ++++++++++ 9 files changed, 140 insertions(+), 18 deletions(-) diff --git a/Docs/Tasks.md b/Docs/Tasks.md index ed147fa..b954710 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -168,7 +168,11 @@ Issues are grouped by category and ordered roughly by severity. Each entry descr --- -### TASK-009 — `resolveFluentToken` calls `getComputedStyle` on every render +### TASK-009 — `resolveFluentToken` calls `getComputedStyle` on every render (done) + +**Where fixed:** `frontend/src/components/BanTrendChart.tsx`, `frontend/src/components/TopCountriesPieChart.tsx`, `frontend/src/components/TopCountriesBarChart.tsx`, `frontend/src/components/JailDistributionChart.tsx` + +**Summary:** Wrapped Fluent token resolution calls in `useMemo([])` in each chart component and added tests verifying token resolution is memoized across rerenders. **Where found:** `frontend/src/utils/chartTheme.ts`, `resolveFluentToken` function. Called 2–3 times per render in `BanTrendChart`, `TopCountriesPieChart`, `TopCountriesBarChart`, and `JailDistributionChart`. diff --git a/frontend/src/components/BanTrendChart.tsx b/frontend/src/components/BanTrendChart.tsx index 32dbfaa..0b38783 100644 --- a/frontend/src/components/BanTrendChart.tsx +++ b/frontend/src/components/BanTrendChart.tsx @@ -4,6 +4,7 @@ * Calls `useBanTrend` internally and handles loading, error, and empty states. */ +import { useMemo } from "react"; import { Area, AreaChart, @@ -134,8 +135,14 @@ function buildEntries( // Custom tooltip // --------------------------------------------------------------------------- -function TrendTooltip(props: TooltipContentProps): React.JSX.Element | null { - const { active, payload } = props; +interface TrendTooltipProps extends Partial { + backgroundColor: string; + borderColor: string; + textColor: string; +} + +function TrendTooltip(props: TrendTooltipProps): React.JSX.Element | null { + const { active, payload = [], backgroundColor, borderColor, textColor } = props; if (!active || payload.length === 0) return null; const entry = payload[0]; if (entry == null) return null; @@ -154,11 +161,11 @@ function TrendTooltip(props: TooltipContentProps): React.JSX.Element | null { return (
@@ -197,11 +204,27 @@ export function BanTrendChart({ const isEmpty = buckets.every((b) => b.count === 0); const entries = buildEntries(buckets, timeRange); - const primaryColour = resolveFluentToken(CHART_PALETTE[0] ?? ""); - const axisColour = resolveFluentToken(CHART_AXIS_TEXT_TOKEN); - const gridColour = resolveFluentToken(CHART_GRID_LINE_TOKEN); + const { primaryColour, axisColour, gridColour } = useMemo( + () => ({ + primaryColour: resolveFluentToken(CHART_PALETTE[0] ?? ""), + axisColour: resolveFluentToken(CHART_AXIS_TEXT_TOKEN), + gridColour: resolveFluentToken(CHART_GRID_LINE_TOKEN), + }), + [], + ); const tickInterval = TICK_INTERVAL[timeRange]; + const tooltipContent = useMemo( + () => ( + + ), + [], + ); + return ( - + ({ + primaryColour: resolveFluentToken(CHART_PALETTE[0] ?? ""), + axisColour: resolveFluentToken(CHART_AXIS_TEXT_TOKEN), + gridColour: resolveFluentToken(CHART_GRID_LINE_TOKEN), + }), + [], + ); return ( ({ + primaryColour: resolveFluentToken(CHART_PALETTE[0] ?? ""), + axisColour: resolveFluentToken(CHART_AXIS_TEXT_TOKEN), + gridColour: resolveFluentToken(CHART_GRID_LINE_TOKEN), + }), + [], + ); + if (entries.length === 0) { return (
@@ -154,10 +164,6 @@ export function TopCountriesBarChart({ const chartHeight = Math.max(entries.length * BAR_HEIGHT_PX, MIN_CHART_HEIGHT); - const primaryColour = resolveFluentToken(CHART_PALETTE[0] ?? ""); - const axisColour = resolveFluentToken(CHART_AXIS_TEXT_TOKEN); - const gridColour = resolveFluentToken(CHART_GRID_LINE_TOKEN); - return (
diff --git a/frontend/src/components/TopCountriesPieChart.tsx b/frontend/src/components/TopCountriesPieChart.tsx index f5ca244..a017a1a 100644 --- a/frontend/src/components/TopCountriesPieChart.tsx +++ b/frontend/src/components/TopCountriesPieChart.tsx @@ -3,6 +3,7 @@ * an "Other" slice aggregating all remaining countries. */ +import { useMemo } from "react"; import { Cell, Legend, @@ -134,7 +135,10 @@ export function TopCountriesPieChart({ }: TopCountriesPieChartProps): React.JSX.Element { const styles = useStyles(); - const resolvedPalette = CHART_PALETTE.map(resolveFluentToken); + const resolvedPalette = useMemo( + () => CHART_PALETTE.map(resolveFluentToken), + [], + ); const slices = buildSlices(countries, countryNames, resolvedPalette); const total = slices.reduce((sum, s) => sum + s.value, 0); diff --git a/frontend/src/components/__tests__/BanTrendChart.test.tsx b/frontend/src/components/__tests__/BanTrendChart.test.tsx index 372a606..06ccd61 100644 --- a/frontend/src/components/__tests__/BanTrendChart.test.tsx +++ b/frontend/src/components/__tests__/BanTrendChart.test.tsx @@ -3,6 +3,7 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { FluentProvider, webLightTheme } from "@fluentui/react-components"; import { BanTrendChart } from "../BanTrendChart"; +import * as chartTheme from "../../utils/chartTheme"; import * as useBanTrendModule from "../../hooks/useBanTrend"; import type { UseBanTrendResult } from "../../hooks/useBanTrend"; import type { BanTrendBucket } from "../../types/ban"; @@ -87,4 +88,22 @@ describe("BanTrendChart", () => { wrap(); expect(screen.getByTestId("area-chart")).toBeInTheDocument(); }); + + it("memoizes token resolution across rerenders", () => { + const spy = vi.spyOn(chartTheme, "resolveFluentToken"); + const props = { timeRange: "24h" as const, origin: "all" as const }; + mockHook({ buckets: [{ timestamp: "2024-01-01T00:00:00Z", count: 5 }] }); + + const { rerender } = wrap(); + expect(spy).toHaveBeenCalledTimes(6); + + rerender( + + + , + ); + expect(spy).toHaveBeenCalledTimes(6); + + spy.mockRestore(); + }); }); diff --git a/frontend/src/components/__tests__/JailDistributionChart.test.tsx b/frontend/src/components/__tests__/JailDistributionChart.test.tsx index 6e84612..17ff3bb 100644 --- a/frontend/src/components/__tests__/JailDistributionChart.test.tsx +++ b/frontend/src/components/__tests__/JailDistributionChart.test.tsx @@ -3,6 +3,7 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { FluentProvider, webLightTheme } from "@fluentui/react-components"; import { JailDistributionChart } from "../JailDistributionChart"; +import * as chartTheme from "../../utils/chartTheme"; import * as useJailDistributionModule from "../../hooks/useJailDistribution"; import type { UseJailDistributionResult } from "../../hooks/useJailDistribution"; @@ -84,4 +85,21 @@ describe("JailDistributionChart", () => { wrap(); expect(screen.getByTestId("bar-chart")).toBeInTheDocument(); }); + + it("memoizes token resolution across rerenders", () => { + const spy = vi.spyOn(chartTheme, "resolveFluentToken"); + const props = { timeRange: "24h" as const, origin: "all" as const }; + + const { rerender } = wrap(); + expect(spy).toHaveBeenCalledTimes(3); + + rerender( + + + , + ); + expect(spy).toHaveBeenCalledTimes(3); + + spy.mockRestore(); + }); }); diff --git a/frontend/src/components/__tests__/TopCountriesBarChart.test.tsx b/frontend/src/components/__tests__/TopCountriesBarChart.test.tsx index f6a1d2c..b130541 100644 --- a/frontend/src/components/__tests__/TopCountriesBarChart.test.tsx +++ b/frontend/src/components/__tests__/TopCountriesBarChart.test.tsx @@ -2,6 +2,7 @@ import { describe, it, expect, vi } from "vitest"; import { render, screen } from "@testing-library/react"; import { FluentProvider, webLightTheme } from "@fluentui/react-components"; import { TopCountriesBarChart } from "../TopCountriesBarChart"; +import * as chartTheme from "../../utils/chartTheme"; vi.mock("recharts", () => ({ ResponsiveContainer: ({ children }: { children: React.ReactNode }) => ( @@ -41,6 +42,26 @@ describe("TopCountriesBarChart", () => { expect(screen.getByTestId("bar-chart")).toBeInTheDocument(); }); + it("memoizes token resolution across rerenders", () => { + const spy = vi.spyOn(chartTheme, "resolveFluentToken"); + const props = { + countries: { DE: 50, US: 30, FR: 15 }, + countryNames: { DE: "Germany", US: "United States", FR: "France" }, + }; + + const { rerender } = wrap(); + expect(spy).toHaveBeenCalledTimes(3); + + rerender( + + + , + ); + expect(spy).toHaveBeenCalledTimes(3); + + spy.mockRestore(); + }); + it("does not render more than 20 bars (TOP_N limit)", () => { // Build 30 countries — only top 20 should appear in the chart const countries: Record = {}; diff --git a/frontend/src/components/__tests__/TopCountriesPieChart.test.tsx b/frontend/src/components/__tests__/TopCountriesPieChart.test.tsx index 493ae7a..f334c54 100644 --- a/frontend/src/components/__tests__/TopCountriesPieChart.test.tsx +++ b/frontend/src/components/__tests__/TopCountriesPieChart.test.tsx @@ -2,6 +2,7 @@ import { describe, it, expect, vi } from "vitest"; import { render, screen } from "@testing-library/react"; import { FluentProvider, webLightTheme } from "@fluentui/react-components"; import { TopCountriesPieChart } from "../TopCountriesPieChart"; +import * as chartTheme from "../../utils/chartTheme"; vi.mock("recharts", () => ({ ResponsiveContainer: ({ children }: { children: React.ReactNode }) => ( @@ -38,4 +39,24 @@ describe("TopCountriesPieChart", () => { ); expect(screen.getByTestId("pie-chart")).toBeInTheDocument(); }); + + it("memoizes palette token resolution across rerenders", () => { + const spy = vi.spyOn(chartTheme, "resolveFluentToken"); + const props = { + countries: { DE: 30, US: 20, CN: 10 }, + countryNames: { DE: "Germany", US: "United States", CN: "China" }, + }; + + const { rerender } = wrap(); + expect(spy).toHaveBeenCalledTimes(5); + + rerender( + + + , + ); + expect(spy).toHaveBeenCalledTimes(5); + + spy.mockRestore(); + }); });