Fix WorldMap hover highlight by memoizing style objects and handlers
Memoize per-Geography style objects with useMemo so React.memo can skip re-renders when only the tooltip position changes. Stabilize mouse event handlers with useCallback using data-* attributes instead of per-Geography closures. This eliminates the state-update race condition that caused hover fill colors to flash back to defaults.
This commit is contained in:
100
Docs/Tasks.md
100
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 `<Divider>`.
|
||||
- In `HistoryPage` (`frontend/src/pages/HistoryPage.tsx`, lines 476–510) the Jail `<Input>` and IP Address `<Input>` 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 `<Divider vertical />` + `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 `<Geography>` (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<string, { default: CSSProperties; hover: CSSProperties; pressed: CSSProperties }> = {};
|
||||
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
|
||||
<Geography style={styleMap[geo.rsmKey]} ... />
|
||||
```
|
||||
|
||||
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 `<div className={styles.divider}>` + `<Divider vertical />` 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 `<Text weight="semibold" size={300}>` on the left and the `<Input>` on the right, separated by `gap: tokens.spacingHorizontalM`.
|
||||
|
||||
2. **`frontend/src/pages/HistoryPage.tsx`**
|
||||
- Remove the two standalone Jail and IP Address `<div>` cards (currently wrapped in `styles.filterLabel` + `cardStyles.card`).
|
||||
- Instead, pass the Jail and IP Address controls into `<DashboardFilterBar>` 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`.
|
||||
|
||||
Reference in New Issue
Block a user