[WIP] Apply to Layer Groups

[scadergit]

Overall applying to layer group went as planned. I have three questions, marked in code with ??? comments.

1. // ??? Rename mapOrGroupOrLayer Not sure readability or consistency is better here
2. // ??? could apply to first layer I might just need the weekend to think this over, but given background is often the first style layer processed, I can't simply apply to the first actual layer in the group
3. // ??? A group wouldn't have a reference I assume this code still important even when used within a group, but not sure how to reference back to the map view when you start at a layer group

[scadergit]

I included an example page named "Apply to Group"

[scadergit]

I don't want to get too far down the rabbit hole, but it seems the approach to implement mapbox background is somewhat limited (currently only color and opacity, and not pattern). When a map is passed to setBackground it relies on styling outside the canvas (css), and I was pondering if it would be more flexible for map and group if background created their own vector layer that would make it more consistent as applying to a vector layer directly--allowing quite a bit of flexibility given fill can be context2d.

A change for another pull request, but perhaps I should have processStyle create a vector layer and use it for setBackground if apply is processing a layer group.

[ahocevar]

I think for now it's fine to leave the background handling the way it is. Better to improve it in OpenLayers than working around with a vector layer here.

[scadergit]

Got one last TS error that I'm perplexed over...as it works just fine. The error says I can't use a string of "change:source" but it's literally in the TS defnition. hmm, it's actually not happy with my use of a pre-defined function as the second parameter...

            layer.on(
              'change:source',
              applyCapturedBackground(
                layer,
                mapOrGroup.get('mapbox-background')
              )
            );

which returns a listener

function applyCapturedBackground(layer, updateStyle) {
  return function (event) {
    if (!event || (event && event.oldValue === null)) {
      layer.setBackground(updateStyle.call(layer));
    }
  };
}

Interestingly, it doesn't mind this:

            layer.on(
              'change:source',
              applyCapturedBackground.call(
                this,
                layer,
                mapOrGroup.get('mapbox-background')
              )
            );

While I could celebrate and commit, any thoughts on why it ts is happy when using a call to invoke?

[ahocevar]

I'm going to commit a change to your branch that simplifies the background handling and makes the TypeScript problem (which comes from using this as context for updateStyle()) go away.

[scadergit]

Ah, that makes sense...apprecate the help and look forward to your suggestions.

[ahocevar]

@scadergit I added two commits. The first one is what I mentioned in my previous comment. It simplifies background handling by using a prerender hook to render the background with fillRect() to the first layer's canvas. According to the spec, the background is applied below all other map content. This is now also implemented correctly and can be seen in your new example. I also added a test for the case of a background applied to a layer group.

The second commit only cleans up the example and changes its name.

if you agree with my changes, please mark the pull request as "Ready for review".

[scadergit]

I like the switch to a renderBackground function, and thanks for catching that I left my Esri source in the example.

[scadergit]

@ahocevar will this be released as 8.2.x or 9.x? Is there a release schedule or as-necessary?

[ahocevar]

@scadergit as-necessary. And it will be in the next release (v9.1.0).

[ahocevar]

v9.1.0 published.

[scadergit]

@ahocevar will you cherry-pick this onto a 8.2.x release so it's picked up as a OL 6.x dep? We use VueLayers, and that project hasn't vetted OL 7 breaking changes.

[ahocevar]

@scadergit We don't do backport releases, but if you're not using the icon-offset layout property (which is the only breaking change that requires OpenLayers 7.x), you can also use v9 with OpenLayers 6.x.

[scadergit]

Had to bump npm to 8.3 so I could use overrides, but yay...

<template>
  <vl-layer-group @created="_created" />
</template>
...
<script>
  import { apply } from 'ol-mapbox-style'
  ...
  _created (group) {
    apply(group.$layer, this._styleUrlWithToken)
  }
</script>