Skip to content

fix(ios): send the current camera when onCameraChanged is registered - #4430

Open
aderiushev wants to merge 1 commit into
rnmapbox:mainfrom
aderiushev:fix/ios-camera-changed-on-register
Open

aderiushev wants to merge 1 commit into
rnmapbox:mainfrom
aderiushev:fix/ios-camera-changed-on-register

Conversation

@aderiushev

Copy link
Copy Markdown

Description

On iOS, MapView sends camera changes to JS only after JS has asked for them (setHandledMapChangedEventsWithEvents, which MapView calls from componentDidMount and again when a callback prop changes). A camera change before that is never sent, and nothing sends the camera afterwards. So when JS asks late, onCameraChanged gets no camera until the map moves:

  • when the screen keeps JS busy while the map mounts, so the start camera comes before componentDidMount (timing-dependent; we hit it every time in our app's map screen, which mounts many layers);
  • when onCameraChanged is set after the map has settled (always).

onMapIdle still reports the right camera, so an app that keeps the camera from onCameraChanged disagrees with the map until the user touches it.

This PR: when .cameraChanged is newly in the handled set, send the current camera once, as applyOnMapChange would (not when .regionIsChanging is registered, which takes precedence there). It goes through withMapView, so it waits for the map view. Registering the same set again (MapView does that on every render with an inline callback) sends nothing.

Android is not changed. There the handled set starts as null, so camera changes before the first registration are sent; a listener added later gets no camera until the map moves there too (from reading RNMBXMapView.kt, not run). I can add the same one-shot on Android if you want both to behave the same. #4426 fixes the Android start-camera loss from the 10 ms drop.

Checklist

  • I've read CONTRIBUTING.md
  • I updated the doc/other generated code with running yarn generate in the root folder (no changes)
  • I have tested the new feature on /example app.
    • In V11 mode/ios
    • In New Architecture mode/ios
    • In V11 mode/android
    • In New Architecture mode/android
  • I added/updated a sample - if a new feature was implemented (/example)

Screenshot OR Video

The reproducer below in the example app (main at 47a736a, React Native 0.84.1, New Architecture, Mapbox 11.33.0), iOS 26.5 simulator (iPhone 16). The map opens at zoom 12 and is never touched; each run is a fresh app launch.

main this PR
'busy' (JS busy 3 s while the map mounts), 10 runs: no onCameraChanged at all 3 of 10 0 of 10
'late' (onCameraChanged set at the first idle), 3 runs: onCameraChanged calls 0, 0, 0 1, 1, 1 (zoom 12.00, 3 ms after the idle)

In every run onMapIdle reported zoom 12.00. Without the busy child the start camera usually comes after componentDidMount, which is why the 'busy' case is timing-dependent; in one run with this PR the one-shot carried the map's first camera (zoom 0.47) and the start camera followed as a normal change. The text area has a fixed height: text that grows would resize the map, and a resize is a camera change of its own.

Component to reproduce the issue you're fixing

BugReportExample.js — 'busy' and 'late' modes, counts onCameraChanged for a map that is never moved
import React, { useLayoutEffect, useRef, useState } from 'react';
import { Text, View } from 'react-native';
import { Camera, MapView } from '@rnmapbox/maps';

// The map opens here and is never moved: the only camera change is the start camera.
const START = { centerCoordinate: [-74.00597, 40.71427], zoomLevel: 12 };

// Two ways JS asks for camera events after the start camera:
// 'busy': the screen keeps JS busy for 3 s while the map mounts (as a screen with many layers and
//         stores does), so MapView's componentDidMount, which asks native for camera events, runs
//         late. Timing-dependent.
// 'late': onCameraChanged is set only once the map is idle. Deterministic.
const MODE = 'busy';

const BusyOnMount = () => {
  useLayoutEffect(() => {
    const end = Date.now() + 3000;
    while (Date.now() < end) {
      // busy
    }
  }, []);

  return null;
};

const BugReportExample = () => {
  const mountedAt = useRef(Date.now());
  const [listening, setListening] = useState(MODE === 'busy');
  const [camera, setCamera] = useState({ calls: 0, first: '-' });
  const [idle, setIdle] = useState('-');

  const onCameraChanged = (state) => {
    const { zoom } = state.properties;

    setCamera((c) => ({
      calls: c.calls + 1,
      first: c.calls === 0 ? `zoom ${zoom.toFixed(2)} at +${Date.now() - mountedAt.current} ms` : c.first,
    }));
  };

  // onMapIdle carries the camera the map really has
  const onMapIdle = (state) => {
    setIdle(`zoom ${state.properties.zoom.toFixed(2)} at +${Date.now() - mountedAt.current} ms`);
    setListening(true);
  };

  return (
    <View style={{ flex: 1 }}>
      <MapView
        style={{ flex: 1 }}
        onCameraChanged={listening ? onCameraChanged : undefined}
        onMapIdle={onMapIdle}
      >
        <Camera defaultSettings={START} />
        {MODE === 'busy' && <BusyOnMount />}
      </MapView>
      {/* A fixed height, so text that grows does not resize the map (a resize is a camera change) */}
      <View style={{ height: 96 }}>
        <Text testID="camera">{`${MODE} | onCameraChanged calls: ${camera.calls} | first: ${camera.first}`}</Text>
        <Text testID="idle">{`onMapIdle: ${idle}`}</Text>
      </View>
    </View>
  );
};

export default BugReportExample;

🤖 Generated with Claude Code

This branch has not been deployed

No deployments
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.

1 participant