Handle conditionals in fill and line tabs

[aparlato]

Closes https://github.com/stamen/mapbox-gl-style-design-system/issues/21

This PR adds conditional (match and case) handling to the fill and line tabs of the legend. It also adds code in expandLayers to handle conditionals inside of scales (interpolate or step expressions), figuring out what any given specified property should look like across all zooms

As a followup task, I think we should address long layer descriptors in a separate PR:

• nested conditionals have long descriptors which we should decide how to break up
  • this might take a little extra thinking since text wrap isn't super easy in SVG and I'm not 100% sure it's the best solution

Let me know if you think we should address here, but I think it makes sense to keep this PR scoped.

Review

Here's a test style I used if you want to try it for testing: test-interpolate-style.json 2.zip

• [ ] Confirm test style (or any style you want to use) breaks up and shows accurately nested conditionals inside scale or other conditional expressions
• [ ] Try to break this!

Note

When testing, note that step expressions do not appear to have handling here at all so will show up black. Also there appear to be some issues handling different color spaces like rgba as opposed to hsl.

I think we should handle both of these issues separately.

[aparlato]

@ebrelsford I think this is functional now, but it ended up using some roundabout logic I'm not in love with to try and expand off of the existing expandLayers logic and handle scenarios where conditionals across zooms don't match. While I haven't been able to break it, it feels brittle. On a side note, you may notice some issues in rendering appropriate colors in the Typography tab, but the output from expandLayers seems right so we should handle whatever is happening there separately I think.

I want to pause here before investing more time in cleaning up this code to see what you think about this path at this point (and if you have other ideas). Happy to look at this with you to discuss since it's admittedly convoluted.

The basic logic here is, if we have a conditional inside of a scale function:

• get all outputs from the scale function
• spread the match expression outputs to follow individual properties across zooms since they might be in an array (we can't do this with case expressions)
• expand all conditional outputs into a nested expanded values object
• use that nested object to create a new expression based on all zooms that we can evaluate
  • we want to do this so that dissimilar conditionals can be followed across zooms and it's easier to not write our own evaluator
• evaluate that expression at each zoom to build another new scale expression with literal outputs
• replace the expanded value from all conditional expanded layers across zooms with that appropriate scale expression

After having tried multiple approaches, I would say if this one feels too convoluted, my current thoughts would be to try and approach conditionals inside of scale expressions separately from the expandLayers logic (although we can use the same data structure output) in a more scoped way.

For example, we might try to parse out what properties to look at (anything following get in an array) and try to suss out values, then use those to do simple evaluations of each property value at each zoom. This would mean if we had a case expression like:

[
  "case",
  ["!=", ["get", "type"], "park"],
  ...
]

we'd be showing the output for type=park.

[ebrelsford]

Thanks for your work on this @aparlato, it's generally creating the output that I would expect!

Not sure if you've tested this but the lines chart doesn't seem to be working for the styles that I have tried. I haven't dug into it too much so I'm not sure how big of an issue this is.

While I haven't been able to break it, it feels brittle.

I agree. How do you feel about adding unit tests for some of these functions, both to more clearly state the inputs/outputs of them and to ensure that future changes here don't break them? I don't see anything in here that makes me want to completely start over but I agree that it can be a little hard to follow at times.

If we get tests in place and run the output by our cartographers to confirm that they match expectations I would like to keep this as it is.

Outside of that, I think we'll want to come up with a better way to show conditions on the fills chart since the text is so long. Do you have ideas for that? I could sketch something and/or we could recruit someone from the team to think through it.

[aparlato]

Not sure if you've tested this but the lines chart doesn't seem to be working for the styles that I have tried. I haven't dug into it too much so I'm not sure how big of an issue this is.

@ebrelsford good catch! This was a copy paste issue. I copied the var name from the fill chart for layers and forgot to change to lineLayers when I was cleaning up trying to remove accidental formatting I had throughout 😅 .

How do you feel about adding unit tests for some of these functions, both to more clearly state the inputs/outputs of them and to ensure that future changes here don't break them? I don't see anything in here that makes me want to completely start over but I agree that it can be a little hard to follow at times.

Makes sense to me! I went ahead and added tests for most of the functions in that file.

I think the main things that will clarify things further is a bit of a refactor to:

• move recursion out of expandLayer and handle that recursion/nesting directly in the expandMatchExpression and expandCaseExpression functions
  • this should let us remove the code to nest values that we're doing specifically for scales, but means we'd need to update our handling in expandLayer
  • I think this will mean we need a new approach for mutually exclusive values to replace this code which relies on metadata from layers.
  • this will also mean expandScaleCondtionals can worry about less and hopefully feel crisper in its handling of conditionals across zooms
• it might also help to break some of these functions out into their own files in some kind of utility directory for expand layers in here and use comments or something to better explain the structure of the intermediate values we're passing around here to eventually get our desired layers and layer values

Depending on how you want to move forward here, I'm open to either:

• doing that refactor here in this PR
• using this PR as a feature branch and refactoring in a PR to merge into this one (for easier review)
• doing as a followup

Outside of that, I think we'll want to come up with a better way to show conditions on the fills chart since the text is so long. Do you have ideas for that? I could sketch something and/or we could recruit someone from the team to think through it.

My initial thoughts in the short term would be to write them as blocks with line breaks like:

test-layer-3
  ↳ ["get","type"]=="park"
    ↳ ["get","class"]=="pitch"

test-layer-3
  ↳ ["get","type"]=="park"
    ↳ ["get","class"]=="residential"

test-layer-3
  ↳ ["get","type"]=="park"
    ↳ ["get","class"]=="fallback"

This would require some changes in our charts to account for different text heights for each layer.

As something that would require a little more effort I could see wanting to do some work to join outputs from the same layer into a single visualization, but I don't have an immediate suggestion for this.

I could sketch something and/or we could recruit someone from the team to think through it.

I would be curious what you and other folks are thinking here!

[aparlato]

Closing in favor of https://github.com/stamen/mapbox-gl-style-design-system/pull/29