Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Layer legends #303

Merged
merged 13 commits into from
Oct 31, 2022
Merged

Layer legends #303

merged 13 commits into from
Oct 31, 2022

Conversation

fschmenger
Copy link
Collaborator

This pull request implements layer legends and builds on previous works by @JakobMiksch (closes #259). After some discussion with my fellow colleagues we decided to choose the layer list module as an anchor for the legend images such that legends can be shown by toggling an expansion button.
Wegue_legend_example

Basics:
To activate legends, set the new boolean configuration attribute legend for your map layer to true. This should work out of the box for (image and tile) WMS layers by utilizing the WMS GetLegendGraphic request.

Legend Params:
For request options, Wegue internally manages the SCALE and LANGUAGE parameter.
If you want to specify additional request params, you can configure the map layer attribute legendOptions. This is an object containing the params passed into the getLegendUrl function, e.g.

"mapLayers": [
    {
          "type": "TILEWMS",
          "lid": "my-layer",
          /* ... */
          "legend": true
          "legendOptions": {
              "transparent": true,
              "width": 14 
         }
    }
]

I also introduced the possibility to specify legend params on an application wide scope: You can configure the top level configuration property legend, describing the same type of object as legendOptions. This can be useful to supply default params like fonts, font-sizes and other stuff common to all legends, e.g.

{
  /* ... wegue top-level-configuration ... */
  "legend": {
      "transparent": true
   }
}

The legend params of the individual layers are then merged with the application wide option, while the layer option takes precedence.

Custom URLs:
In order to support custom legend URLs (e.g. for non WMS layers, static layer images etc.), the map layer configuration supports a 3rd parameter legendUrl. This is an argument of type string, which may contain format placeholders for the various legendOptions. Placeholders may appear anywhere in the URL. Those are typically useful to support a localized or scale dependent custom legend, but work for every other of your supplied options e.g.

"mapLayers": [
    {
         "type": "TILEWMS",
         "lid": "my-layer",
         /* .. */.
         "legend": true
         "legendOptions": {
              "transparent": true,
              "width": 14 
         },
        "legendUrl": "https://somewhere//my_legend_image_${LANGUAGE}_${WIDTH}.png?transparent=${TRANSPARENT}&&scale=${SCALE}"
    }
]

Disabling legends in layer list
Since anchoring the legends in the layer list is probably an opinionated decision I added a boolean attribute showLegends to the layer list module configuration (defaults to true).
If you want to provide your custom legend implementation, this can now be done much easier by reusing the wgu-layerlegendimage template. It should be straightforward to configure and already takes care of implementing all the configuration options described above.

Example configurations
For the app-starters I enabled legends for the ahocevar-wms layer.

@fschmenger
Copy link
Collaborator Author

fschmenger commented Oct 24, 2022

I would kindly ask you to review it first, before I provide some unit tests and a documentation.

Some of the points to think about:

  • Are we good with the naming and structuring of the config properties? e.g. I introduced legend for top-level configuration which basically corresponds to legendOptions on the layer level.
  • We dont have format placeholders in Wegue yet. Using the ${VAR_NAME} syntax was more like an ad-hoc decision. Later on I realized that this may lead to lint errors when assigning a format URL from code. So we might be better off changing it to %VAR_NAME% syntax ?!
  • The dark mode dilemma: There seems to be no good generic way of supplying colorization options for legends. I therefore decided to not request images as transparent, otherwise the foreground color might not be readable. This may currently result in something like this on dark mode:
    Wegue_legends_dark_mode_problem.
    I can only leave it to the app developer to deal with it.
    E.g. the workaround I use in my app for the GeoServer based layers is specifying a gray tone as foreground color and then providing the transparent flag. This can be done via GeoServers LEGEND_OPTIONS param:
"legend": {
    "legend_options": "fontColor:666666",
    "transparent": true
 }

src/components/layerlist/LayerLegendImage.vue Outdated Show resolved Hide resolved
}
}
};
</script>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add newline at end of file

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in 5e2e02d

src/components/layerlist/LayerListItem.vue Show resolved Hide resolved
@@ -0,0 +1,62 @@
<template>
<!-- Remarks:
As we need none of the responsive functionality of v-img, use a simple
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
As we need none of the responsive functionality of v-img, use a simple
As we need none of the responsive functionality of v-img, we use a simple

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in 5e2e02d

src/components/layerlist/LayerLegendImage.vue Show resolved Hide resolved
@JakobMiksch
Copy link
Collaborator

Some of the points to think about:

* Are we good with the naming and structuring of the config properties? e.g. I introduced `legend` for top-level configuration which basically corresponds to `legendOptions` on the layer level.

fine for me.

* We dont have format placeholders in Wegue yet. Using the `${VAR_NAME}` syntax was more like an ad-hoc decision. Later on I realized that this may lead to lint errors when assigning a format URL from code. So we might be better off changing it to `%VAR_NAME%` syntax ?!

I do not have a strong opinion here. The pattern {{my_variable}} is used very often - also within Vue. Maybe we could use that here as well.

@JakobMiksch
Copy link
Collaborator

JakobMiksch commented Oct 26, 2022

Thanks @fschmenger for this useful feature, I like it! Code quality is great like always 👍
So far I could only try it with a custom layer, because the default configs did not show any image:

image

My custom config:

{
      "type": "IMAGEWMS",
      "lid": "mgsm-places",
      "ratio": 1.5,
      "format": "image/png",
      "layers": "mgsm-world:ne_10m_populated_places",
      "url": "https://services.meggsimum.de/geoserver/mgsm-world/wms?",
      "transparent": true,
      "projection": "EPSG:3857",
      "isBaseLayer": false,
      "visible": true,
      "displayInLayerList": true,
      "legend": true
    },

