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

🎉 Add prompt before exiting the program #271

Merged
merged 4 commits into from
Feb 16, 2022

Conversation

FabienRoger
Copy link
Contributor

Fixes #216

Copy link
Member

@MathisFederico MathisFederico left a comment

Choose a reason for hiding this comment

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

A few changes and we are good to go !
Thanks for the bugfix !

Comment on lines 463 to 479
quit_msg = "Exit without saving?"
msgbox = QMessageBox(self)
msgbox.setText(quit_msg)
msgbox.setWindowTitle("Exit?")
msgbox.addButton(QMessageBox.Yes)
msgbox.addButton(QMessageBox.No)
cb = QCheckBox("Never show this again")
msgbox.setCheckBox(cb)
msgbox.exec()

if msgbox.checkBox().checkState() == Qt.CheckState.Checked:
self.never_show_exit_prompt = True

if msgbox.result() == int(str(QMessageBox.No)):
event.ignore()
return

Copy link
Member

Choose a reason for hiding this comment

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

This should replace self.maybeSave

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel like maybeSave is not exactly what we want. First, it doesn't have the checkbox, (and I really don't want to have the exit prompt when trying out the new features I am coding). Second, it isn't clear what should be saved, and how, when there are multiple open widgets, especially when some of them have never been saved.

I believe maybeSave is an outdated function that only worked when it was developed, when there was only one single widget.

Copy link
Member

Choose a reason for hiding this comment

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

True

Copy link
Member

Choose a reason for hiding this comment

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

But we should delete it and not comment it then !

pyflow/graphics/window.py Show resolved Hide resolved
Copy link
Member

@MathisFederico MathisFederico left a comment

Choose a reason for hiding this comment

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

Just remove maybeSave and we are good to go!

Comment on lines 503 to 504
# def maybeSave(self) -> bool:
# """Ask for save and returns if the file should be closed.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# def maybeSave(self) -> bool:
# """Ask for save and returns if the file should be closed.

We should delete it I guess

Copy link
Member

@MathisFederico MathisFederico left a comment

Choose a reason for hiding this comment

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

Nice ! Thanks for the bugfix !

@MathisFederico MathisFederico merged commit 584b84c into dev Feb 16, 2022
@MathisFederico MathisFederico deleted the feature/exit-without-saving branch February 16, 2022 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exiting Pyflow should prompt "Exit without saving?"
2 participants