mapbox / mapbox-gl-style-spec

76 stars 38 forks source link

un-nest `visibility` from `layout`. #428

Closed tristen closed 8 years ago

tristen commented 8 years ago

visibility currently falls under layout which is an object collection of properties that effect the style/position of features in a given source.

But visibility doesn't behave the way it's siblings do. It hides/shows the entire layer. That fact that it does this makes the filter aspects of this example:

map.addLayer({
  "id": "foo",
  "source": "foo",
  "type": "symbol",
    "layout": {
      "visibility": "none"
    },
    "filter": ["in", "name", ""]
});

unusable. Here's what I hope demonstrates this: https://jsbin.com/xirorazabe/ Implementation details aside - I'd expect filtering features into this layer to work.

propose


Mapbox GL JS would need a set method for layer visibility i.e. map.setVisibility(layerID, 'none');

jfirebaugh commented 8 years ago

I'm not clear on what the issue here is and how moving visibility would address it. Can you clarify?

tristen commented 8 years ago

I'm not clear on what the issue here is and how moving visibility would address it. Can you clarify?

I'll try :)

I expect properties in layout or paint to effect the makeup of features in a source (i.e colour, positioning). The visibility property however does not work this way.

It's not like, display: none in CSS where you can give elements this css rule and it takes effect. When you set this property the layer is not there. You can't (like in the jsbin example I linked to) filter features into this layer dynamically because it's no longer around.

Pulling visibility out of layout clarifies and removes assumptions that visibility doesn't just dress up features.

tmcw commented 8 years ago

When you set this property the layer is not there.

What does "not there" mean? Visibility does not remove layers, it hides them.

tristen commented 8 years ago

I'm finding this challenging to explain. Or maybe I'm just over-explaining :p

Does the example I linked to not illustrate it? I want to be able to shove features into a layer that has it's visibility turned off.

tmcw commented 8 years ago

Yes... just like other layout & paint properties, visibility affects everything in a layer. If you set fill-color to green and wanted to shove new features into it, you'd expect those features to be green, right? It sounds like you have an expectation of layer styles as an imperative api that applies to features individually, instead of acting like a stylesheet, like they do right now.

tristen commented 8 years ago

sounds like you have an expectation of layer styles as an imperative api that applies to features individually, instead of acting like a stylesheet, like they do right now.

It sounds like that but I swear I don't! Let me put it this way: visibility sounds like it would hide features in a layer (and you could still add things to it). But it doesn't work that way.

It works more like something that should be named remove. Which is much more drastic than other property siblings.

lucaswoj commented 8 years ago

I think we cleared up this misunderstanding now. Some weird behaviour caused by collisions was throwing all of us for a loop. Feel free to re-open if I'm mistaken.

tristen commented 8 years ago

I'd like to keep this open. Visibility doesn't behave the way all other layout (or paint for that matter) properties work.

Here's a few examples. (Note: pan the map to initiate the featuresIn call):

And yes: changing these properties effects the entire layer but it effects features within the layer and visibility is the outlier. This will especially become more evident when variables are introduced (data-driven styling) to values.

The solution is to move visibility out of the layers object.

peterqliu commented 8 years ago

I'm pretty baffled, too. @tristen what is the difference between "the entire layer," and the sum of all features in a layer?

When I turn fill to green, am I "turning the entire layer green," or going through feature by feature and making each green?

When I turn visibility to none, am I "turning the entire layer invisible," or going through, feature by feature, and making each invisible?

Are these different?

tristen commented 8 years ago

OK I just talked this out with @tmcw and I'm going to close this. This example illustrates weird behaviour caused by collisions and I'm willing to accept my model of how this should work is not quiet correct :p