tangrams / walkabout-style

Preview map:
https://tangrams.github.io/walkabout-style
MIT License
32 stars 14 forks source link

WIP: dimmed routing for transit overlay, bike map #189

Open nvkelso opened 7 years ago

nvkelso commented 7 years ago

See also:

Transit example:

screen shot 2017-08-07 at 17 05 46 screen shot 2017-08-07 at 17 05 38

Bike map example:

screen shot 2017-08-07 at 17 03 16 screen shot 2017-08-07 at 17 03 05
nvkelso commented 7 years ago

@bcamper Can you comment on enabled allowing functions, please? I think now it just accepts a boolean? (When I set it to a function it always evaluates to true.)

Also, Tangram JS seems to complain when I set a data source to be {} like scene.config.sources.mz_route_start = {} and complains Tangram v0.13.0 [warn]: Scene: Could not create data source: undefined.... {url: undefined} Is there another way to null it out via the Tangram JS interface?

nvkelso commented 7 years ago

/cc @msmollin

nvkelso commented 7 years ago

/cc @burritojustice

bcamper commented 7 years ago

@nvkelso

enabled For layer enabled, it is only intended to accept a boolean. In fact, since layers default on, it only checks for an explicit false value, so any other value will keep the layer enabled -- @matteblair do you think this is an issue with ES? We could add more explicit warnings for non-boolean values (like enabled: xyz), it is just the most natural thing in JS to check for explicit false like this.enabled = (enabled !== false);.

I would like to not have enabled accept functions, because they are evaluated in a different context / stage in the scene pipeline (almost all the other cases where we evaluate functions, they are in the context of a specific feature), and because I think cases where that's needed can be handled by putting the necessary logic into a filter instead.

enabled does however accept a global, like enabled: global.show_layer, for basic toggling cases.

data sources Currently Tangram JS expects every value in the config.sources object to itself be a valid data source object. However it is allowable to have style layers that reference a data source that doesn't exist (maybe because it will be added at run-time), so the current proper way to "remove" a data source is to delete it from the object entirely (since setting it to an empty object or null won't work):

delete scene.config.sources.mz_route_start;

We can also look at adding logic to treat a null value as an empty data source (including if null is passed to scene.setDataSource, which currently throws a warning/error message). This might be more natural expectation than using delete.

matteblair commented 7 years ago

Re: enabled Tangram ES behaves the same way, it will only disable the layer if the value is false. I agree with the reasoning above for disallowing functions.

nvkelso commented 7 years ago

This PR should also add colorized route line based on additional Valhalla properties (like cycleway) that are kinda the same as Tilezen but different.