Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PB-878: Filter out KML outside print extent #1055

Open
wants to merge 23 commits into
base: develop
Choose a base branch
from
Open

PB-878: Filter out KML outside print extent #1055

wants to merge 23 commits into from

Conversation

ismailsunni
Copy link
Contributor

@ismailsunni ismailsunni commented Sep 2, 2024 •
edited by github-actions bot
Loading

Test link

Tested with this file:

super-big.zip

Test link

@cypress Cypress
Copy link

cypress bot commented Sep 2, 2024 •
edited
Loading

web-mapviewer    Run #3490

Run Properties:  status check failed Failed #3490  •  git commit 2f20e4a07e: PB-878: Remove unused comment.
Project web-mapviewer
Branch Review pb-878-filter-out-kml
Run status status check failed Failed #3490
Run duration 04m 29s
Commit git commit 2f20e4a07e: PB-878: Remove unused comment.
Committer Ismail Sunni
View all properties for this run ↗︎
Test results
Tests that failed  Failures 2
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 22
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 209
View all changes introduced in this branch ↗︎

Tests for review

Failed  tests/cypress/tests-e2e/print.cy.js • 2 failed tests • e2e/electron/mobile

View Output

Test Artifacts
Testing print > Send print request with layers > should send a print request correctly to mapfishprint (with KML layer) - with import layer Test Replay Screenshots
Testing print > Send print request with layers > should send a print request correctly to mapfishprint with GPX layer Test Replay Screenshots

@ismailsunni ismailsunni marked this pull request as ready for review September 4, 2024 09:04
@ismailsunni ismailsunni requested review from ltshb and pakb and removed request for ltshb September 4, 2024 09:04
pakb
pakb reviewed Sep 4, 2024
View reviewed changes
src/api/print.api.js Show resolved Hide resolved
src/modules/map/components/openlayers/utils/usePrintAreaRenderer.composable.js Outdated
Comment on lines 121 to 135
const topLeftCoordinate = map.getCoordinateFromPixel([printRectangle[0], printRectangle[1]])
const rightBottomCoordinate = map.getCoordinateFromPixel([
printRectangle[2],
printRectangle[3],
])

store.commit('setPrintExtent', {
printExtent: [
topLeftCoordinate[0],
rightBottomCoordinate[1],
rightBottomCoordinate[0],
topLeftCoordinate[1],
],
dispatcher,
})
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if this would help, but there are a couple extent utils in the file src/utils/coordinateUtils.js that might be of use here.
There's a function to flatten an extent that is expressed as [ [x0, y0], [x1, y1] ] and the opposite to make it a double coordinate array

If you want to use them, you could move these two function in the newly created src/utils/extentUtils.js file instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's different I guess, since this code compute the coordinate-based extent from the pixel-based extent. The order is in different direction (e.g. pixel is from top-left while the coordinate is from left-bottom).

If it's ok, I will keep it in this location because it's very specific. I have added a comment to explain the order.

@ismailsunni ismailsunni force-pushed the pb-878-filter-out-kml branch 3 times, most recently from 86980db to 48d6f92 Compare September 5, 2024 06:45
@ismailsunni ismailsunni force-pushed the pb-878-filter-out-kml branch 3 times, most recently from f297b8c to c08205a Compare October 7, 2024 04:57
@ismailsunni
PB-878: Use print extent to filter out features to be printed.
bc48303
@ismailsunni
PB-878: Fix unit test by using higher zoom level so that layer can be…
997fa6a 
… seen (not filtered out).
@ismailsunni
PB-878: Fix unit test on importing router.
4df8b3e
@ismailsunni
PB-878: Add JSDoc for GeoAdminCustomizer.
f07d4c0
@ismailsunni
PB-878: Add comment and try to fix failed CI build.
dedf1a9
@ismailsunni
PB-878: Ignore simple-import-sort/imports to fix import issue.
ec63949
@ismailsunni
PB-878: Use different zoom level for failed test in CI.
d5de3e4
@ismailsunni
PB-878: Set center to fix failed test in CI.
b7a8d66
@ismailsunni
PB-878: Check center and zoom level for failed test.
296c2b7
@ismailsunni
PB-878: Also test the zoom level
fb56a9c
@ismailsunni
PB-878: Add check for layers loaded in OpenLayers.
6851bb9
@ismailsunni
PB-878: Add wait (test only, removed later).
6e44ca5
@ismailsunni
PB-878: Add render check on checkOlLayer.
a75cdc2
@ismailsunni
PB-878: Check if there are features and the extent is correct.
b4c8e60
@ismailsunni
PB-878: remove check on rendering.
aafb7ae
@ismailsunni
PB-878: Log the layer type.
252fe6a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pakb pakb pakb left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ismailsunni @pakb