feat: adds wms styling using geostyler #1771
Conversation
There was a problem hiding this comment.
Thank you very much for this addition @FritzHoing!
Overall the functionality is already awesome and is working for a defined context obviously. But for the production scenarios I think we need to adjust some architectural assumptions before we can merge this:
- Guessing the layer's map server by its path is very prone to errors since it might not match non-default scenarios. Probably it's the easiest way to just make the style-function configurable by the layer, e.g. by introducing a property in the layer configuration like
styleable(to match the existing keys likesearchableoreditable). Astyleablelayer might then be defined as layer coming from GeoServer (this is open for discussion of course) or we might add an additional config likestyleConfigcontaining the URL where to get the default style from. - Using the internal GeoServer REST interfaces to get the style is generally fine, but in default scenarios this is accessible for admin users only. Default app users won't be able to access them. An easy approach would be to introduce a new proxy interface for the style URLS in the SHOGun backend providing access to them. An alternate approach could be using the
WMS GetStylesrequest, but I'm not sure if this is actually supported as expected by GeoServer.
But again, I really like how this is looking so far and it would be really awesome to have this included!
| export const checkIfGeoserverLayer = (layerSource: Source | undefined): boolean => { | ||
| const url = getLayerUrl(layerSource); | ||
| return url ? url.includes('/geoserver/') : false; | ||
| }; |
There was a problem hiding this comment.
This check is quite unreliable since there a GeoServers out there that aren't served from the default path.
| return false; | ||
| } | ||
| let url: string | undefined; | ||
| if (layerSource instanceof TileWMS || layerSource instanceof ImageWMS) { |
There was a problem hiding this comment.
Very minor, but we might want to reuse the isWmsLayer typeguard instead.
| if (layerSource instanceof TileWMS || layerSource instanceof ImageWMS) { | ||
| url = layerSource instanceof TileWMS ? layerSource.getUrls()?.[0] : layerSource.getUrl(); | ||
| } | ||
| return url || false; |
There was a problem hiding this comment.
I would assume that it's very uncommon to return false here. If no url can be found, we should just return undefined or null.
| if (!layerSource) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
This check can be omitted if we would make the input argument mandatory.
| client: SHOGunAPIClient | null | ||
| ): Promise<string | undefined> => { | ||
| const cleanLayerUrl = layerUrl.replace('/ows?', ''); | ||
| const url = `${cleanLayerUrl}/rest/layers/${layerWorkspace}:${layerName}.json`; |
There was a problem hiding this comment.
This URL (and also the one used in fetchSld()) is usually accessible for users holding the role ROLE_ADMINISTRATOR in GeoServer only.
| source.updateParams({ | ||
| SLD_BODY: sld, | ||
| STYLES: style.name | ||
| }); |
There was a problem hiding this comment.
Just wondering, but what happens if:
- a layer has been opened in the drawer,
- got a new style via
SLD_BODY, - the drawer has been closed and reopened with the same layer.
What style would be shown in the styler UI? The default one or the updated one?
|
|
||
| const { | ||
| t | ||
| } = useTranslation(); | ||
|
|
||
| useEffect(() => { | ||
|
|
| @@ -293,6 +316,18 @@ export const LayerTreeContextMenu: React.FC<LayerTreeContextMenuProps> = ({ | |||
| }); | |||
| } | |||
|
|
|||
| if (isGeoserverLayer) { | |||
| dropdownMenuItems.push({ | |||
| label: 'Geostyler', | |||
There was a problem hiding this comment.
The label should be translatable and the current one might be confusing to users. We might want to replace it with something like Layer style.
| dropdownMenuItems.push({ | ||
| label: t('LayerTreeContextMenu.layerDetails'), | ||
| key: 'layerDetails' | ||
| }); |
There was a problem hiding this comment.
Why is this needed? This will be added further down (conditionally).
| dispatch(setStylingDrawerVisibility(true)); | ||
| dispatch(setStylingDrawerLayerUid(getUid(layer))); |
There was a problem hiding this comment.
Should we show the styler if there is no layer present?
This MR adds wms styling via the geostyler using the SLD_BODY.
Please note:
The geostyler has a css issue in the current version, that needs to be fixed. The canvas of the style is dislocated.