API for mutating style properties

[jfirebaugh]

So, it appears that it's only possible to cascade style changes made from an external tool if you change the stylesheet passed into mapbox-gl-js by reference, which is yucky.

Either let's have an API to do this, or maybe it'll be necessary to hack this in by using _.assign with map.style.stylesheet

[mourner]

How would an imaginary API for this look like?

We probably want to refactor style implementation in a way that makes it possible to do small atomic changes quickly. Like, changing one constant or adding one layer rule doesn't need to force reinterpretation of the whole style.

[jfirebaugh]

One potential imaginary API would be for updates to properties in the original object passed to map.setStyle() to be automatically cascaded. Good use case for Object.observe!

[tmcw]

For my use cases, I really really do not want to use the stylesheet object in the map object as the point of truth. A good API would provide a CSSOM-esque API like map.stylesheet.setStyle('layerid', 'property', 'value') that supported transitioning and covered constants as well.

[jfirebaugh]

For clarity, the API I was imagining is:

var style = {
  ...
  layers: {
    layerid: {
      style: {
        property: "value"
      }
    }
  }
};

map.setStyle(style);

style.layers.layerid.style.property = "new_value"; // cascades automatically

But since Object.observe support isn't widespread, it's probably moot.

In any case this seems like it's going to be a difficult/complex implementation, with lots of corner cases.

[tmcw]

Hm, yeah - that isn't the kind of API that would fit my use case, though I guess it could, if it had as part of its design that the map object does not mutate the style object internally.

[incanus]

@jfirebaugh's idea of an API is what I was envisioning over here:

cutting-room-floor/mapbox-gl-cocoa#31 (comment)

We probably want to refactor style implementation in a way that makes it possible to do small atomic changes quickly.

Precisely. Though this has been off my radar for a while, it's especially important on mobile, so the style of modification will likely percolate back over here.

[jfirebaugh]

the map object does not mutate the style object internally

Does it still do that? I would consider that a bug.

[mourner]

Yeah, it shouldn't.

[mourner]

But since Object.observe support isn't widespread, it's probably moot.

We could do something like http://vuejs.org/ (and some other MVC framework) does, — preprocess the given style object, replacing every property with a getter/setter with Object.defineProperty.

[tmcw]

I'm much more in favor of an explicit setter/getter implementation than using defineProperty or Object.observe

[mourner]

Yeah, I think I'm in favor of map.stylesheet.setStyle('layerid', 'property', 'value')-like API as well. Especially given that layers is an array, we can't make it an object, so it won't be as easy as this:

style.layers.layerid.style.property = "new_value"; // cascades automatically

[lbud]

Also, paint properties can cascade, which is super nice, but changing any other properties requires a reload. Should this be a catch-all API, so you could just as easily do

map.style.setIn(['layerId', 'paint', 'fill-color', '#f00'])    // cascades
map.style.setIn(['layerId', 'layout', 'text-font', 'New Font'])    // reloads
map.style.setIn(['layerId', 'filter', ['==', 'scalerank', 2])    // reloads

— or should it be limited to nice cascading paint properties?

[jfirebaugh]

Some questions about how y'all would like this API to behave.

So far, I have added map.setPaintProperty(layer, name, value[, class]) and map.getPaintProperty(layer, name[, class]).

Would you expect/require this to be perfectly symmetrical? I.e., if you call map.setPaintProperty('background', 'background-color', 'red'), do you expect the getter to return 'red', or is normalization to [1, 0, 0, 1] acceptable? And same question but for constants; must get after set("@red") return "@red" rather than the resolved value?

Which of the following are needed:

• removePaintProperty
• setLayoutProperty
• getLayoutProperty
• removeLayoutProperty
• setConstant
• getConstant
• removeConstant

[edenh]

Are we going to support getting and setting non-default paint properties, like paint.night?

I would expect the getter to return the set value (red) and if a constant, @red and use getConstant if necessary.

[jfirebaugh]

Are we going to support getting and setting non-default paint properties, like paint.night?

Yes, the last argument, class is an optional string like "night".

[tmcw]

I.e., if you call map.setPaintProperty('background', 'background-color', 'red'), do you expect the getter to return 'red', or is normalization to [1, 0, 0, 1] acceptable?

I'm cool with normalization but mostly because I don't see a good way that an alternate path could be implemented without kludge. Fwiw browsers do a little bit of normalization in CSS properties, especially complex ones like matrix transforms that are reformatted and mushed internally.

[jfirebaugh]

Yeah, the alternatives are to normalize at a later step (during cascade or recalculate, making them slower) or to store both the normalized and original values. Neither is very appealing.

[peterqliu]

@jfirebaugh is there a rationale for separate methods for layout and paint, instead of general setProperty and getProperty?

The catch-alls could avoid accidental mismatches, like map.setLayoutProperty('background', 'background-color', 'red')

[jfirebaugh]

Yes, it's essentially the same rationale as having "paint" and "layout" sections in styles -- they are different things in the features they support, how they behave, and in their performance characteristics. Changes to paint properties will transition, changes to layout properties will result in reparsing all tiles and will not transition. Also, setPaintProperty supports a class argument to set the property for a specific paint class, and setLayoutProperty will not. And setting a layout property on any one layer will affect all other layers that ref that layer.

[mourner]

@jfirebaugh lets not normalize, I don't see it as a problem in practice.

[robschley]

I see that you added an API to set layout and paint properties, that's very handy, thank you. But, as far as I can tell, filter settings are neither paint nor layout properties, they're layer properties. Do you intend to have a similar API for updating filters?

[jfirebaugh]

@robschley Correct, we'll followup in future releases with more APIs. I've opened a ticket for filters: #985. Could you elaborate on your use case there, so I can make sure it's covered by the API we design? Thanks!