Skip to content

Conversation

@ltkum
Copy link
Contributor

@ltkum ltkum commented Jul 31, 2025

  • 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.

Test link

@github-actions github-actions bot added the bug label Jul 31, 2025
@cypress
Copy link

cypress bot commented Jul 31, 2025

web-mapviewer    Run #5564

Run Properties:  status check passed Passed #5564  •  git commit 6c5622a334: PB-1875: add tests
Project web-mapviewer
Branch Review fix-PB-1875-wrong-center-on-startup-with-selected-layers
Run status status check passed Passed #5564
Run duration 05m 06s
Commit git commit 6c5622a334: PB-1875: add tests
Committer Martin Künzi
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
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 261
View all changes introduced in this branch ↗︎

@ltkum ltkum requested a review from pakb August 4, 2025 08:55
@pakb pakb force-pushed the fix-PB-1875-wrong-center-on-startup-with-selected-layers branch from 1948be4 to 0001d31 Compare August 5, 2025 10:22
Copy link
Contributor

@pakb pakb left a comment

Choose a reason for hiding this comment

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

A little test to cover this new addition?

- 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.
@ltkum ltkum force-pushed the fix-PB-1875-wrong-center-on-startup-with-selected-layers branch from 0001d31 to 46a10b5 Compare August 7, 2025 07:36
Copy link
Contributor

@pakb pakb left a comment

Choose a reason for hiding this comment

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

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

@ltkum ltkum force-pushed the fix-PB-1875-wrong-center-on-startup-with-selected-layers branch 2 times, most recently from 0cdd051 to b81b170 Compare August 8, 2025 12:26
@ltkum ltkum requested a review from pakb August 8, 2025 12:33
@ltkum ltkum force-pushed the fix-PB-1875-wrong-center-on-startup-with-selected-layers branch 2 times, most recently from bb98f2c to 2c05c10 Compare August 11, 2025 12:33
- 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.
@ltkum ltkum force-pushed the fix-PB-1875-wrong-center-on-startup-with-selected-layers branch from 2c05c10 to 6c5622a Compare August 11, 2025 12:56
@ltkum ltkum requested a review from pakb August 11, 2025 13:13
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.

2 participants