fix(charts): preserve map pan/zoom on re-render + reflect graph node selection

[alfredo1996]

Graph + map — deep functional review

Continuation of the v1.1 component review. After the per-chart design/a11y pass, this is a deep functional review of the two most complex charts, where remaining bugs were likeliest to hide.

🔴 P1 — Map: pan/zoom reset + full marker rebuild on every parent re-render

The marker-build useEffect re-ran on every render because two deps got a fresh identity each render: the plugin's inline onMarkerClick arrow, and the inline [20, 20] fitBoundsPadding default. Each run tore down + rebuilt every marker and called map.fitBounds(...) — snapping the user's pan/zoom back to the data bounds. autoFitBounds defaults to true in the map plugin, so any unrelated re-render (parameter change, auto-refresh poll, hover) yanked the map back.

Fix: latest-ref the click handler (no longer an effect dep); hoist the padding default to a module constant (DEFAULT_FIT_PADDING); split auto-fit into its own effect keyed on [markers, autoFitBounds, fitBoundsPadding]. Re-fit now happens only when markers actually change → user pan/zoom is preserved.

🟠 P2 — Graph: selectedNodeIds had no visual effect

graph-exploration-wrapper passes selectedNodeIds and the click handler toggles it, but toNvlNode never mapped it to NVL's selected field — so a restored/programmatic selection never highlighted the node (selection was one-way-out only).

Fix: build a Set from selectedNodeIds and set selected in toNvlNode; add to the nvlNodes memo deps. BasicNvlWrapper diffs node attributes and applies via addAndUpdateElementsInGraph, so selection updates incrementally without re-running layout (no graph reshuffle on click — verified against the NVL wrapper source).

⚪ Verified, not bugs

• Raw popup HTML binding is a trusted public-API choice — the app's transformToMapData never sets popup, and the data-driven properties tooltip path is escaped. No app XSS.

Verification

• component unit suite: 1474 pass (+7 new covering exactly the changed behavior) · tsc --noEmit clean · npm run lint 0 errors.
• E2E (local): ran charts/heavy-widgets/widget-states/styling-rules → 61 passed, 6 failed. All 6 failures are AuthPage.login / waitForURL / pre-render timeouts — the documented [P2] e2e: local suite flaky above 2 workers — cap local parallelism or document it #994 server/DB-contention flakes, failing at the auth/navigation stage before any graph/map widget renders (categorically unreachable by these component changes). Isolation re-runs were blocked by a 1-CPU Docker host that can't start the Neo4j testcontainer within 120s (Neo4j took >6 min to start standalone here). CI E2E is the authoritative gate.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Graph chart selection state now visually updates correctly when nodes are selected
  • Map chart pan/zoom no longer snaps back unexpectedly when interacting with markers

• Tests

  • Added test coverage for graph node selection behavior
  • Added regression tests to ensure map pan/zoom stability during re-renders

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 20e23cf5-6fdb-4940-a079-9b0e436a0e55

📥 Commits

Reviewing files that changed from the base of the PR and between d0e312c and c7d064d.

📒 Files selected for processing (5)

• claude_code_docs/component-review/graph-map-functional-findings.md
• component/src/charts/__tests__/graph-chart.test.tsx
• component/src/charts/__tests__/map-chart.test.tsx
• component/src/charts/graph-chart.tsx
• component/src/charts/map-chart.tsx

---

Walkthrough

Two bug fixes with accompanying tests: map-chart.tsx prevents pan/zoom resets by storing onMarkerClick in a ref and splitting auto-fit bounds into a separate useEffect with narrower dependencies; graph-chart.tsx makes selectedNodeIds visually reflect by threading a memoized selectedIds set into toNvlNode to set NVL node selected flags.

Changes

MapChart pan/zoom stability fix

Layer / File(s)	Summary
Stabilize click handler ref, padding constant, and auto-fit effect component/src/charts/map-chart.tsx, claude_code_docs/component-review/graph-map-functional-findings.md	Adds DEFAULT_FIT_PADDING module-level constant, stores onMarkerClick in onMarkerClickRef updated each render, rewires marker click binding to call ref.current, and moves fitBounds into a dedicated useEffect with [markers, autoFitBounds, fitBoundsPadding] deps. Docs record the P1 fix and "verified—NOT bugs" items.
MapChart regression tests component/src/charts/__tests__/map-chart.test.tsx	Four new tests assert fitBounds is not re-called on handler-only re-renders, markers are not rebuilt on callback identity change, fitBounds re-runs on coordinate changes, and the latest callback is invoked via the bound handler without stale closure.

GraphChart selection state visual fix

Layer / File(s)	Summary
toNvlNode selectedIds injection and memo wiring component/src/charts/graph-chart.tsx	Extends toNvlNode to accept selectedIds: Set<string> and sets selected: selectedIds.has(node.id); GraphChartInner memoizes the set from selectedNodeIds and passes it into nvlNodes, adding selectedIds to the useMemo dep array.
GraphChart selection tests component/src/charts/__tests__/graph-chart.test.tsx	Three new tests confirm selectedNodeIds maps correctly to NVL selected flags when provided, absent, and updated via rerender.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• alfredo1996/neoboard#278: Modifies graph-chart.tsx's toNvlNode inputs and map-chart.tsx's marker rendering/effect dependency behavior — directly overlaps with this PR's code areas.
• alfredo1996/neoboard#467: Modifies map-chart.tsx's marker-layer building and click wiring logic, intersecting with the ref-based click handler and effect dependency changes here.

Suggested labels

bug, pkg:component, area:charts

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and specifically summarizes the two main fixes in the changeset: preserving map pan/zoom on re-render and reflecting graph node selection, matching the PR's core objectives.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/graph-map-functional-review

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: one or more packages not found in the registry.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud