Conversation
web-mapviewer
|
||||||||||||||||||||||||||||
| Project |
web-mapviewer
|
| Branch Review |
fix-PB-1875-wrong-center-on-startup-with-selected-layers
|
| Run status |
|
| Run duration | 06m 03s |
| Commit |
|
| Committer | Martin Künzi |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
20
|
Skipped
|
0
|
Passing
|
261
|
| View all changes introduced in this branch ↗︎ | |
1948be4 to
0001d31
Compare
pakb
reviewed
Aug 5, 2025
Contributor
pakb
left a comment
There was a problem hiding this comment.
A little test to cover this new addition?
0001d31 to
46a10b5
Compare
pakb
reviewed
Aug 8, 2025
Contributor
pakb
left a comment
There was a problem hiding this comment.
AFAIK you aren't really testing what you changed in the code here, please change the test so that you test that:
- The map centers on a (single) feature when it is loaded with a pre-selected layer feature, and without defining a center param
- The map loads the view on the center param, even if the feature is also declared alongside it
0cdd051 to
b81b170
Compare
pakb
reviewed
Aug 8, 2025
packages/mapviewer/tests/cypress/tests-e2e/featureSelection.cy.js
Outdated
Show resolved
Hide resolved
packages/mapviewer/tests/cypress/tests-e2e/featureSelection.cy.js
Outdated
Show resolved
Hide resolved
2c05c10 to
6c5622a
Compare
- Issue: In some occasions, opening a link containing both a center and pre-selected features would prioritize centering on the features rather than centering on the pre-established center. This would only occur at startup. Once the application is running, changing the URL to the one we want would set all parameters correctly - Cause: On startup, we go through the store sync loop multiple times, and at one point, we set the features after the center, and thus override the center parameter - Fix: If there is a `center` parameter in the query, we don't set the center on the features extent.
- Added tests to ensure that, whenever both center and features are present at startup, we prioritize the center - Modifying a test, ensuring the synchronisation test between store and URL tells us if reloading the app still keeps the selected features, but we'll be investigating the flaky test behaviour at a later time.
6c5622a to
50c3ef3
Compare
pakb
approved these changes
Feb 23, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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: In some occasions, opening a link containing both a center and pre-selected features would prioritize centering on the features rather than centering on the pre-established center. This would only occur at startup. Once the application is running, changing the URL to the one we want would set all parameters correctly
Cause: On startup, we go through the store sync loop multiple times, and at one point, we set the features after the center, and thus override the center parameter
Fix: If there is a
centerparameter in the query, we don't set the center on the features extent.Test link