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

feat: added second layer confirmation for Cancel Job #1978

Merged
merged 10 commits into from
Sep 6, 2024

Conversation

FatalBulletHit
Copy link
Contributor

@FatalBulletHit FatalBulletHit commented Aug 17, 2024

Description

This PR adds a second layer confirmation dialog for the Cancel Job button.

Related Tickets & Documents

None.

Mobile & Desktop Screenshots/Recordings

image

Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 0 0
en.json 0 0

Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 0 0
en.json 0 0

2 similar comments
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 0 0
en.json 0 0

Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 0 0
en.json 0 0

@FatalBulletHit FatalBulletHit marked this pull request as ready for review August 17, 2024 20:34
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Aug 17, 2024
@meteyou
Copy link
Member

meteyou commented Aug 18, 2024

Thx for your PR!

its fine, that you only add languages, which you are know/speak. all other languages should be translated by the translators. (missing translations are better then wrong translations, because the translators will find them faster).

you don't need docker for testing it. you just have to:

  • install npm on your pc
  • run npm ci
  • rename and modify the .env file
  • add http://localhost:8080 to the cors_domains in your moonraker.conf of your printer
  • run Mainsail in dev mode npm run serve
  • open Mainsail with http://localhost:8080

then you should be able to test it.

please change the toolbar from red to normal. the e-stop dialog is the only one that has this toolbar color. every other confirm dialog does not have this.

furthermore, i am still thinking about how useful this dialog is, because it is actually a third level confirm (pause -> cancel -> dialog). but it is optional, and there are users who deactivate the pause -> cancel, so it could be useful especially for these users.

Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 0 0
en.json 0 0

Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 0 0
en.json 0 0

@FatalBulletHit
Copy link
Contributor Author

Fixed, tested and working, thanks for the help!
Will use this with the "always display cancel print" option and hopefully not accidentally cancel prints anymore. 😄
image

@meteyou
Copy link
Member

meteyou commented Aug 18, 2024

pls also change the icon from the dialog. this one looks like a "loading issue" from the icon.

@FatalBulletHit
Copy link
Contributor Author

How about mdiStopCircleOutline? Or would you prefer another one or no icon at all?
image

Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 0 0
en.json 0 0

@meteyou meteyou added this to the v2.13.0 milestone Sep 4, 2024
Copy link
Contributor

github-actions bot commented Sep 6, 2024

Language file analysis report:

File Missing Keys Unused Keys
de.json 2 0
en.json 0 0

@meteyou meteyou self-requested a review September 6, 2024 09:07
Signed-off-by: Stefan Dej <meteyou@gmail.com>
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Sep 6, 2024
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 6, 2024
Copy link
Contributor

github-actions bot commented Sep 6, 2024

Language file analysis report:

File Missing Keys Unused Keys
de.json 2 0
en.json 0 0

@meteyou meteyou merged commit f6f3c77 into mainsail-crew:develop Sep 6, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants