Skip to content

feat(map): Follow & Auto-zoom on Map Analysis (#3788 Phase 2) - #4012

Merged
Yeraze merged 5 commits into
mainfrom
feature/3788-follow-autozoom
Jul 8, 2026
Merged

feat(map): Follow & Auto-zoom on Map Analysis (#3788 Phase 2)#4012
Yeraze merged 5 commits into
mainfrom
feature/3788-follow-autozoom

Conversation

@Yeraze

@Yeraze Yeraze commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Epic #3788 — "Follow nodes on the map", Phase 2 of 2 (builds on Phase 1's node selection, merged in #4010). Plan: docs/internal/dev-notes/MAP_FOLLOW_AUTOZOOM_EPIC.md.

What

Adds Follow and Auto-zoom toggles to the /analysis toolbar that keep the selected nodes framed as their positions update (15s cross-source poll):

  • Follow → recenter to the selected nodes' average position, preserving zoom.
  • Auto-zoomfitBounds of the selection +15% margin. When both on, Auto-zoom governs.
  • A manual pan/zoom pauses and shows a ⟳ Resume follow overlay; no yank-back until you resume or retarget the selection.

How (frontend-only, builds on Phase 1)

  • followMath.ts (new) — pure, leaflet-free averageLatLng + planAutoZoom (discriminated none/single/multi union encoding empty/single/coincident + the 15% pad). All edge-case branching is here and unit-tested.
  • FollowController.tsx (new) — useMap() child. Derives the selected points from Phase 1's useAnalysisNodes()/selectedNodeIds (shared query cache, no refetch). Applies Follow/Auto-zoom on each position-signature change. Manual-pan detection: a single moveend gate + programmaticRef flag + {animate:false} (programmatic moveend fires synchronously and is consumed) + a requestAnimationFrame safety-net for no-op moves. Pause auto-resets on selection membership change or toggle change — never on position drift (so the 15s poll can't silently un-pause).
  • FollowResumeButton.tsx (new) — overlay shown only while a mode is active and paused.
  • Config: followMode/autoZoom added to MapAnalysisConfig (persisted, load-safe); followPaused is transient context state (not persisted).
  • Toolbar toggles mirror the existing layer-toggle chrome.

Tests

New/extended: followMath.test.ts, FollowController.test.tsx (all Follow/Auto-zoom/both/single/empty cases + programmatic-move-doesn't-self-pause + user-move-pauses + paused-inert + no-op guard), FollowResumeButton.test.tsx, useMapAnalysisConfig.test.ts, MapAnalysisToolbar.test.tsx. Full suite: 2744/2744 suites, 0 failures; typecheck + lint:ci clean.

Browser-validated on the deployed build

Follow recentered [25,-95]z9 → [25.978,-80.137]z9 (selection average, zoom preserved); Auto-zoom fit a wide select-all from z14 → z0; a near-coincident pair correctly stayed put (single-case, no zoom-to-max); a user zoom paused + surfaced "Resume follow" + did not yank back; Resume re-engaged (refit to z0); toggles persisted across reload. No feature console errors.

Closes #3788 (completes the epic: Phase 1 selection #4010 + this).

🤖 Generated with Claude Code

https://claude.ai/code/session_015AFBA76hsjqhsXe1BdnYub

Yeraze and others added 5 commits July 8, 2026 14:05
…Paused (#3788 P2 WP-A)

Foundation for Phase 2 Follow/Auto-zoom: persisted followMode/autoZoom
flags on MapAnalysisConfig (mirroring selectedNodeIds), hardened
load() coercion for old/garbage config, and transient followPaused
state on MapAnalysisContext for the not-yet-built follow controller.
…WP-B)

Leaflet-free averageLatLng + planAutoZoom (empty/single/coincident/
multi with 15% pad) for the upcoming FollowController; kept pure and
unit-tested so the branching isn't buried in a Leaflet effect.
…updates (#3788 P2 WP-C)

Add the `useMap()` view controller (spec §3c) that recenters/fits the map
onto the selected nodes' current positions each live update, and wire it into
the MapAnalysisCanvas `<MapContainer>` (§3e). Toolbar toggles and the Resume
overlay are WP-D and intentionally excluded here.

- FollowController.tsx: selected-points/`sig`/`selKey` derivation; the
  `programmaticRef` + `animate:false` + rAF safety-net lifecycle so our own
  moves don't self-pause; single `moveend` gate that pauses on genuine user
  pan/zoom; apply effect (Follow / Auto-zoom / both-on precedence, epsilon
  no-op guard); pause auto-reset on selection membership and mode toggles
  (never on position drift).
- Wire `<FollowController/>` after `<TileLayer/>` inside `<MapContainer>`.
- MapAnalysisCanvas.test.tsx: mock FollowController as a no-op there (its
  behavior is covered by the dedicated suite) so the existing `useMap()->null`
  spiderfier path is preserved.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015AFBA76hsjqhsXe1BdnYub
…rlay (#3788 P2 WP-D)

Adds the Follow/Auto-zoom toggle buttons to MapAnalysisToolbar and a
FollowResumeButton overlay (sibling of MapContainer, alongside MapLegend)
that surfaces "Resume follow" only while a mode is active and paused,
clearing the pause on click. Extends toolbar tests for the new toggles.
…se 2 spec (#3788)

- User docs: map-analysis.md gains a "Follow & Auto-zoom" section, a toolbar
  row, and a persistence-list mention.
- Internal: epic plan Phase 2 marked done with browser-validation evidence;
  commit the Phase 2 implementation spec.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015AFBA76hsjqhsXe1BdnYub
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

Claude finished @Yeraze's task —— View job


PR Review: feat(map): Follow & Auto-zoom on Map Analysis (#3788 Phase 2)

Tasks:

  • Gather context and understand the PR
  • Read new core files (followMath.ts, FollowController.tsx, FollowResumeButton.tsx)
  • Read modified files (Context, Canvas, Toolbar, Config hook)
  • Read test files
  • Post comprehensive review

This is a well-structured, clean feature implementation. The architecture decisions are sound and the test coverage is thorough. Below are findings ranging from critical issues to minor suggestions.

Potential Bug: Race Condition in applyView + RAF Flag Clearing

FollowController.tsx:29-44 — The programmaticRef flag can be cleared by the RAF before the synchronous moveend fires in some Leaflet edge cases. The code comments explain that { animate: false } makes moveend fire synchronously consuming the flag before the RAF runs, and the RAF serves as a safety net for no-op moves. This logic is correct when animate: false is honored.

However, there's a subtlety: if a third-party Leaflet plugin or future version changes this behavior, both the RAF and the moveend handler could race to clear the flag. This is a known trade-off that's well-documented in the comments. The existing test coverage for the "programmatic move does not self-pause" case validates the current behavior. Consider adding a code comment explicitly noting this is an implementation contract with Leaflet's animate: false behavior.

Minor Bug: selKey Computed Outside useMemo

FollowController.tsx:27

const selKey = config.selectedNodeIds.join('|');

This is computed directly during render, not inside a useMemo. This is fine for correctness since useEffect captures the value correctly, but it creates a new string on every render even when the array hasn't changed. Since this is used as a useEffect dependency at line 88, it could trigger extra re-runs if config re-renders for unrelated reasons. Consider memoizing:

const selKey = useMemo(() => config.selectedNodeIds.join('|'), [config.selectedNodeIds]);

Observation: eslint-disable on react-hooks/exhaustive-deps

FollowController.tsx:82-83

// eslint-disable-next-line react-hooks/exhaustive-deps -- #3788 keyed on sig to fire only on coordinate change
}, [sig, followPaused, config.followMode, config.autoZoom, map]);

The intentional omission of points and applyView from the dependency array is the right design (keying on sig prevents firing on position-equal re-renders). The reason is well-documented. This is correct behavior. The CLAUDE.md warns against auto-fixing exhaustive-deps and this is a careful per-site disable — ✓

Design Question: Toolbar Position of Follow/Auto-zoom Toggles

MapAnalysisToolbar.tsx:142-157 — The Follow and Auto-zoom buttons appear between NodeMultiSelect and Time Slider. Given that Follow/Auto-zoom are only meaningful when nodes are selected, positioning them immediately after the node picker is natural. However, there's no visual indication in the toolbar when Follow/Auto-zoom is active but paused. Users might toggle Follow on, pan the map, see no "Resume" button (if they pan and it disappears from view), and wonder why nothing is happening. The FollowResumeButton overlay appears centered at the top of the canvas, which should be visible — this is likely fine in practice.

Minor: No Accessibility Label on Resume Button

FollowResumeButton.tsx:15-21 — The button renders ⟳ Resume follow as text content. This is accessible by text name (confirmed by the test using { name: /resume follow/i }). The character may be announced as "clockwise open circle arrow" or similar by screen readers. Consider wrapping it in <span aria-hidden="true">⟳</span> if screen reader verbosity is a concern, though this is a low-priority polish item.

Test Coverage: Excellent

The tests cover all the critical behavioral cases:

  • followMath.test.ts — pure function edge cases (empty, single, coincident, multi, custom pad) ✓
  • FollowController.test.tsx — Follow, Auto-zoom, both, single-point, empty, programmatic-no-pause, user-move-pauses, paused-inert, no-op guard ✓
  • FollowResumeButton.test.tsx — all 4 visibility states + click behavior ✓
  • useMapAnalysisConfig.test.ts — default values, persistence, load-safe coercion, reset ✓
  • MapAnalysisToolbar.test.tsx — toggle independence, active class, persistence ✓

The test for "programmatic move does not self-pause" (line 157-166) is particularly important and well-written — it validates the programmaticRef flag mechanism without relying on real timing.

Code Quality Observations

  • followMath.ts: Clean separation of pure math from Leaflet. The discriminated union FitPlan type is idiomatic and testable. ✓
  • FollowController.tsx: Correctly uses useMap() as a child of <MapContainer>. The three-effect design (apply view, pause detection, reset-on-change) is clean. ✓
  • MapAnalysisContext.tsx: followPaused as transient (not persisted) context state is the right call. ✓
  • useMapAnalysisConfig.ts: Load-safe coercion pattern (typeof parsed.followMode === 'boolean' ? ... : false) is consistent with the existing pattern and handles old configs that lack these fields. ✓
  • MapAnalysisCanvas.tsx: <FollowController /> placed as first child inside <MapContainer> ensures it registers its event listeners early. <FollowResumeButton /> placed outside <MapContainer> (sibling of the map div) so it's not affected by Leaflet pane stacking. ✓

Summary

This is a high-quality implementation. The one actionable improvement is memoizing selKey to avoid unnecessary effect triggers. Everything else is either correct-by-design or cosmetic. The browser validation described in the PR confirms the full end-to-end flow works. ✅ Ready to merge pending the minor suggestion.

@Yeraze
Yeraze merged commit 6b6b039 into main Jul 8, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEAT] Ability to follow nodes on the map

1 participant