Skip to content

PB-1355 : don't show drawing lost warning when we open download PDFs#1210

Merged
pakb merged 1 commit intodevelopfrom
bug-PB-1355-no-warning-after-print
Jan 20, 2025
Merged

PB-1355 : don't show drawing lost warning when we open download PDFs#1210
pakb merged 1 commit intodevelopfrom
bug-PB-1355-no-warning-after-print

Conversation

@pakb
Copy link
Contributor

@pakb pakb commented Jan 20, 2025

using window.open to trigger a download (of the print results) was triggering the beforeUnload event in MapView.vue's code, and showing the "you'll lose your drawing" warning, while we stay on the page in the end.

Also fixing some usage of the old $t to the new Composition API equivalent

Test link

@github-actions github-actions bot added the bug label Jan 20, 2025
@cypress
Copy link

cypress bot commented Jan 20, 2025

web-mapviewer    Run #4335

Run Properties:  status check passed Passed #4335  •  git commit e38e47d52b: Merge pull request #1210 from geoadmin/bug-PB-1355-no-warning-after-print
Project web-mapviewer
Branch Review develop
Run status status check passed Passed #4335
Run duration 04m 51s
Commit git commit e38e47d52b: Merge pull request #1210 from geoadmin/bug-PB-1355-no-warning-after-print
Committer Pascal Barth
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 231
View all changes introduced in this branch ↗︎

@pakb pakb requested a review from ltkum January 20, 2025 14:56
using window.open to trigger a download (of the print results) was triggering the beforeUnload event in MapView.vue's code, and showing the "you'll lose your drawing" warning, while we stay on the page in the end.

Also fixing some usage of the old $t to the new Composition API equivalent
@pakb pakb force-pushed the bug-PB-1355-no-warning-after-print branch from 1f76846 to 90947f4 Compare January 20, 2025 14:59
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 the long run, I woulld prefer a cleaner solution without using the store, but for a quick fix, I can let this go :)

Comment on lines +179 to +188
/**
* Flag set to true when the app is opening a new tab.
*
* This helps us decide to show or not show a warning popup for lost changes if the user
* closes the tab. In some cases, we are opening a new tab ourselves (print result) and
* don't want this popup to show up.
*
* @type Boolean
*/
isOpeningNewTab: false,
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 like this approach, and would prefer we play around with the closeTab event interception, but I can see why this would be simpler to do this way :) We can go with this if you're satisfied with it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for now we will live with it, I'll create a task to sort this out for the next release

@pakb pakb merged commit e38e47d into develop Jan 20, 2025
3 checks passed
@pakb pakb deleted the bug-PB-1355-no-warning-after-print branch January 20, 2025 15: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