-
Notifications
You must be signed in to change notification settings - Fork 10
Move to ES6 #2
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 to ES6 #2
Conversation
const Mapbox = require('com.mapbox.mapboxsdk.Mapbox'); | ||
const Activity = require('android.app.Activity'); | ||
|
||
Mapbox.getInstance(new Activity(Ti.Android.currentActivity), 'YOUR_MAPBOX_ACCESS_TOKEN'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this will work correctly here, since it may have to run delayed for the current activity stuff to work. That's why I did it lazily inside the createView call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hoping it is executed once anything is imported, which seems to work okay. The problem is that there are mapbox API's that do not require a map-view, like geocoders. Maybe do an initialize
method? iOS can actually also be configured by code, so I'd remove the plist-key and use one central method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing this one:
import MapView from './mapview';
import Annotation from './annotation';
const MGLAccountManager = require('Mapbox/MGLAccountManager');
class Mapbox {
static initialize(apiKey) {
if (!apiKey) { throw 'No API-key provided!'; }
MGLAccountManager.setAccessToken(apiKey);
}
}
export { Annotation, MapView, Mapbox }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
Mapbox.getInstance(new Activity(Ti.Android.currentActivity), 'YOUR_MAPBOX_ACCESS_TOKEN'); | ||
class Mapbox { | ||
static initialize(apiKey) { | ||
if (!apiKey) { throw 'No API-key provided!'; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're not supposed to throw raw strings, but wrap them in an Error object:
if (!apiKey) { throw new Error('No API-key provided!'); }
|
||
class Mapbox { | ||
static initialize(apiKey) { | ||
if (!apiKey) { throw 'No API-key provided!'; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
I don't have write access, so can't merge. CR seems fine. |
Thanks for the review @sgtcoolguy! We are blocked by the Hyperloop 3.1.0 release but can merge once released (pending QE testing). |
Ready to review! Requires Hyperloop 3.1.0 Beta 2 (or later).