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.

Sign up for GitHub

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

Jump to bottom

Fixed onMapReady event on iOS to resemble onMapReady on Android #1797

Merged
merged 1 commit into from
Nov 28, 2017
Merged

Fixed onMapReady event on iOS to resemble onMapReady on Android #1797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/ios/AirGoogleMaps/AIRGoogleMap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
@property (nonatomic, assign) BOOL showsMyLocationButton;
@property (nonatomic, assign) BOOL showsIndoorLevelPicker;

- (void)didFinishTileRendering;
- (void)didPrepareMap;
- (BOOL)didTapMarker:(GMSMarker *)marker;
- (void)didTapPolyline:(GMSPolyline *)polyline;
- (void)didTapPolygon:(GMSPolygon *)polygon;
Expand Down
2 changes: 1 addition & 1 deletion lib/ios/AirGoogleMaps/AIRGoogleMap.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ - (void)setRegion:(MKCoordinateRegion)region {
self.camera = [AIRGoogleMap makeGMSCameraPositionFromMap:self andMKCoordinateRegion:region];
}

- (void)didFinishTileRendering {
- (void)didPrepareMap {
if (self.onMapReady) self.onMapReady(@{});
}

Expand Down
18 changes: 10 additions & 8 deletions lib/ios/AirGoogleMaps/AIRGoogleMapManager.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@


@interface AIRGoogleMapManager() <GMSMapViewDelegate>

{
BOOL didCallOnMapReady;
}

@end

@implementation AIRGoogleMapManager
Expand Down Expand Up @@ -263,13 +266,12 @@ - (NSDictionary *)constantsToExport {
return @{ @"legalNotice": [GMSServices openSourceLicenseInfo] };
}

+ (BOOL)requiresMainQueueSetup {
return YES;
}

- (void)mapViewDidFinishTileRendering:(GMSMapView *)mapView {
AIRGoogleMap *googleMapView = (AIRGoogleMap *)mapView;
[googleMapView didFinishTileRendering];
- (void)mapViewDidStartTileRendering:(GMSMapView *)mapView {
if (didCallOnMapReady) return;
didCallOnMapReady = YES;

AIRGoogleMap *googleMapView = (AIRGoogleMap *)mapView;
[googleMapView didPrepareMap];
}

- (BOOL)mapView:(GMSMapView *)mapView didTapMarker:(GMSMarker *)marker {
Expand Down
14 changes: 10 additions & 4 deletions lib/ios/AirMaps/AIRMapManager.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ @interface AIRMapManager() <MKMapViewDelegate>
@end

@implementation AIRMapManager

{
BOOL didCallOnMapReady;
}

RCT_EXPORT_MODULE()

- (UIView *)view
Expand Down Expand Up @@ -708,6 +711,12 @@ - (void)mapView:(AIRMap *)mapView regionDidChangeAnimated:(__unused BOOL)animate

- (void)mapViewWillStartRenderingMap:(AIRMap *)mapView
{
if (!didCallOnMapReady)
{
didCallOnMapReady = YES;
mapView.onMapReady(@{});
}

mapView.hasStartedRendering = YES;
[mapView beginLoading];
[self _emitRegionChangeEvent:mapView continuous:NO];
Expand All @@ -716,9 +725,6 @@ - (void)mapViewWillStartRenderingMap:(AIRMap *)mapView
- (void)mapViewDidFinishRenderingMap:(AIRMap *)mapView fullyRendered:(BOOL)fullyRendered
{
[mapView finishLoading];
[mapView cacheViewIfNeeded];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was also an issue: cacheViewIfNeeded is called on finishLoading also, which is a tremendous overhead that is not necessary.
As caching an image is a CPU and RAM consuming job.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I'm sorry to say but this commit is quite flawed. It creates a global variable called didCallOnMapReady which once set to YES is never reset to NO. This breaks onMapReady quite badly, causing it now to only fire on the first rendered MapView and after that never again (not when MapView is unmounted/mounted, or for any other MapViews).

@danielgindi @christopherdro

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by PR #1853


mapView.onMapReady(@{});
}

#pragma mark Private
Expand Down