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

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

Open
wants to merge 5 commits into
base: gh-pages
Choose a base branch
from

Conversation

nvkelso
Copy link
Member

@nvkelso nvkelso commented Aug 7, 2017

  • Adds SDK support for 4 application state:
    • browse (default)
    • search - when showing search result pins
    • route-plan - preview a route before starting navigation
    • route-navigate - once you're in navigation (to turn on the big route arrow icon)
  • When the user is planning a route the bike map should "dim" to emphasize the route line.
  • Same thing for the transit overlay: transit lines should dim out so the multi-modal routeline is emphasized.
  • ¿Tangram JS only allows booleans for layer enabled setting?
    • Ideally this would support a function so we could have one application state with multiple values since enabled could be true for various application states (like route-plan and route-navigate).

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
Copy link
Member Author

nvkelso commented Aug 8, 2017

@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
Copy link
Member Author

nvkelso commented Aug 8, 2017

/cc @msmollin

@nvkelso nvkelso changed the title WIP: dimmed routing WIP: dimmed routing for transit overlay, bike map Aug 8, 2017
@nvkelso
Copy link
Member Author

nvkelso commented Aug 8, 2017

/cc @burritojustice

@bcamper
Copy link
Member

bcamper commented Aug 8, 2017

@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
Copy link
Member

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
Copy link
Member Author

nvkelso commented Aug 24, 2017

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

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

Successfully merging this pull request may close these issues.

3 participants