tangrams / walkabout-style

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

Toggle bike interlay with 'visible' flag instead of filters #154

Closed matteblair closed 7 years ago

matteblair commented 7 years ago

@nvkelso @sensescape The visible layer flag seems to do what you are trying to achieve by querying global flags in filter functions. Using visible is more efficient for the engine and (in my opinion) reads more naturally.

I believe this changes preserves the previous behavior, but I haven't been looking at this scene as long as either of you.

nvkelso commented 7 years ago

I'll have a look...

nvkelso commented 7 years ago

I think there's a path forward here (to use more visible properties rather than always filter options), but the current PR ends up showing bike map stuff by default (oops), and removes most minor roads (by default, but restored in bike mode?!). I'll have a closer look tomorrow.

screen shot 2017-03-13 at 17 45 03 screen shot 2017-03-13 at 17 44 54
matteblair commented 7 years ago

Whoa, that is not at all what happens in tangram-es. This definitely deserves a closer look!

screen shot 2017-03-13 at 10 24 11 pm
matteblair commented 7 years ago

@bcamper and I were puzzling through this and we discovered that the primary reason this change behaves differently on JS and ES is that the two engines have different interpretations of the layer visible flag.

In tangram-es if a layer has visible: false, that layer and its sublayers will not be visible, regardless of any visible flags in the sublayers.

In tangram-js if a layer has visible: false, that layer and its sublayers will not be visible unless a sublayer has visible: true.

The syntax documentation doesn't specify one way or the other: https://mapzen.com/documentation/tangram/layers/#visible, so before going any further with this PR, we should standardize this behavior.

nvkelso commented 7 years ago

Once Tangram JS v0.12 lands I'll refresh this PR...

nvkelso commented 7 years ago

JS and ES now support enabled, I'll have another look at this soon.

nvkelso commented 7 years ago

Fixed in https://github.com/tangrams/walkabout-style/pull/165/commits/82e4c860b0439e1e46d165ae4024134598cae889 via the other PR. Closing this one.