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

Move gesture recognizers from GLKView to TGMapView itself #2127

Merged
merged 1 commit into from
Jan 19, 2020

Conversation

matteblair
Copy link
Member

What's different?

The gesture recognizers used in TGMapView used to be attached to the GLKView that gets created as a subview. They are now attached to the TGMapView itself. There should be no differences in behavior from this change.

Why?

If the app is still in the background during initial setup of TGMapView, the GLKView will be nil when we try to attach gesture recognizers to it - leaving us with no working gestures.

To avoid this, we can instead attach the recognizers to the TGMapView itself.

This situation seems to occur in apps using the "scene" based UI framework introduced in iOS 13 (https://developer.apple.com/documentation/uikit/app_and_environment/scenes). This is the default UI paradigm in iOS going forward, so Tangram should work correctly within it.

Why was it done differently before?

I think when this was implemented the first time, I misunderstood how touch events are handled in iOS view hierarchies. I probably thought that the gesture recognizers had to be on the front-most view to receive touch events, but in fact they'll receive any touch events that aren't handled by subviews (https://developer.apple.com/documentation/uikit/touches_presses_and_gestures/using_responders_and_the_responder_chain_to_handle_events).

If the app is still in the background during initial setup of TGMapView, the GLKView will be nil when we try to attach gesture recognizers to it - leaving us with no working gestures.

To avoid this, we can instead attach the recognizers to the TGMapView itself.
@matteblair matteblair merged commit 4fed5d9 into master Jan 19, 2020
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.

1 participant