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

Use the defaultStyles constant on Android apps #1462

Closed
tmpsantos opened this issue May 7, 2015 · 14 comments
Closed

Use the defaultStyles constant on Android apps #1462

tmpsantos opened this issue May 7, 2015 · 14 comments
Labels
Android Mapbox Maps SDK for Android refactor

Comments

@tmpsantos
Copy link
Contributor

The MacOS and Linux app are now using the defaultStyles as the list of styles we can select from. The iOS and Android apps should use the same list.

defaultStyles was added at #1443

/cc @incanus

@tmpsantos tmpsantos added iOS Mapbox Maps SDK for iOS refactor Android Mapbox Maps SDK for Android starter-task labels May 7, 2015
@incanus incanus added this to the iOS Beta 2 milestone May 7, 2015
@jfirebaugh
Copy link
Contributor

default_styles.hpp should be a header in include/mbgl/platform so it can be included via #include <mbgl/platform/default_styles.hpp>. There shouldn't be relative includes like this in the test/sample apps.

@kkaefer
Copy link
Contributor

kkaefer commented May 13, 2015

There shouldn't be relative includes like this in the test/sample apps.

Why not? My reason for not moving it into the include/mbgl/platform is that it's technically not part of the library.

@jfirebaugh
Copy link
Contributor

I think it should be considered part of the library. If a third-party wanted to build an app with style-switching functionality, they should #include <mbgl/platform/default_styles.hpp> and then use it.

@kkaefer
Copy link
Contributor

kkaefer commented May 15, 2015

The bundled styles aren't technically part of the library either; they're just there because we package them with the apps.

@incanus
Copy link
Contributor

incanus commented May 15, 2015

They are part of the library on iOS — we package a MapboxGL.bundle with the static library and headers which contains resources like the compass image and Mapbox logo as well as the included styles. It's a first-class part of the library. Currently we recommend users query for the included styles with an API call, which we hook up by iterating the filesystem directly. This should be a first-class API internal to mbgl that bindings like iOS can query.

@ljbade
Copy link
Contributor

ljbade commented May 18, 2015

Hmm good idea @incanus.

@incanus
Copy link
Contributor

incanus commented Jun 15, 2015

Punting off of b2 since the bundled styles haven't changed since b1 and nothing under the hood has moved here that makes this any more urgent.

@incanus incanus removed this from the iOS Beta 2 milestone Jun 15, 2015
@ljbade ljbade added this to the Android b1 milestone Jul 28, 2015
@bleege bleege modified the milestones: v0.2.0-android, v0.1.0-android Aug 26, 2015
@bleege bleege modified the milestones: android-v2.1.0, android-v2.2.0 Oct 2, 2015
@ljbade
Copy link
Contributor

ljbade commented Oct 24, 2015

@1ec5 is looking to implement this in #2746 (comment) for iOS.

On Android I decided to just have a class that provides static constants in the Styles class.

1ec5 added a commit that referenced this issue Oct 30, 2015
Moved mbgl::util::default_styles to a more appropriate location, where iOS platform code can also find it. Moved -[MGLMapView bundledStyleURLs] (which is now deprecated) and the style switcher in iosapp to default_styles.

Added a collection of convenience methods for getting style URLs. It makes little sense to layer an enum atop this, as MapKit does, because MGLMapView styles aren’t limited to this set. A good analogy is UIColor. This also makes for a good entry point for future runtime styling APIs.

Introduced independent constants for each default style, because it’s more common to need access to a particular style than to iterate over them. This fact is apparent in the MGLStyle class, which now uses macros and assertions to ensure that it’s kept up-to-date with changes in default_styles.

/ref #1462
AndwareSsj pushed a commit to AndwareSsj/mapbox-gl-native that referenced this issue Nov 6, 2015
Moved mbgl::util::default_styles to a more appropriate location, where iOS platform code can also find it. Moved -[MGLMapView bundledStyleURLs] (which is now deprecated) and the style switcher in iosapp to default_styles.

