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

feat: custom location provider & native user location styling #3134

Closed
wants to merge 24 commits into from

Conversation

g4rb4g3
Copy link
Contributor

@g4rb4g3 g4rb4g3 commented Oct 24, 2023

Description

Added setCustomLocation to MapView that allows overriding the location used for the native user location component. It takes advantage of all the functionality of mapbox-maps to animate the native user location.
Added native user location images and scale props that allow styling the native user location component and not only use the stock images (like the blue puck).

Checklist

  • I have tested this on a device/simulator for each compatible OS
  • I updated the documentation with running yarn generate in the root folder
  • I added/updated a sample - if a new feature was implemented (/example)

Screenshot OR Video

The red arrow with the white circle is our custom styled user location using nativeTopImage and nativeShadowImage and the animation is done with setCustomLocation on the map view.
https://github.com/rnmapbox/maps/assets/646435/650d893f-c4dd-4a05-af5f-d7af98abcbeb

@mfazekas
Copy link
Contributor

@g4rb4g3 thanks much, looks nice, for first glance will look into it more in details

One note that instead of topImage I'd use topNativeImage to avoid confusion. topImage needs to be native resource name. not a mapbox image name, which we define using the Images component.

We call those nativeAssetImages there - https://github.com/rnmapbox/maps/blob/main/docs/Images.md#nativeassetimages

What do you think?

@g4rb4g3
Copy link
Contributor Author

g4rb4g3 commented Oct 25, 2023

@mfazekas
Glad to hear that you like it 🙂
I have named them nativeTopImage, nativeBearingImage and nativeShadowImage in the UserLocation component.
Only in the NativeUserLocation I have skipped the native part since the whole component is already native and that component is not exported anyways. Is that sufficient or you still want them to be named differently?

@mfazekas
Copy link
Contributor

@mfazekas
Glad to hear that you like it 🙂
I have named them nativeTopImage, nativeBearingImage and nativeShadowImage in the UserLocation component.
Only in the NativeUserLocation I have skipped the native part since the whole component is already native and that component is not exported anyways. Is that sufficient or you still want them to be named differently?

I'm thinking on deprecate renderMode={'native'}. And making user use NativeUserLocation instead of UserLocation if they want a native user location. Therefore nativeTopImage is not neccesary.

I still find 'topImage' a bit confusing as those are not mapbox image names added to style, but native asset image names.

so maybe something like:
topImageAsset
would clarify that. We should also update the documentation.

@g4rb4g3
Copy link
Contributor Author

g4rb4g3 commented Oct 25, 2023

@mfazekas
Not sure on deprecating the renderMode, the location provider is set on the MapView anyways since it replaces the maps location provider.

I took the names from the iOS PuckType of mapbox-maps.
Let me know how we want to continue on that. 🙂

src/components/NativeUserLocation.tsx Outdated Show resolved Hide resolved
src/components/NativeUserLocation.tsx Outdated Show resolved Hide resolved
@mfazekas
Copy link
Contributor

@g4rb4g3
please remove nativeTopImage etc from UserLocation.

Those features will be available only when using NativeUserLocation, directly. I'll deprecate native mode on UserLocation later.

@g4rb4g3 g4rb4g3 requested a review from mfazekas October 27, 2023 08:12
@g4rb4g3
Copy link
Contributor Author

g4rb4g3 commented Nov 1, 2023

@mfazekas I did a few updates to this PR and it is ready your review. 🙂

@mfazekas
Copy link
Contributor

@g4rb4g3 thanks much, great work,

I've made some changes, see #3166

Changes:

  • removed custom location provider. It needs to be refactored to a component, as MapView is already a huge we don't want to add new stuff to it unless needed. Also I think we can just use interface like this:
    <MapVIew>
       <CustomLocationProvider location={[lng,lat]} heading={heading} />
    </MapView>
    
  • topImage, bearingImage etc. is now accepts an image name instead of a native asset name. So it supports any image defined by the Images component.
    <MapView>
       <Images nativeAssetImages={['pin']}>
          <Image name={'bearing'}>
             <View style={{borderColor: 'blue', borderWidth: 2, width: 16, height: 16, borderRadius: 8, backgroundColor: 'red'}}></View>
          </Image>
       </Images>
       <NativeUserLocation topImage="pin" bearingImage="bearing" />
    </MapView>
    
  • scale now support an expression:
    <NativeUserLocation topImage="topImage" visible={true}
       scale={
            [
              "interpolate",
              ["linear"],
              ["zoom"],
              10, 1.0,
              20, 4.0
            ]
          }
          />

@g4rb4g3
Copy link
Contributor Author

g4rb4g3 commented Nov 14, 2023

@mfazekas Great job! Thanks for putting everything together nicely. 🙂

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.

2 participants