Maybe the the layer from ahocevar was only temporarily disabled. If not, could you add a working example to the config(s), ideally also for a custom image?

Do I understand correctly that you designed src/components/layerlist/LayerListItem.vue to be used in custom modules as well?

@fschmenger
Copy link
Collaborator Author

Hi Jakob, thanks a lot for the review!

Regarding the ahocevar layers:
I had some trouble too, as those were temporarily not working. At some point they did and I could figure out, that the TILEWMS was able to produce a legend image, while the IMAGEWMS doesnt have any legend configured. This is how it should look like
Wegue_legends_ahocevar

For a custom URL I could put a static image into static/icons (maybe even 2 to demonstrate the legend localization feature), however that would increase the package size. But I`m not sure which layer should be suitable to do this for?!

Regarding the LayerListItem: My initial motivation to factor those out was to manage an open state per layer to stop them from requesting the legend image, when the extender is closed - see this commit. This would otherwise get a bit messy. It seems to be a good side effect, that you can now use the LayerListItem as a component for a custom LayerList. I also intended to make the LayerPreviewImage component reusable.

Meanwhile I finished the unit tests and documentation. I will now address the formatting issues and finalize it.

@fschmenger
Copy link
Collaborator Author

fschmenger commented Oct 27, 2022

OK, I updated the docs, unit tests and fixed the formatting.

Regarding the placeholders
For now I decided to leave in the ${VAR_NAME} placeholder syntax until we make up our mind. This is probably the most common for named parameters. However the downside/workaround when having to assign it from code can be seen here:

/* eslint-disable no-template-curly-in-string */
const layer = new TileLayer({
source: new OSM(),
legendUrl: 'http://my-image.png?transparent=${TRANSPARENT}&width=${WIDTH}&SCALE=${SCALE}&language=${LANGUAGE}',
legendOptions: {
transparent: true,
width: 14
}
});
/* eslint-enable no-template-curly-in-string */
{{VAR_NAME}} is very specific to vue templates but definitely a possibility. Another option would be to use {VAR_NAME} (which is more common for positional placeholders) or %VAR_NAME% like e.g. in SQL.
Sooner or later we might have to introduce more placeholders in the configuration, so this should be future proof. Therefore opinions on this are strongly appreciated (@chrismayer).

@JakobMiksch
Copy link
Collaborator

For a custom URL I could put a static image into static/icons (maybe even 2 to demonstrate the legend localization feature), however that would increase the package size. But I`m not sure which layer should be suitable to do this for?!

we can just add a dummy image as static legend, maybe to the shop layer (OSM). But this is not that important. Otherwise we can include a image from a site like https://dummyimage.com/

Copy link
Collaborator

@JakobMiksch JakobMiksch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it and it works well, code looks also fine. Can be merged.

A non-blocking question:
When I run the tests locally I get the errors below. It is strange that only the new tests fail. But obviously in the CI they are running. Do you have an idea why it could be like this?

I use Node version 16.

HeadlessChrome 105.0.5195 (Linux 0.0.0): Executed 274 of 274 (5 FAILED) (3.14 secs / 2.41 secs)
TOTAL: 5 FAILED, 269 SUCCESS


1) has correct previewURL for static layer image
     bglayerswitcher/LayerPreviewImage.vue computed properties
     AssertionError: expected 'https://b.tile.openstreetmap.org/2/2/…' to equal 'http://my-image.png'
    at Context.<anonymous> (webpack:///test/unit/specs/components/bglayerswitcher/LayerPreviewImage.spec.js:93:32 <- index.js:201147:32)

2) has correct legendURL for static legend URL
     layerlist/LayerLegendImage.vue computed properties
     AssertionError: expected undefined to equal 'http://my-image.png'
    at Context.<anonymous> (webpack:///test/unit/specs/components/layerlist/LayerLegendImage.spec.js:100:31 <- index.js:201804:31)

3) has correct legendURL for legend format URL
     layerlist/LayerLegendImage.vue computed properties
     AssertionError: expected undefined to equal 'http://my-image.png?transparent=true&…'
    at Context.<anonymous> (webpack:///test/unit/specs/components/layerlist/LayerLegendImage.spec.js:113:31 <- index.js:201818:31)

4) has correct legendURL for WMS
     layerlist/LayerLegendImage.vue computed properties
     AssertionError: expected undefined to equal 'https://ahocevar.com/geoserver/wms?SE…'
    at Context.<anonymous> (webpack:///test/unit/specs/components/layerlist/LayerLegendImage.spec.js:118:31 <- index.js:201824:31)

5) legendURL supports localization and scale
     layerlist/LayerLegendImage.vue computed properties
     AssertionError: expected undefined to equal 'https://ahocevar.com/geoserver/wms?SE…'
    at Context.<anonymous> (webpack:///test/unit/specs/components/layerlist/LayerLegendImage.spec.js:125:31 <- index.js:201830:31)



@fschmenger
Copy link
Collaborator Author

Hi @JakobMiksch,

I suggest we provide an example with a static legend URL in a separate PR. Do you have a meaningful image, that we could use for the shop layer? Maybe 2 images with localized texts, so we can demonstrate the placeholder with a {{language}} parameter ?!

Regarding the unit tests: It's not only the new tests failing but also an older test regarding previewURL.
According to Karma issue tracker 3571 and 3588 there is a compatibility issue with newer Node versions.

..because Karma Runner is not compatible with Node Version 15. You have to downgrade your node version below or equal to 14.

Possibly you could try to make those tests asynchronous and wrap the assertion by a vm.$nextTick ?

Anyway, thanks a lot for the review, will merge now!

@fschmenger fschmenger merged commit 6f444a8 into wegue-oss:master Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for legends
2 participants