[ASMapNode] bug fixes + added the ability to zoom in on the annotations + example project

[george-gw]

• Toggle user interaction when liveMap changes ([ASMapNode] Toggling liveMap on does not enable touches #1753)
• Change map snapshot when updating it with a new region ([ASMapNode] Setting region on ASMapNode doesn't work when liveMap = false #1754)
• Commented out code that is causing inaccurate behavior, this was wrapped with a FIXME
• Added the ability to zoom in and show annotations, similar to showAnnotations:animated: of MKMapView.
• Added a basic example for ASMapNode to try out the different changes

A minor change: Fixed typo in some comment

[ghost]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

[ghost]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[Adlai-Holler]

What's the motivation for this change?

[Adlai-Holler]

Oh I see, it's to trigger a new snapshot. Hmm. Let's use [self.options copy] instead.

[appleguy]

@george-gw yes, @Adlai-Holler is right - we should copy the options, then just write to the region value, and set it back to self.options.

[george-gw]

@appleguy @Adlai-Holler I had written it with [self.options copy] before, but it didn't work for some reason, that's why I did what I did there. But I must have done something wrong somewhere else because it seems to work fine now with [self.options copy].

Thanks!

[appleguy]

Looks like a number of revisions are required here. Thanks for contributing, @george-gw! This is an important component, so we should get at least two ASDK accepts to land.

[george-gw]

@appleguy I fixed everything except setting userInteractionEnabled = YES by default because of the pending discussion, I hope that's ok!

Thank you for the revisions and this library!

[hannahmbanana]

@Adlai-Holler @maicki - time for a 2nd review on this.

[Adlai-Holler]

Would you be willing to protect the new showAnnotationsOptions flag with a lock? I doubt it will be contended often, but better safe than sorry. And then once it's protected, ensure that the property is only read once during setAnnotations:.

Once that is in, 👍 from me!

[hannahmbanana]

@george-gw

[george-gw]

@Adlai-Holler: Wouldn't it potentially cause a problem in addLiveMap if I protect it with a mutex, since it requires to be called on the main thread?

PS: I added the lock

[george-gw]

That's odd! Why would the test fail based on the new commit?
I tried running the test locally, it worked fine for me.

[Adlai-Holler]

@george-gw It should be OK because it's acceptable for the main thread to wait on background threads.

Strange that an arbitrary ASDisplayLayer test failed. I've restarted the tests – let's see if the failure occurs again.

@appleguy @hannahmbanana Maybe we have a fragile test? Here's the failure:

_ASDisplayLayerTests
  testDelegateDrawInContextAsync, ((asyncDelegate.drawRectCount) equal to (0u)) failed: ("1") is not equal to ("0")
  /Users/travis/build/facebook/AsyncDisplayKit/AsyncDisplayKitTests/ASDisplayLayerTests.m:361
    XCTAssertEqual(layer.drawInContextCount, 0u);
    XCTAssertEqual(asyncDelegate.drawRectCount, 0u);
    dispatch_resume([_ASDisplayLayer displayQueue]);
     Executed 308 tests, with 1 failure (0 unexpected) in 26.641 (27.338) seconds

[george-gw]

@Adlai-Holler Alright, cool! And thanks for restarting the test, seems good now!

[Adlai-Holler]

Thanks for following through on this one @george-gw ! Glad to have this in.

[george-gw]

I enjoyed the collaboration, thank you for your input @Adlai-Holler :)