PB-1383: initial values not included in external layers #1509
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pakb
reviewed
Nov 18, 2025
Contributor
pakb
left a comment
pakb
left a comment
There was a problem hiding this comment.
you broke a unit test, please fix it
ltkum
force-pushed
the
fix-PB-1383-no-external-layers-params
branch
3 times, most recently
from
8d8a697 to
6c7fe50
Compare
pakb
reviewed
Nov 27, 2025
ltkum
force-pushed
the
fix-PB-1383-no-external-layers-params
branch
2 times, most recently
from
7c9e72d to
a1df253
Compare
ltkum
force-pushed
the
fix-PB-1383-no-external-layers-params
branch
2 times, most recently
from
7c4ddd7 to
a287cb9
Compare
pakb
reviewed
Dec 1, 2025
| try { | ||
| layerOptions = optionsFromCapabilities(capabilities, {layer: attributes.id, projection: outputProjection.epsg}) | ||
| } catch (exception) { | ||
| const msg = `issue while getting the options from capabilities : ${exception?.toString()}` |
Contributor
There was a problem hiding this comment.
no need to create a variable as you are only using it once
Contributor
Author
There was a problem hiding this comment.
the optionsFromCapabilities can create exceptions (for example: when getAxisOrientation() returns null, which happened in some tests). Without the try catch, layers which are defined in the capabilities won't load, and won't be shown.
If the axisOrientation is a mandatory part of any wmts layer, then we can revert this change and fix our fixtures instead.
Contributor
Author
There was a problem hiding this comment.
so, turns out the issue was linked to a lack of registration of the swiss projection. We can either keep a try catch around the options to have some more logging, or consider the existing logging is sufficient enough.
packages/viewer/src/store/plugins/storeSync/params/bgLayer.param.ts
Outdated
Show resolved
Hide resolved
sommerfe
force-pushed
the
feat-PB-1383-pinia-store
branch
from
a1cccff to
9e6d8d7
Compare
ltkum
force-pushed
the
fix-PB-1383-no-external-layers-params
branch
2 times, most recently
from
b7a7534 to
3228bdd
Compare
ltkum
force-pushed
the
fix-PB-1383-no-external-layers-params
branch
3 times, most recently
from
20fb715 to
ac5fdc4
Compare
pakb
approved these changes
Dec 10, 2025
Issue: When loading an external WMS layer, the initial values (like the visibility or opacity) were not passed to the layer, which meant that on startup, we would automatically lose their initial state. Fix: We add the initial values as parameters to the `makeExternalWMSLayer` function.
Issue: When starting the app for the first time, the URL sync plugin would start being active before the topics were fully set, causing (mainly) the background layer parameter to be ignored and replaced with the topic's default one Fix: The `ConfigLoaded` state now wait for the background layer to be set before being fulfilled.
small modifications to the loading of topics, there was a case where it would refuse to set up the default background on startup
…d crash and stops a layer from loading
- some more tests coverage corrections - removal of some useless comments - adding some comments to tell people why some changes have happened
- some more tests coverage corrections - removal of some useless comments - adding some comments to tell people why some changes have happened PB-1383: adding proj4 registration to layers so openlayers supports layers with only swiss projections - Issue with the drag and drop reordering test: when it was called alone, it would pass, when it would be called after the one reordering layers with the buttons, it would fail because a menu popup would show which was not planned. Rather than place a "force" which could pass even if the element is somehow obscured for a non legitimate reason, I switched the order of the tests - Issue with the call of layer capabilities: We've changed the number of expected calls in the tests, as the capabilities are called once per layer, not once in total. If we need to rework the capabilities handling, we'll have to change this test once again.
ltkum
force-pushed
the
fix-PB-1383-no-external-layers-params
branch
from
ac5fdc4 to
ecda228
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue: When loading an external WMS layer, the initial values (like the visibility or opacity) were not passed to the layer, which meant that on startup, we would automatically lose their initial state.
Fix: We add the initial values as parameters to the
makeExternalWMSLayerfunction.Test link