Skip to content

Conversation

@sommerfe
Copy link
Contributor

@sommerfe sommerfe commented Nov 26, 2025

This fixes the behavior of having selected active layers and background layer and then reloading.
Also when changing the language it now replaces the currently active layers with the ones in the new language.
On topic change the new topic layers are set as active layers.

Test link

@sommerfe sommerfe self-assigned this Nov 26, 2025
@github-actions github-actions bot added the bug label Nov 26, 2025
@cypress
Copy link

cypress bot commented Nov 26, 2025

web-mapviewer    Run #6067

Run Properties:  status check failed Failed #6067  •  git commit 183a422a41: PB-2064: refactor load topic
Project web-mapviewer
Branch Review fix-pb-2064-load-layers-lang-topic-reload
Run status status check failed Failed #6067
Run duration 24m 33s
Commit git commit 183a422a41: PB-2064: refactor load topic
Committer Felix Sommer
View all properties for this run ↗︎

Test results
Tests that failed  Failures 78
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 20
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 129
View all changes introduced in this branch ↗︎

Tests for review

Failed  timeSlider.cy.ts • 3 failed tests • e2e/chrome/mobile

View Output

Test Artifacts
Cypress tests covering the time slider, its functionalities and its URL parameter > checking the time slider behavior, both on startup and during use > checks that the time slider is functional and behave correctly part 1 Test Replay Screenshots
Cypress tests covering the time slider, its functionalities and its URL parameter > checking the time slider behavior, both on startup and during use > checks that the time slider is functional and behave correctly with the timeSlider param at startup Test Replay Screenshots
Cypress tests covering the time slider, its functionalities and its URL parameter > checking the time slider behavior, both on startup and during use > behaves correctly when years are being entered in the input Test Replay Screenshots
Failed  3d/layers.cy.ts • 1 failed test • e2e/chrome/mobile

View Output

Test Artifacts
An uncaught error was detected outside of a test Test Replay Screenshots
Failed  menuTray.cy.ts • 1 failed test • e2e/chrome/mobile

View Output

Test Artifacts
An uncaught error was detected outside of a test Test Replay Screenshots
Failed  drawing.cy.ts • 1 failed test • e2e/chrome/mobile

View Output

Test Artifacts
An uncaught error was detected outside of a test Test Replay Screenshots
Failed  shareShortLink.cy.ts • 1 failed test • e2e/chrome/mobile

View Output

Test Artifacts
An uncaught error was detected outside of a test Test Replay Screenshots

The first 5 failed specs are shown, see all 26 specs in Cypress Cloud.

@sommerfe sommerfe requested review from ltkum and pakb November 26, 2025 14:17
Comment on lines 58 to 81
function updateActiveLayersFromTopics(this: TopicsStore): Layer[] {
const layersStore = useLayersStore()
// Get IDs of currently active layers
const activeLayerIds = new Set(layersStore.activeLayers.map(layer => layer.id))

// Create a map of active layers by ID for quick lookup with their time configs
const activeLayersMap = new Map(
layersStore.activeLayers.map(layer => [layer.id, layer])
)

// Filter layersToActivate to only include layers that are currently active
const layersToKeep = this.currentTopic!.layersToActivate.filter(layer =>
activeLayerIds.has(layer.id)
).map(layer => {
const activeLayer = activeLayersMap.get(layer.id)
layer.isVisible = activeLayer?.isVisible || false
// If the active layer has a time config and current time entry, update the new layer's time config
if (activeLayer?.timeConfig?.currentTimeEntry && layer.timeConfig) {
layer.timeConfig.currentTimeEntry = activeLayer.timeConfig.currentTimeEntry
}

return layer
})
return layersToKeep
Copy link
Contributor

@pakb pakb Nov 27, 2025

Choose a reason for hiding this comment

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

IMHO you could move that inside the action above, I don't really like the way we need to bind it to have access to this here. (we don't have to deal with this usually, better keep it to the minimum complexity possible)

If you want to keep a bit of "compartments" you can add the name you've given this function as comment just before the logic it contains.

Or if you really think it makes sense to have that as a dedicated function, I'd move it to the utils/ folder, and have it import the topic store with the standard useTopicsStore() instead of relying on this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i removed the function and put it all back into the main loadTopic function

Copy link
Contributor

@ltkum ltkum left a comment

Choose a reason for hiding this comment

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

In this PR : #1509 , I managed to have an adequate behavior for layers when changing languages with the following change in the setLang action : the changeLayersOnTopicChange option is set to false.

We still have an issue (changing the language open the topic section, maybe we should have an option for that too), but I don't think we need that many changes to achieve what we want.

@sommerfe
Copy link
Contributor Author

In this PR : #1509 , I managed to have an adequate behavior for layers when changing languages with the following change in the setLang action : the changeLayersOnTopicChange option is set to false.

We still have an issue (changing the language open the topic section, maybe we should have an option for that too), but I don't think we need that many changes to achieve what we want.

@ltkum yes your PR seems to fix the language changes, my PR makes it now that the topic changes also reload/set the layers

@sommerfe sommerfe force-pushed the fix-pb-2064-load-layers-lang-topic-reload branch from 160d1d2 to 183a422 Compare November 27, 2025 14:14
@sommerfe sommerfe requested review from ltkum and pakb November 27, 2025 14:15
@sommerfe sommerfe merged commit e434005 into feat-PB-1383-pinia-store Nov 27, 2025
4 of 6 checks passed
@sommerfe sommerfe deleted the fix-pb-2064-load-layers-lang-topic-reload branch November 27, 2025 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants