Skip to content
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

Traffic plugin #4

Closed
zugaldia opened this issue Apr 5, 2017 · 11 comments
Closed

Traffic plugin #4

zugaldia opened this issue Apr 5, 2017 · 11 comments
Assignees
Labels
Milestone

Comments

@zugaldia
Copy link
Member

zugaldia commented Apr 5, 2017

We'd like to add a traffic plugin that lets you add a traffic layer to any Mapbox basemap. To do that, we could use runtime styling.

We'd also like to add a sample activity to the test app that lets you switch between a few basemaps, including a toggle to turn traffic on and off.

cc: @ericrwolfe

@zugaldia zugaldia modified the milestone: v0.1 Apr 5, 2017
@tobrun
Copy link
Member

tobrun commented Apr 6, 2017

Missing an API to completely port the iOS code mapbox/mapbox-gl-native#8663.

@tobrun
Copy link
Member

tobrun commented Apr 6, 2017

ezgif com-video-to-gif 14

@zugaldia
Copy link
Member Author

zugaldia commented Apr 6, 2017

Missing an API to completely port the iOS code mapbox/mapbox-gl-native#8663

👍

@tobrun Is here a branch/PR we can play with?

@tobrun
Copy link
Member

tobrun commented Apr 6, 2017

The code isn't PR ready yet, most of implementation is done but hitting some unforeseen issues.

  • MapboxMap doesn't expose OnMapChangeListener anymore, we probably need to rely on both MapView and MapboxMap as a dependency.
  • Missing API to get layer positioning completely right
  • Crash when switching styles, on iOS it's allowed to reuse sources and layers while on Android you are required to do the following: remove/switch style/readd. The means we need a hook into style loading while we don't have that atm.

@ericrwolfe
Copy link
Contributor

Missing API to get layer positioning completely right

If you're looking for a workaround until there's a sourceLayerIdentifier, might be easy to just hard-code a few layer ids to check for.

Looking through the Mapbox core styles, the top road layer is generally one of the following two:

  • bridge-construction
  • bridge-motorway

Crash when switching styles

On iOS, runtime styling layers can't be reused when switching styles.

Generally every time the style changes, we listen for a style did finish loading callback, and re-invoke any runtime styling there:

extension TrafficOverlayViewController: MGLMapViewDelegate {
    func mapView(_ mapView: MGLMapView, didFinishLoading style: MGLStyle) {
        Traffic.setVisible(self.trafficEnabled, inStyle: self.mapView.style!)
    }
}

@tobrun
Copy link
Member

tobrun commented Apr 7, 2017

If you're looking for a workaround until there's a sourceLayerIdentifier, might be easy to just hard-code a few layer ids to check for. Looking through the Mapbox core styles, the top road layer is generally one of the following two:
bridge-construction
bridge-motorway

I ended up using these in the meantime, but the plugin should be able to handle any kind of setup. I'm working today on adding that feature in the SDK so we can continue working from a SNAPSHOT.

On iOS, runtime styling layers can't be reused when switching styles.

On Android neither.

Generally every time the style changes, we listen for a style did finish loading callback, and re-invoke any runtime styling there:

This is the part that is resulting in a crash, ticketed in mapbox/mapbox-gl-native#8675

@tobrun
Copy link
Member

tobrun commented Apr 7, 2017

Ok, think the crash was an issue from my end:

ezgif com-video-to-gif 15

@tobrun tobrun mentioned this issue Apr 7, 2017
@tobrun tobrun added the feature label Apr 7, 2017
@tobrun
Copy link
Member

tobrun commented Apr 10, 2017

Current state:

ezgif com-video-to-gif 32

@zugaldia
Copy link
Member Author

@tobrun Looking great. Just two quick UI-suggestions:

  1. How about the traffic toggle includes a car icon to make it more explicit? Or we replace the FAB with a lower button centered that simply says "toggle traffic".

  2. Instead of rotating through the base styles one by one, how about we add a button row on the top of the screen with the names of the styles?

@ericrwolfe Anything else you'd like to see it here?

@ericrwolfe
Copy link
Contributor

@tobrun Awesome work, this looks great. I think what we have in terms of changing the style and toggling the traffic is all we need in terms of the example.

@tobrun
Copy link
Member

tobrun commented Apr 11, 2017

I think what we have in terms of changing the style and toggling the traffic

Remaining item to hit from #4 (comment) is changing icons.

device-2017-04-11-084657

This was referenced Apr 11, 2017
@tobrun tobrun closed this as completed in #6 Apr 11, 2017
cammace pushed a commit that referenced this issue Jul 14, 2017
# This is the 1st commit message:
add Building plugin

# The commit message #2 will be skipped:

#	fixed issues reviewed by me

# The commit message #3 will be skipped:

#	fixed bitrise script

# The commit message #4 will be skipped:

#	added DDS for building height

# The commit message #5 will be skipped:

#	removed commented out code
cammace pushed a commit that referenced this issue Jul 18, 2017
* # This is a combination of 5 commits.
# This is the 1st commit message:
add Building plugin

# The commit message #2 will be skipped:

#	fixed issues reviewed by me

# The commit message #3 will be skipped:

#	fixed bitrise script

# The commit message #4 will be skipped:

#	added DDS for building height

# The commit message #5 will be skipped:

#	removed commented out code

* cleaned up gradle file

* updated Makefile

* added optional constructor param to place buildings below layer

* added comment with more information
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants