Android backbutton handling issue

[cusspvz]

This needs to be reviewed, moved aside from the #42.

@Anothar I had issues before with the addMarkerCallback because of the way it is designed to work (by a class shared variable).

Will let this aside for us to think on a better way to implement it.

The first idea that comes into my mind is to implement an event dispatcher natively on each map creation that is bind to a Map instance on the JS side. This would make future implementations easier, but not sure how difficult it could be ti implement.

[risinghero]

The first idea that comes into my mind is to implement an event dispatcher natively on each map creation that is bind to a Map instance on the JS side. This would make future implementations easier, but not sure how difficult it could be ti implement.

Interesting idea. Let's try to go a little further in your idea. If we make independent class for map and create it on map show. Then we can add all handlers to it. And what then we get? A multiple map support. As I see it will also require some feasible (not that hard) changes in code. Actually we have to add list of maps to code and generate for each map unique id. Then add to each method id as parameter. And write useful abstraction in js code.

[cusspvz]

If we make independent class for map and create it on map show. Then we can add all handlers to it. And what then we get? A multiple map support. As I see it will also require some feasible (not that hard) changes in code. Actually we have to add list of maps to code and generate for each map unique id. Then add to each method id as parameter. And write useful abstraction in js code.

Exactly what I was thinking. The issue with that is this could take longer to implement. If we could open up the way for it by the next minor release (by placing only the native event dispatcher and the evented class under JS) we could solve this issue, release, and start thinking over mulltiple map support.

If you think further on that idea, you will have a better API for managing maps and markers as well, since you could atomically bind/unbind markers to a map, the click event on markers could be handled trough the map instance event dispatcher, which would fire an event on a respectively marker instance.

Seems we do both can see and agree over it, but does the native event emitter (without multiple map for now, just an added callback) is difficult to implement over both Android and iOS?

[dagatsoin]

Hello guys,
multi maps are part of my branch #29. I did not finish the implementation yet (I have to handle the overlays zone of each map during touch). So +1 for thinking about it for the next major release.

What is exactly the issue with markercallback and how it is related to the backbutton?

[cusspvz]

@dagatsoin Hello!

multi maps are part of my branch #29. I did not finish the implementation yet (I have to handle the overlays zone of each map during touch). So +1 for thinking about it for the next major release.

Oh really!?! Nice! Are you capturing the touch event per map view?

What is exactly the issue with markercallback and how it is related to the backbutton?

It happened with me, since markerCallback does sit under a global space instead of a map instance, it will sit there until we change. I have two kinds of maps on the project I'm using it. The first one does have markers and I need the callback for navigation. The second one is a location dragger, it has only one map, you move the map anywhere, it places the marker always in the middle (since we don't have dragged markers) and this allows me to give the user a way to choose a location. The issue was, whenever an user clicked on the second map's marker, it tried to execute things from the first one. I've workaround it by placing a noop js function () => {} whenever the first map was destroyed. The solution is to keep callbacks only during while the map view is opened.

[cusspvz]

This line should be removed, cannot forget it into here.

[dagatsoin]

The issue was, whenever an user clicked on the second map's marker, it tried to execute things from the first one.

Ok I see. For a minor release, what about making a map of callbacks identified by the markerId?
You could keep the actual JS api and add another optional parameter. Quickly, this how I see the implementation:

JS API:

addMarkerCallback: function (callback, markerId) {
  cordova.exec(callback, null, "Mapbox", "addMarkerCallback", [markerId || ""]);
},

Android:

• change the MarkerClickListener Class to accept a specific success function
• replace https://github.com/Telerik-Verified-Plugins/Mapbox/blob/master/src/android/Mapbox.java#L393 by a map.
• and add the callback with this: mapView.setOnInfoWindowClickListener(new MarkerClickListener(String id));

Back to the features list. To be sure to understand how will be organize the next minor/major release how can we do a roadmap or any other document to decide which feature for which release? On my side the main features are:

• overlay DOM html elements
• multi maps

[risinghero]

+1 for feature list
+1 for multi maps

• refactor: separate each class to different file.
• move to swift on iOS to make code simpler

[cusspvz]

Added milestone and roadmap under #68

[cusspvz]

I would opt-in for removing the callback natively for now while hiding a map, I propose this because it seems easier than changing the API, could fix by now. And we could release it as a patch. :D

Android

this.markerCallbackContext = null

iOS

self.markerCallbackId = nil

What do you think?

[risinghero]

@cusspvz , okay fix by now.