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.

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

PB-580: add warning when leaving drawing mode without export or share #1121

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

sommerfe
Copy link
Contributor

@sommerfe sommerfe commented Nov 8, 2024

Copy link

cypress bot commented Nov 8, 2024

web-mapviewer    Run #3802

Run Properties:  status check passed Passed #3802  •  git commit ed2c536288: PB-580: add warning when leaving drawing mode without export or share
Project web-mapviewer
Branch Review feat-pb-580-add-warning-leaving-drawing-mode
Run status status check passed Passed #3802
Run duration 04m 31s
Commit git commit ed2c536288: PB-580: add warning when leaving drawing mode without export or share
Committer Felix Sommer
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 21
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 211
View all changes introduced in this branch ↗︎

@sommerfe sommerfe requested review from ltshb, pakb and ltkum November 8, 2024 09:48
@sommerfe sommerfe marked this pull request as ready for review November 8, 2024 09:48
Copy link
Contributor

@ltshb ltshb left a comment

Choose a reason for hiding this comment

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

Implementation looks just, just an issue regarding the store import.

Otherwise I think the warning message should be improved. @hansmannj
@Luke252 a warning about non exported drawing might be a good idea, but in my opinion the warning should say why we should export it or copy the share link. We should also differentiate if the drawing has only be shared via normal link without admin link. So if the user don't click on export and/or copy admin link the warning should say that the drawing cannot be edited anymore once the webpage is reloaded !

src/modules/drawing/components/SharePopup.vue Outdated Show resolved Hide resolved
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.

Implementation looks good, but I'm against this message.
It will confuse more than help IMHO.

@sommerfe sommerfe force-pushed the feat-pb-580-add-warning-leaving-drawing-mode branch 2 times, most recently from a94be4d to c1fa52b Compare November 12, 2024 08:18
@sommerfe sommerfe force-pushed the feat-pb-580-add-warning-leaving-drawing-mode branch from c1fa52b to ed2c536 Compare November 12, 2024 15:07
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.

3 participants