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

Typings for v0.21.0 #2165

Merged
merged 1 commit into from
Apr 5, 2018
Merged

Typings for v0.21.0 #2165

merged 1 commit into from
Apr 5, 2018

Conversation

aMarCruz
Copy link
Contributor

@aMarCruz aMarCruz commented Apr 4, 2018

Does any other open PR do the same thing?

I think so, but this PR is updated and more refined.

What issue is this PR fixing?

Mainly, event interface and missing methods and properties.

How did you test this PR?

Tested with VS Code in real work, but not all the typings (We are not using all).

@rborn
Copy link
Collaborator

rborn commented Apr 5, 2018

@aMarCruz could you please let me know what other PRs do the same but are outdated so I can close them? 🤗

@aMarCruz
Copy link
Contributor Author

aMarCruz commented Apr 5, 2018

@rborn , I got confused, it's #2141 but it's already merged, there's no problem with that.

@rborn rborn merged commit 515a99c into react-native-maps:master Apr 5, 2018
rborn pushed a commit that referenced this pull request Apr 9, 2018
* 1. Allow canReplaceMapContent to be set on the JS side (via prop shouldReplaceMapContent) for iOS, MapKit only
2. Allow GoogleMaps on iOS to obey maximumZ
3. Added prop minimumZ for MapKit and GoogleMaps on iOS and Android

* Removed debug NSLogs

* MaximumZ was not being obeyed correctly for google maps (convert to long to compare). Get rid of compiler warnings (accidentally wrote NSUInteger instead of NSInteger)

* Typings for v0.21.0 (#2165)

* Update mapview.md (#2171)

Add 'none' option to docs for mapType

* Fixed crash for Android API level below 18 on isFromMockProvider (#2172)

* Add Mock Provider boolean on each location update

* Update mapview.md

Update docs to specify that coordinate includes mock provider boolean

* Check API is 18 or above for isFromMockProvider

* Update docs to mention API

* 1. Allow canReplaceMapContent to be set on the JS side (via prop shouldReplaceMapContent) for iOS, MapKit only
2. Allow GoogleMaps on iOS to obey maximumZ
3. Added prop minimumZ for MapKit and GoogleMaps on iOS and Android

* Removed debug NSLogs

* MaximumZ was not being obeyed correctly for google maps (convert to long to compare). Get rid of compiler warnings (accidentally wrote NSUInteger instead of NSInteger)
@firefueled
Copy link

Hey @aMarCruz I'm wondering why you wrote the type definition for MapEvent the way you did

export interface MapEvent<T = {}> extends NativeSyntheticEvent<T & {
    coordinate: LatLng;
    position: Point;
}> { }

, instead of

export interface MapEvent<T = {}> extends NativeSyntheticEvent<T> {
    coordinate: LatLng;
    position: Point;
}

The way it is now, the coordinate and position end up hidden under the nativeEvent key. Shouldn't they be added to the parent object so that one could access them more directly?
e => e.coordinate.latitude

Am I missing something?

@aMarCruz
Copy link
Contributor Author

@firefueled I really don't know. May be a typo. Right now I'm not using maps but will check it next week.

@firefueled
Copy link

I was thinking about submitting a PR like this:

type MapEventType = {
    coordinate: LatLng;
    position: Point;
}
export interface MapEvent<T = {}> extends NativeSyntheticEvent<T & MapEventType>, MapEventType { }

but I wasn't sure if the thing you did was on purpose so I decided to ask first.
The redundancy there is to make sure people's builds don't break.
Also, we need to check that those values really are NOT expected to be included into NativeSyntheticEvent

@aMarCruz
Copy link
Contributor Author

yeah, it needs testing.

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