Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

MapActivity in SDK #2616

Closed
tobrun opened this issue Oct 14, 2015 · 8 comments
Closed

MapActivity in SDK #2616

tobrun opened this issue Oct 14, 2015 · 8 comments
Assignees
Labels
Android Mapbox Maps SDK for Android

Comments

@tobrun
Copy link
Member

tobrun commented Oct 14, 2015

Acknowledged by both @bleege and @ljbade that this would be a nice addition to the SDK.
Current setup requires overriding multiple lifecycle methods of Activity.
In the past I forgot to add one or I had overridden the wrong one.. imo this setup is error prone.
Besides that I would also simplify the code in the test application, which makes it easier to understand,

Proposition:

  • in similar fashion as ListActivitity in the Android SDK
    • uses a default layout file, allows setting a different one but enforces a certain id (android.R.id.list)
@bleege bleege added the Android Mapbox Maps SDK for Android label Oct 14, 2015
@bleege bleege added this to the android-v2.2.0 milestone Oct 14, 2015
@ljbade
Copy link
Contributor

ljbade commented Oct 14, 2015

So the idea is that for a quick map activity one just uses our MapActivity or possibly subclass/custom XML to adapt it to their specific needs.

@ljbade
Copy link
Contributor

ljbade commented Oct 24, 2015

Been working on this in 2ad5e3d

@ljbade
Copy link
Contributor

ljbade commented Oct 30, 2015

Discovered we also need a AppCompatMapActivity

@tobrun
Copy link
Member Author

tobrun commented Nov 20, 2015

Picking this up again

@tobrun
Copy link
Member Author

tobrun commented Nov 20, 2015

@bleege @zugaldia:
I created another approach to this based on ListActivity implementation in the Android SDK.
Would love some 👀 on this hence changing this afterwards could impact users.

MapActivity itself and implemented examples in InfoWindowActivity and VisibleCoordinateBoundsActivity

@zugaldia
Copy link
Member

Another possibility is to follow Google's approach to AppCompatActivity (MapActivity) for consistency with the rest of the Android framework.

The idea is not just having a base class but also a AppCompatDelegate (MapDelegate?):

In this release, ActionBarActivity has been deprecated in favor of the new AppCompatActivity. However, this wasn’t just a rename. In fact, the internal logic of AppCompat is now available via AppCompatDelegate - a class you can include in any Activity, hook up the appropriate lifecycle methods, and get the same consistent theming, color tinting, and more without requiring you to use AppCompatActivity (although that remains the easiest way to get started).

@bleege
Copy link
Contributor

bleege commented Nov 29, 2015

@tobrun @zugaldia The more I think about this, the more I'm not sure that MapActivity is a useful thing for developers using the Mapbox SDK. It appears to shoe horn developers into a preset settings for Activities (having to use App Bar, etc) that may be difficult for them to change / customize. Granted if everyone is using Google recommend Material Design constructs these days then it's likely not too big an impact, but it still forces an opinion on them. Forcing opinions at the Activity level seems a step too far IMHO.

I'm thinking that Google's approach in the Google Maps API is the one we should follow. Specifically, the core class is GoogleMap that can be accessed from MapFragment and MapView. In Mapbox's case we'd likely call it MapboxMap instead. It'd be keeping with the Mapbox SDK design goals to mirror Google Maps and it'd decouple the Map object from Android lifecycle and chrome implementation.

@tobrun
Copy link
Member Author

tobrun commented Nov 30, 2015

Closing this down for now, will create a separate issue for introducing MapboxMap.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

No branches or pull requests

5 participants