Skip to content

fix: fly_to in overlay/interleaved basemap mode#1123

Open
tylere wants to merge 4 commits intodevelopmentseed:mainfrom
tylere:fix/fly-to-overlay-renderer
Open

fix: fly_to in overlay/interleaved basemap mode#1123
tylere wants to merge 4 commits intodevelopmentseed:mainfrom
tylere:fix/fly-to-overlay-renderer

Conversation

@tylere
Copy link

@tylere tylere commented Mar 10, 2026

Summary

  • Fix fly_to() having no effect when the basemap mode is "overlaid" or "interleaved" (the default). The OverlayRenderer passes initialViewState to react-map-gl's <MapGL>, which only reads that prop on initial mount and ignores subsequent updates.
  • Use a MapGL ref to call MapLibre's native map.flyTo() API directly when a fly-to request arrives, keeping the existing uncontrolled view state management unchanged for normal pan/zoom.
  • Filter out transition_* fields from view state dicts sent by the frontend before constructing Python ViewState dataclasses, preventing TypeError on unexpected keyword arguments like transition_duration.

Test plan

  • npm run build completes without errors
  • In a notebook with default basemap (overlay mode): m = viz(gdf) then m.fly_to(longitude=..., latitude=..., zoom=...) — verify the map animates
  • Verify deck-first mode still works (no basemap): m = Map(layers=[...]) then m.fly_to(...)
  • Verify normal pan/zoom still syncs view state back to Python in overlay mode

🤖 Generated with Claude Code

The OverlayRenderer passes initialViewState to react-map-gl's <MapGL>,
which only reads that prop on mount and ignores subsequent updates.
This meant fly_to() calls from Python had no visible effect when using
overlaid or interleaved basemap modes.

Fix by using a MapGL ref to call MapLibre's native map.flyTo() API
directly when a fly-to request arrives, while keeping the existing
uncontrolled view state management unchanged for normal pan/zoom.

Also filter out transition_* fields from view state dicts sent by the
frontend before constructing Python ViewState dataclasses, preventing
TypeError on unexpected keyword arguments.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@tylere
Copy link
Author

tylere commented Mar 10, 2026

Fixes #1064.

@tylere tylere changed the title Fix fly_to in overlay/interleaved basemap mode fix: fly_to in overlay/interleaved basemap mode Mar 10, 2026
@github-actions github-actions bot added the fix label Mar 10, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes fly_to() not animating the map when using the default basemap modes (“overlaid” / “interleaved”) by routing fly-to requests through MapLibre’s native map.flyTo() in the overlay renderer, and hardens Python view state validation against frontend transition* fields.

Changes:

  • Add flyToRequest plumbing to the overlay renderer and invoke map.flyTo() via a MapGL ref.
  • Route "fly-to" custom messages to the overlay renderer in overlaid/interleaved basemap modes; keep existing deck-first behavior.
  • Filter out transition* keys from frontend-sent view state dicts before constructing Python ViewState dataclasses.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/renderers/types.ts Extends overlay renderer prop types to accept a fly-to request and completion callback.
src/renderers/overlay.tsx Uses a MapGL ref + effect to call MapLibre flyTo() in overlay mode.
src/index.tsx Stores fly-to requests in React state for overlay/interleaved modes and passes them to OverlayRenderer.
lonboard/traits/_map.py Drops transition* fields from view state dict inputs to avoid dataclass constructor errors.
Comments suppressed due to low confidence (1)

src/index.tsx:183

  • model.on("msg:custom", ...) is being called during render, which will register a new event handler on every re-render and can cause duplicated fly-to handling and memory leaks. Move this subscription into a useEffect with a stable handler and cleanup via model.off("msg:custom", handler) in the effect's return function.
  // Handle custom messages
  model.on("msg:custom", (msg: Message) => {
    switch (msg.type) {
      case "fly-to":
        if (
          basemapState?.mode === "overlaid" ||
          basemapState?.mode === "interleaved"
        ) {
          setFlyToRequest(msg);
        } else {
          flyTo(msg, setViewState);
        }
        break;

      default:
        break;
    }
  });

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...deckProps
} = mapProps;

const mapRef = React.useRef<MapRef>(null);
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React.useRef<MapRef>(null) is a TypeScript error under strictNullChecks because the initial value is null but the ref type is non-nullable. Make the ref type nullable (e.g., MapRef | null) or omit the explicit generic so the inferred type matches the null initializer.

Suggested change
const mapRef = React.useRef<MapRef>(null);
const mapRef = React.useRef<MapRef | null>(null);

Copilot uses AI. Check for mistakes.
Comment on lines +99 to +102
# Filter out transition fields that are not part of the view state dataclass
snake_case_kwargs = {
k: v for k, v in snake_case_kwargs.items() if not k.startswith("transition")
}
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new filtering of transition* keys should be covered by a unit test to prevent regressions (e.g., ensure a camelCase transitionDuration/transitionInterpolator key in an incoming view_state dict is ignored rather than raising TypeError). There are already view_state validation tests in tests/test_map.py that could be extended for this case.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants