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

Bearing missing from XML #6482

Closed
cammace opened this issue Sep 27, 2016 · 6 comments · Fixed by #6894
Closed

Bearing missing from XML #6482

cammace opened this issue Sep 27, 2016 · 6 comments · Fixed by #6894
Assignees
Labels
Android Mapbox Maps SDK for Android SEMVER-MAJOR Requires a major release according to Semantic Versioning rules

Comments

@cammace
Copy link
Contributor

cammace commented Sep 27, 2016

When creating the Mapview in XML you can set the initial tilt, position, and zoom but there isn't an option for bearing/rotation.

cc: @tobrun

@cammace cammace added the Android Mapbox Maps SDK for Android label Sep 27, 2016
@tobrun
Copy link
Member

tobrun commented Sep 27, 2016

Because it's called direction :trollface:

@tobrun tobrun closed this as completed Sep 27, 2016
@tobrun
Copy link
Member

tobrun commented Sep 27, 2016

Feel free to (re)open this/an issue if you want to rename it, we have currently 3 names for same concept (mainly mixture of naming in core vs google parity vs legacy code). This would introduce a semver change.

@cammace cammace reopened this Sep 27, 2016
@cammace cammace added this to the android-future milestone Sep 27, 2016
@cammace cammace added the SEMVER-MAJOR Requires a major release according to Semantic Versioning rules label Sep 27, 2016
@tobrun
Copy link
Member

tobrun commented Nov 2, 2016

Noticed that we use the following:

<!--Camera-->
<attr name="mapbox_center_longitude" format="float" />
<attr name="mapbox_center_latitude" format="float" />
<attr name="mapbox_zoom" format="float" />
<attr name="mapbox_direction" format="float" />
<attr name="mapbox_tilt" format="float" />

If we are breaking the atttribute mapbox_direction already, we should also look into renaming the other attrs to match the fields from CameraPosition class:

    /**
     * Direction that the camera is pointing in, in degrees clockwise from north.
     */
    public final double bearing;

    /**
     * The location that the camera is pointing at.
     */
    public final LatLng target;

    /**
     * The angle, in degrees, of the camera angle from the nadir (directly facing the Earth). See tilt(float) for details of restrictions on the range of values.
     */
    public final double tilt;

    /**
     * Zoom level near the center of the screen. See zoom(float) for the definition of the camera's zoom level.
     */
    public final double zoom;

This means renaming mapbox_center_longitude to mapbox_target_longitude and mapbox_center_latitiude to mapbox_target_longitude.

@mariusboepple
Copy link

Are you serious?! First, these are breaking changes - please mention this in the changelog next time! Second, now I have to spend half an hour on how you renamed the attributes to change them all, because we're using your maps in many places of our app, but maybe that's not the recommended use-case... Please at least provide some additional information how to migrate (I hope that was planned for the stable release)... And third: I don't think that is really how it should be designed for the long-term, attributes do/should not have to be prefixed in Android.

@tobrun
Copy link
Member

tobrun commented Mar 20, 2017

@mariusboepple

On first: We follow semantic versioning, this allows us to only change the api on a major version bump. These changes are items we have been postponing for a long time since we were committed to the public api of 4.x. On second: We have a migration guide but it seems we need to look into improving the visibility of it. On third: when writing Android code for an aar it's recommend to prefix resources:

you configure a "resource prefix" for your Android library (see the build system docs), in order to avoid accidental name clashes, Studio will flag all resources that do not conform to the given prefix, and it will also default newly suggested identifiers in the layout editor, create resource dialogs etc to identifiers which begin with the prefix.

Apologies if the update has been a burden, thank you for reaching out and reporting this back to us!

@mariusboepple
Copy link

Okay, thanks for the quick response. Nevertheless it would be nice if breaking changes are marked and the migration guide is linked in the changelog (because I'm checking the changelog and not if there's any guide in the wiki when I'm updating)...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android SEMVER-MAJOR Requires a major release according to Semantic Versioning rules
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants