Repository navigation
feat: add focalPoint - #4163
feat: add focalPoint#4163
focalPoint#4163Conversation
…val, add example Android converts the dp focal point to pixels for GesturesSettings.focalPoint. Both platforms clear the focal point when it's removed from gestureSettings. The doc generator now expands nested object types instead of printing 'signature'.
|
Thanks @ted-mundy, this is a nice addition! I pushed a commit on top of yours (your commits are unchanged; I merged
Both SDKs use the focal point for pinch, rotate, double-tap zoom-in and two-finger-tap zoom-out. Verification: the gestures were run away from the marker, then I checked whether the coordinate under the marker moved.
|
|
Am I ok to undraft this @mfazekas ? |
mfazekas
left a comment
There was a problem hiding this comment.
Thanks, this works well. I checked it on the iOS simulator and Android emulator: pinch, double-tap and rotate keep the focal point fixed on both, the dp to px conversion on Android lands in the same spot as iOS, and turning it off via focalPoint: undefined resets it. A couple of things before merging:
- Android: removing the whole
gestureSettingsprop shows a redbox (ClassCastException: Dynamic value from Object is not a ReadableMap).settings.asMap()throws on a nullDynamicinstead of returning null, so themap == nullreset branch never runs. Checkingsettings.isNull(orsettings.type != ReadableType.Map) beforeasMap()and clearingfocalPointthere should fix it.maincrashes the same way, but since this PR makes removal reset the focal point it'd be good to make that work on Android too. - Docs: the iOS SDK ignores
focalPointfor pinch whilepinchPanEnabledis true, which is the default. Worth one sentence in thefocalPointdoc saying to setpinchPanEnabled: falseto keep pinch anchored (e.g. under a navigation puck). - Small one: the
DocJSONBuilder.mjschange also changes the generatedpaddingtypes inCamera.md. That's an improvement, just mention it in the PR description so it isn't a surprise.
Optional: RNMBXMapView already has toPx(ScreenCoordinate) for dp to px; making it internal and reusing it would avoid a second copy of the density math in the manager.
# Conflicts: # android/src/main/java/com/rnmapbox/rnmbx/components/mapview/RNMBXMapViewManager.kt # example/src/examples/Map/index.js
…Settings is removed Also document that iOS pinch ignores focalPoint while pinchPanEnabled is true.
|
Sorry for missing your question, @ted-mundy. Yes, undrafting was fine. We've finished the remaining review items on top of your branch: merged |
mfazekas
left a comment
There was a problem hiding this comment.
Review fixes are in; verified on Android.
Description
Added
focalPointinto the React Native version of the package. Functionality already exists in both Android and iOS, but no bridge was ever built. This builds that bridge 🙂The
DocJSONBuilder.mjschange expands nested object types in the generated docs, which also changes thepaddingtype shown inCamera.md.Checklist
CONTRIBUTING.mdyarn generatein the root folder/exampleapp./example)Screenshot OR Video
See the
Map > Gesture Focal Pointexample and the verification notes in the comments.