Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(android): camera bounds, zoom, and animation mode handling #3375

Merged
merged 7 commits into from Feb 18, 2024

Conversation

naftalibeder
Copy link
Collaborator

@naftalibeder naftalibeder commented Feb 13, 2024

This is a refactor to fix bounds and zoom handling on Android, similar to #3354 on iOS.

There wasn't a double-padding issue like on iOS, but Android did have these problems:

  • couldn't zero out min or max zoom levels
  • the zoom value was getting overridden after setting bounds
  • the "move" camera animation mode just fell back to easing

See before and after (using example at this commit):

Before:

old.mov

After:

new.mov

@mfazekas
Copy link
Contributor

@mfazekas @naftalibeder Since the latest update im getting and error that rnmapboxcameramodule doesnt exist on registry. Screenshot 2024-02-15 at 12 42 34
The version of mapbox before that has a problem for android devices. When the map opens on android the animationMode: flyTo doesnt work, but for IOS works.
Is that the issue you were talking about?

For the camera module doesn't exists, have you executed pod install after updating the @rnmapbox/maps in node_modules? Then you'll need to rebuild the app in Xcode, or use npx react-native run-ios or similar

Actually no, I'm using the rnmapbox with Expo managed workflow, and before the update to 10.1.13 it was working allright with EAS build. Do I need to do pod install even with managed workflow?

I could not repro the issue in a new expo project, yes you'll need to run eas build again after @rnmapbox/maps update. Pls open a discussion, if it doesn't fix as it's not related to this PR

@naftalibeder
Copy link
Collaborator Author

@mfazekas Updated the description at the top and simplified changes - let me know what you think.

@naftalibeder
Copy link
Collaborator Author

@mfazekas Weird, it seems like my edit to the description didn't save properly before, but it was still cached locally. I just resubmitted it.

@vasilring
Copy link

@mfazekas @naftalibeder Since the latest update im getting and error that rnmapboxcameramodule doesnt exist on registry. Screenshot 2024-02-15 at 12 42 34
The version of mapbox before that has a problem for android devices. When the map opens on android the animationMode: flyTo doesnt work, but for IOS works.
Is that the issue you were talking about?

For the camera module doesn't exists, have you executed pod install after updating the @rnmapbox/maps in node_modules? Then you'll need to rebuild the app in Xcode, or use npx react-native run-ios or similar

Actually no, I'm using the rnmapbox with Expo managed workflow, and before the update to 10.1.13 it was working allright with EAS build. Do I need to do pod install even with managed workflow?

I could not repro the issue in a new expo project, yes you'll need to run eas build again after @rnmapbox/maps update. Pls open a discussion, if it doesn't fix as it's not related to this PR

I see thank you! Creating a new project and installing all dependencies fixed the issue.

@mfazekas mfazekas changed the title Refactor camera bounds, zoom, and animation mode handling on Android refactor(android): camera bounds, zoom, and animation mode handling Feb 16, 2024
@mfazekas mfazekas merged commit 4f91f2f into rnmapbox:main Feb 18, 2024
10 checks passed
@naftalibeder naftalibeder deleted the truckmap/fix/camera branch February 21, 2024 15:46
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.

None yet

3 participants