diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 5e2cf75..90e7088 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -8,37 +8,79 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue. ## Open Issues -### 1. History Screen — Move Jail & IP Address Filters Into the Time Range Bar +### ~~WorldMap hover highlight: fill color sometimes does not change~~ ✅ Done -**Goal:** Unify all History-page filters into a single bordered bar so that Jail and IP Address sit inside the same card/border as Time Range and Filter, separated by vertical dividers. +**Status:** Completed. -**Current state:** -- `DashboardFilterBar` (`frontend/src/components/DashboardFilterBar.tsx`) renders a single bordered card (`cardStyles.card`) that contains two groups — **Time Range** (toggle buttons) and **Filter** (origin toggle buttons) — separated by a vertical ``. -- In `HistoryPage` (`frontend/src/pages/HistoryPage.tsx`, lines 476–510) the Jail `` and IP Address `` are rendered **outside** that bar as separate cards, each wrapped in their own `cardStyles.card` div, laid out horizontally via `styles.filterRow` (flexbox row with gap). +**Summary:** Memoized Geography style objects via `useMemo` keyed by `geographies`, `countries`, `selectedCountry`, and threshold props. Stabilized `onMouseEnter`, `onMouseMove`, and `onMouseLeave` handlers with `useCallback` using `data-*` attributes to avoid per-Geography closures. This ensures `React.memo(Geography)` correctly skips re-renders when only tooltip position changes, eliminating the state-update race that caused hover fills to flash back to defaults. -**Desired state:** -- The Jail and IP Address inputs must move **inside** the `DashboardFilterBar` card border (or the equivalent combined container) so the entire filter strip looks like one cohesive section. -- Each new group (Jail, IP Address) is separated from its neighbor by a vertical divider (`|`), using the same `` + `styles.divider` pattern already used between Time Range and Filter. -- Inside each group the label text ("Jail", "IP Address") must appear **to the left** of its input field (i.e. `flexDirection: "row"` with `alignItems: "center"`, not above it). This matches the existing group style where the title text sits to the left of the toolbar buttons. -- The visual order inside the bar is: **Time Range** | **Filter** | **Jail** | **IP Address**. +**Symptom:** On mouse-over, a country's stroke correctly becomes bold (`strokeWidth: 1`), but the background fill colour sometimes stays at the default value instead of switching to the hover colour. + +**Root cause — `React.memo` is defeated by new style objects on every render:** + +`Geography.js` (`react-simple-maps-main/src/components/Geography.js`) is wrapped in `memo()`. It keeps an internal `isFocused` state that toggles between `style.default` and `style.hover`. This works correctly in isolation. + +However, in `WorldMap.tsx` (`frontend/src/components/WorldMap.tsx`): + +1. **`onMouseMove`** calls `setTooltip(...)` on every pixel of mouse movement, triggering a GeoLayer re-render. +2. Each GeoLayer render creates a **new `style` object literal** for every `` (lines ~199–222). Because `memo()` does a shallow reference comparison, it sees a "new" `style` prop every time and **re-renders all ~200 Geography components** on every mouse-move event. +3. During these rapid cascading re-renders, React's batched state updates can cause `Geography`'s internal `setFocus(true)` (fired in `handleMouseEnter`) and the parent's `setTooltip` to race. When the parent re-render is processed before `isFocused` is committed, Geography momentarily renders with `isFocused = false` and picks `style.default` — the fill stays at the default colour while the stroke (which changes less dramatically) appears correct. + +A secondary contributing factor: for countries with `count === 0` that are not selected, `style.hover.fill` equals `style.default.fill` (both resolve to `fillColor`), so there is intentionally no visible fill change on hover for those countries. This may overlap with the perceived bug if the user expects all countries to highlight. + +**Fix — memoize the style objects so `memo()` can skip re-renders:** + +In `WorldMap.tsx`, compute each Geography's style object with `useMemo` (or move it outside the `.map()` callback with stable references). Concretely: + +1. **Create a `useMemo`-based style map** keyed by `geo.rsmKey` that only recomputes when `countries`, `selectedCountry`, or the threshold props change — not on every tooltip update. + + ```tsx + const styleMap = useMemo(() => { + const map: Record = {}; + for (const geo of geographies as { rsmKey: string; id: string | number }[]) { + const numericId = String(geo.id); + const cc = ISO_NUMERIC_TO_ALPHA2[numericId] ?? null; + const count = cc !== null ? (countries[cc] ?? 0) : 0; + const isSelected = cc !== null && selectedCountry === cc; + const fillColor = getBanCountColor(count, thresholdLow, thresholdMedium, thresholdHigh); + map[geo.rsmKey] = { + default: { + fill: isSelected ? tokens.colorBrandBackground : fillColor, + stroke: tokens.colorNeutralStroke2, + strokeWidth: 0.75, + outline: "none", + }, + hover: { + fill: isSelected + ? tokens.colorBrandBackgroundHover + : cc && count > 0 + ? tokens.colorNeutralBackground3 + : fillColor, + stroke: tokens.colorNeutralStroke1, + strokeWidth: 1, + outline: "none", + }, + pressed: { + fill: cc ? tokens.colorBrandBackgroundPressed : fillColor, + stroke: tokens.colorBrandStroke1, + strokeWidth: 1, + outline: "none", + }, + }; + } + return map; + }, [geographies, countries, selectedCountry, thresholdLow, thresholdMedium, thresholdHigh]); + ``` + +2. **Pass the memoized style** in the `.map()` callback: + ```tsx + + ``` + +3. **Wrap event handlers in `useCallback`** (`onMouseEnter`, `onMouseMove`, `onMouseLeave`) or move them outside the `.map()` so they are stable references and do not defeat `memo()`. Consider passing `cc`, `count`, and `countryNames` as data attributes and reading them from `e.currentTarget` inside a single shared handler. + +With stable `style` references, `memo(Geography)` will correctly skip re-renders for countries whose data has not changed, eliminating the state-update race condition and the unnecessary rendering of ~200 SVG paths on every mouse-move event. **Files to change:** - -1. **`frontend/src/components/DashboardFilterBar.tsx`** - - Accept two new optional props (e.g. `jailSlot?: React.ReactNode` and `ipSlot?: React.ReactNode`, or pass the value+onChange pairs directly). Keep the component reusable — the Dashboard page uses the same component but does not need the Jail/IP inputs, so these slots must be optional. - - After the existing Filter group, conditionally render a `
` + `` followed by a new group for Jail, and repeat for IP Address. - - Each new group should follow the existing `styles.group` layout: a row with the label `` on the left and the `` on the right, separated by `gap: tokens.spacingHorizontalM`. - -2. **`frontend/src/pages/HistoryPage.tsx`** - - Remove the two standalone Jail and IP Address `
` cards (currently wrapped in `styles.filterLabel` + `cardStyles.card`). - - Instead, pass the Jail and IP Address controls into `` via the new props/slots. - - The `styles.filterLabel` style can be removed if no longer used elsewhere. - -**Acceptance criteria:** -- All four filter groups (Time Range, Filter, Jail, IP Address) render inside a single bordered bar. -- Each group is separated by a vertical divider identical to the existing one between Time Range and Filter. -- The labels "Jail" and "IP Address" sit to the **left** of their respective input fields (horizontal layout), not above them. -- The Dashboard page's usage of `DashboardFilterBar` is unaffected (no Jail/IP inputs shown there). -- Existing filter functionality (debounced input, query params, pagination reset) remains unchanged. - -Status: completed +- `frontend/src/components/WorldMap.tsx` — memoize style objects and stabilize event handlers. +- No changes needed in `react-simple-maps-main/src/components/Geography.js`. diff --git a/frontend/src/components/WorldMap.tsx b/frontend/src/components/WorldMap.tsx index 21541f3..d9b4871 100644 --- a/frontend/src/components/WorldMap.tsx +++ b/frontend/src/components/WorldMap.tsx @@ -8,10 +8,11 @@ */ import { createPortal } from "react-dom"; -import { useCallback, useState } from "react"; +import { useCallback, useMemo, useState } from "react"; import { ComposableMap, ZoomableGroup, Geography, useGeographies } from "react-simple-maps"; import { Button, makeStyles, tokens } from "@fluentui/react-components"; import { useCardStyles } from "../theme/commonStyles"; +import type { CSSProperties } from "react"; import type { GeoPermissibleObjects } from "d3-geo"; import { ISO_NUMERIC_TO_ALPHA2 } from "../data/isoNumericToAlpha2"; import { getBanCountColor } from "../utils/mapColors"; @@ -117,6 +118,73 @@ function GeoLayer({ [selectedCountry, onSelectCountry], ); + // Stable event handlers — shared across all Geography components so + // React.memo is not defeated by new function references each render. + const handleMouseEnter = useCallback( + (e: React.MouseEvent): void => { + const cc = e.currentTarget.dataset.cc; + if (!cc) return; + const count = Number(e.currentTarget.dataset.count); + const name = e.currentTarget.dataset.name ?? cc; + setTooltip({ cc, count, name, x: e.clientX, y: e.clientY }); + }, + [], + ); + + const handleMouseMove = useCallback( + (e: React.MouseEvent): void => { + setTooltip((current) => + current + ? { ...current, x: e.clientX, y: e.clientY } + : current, + ); + }, + [], + ); + + const handleMouseLeave = useCallback((): void => { + setTooltip(null); + }, []); + + // Memoize style objects so Geography's React.memo can skip re-renders + // when only the tooltip position changes. + type GeoStyle = { default: CSSProperties; hover: CSSProperties; pressed: CSSProperties }; + const styleMap = useMemo(() => { + const map: Record = {}; + for (const geo of geographies as { rsmKey: string; id: string | number }[]) { + const numericId = String(geo.id); + const cc: string | null = ISO_NUMERIC_TO_ALPHA2[numericId] ?? null; + const count: number = cc !== null ? (countries[cc] ?? 0) : 0; + const isSelected = cc !== null && selectedCountry === cc; + const fillColor = getBanCountColor(count, thresholdLow, thresholdMedium, thresholdHigh); + map[geo.rsmKey] = { + default: { + fill: isSelected ? tokens.colorBrandBackground : fillColor, + stroke: tokens.colorNeutralStroke2, + strokeWidth: 0.75, + outline: "none", + }, + hover: { + fill: isSelected + ? tokens.colorBrandBackgroundHover + : cc && count > 0 + ? tokens.colorNeutralBackground3 + : fillColor, + stroke: tokens.colorNeutralStroke1, + strokeWidth: 1, + outline: "none", + }, + pressed: { + fill: cc ? tokens.colorBrandBackgroundPressed : fillColor, + stroke: tokens.colorBrandStroke1, + strokeWidth: 1, + outline: "none", + }, + }; + } + return map; + }, [geographies, countries, selectedCountry, thresholdLow, thresholdMedium, thresholdHigh]); + if (geographies.length === 0) return <>; // react-simple-maps types declare path as always defined, but it can be null @@ -133,14 +201,6 @@ function GeoLayer({ const count: number = cc !== null ? (countries[cc] ?? 0) : 0; const isSelected = cc !== null && selectedCountry === cc; - // Compute the fill color based on ban count - const fillColor = getBanCountColor( - count, - thresholdLow, - thresholdMedium, - thresholdHigh, - ); - // Only calculate centroid if path is available let cx: number | undefined; let cy: number | undefined; @@ -172,54 +232,13 @@ function GeoLayer({ handleClick(cc); } }} - onMouseEnter={(e): void => { - if (!cc) return; - setTooltip({ - cc, - count, - name: countryNames?.[cc] ?? cc, - x: e.clientX, - y: e.clientY, - }); - }} - onMouseMove={(e): void => { - setTooltip((current) => - current - ? { - ...current, - x: e.clientX, - y: e.clientY, - } - : current, - ); - }} - onMouseLeave={(): void => { - setTooltip(null); - }} - style={{ - default: { - fill: isSelected ? tokens.colorBrandBackground : fillColor, - stroke: tokens.colorNeutralStroke2, - strokeWidth: 0.75, - outline: "none", - }, - hover: { - fill: isSelected - ? tokens.colorBrandBackgroundHover - : cc && count > 0 - ? tokens.colorNeutralBackground3 - : fillColor, - stroke: tokens.colorNeutralStroke1, - strokeWidth: 1, - outline: "none", - }, - pressed: { - fill: cc ? tokens.colorBrandBackgroundPressed : fillColor, - stroke: tokens.colorBrandStroke1, - strokeWidth: 1, - outline: "none", - }, - }} + onMouseEnter={handleMouseEnter} + onMouseMove={handleMouseMove} + onMouseLeave={handleMouseLeave} + data-cc={cc ?? undefined} + data-count={cc ? String(count) : undefined} + data-name={cc ? (countryNames?.[cc] ?? cc) : undefined} + style={styleMap[geo.rsmKey]} /> {count > 0 && cx !== undefined && cy !== undefined && isFinite(cx) && isFinite(cy) && (