Added a collection of convenience methods for getting style URLs. It makes little sense to layer an enum atop this, as MapKit does, because MGLMapView styles aren’t limited to this set. A good analogy is UIColor. This also makes for a good entry point for future runtime styling APIs.

Introduced independent constants for each default style, because it’s more common to need access to a particular style than to iterate over them. This fact is apparent in the MGLStyle class, which now uses macros and assertions to ensure that it’s kept up-to-date with changes in default_styles.

/ref mapbox#1462
@bleege bleege modified the milestones: android-v2.3.0, android-v2.4.0 Dec 4, 2015
@bleege bleege modified the milestones: android-v3.0.0, android-v3.1.0 Dec 21, 2015
@bleege bleege modified the milestones: android-v3.1.0, android-v4.0.0 Jan 20, 2016
@friedbunny friedbunny changed the title Use the defaultStyles constant on iOS and Android apps Use the defaultStyles constant on Android apps Jan 21, 2016
@friedbunny friedbunny added Android Mapbox Maps SDK for Android and removed iOS Mapbox Maps SDK for iOS labels Jan 21, 2016
@tobrun
Copy link
Member

tobrun commented Feb 16, 2016

On Android we have:

/**
 * <p>
 * Style provides URLs to several professional styles designed by Mapbox.
 * </p>
 * These styles are all ready to go in your app. To load one, pass it into {@link MapView#setStyleUrl(String)}
 *
 * @see MapView#setStyleUrl(String)
 */
public class Style {

    /**
     * Indicates the parameter accepts one of the values from {@link Style}.
     */
    @StringDef({MAPBOX_STREETS, EMERALD, LIGHT, DARK, SATELLITE, SATELLITE_STREETS})
    @Retention(RetentionPolicy.SOURCE)
    public @interface StyleUrl {
    }

    // IMPORTANT: If you change any of these you also need to edit them in strings.xml

    /**
     * Mapbox Streets: A complete basemap, perfect for incorporating your own data.
     */
    public static final String MAPBOX_STREETS = "mapbox://styles/mapbox/streets-v8";
    /**
     * Emerald: A versatile style, with emphasis on road networks and public transit.
     */
    public static final String EMERALD = "mapbox://styles/mapbox/emerald-v8";
    /**
     * Light: Subtle light backdrop for data visualizations.
     */
    public static final String LIGHT = "mapbox://styles/mapbox/light-v8";
    /**
     * Dark: Subtle dark backdrop for data visualizations.
     */
    public static final String DARK = "mapbox://styles/mapbox/dark-v8";
    /**
     * Satellite: A beautiful global satellite and aerial imagery layer.
     */
    public static final String SATELLITE = "mapbox://styles/mapbox/satellite-v8";

    /**
     * Satellite Streets: Global satellite and aerial imagery with unobtrusive labels.
     */
    public static final String SATELLITE_STREETS = "mapbox://styles/mapbox/satellite-hybrid-v8";

}

Since @friedbunny removed iOS label, I think this is resolved for both platforms!

@tobrun tobrun closed this as completed Feb 16, 2016
@jfirebaugh jfirebaugh removed the ready label Feb 16, 2016
@friedbunny
Copy link
Contributor

Yup, iOS happened in #2746.

@tobrun tobrun reopened this Feb 17, 2016
@tobrun
Copy link
Member

tobrun commented Feb 17, 2016

It seems the current way is not as requested in this issue.

We should fetch:

and match those urls on the Style class shown above.

-> if we change default_styles.hpp -> our app urls should change automatically.

@1ec5
Copy link
Contributor

1ec5 commented Feb 17, 2016

On the iOS side, we're relying on a compile-time assertion that the number of default styles hasn't changed since we last updated the iOS-specific list. We also have a unit test to the same effect.

@tobrun
Copy link
Member

tobrun commented Feb 18, 2016

I was also thinking about handling this at compile time (integrating this as a task in gradle) but instead of asserting, I was thinking about generating the java file.
\cc @bleege @zugaldia

@jfirebaugh
Copy link
Contributor

Obsolete.

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

No branches or pull requests

9 participants