From aac2d0edfc5eabf4aba521ca228994c717a01867 Mon Sep 17 00:00:00 2001 From: Rebecca Breu Date: Mon, 20 May 2024 19:31:06 +0200 Subject: [PATCH] Add confirmation dialog when discarding an unsaved file --- .github/workflows/pytest.yml | 2 +- CHANGELOG.rst | 3 + beeref/actions/actions.py | 2 +- beeref/actions/mixin.py | 2 +- beeref/config/settings.py | 4 ++ beeref/fileio/errors.py | 2 +- beeref/view.py | 40 ++++++++++- beeref/widgets/settings.py | 80 +++++++++++++++------- pyproject.toml | 4 +- setup.py | 3 + tests/actions/test_mixin.py | 2 +- tests/test_view.py | 117 ++++++++++++++++++++++++++++++++- tests/widgets/test_settings.py | 45 +++++++++++-- 13 files changed, 270 insertions(+), 36 deletions(-) create mode 100644 setup.py diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index c1e4135..c2f7f91 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -8,7 +8,7 @@ jobs: strategy: matrix: python-version: ["3.9", "3.10", "3.11", "3.12"] - pyqt-version: ["6.5.3", "6.7.0"] + pyqt-version: ["6.7.0"] steps: - uses: actions/checkout@v4 - name: Set up Python diff --git a/CHANGELOG.rst b/CHANGELOG.rst index bc2756b..06aa006 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -10,6 +10,9 @@ Added QT_IMAGEIO_MAXALLOC * Display error messages when images can't be loaded from bee files * Added option to export all images from scene (File -> Export Images) +* Added a confirmation dialog when attempting to close unsaved files. + The confirmation dialog can be disalbed in: + Settings -> Miscellaneous -> Confirm when closing an unsaved file Fixed diff --git a/beeref/actions/actions.py b/beeref/actions/actions.py index 5dab777..c45fb64 100644 --- a/beeref/actions/actions.py +++ b/beeref/actions/actions.py @@ -312,7 +312,7 @@ def get_default_shortcut(self, index): id='new_scene', text='&New Scene', shortcuts=['Ctrl+N'], - callback='clear_scene', + callback='on_action_new_scene', ), Action( id='fit_scene', diff --git a/beeref/actions/mixin.py b/beeref/actions/mixin.py index 7e1e22e..622a3ae 100644 --- a/beeref/actions/mixin.py +++ b/beeref/actions/mixin.py @@ -123,7 +123,7 @@ def _build_recent_files(self, menu=None): qaction = QtGui.QAction(os.path.basename(filename), self) qaction.setShortcuts(action.get_shortcuts()) qaction.triggered.connect( - partial(self.open_from_file, filename)) + partial(self.on_action_open_recent_file, filename)) self.addAction(qaction) action.qaction = qaction self._recent_files_submenu.addAction(qaction) diff --git a/beeref/config/settings.py b/beeref/config/settings.py index 88fe3a4..917bdde 100644 --- a/beeref/config/settings.py +++ b/beeref/config/settings.py @@ -109,6 +109,10 @@ class BeeSettingsEvents(QtCore.QObject): class BeeSettings(QtCore.QSettings): FIELDS = { + 'Save/confirm_close_unsaved': { + 'default': True, + 'cast': bool, + }, 'Items/image_storage_format': { 'default': 'best', 'validate': lambda x: x in ('png', 'jpg', 'best'), diff --git a/beeref/fileio/errors.py b/beeref/fileio/errors.py index 04235d7..55e7dff 100644 --- a/beeref/fileio/errors.py +++ b/beeref/fileio/errors.py @@ -16,7 +16,7 @@ IMG_LOADING_ERROR_MSG = ( 'Unknown format or too big?\n' - 'Check Settings -> Miscellaneous -> Maximum Image Size') + 'Check Settings -> Images & Items -> Maximum Image Size') class BeeFileIOError(Exception): diff --git a/beeref/view.py b/beeref/view.py index dcfb8a0..63cb38b 100644 --- a/beeref/view.py +++ b/beeref/view.py @@ -200,6 +200,26 @@ def fit_rect(self, rect, toggle_item=None): self.fitInView(rect, Qt.AspectRatioMode.KeepAspectRatio) logger.trace('Fit view done') + def get_confirmation_unsaved_changes(self, msg): + confirm = self.settings.valueOrDefault('Save/confirm_close_unsaved') + if confirm and not self.undo_stack.isClean(): + answer = QtWidgets.QMessageBox.question( + self, + 'Discard unsaved changes?', + msg, + QtWidgets.QMessageBox.StandardButton.Yes | + QtWidgets.QMessageBox.StandardButton.Cancel) + return answer == QtWidgets.QMessageBox.StandardButton.Yes + + return True + + def on_action_new_scene(self): + confirm = self.get_confirmation_unsaved_changes( + 'There are unsaved changes. ' + 'Are you sure you want to open a new scene?') + if confirm: + self.clear_scene() + def on_action_fit_scene(self): self.fit_rect(self.scene.itemsBoundingRect()) @@ -388,6 +408,13 @@ def on_loading_finished(self, filename, errors): self.scene.add_queued_items() self.on_action_fit_scene() + def on_action_open_recent_file(self, filename): + confirm = self.get_confirmation_unsaved_changes( + 'There are unsaved changes. ' + 'Are you sure you want to open a new scene?') + if confirm: + self.open_from_file(filename) + def open_from_file(self, filename): logger.info(f'Opening file {filename}') self.clear_scene() @@ -402,6 +429,12 @@ def open_from_file(self, filename): self.worker.start() def on_action_open(self): + confirm = self.get_confirmation_unsaved_changes( + 'There are unsaved changes. ' + 'Are you sure you want to open a new scene?') + if not confirm: + return + self.cancel_active_modes() filename, f = QtWidgets.QFileDialog.getOpenFileName( parent=self, @@ -528,8 +561,11 @@ def on_export_images_file_exists(self, filename): self.worker.start() def on_action_quit(self): - logger.info('User quit. Exiting...') - self.app.quit() + confirm = self.get_confirmation_unsaved_changes( + 'There are unsaved changes. Are you sure you want to quit?') + if confirm: + logger.info('User quit. Exiting...') + self.app.quit() def on_action_settings(self): widgets.settings.SettingsDialog(self) diff --git a/beeref/widgets/settings.py b/beeref/widgets/settings.py index 449c639..b8c2b3f 100644 --- a/beeref/widgets/settings.py +++ b/beeref/widgets/settings.py @@ -17,6 +17,7 @@ import logging from PyQt6 import QtWidgets +from PyQt6.QtCore import Qt from beeref import constants from beeref.config import BeeSettings, settings_events @@ -26,6 +27,9 @@ class GroupBase(QtWidgets.QGroupBox): + TITLE = None + HELPTEXT = None + KEY = None def __init__(self): super().__init__() @@ -50,16 +54,24 @@ def on_value_changed(self, value): if self.ignore_value_changed: return + value = self.convert_value_from_qt(value) if value != self.settings.valueOrDefault(self.KEY): logger.debug(f'Setting {self.KEY} changed to: {value}') self.settings.setValue(self.KEY, value) self.update_title() + def convert_value_from_qt(self, value): + return value + + def on_restore_defaults(self): + new_value = self.settings.valueOrDefault(self.KEY) + self.ignore_value_changed = True + self.set_value(new_value) + self.ignore_value_changed = False + self.update_title() + class RadioGroup(GroupBase): - TITLE = None - HELPTEXT = None - KEY = None OPTIONS = None def __init__(self): @@ -79,19 +91,12 @@ def __init__(self): self.ignore_value_changed = False self.layout.addStretch(100) - def on_restore_defaults(self): - new_value = self.settings.valueOrDefault(self.KEY) - self.ignore_value_changed = True - for value, btn in self.buttons.items(): - btn.setChecked(value == new_value) - self.ignore_value_changed = False - self.update_title() + def set_value(self, value): + for old_value, btn in self.buttons.items(): + btn.setChecked(old_value == value) class IntegerGroup(GroupBase): - TITLE = None - HELPTEXT = None - KEY = None MIN = None MAX = None @@ -99,18 +104,33 @@ def __init__(self): super().__init__() self.input = QtWidgets.QSpinBox() self.input.setRange(self.MIN, self.MAX) - self.input.setValue(self.settings.valueOrDefault(self.KEY)) + self.set_value(self.settings.valueOrDefault(self.KEY)) self.input.valueChanged.connect(self.on_value_changed) self.layout.addWidget(self.input) self.layout.addStretch(100) self.ignore_value_changed = False - def on_restore_defaults(self): - new_value = self.settings.valueOrDefault(self.KEY) - self.ignore_value_changed = True - self.input.setValue(new_value) + def set_value(self, value): + self.input.setValue(value) + + +class SingleCheckboxGroup(GroupBase): + LABEL = None + + def __init__(self): + super().__init__() + self.input = QtWidgets.QCheckBox(self.LABEL) + self.set_value(self.settings.valueOrDefault(self.KEY)) + self.input.checkStateChanged.connect(self.on_value_changed) + self.layout.addWidget(self.input) + self.layout.addStretch(100) self.ignore_value_changed = False - self.update_title() + + def set_value(self, value): + self.input.setChecked(value) + + def convert_value_from_qt(self, value): + return value == Qt.CheckState.Checked class ImageStorageFormatWidget(RadioGroup): @@ -144,6 +164,15 @@ class AllocationLimitWidget(IntegerGroup): MAX = 10000 +class ConfirmCloseUnsavedWidget(SingleCheckboxGroup): + TITLE = 'Confirm when closing an unsaved file:' + HELPTEXT = ( + 'When about to close an unsaved file, should BeeRef ask for ' + 'confirmation?') + LABEL = 'Confirm when closing' + KEY = 'Save/confirm_close_unsaved' + + class SettingsDialog(QtWidgets.QDialog): def __init__(self, parent): super().__init__(parent) @@ -154,11 +183,18 @@ def __init__(self, parent): misc = QtWidgets.QWidget() misc_layout = QtWidgets.QGridLayout() misc.setLayout(misc_layout) - misc_layout.addWidget(ImageStorageFormatWidget(), 0, 0) - misc_layout.addWidget(ArrangeGapWidget(), 0, 1) - misc_layout.addWidget(AllocationLimitWidget(), 1, 0) + misc_layout.addWidget(ConfirmCloseUnsavedWidget(), 0, 0) tabs.addTab(misc, '&Miscellaneous') + # Images & Items + items = QtWidgets.QWidget() + items_layout = QtWidgets.QGridLayout() + items.setLayout(items_layout) + items_layout.addWidget(ImageStorageFormatWidget(), 0, 0) + items_layout.addWidget(ArrangeGapWidget(), 0, 1) + items_layout.addWidget(AllocationLimitWidget(), 1, 0) + tabs.addTab(items, '&Images && Items') + layout = QtWidgets.QVBoxLayout() self.setLayout(layout) layout.addWidget(tabs) diff --git a/pyproject.toml b/pyproject.toml index e1cb30e..f60a2af 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,8 +21,8 @@ requires-python = ">=3.9,<3.13" dependencies = [ "exif>=1.3.5,<=1.6.0", "lxml==5.1.0", - "pyQt6-Qt6>=6.5.3,<=6.7.0", - "pyQt6>=6.5.0,<=6.7.0", + "pyQt6-Qt6>=6.7.0,<=6.7.0", + "pyQt6>=6.7.0,<=6.7.0", "rectangle-packer>=2.0.1,<=2.0.2", ] diff --git a/setup.py b/setup.py new file mode 100644 index 0000000..6068493 --- /dev/null +++ b/setup.py @@ -0,0 +1,3 @@ +from setuptools import setup + +setup() diff --git a/tests/actions/test_mixin.py b/tests/actions/test_mixin.py index ef400b6..d6e9ed7 100644 --- a/tests/actions/test_mixin.py +++ b/tests/actions/test_mixin.py @@ -20,7 +20,7 @@ def on_foo(self): def on_bar(self): pass - def open_from_file(self): + def on_action_open_recent_file(self, filename): pass diff --git a/tests/test_view.py b/tests/test_view.py index fadf4de..913ed9b 100644 --- a/tests/test_view.py +++ b/tests/test_view.py @@ -7,7 +7,7 @@ from PyQt6 import QtCore, QtGui, QtWidgets from PyQt6.QtCore import Qt -from beeref import widgets +from beeref import commands, widgets from beeref.actions import actions from beeref.config import logfile_name from beeref.items import BeePixmapItem, BeeTextItem @@ -153,6 +153,88 @@ def test_fit_rect_toggle_when_previous(center_mock, fit_mock, view): assert view.get_scale() == 2 +@patch('PyQt6.QtWidgets.QMessageBox.question') +def test_get_confirmation_unsaved_changes_when_no_changes( + dlg_mock, settings, view, item): + view.scene.addItem(item) + assert view.undo_stack.isClean() + assert view.get_confirmation_unsaved_changes('foo') is True + dlg_mock.assert_not_called() + + +@patch('PyQt6.QtWidgets.QMessageBox.question') +def test_get_confirmation_unsaved_changes_when_changes_confirmation_disabled( + dlg_mock, settings, view, item): + settings.setValue('Save/confirm_close_unsaved', False) + view.undo_stack.push( + commands.InsertItems(view.scene, [item], QtCore.QPointF(0, 0))) + assert view.undo_stack.isClean() is False + assert view.get_confirmation_unsaved_changes('foo') is True + dlg_mock.assert_not_called() + + +@patch('PyQt6.QtWidgets.QMessageBox.question', + return_value=QtWidgets.QMessageBox.StandardButton.Yes) +def test_get_confirmation_unsaved_changes_when_changes_confirmed( + dlg_mock, settings, view, item): + view.undo_stack.push( + commands.InsertItems(view.scene, [item], QtCore.QPointF(0, 0))) + assert view.undo_stack.isClean() is False + assert view.get_confirmation_unsaved_changes('foo') is True + dlg_mock.assert_called_once() + + +@patch('PyQt6.QtWidgets.QMessageBox.question', + return_value=QtWidgets.QMessageBox.StandardButton.Cancel) +def test_get_confirmation_unsaved_changes_when_changes_not_confirmed( + dlg_mock, settings, view, item): + view.undo_stack.push( + commands.InsertItems(view.scene, [item], QtCore.QPointF(0, 0))) + assert view.undo_stack.isClean() is False + assert view.get_confirmation_unsaved_changes('foo') is False + dlg_mock.assert_called_once() + + +@patch('beeref.view.BeeGraphicsView.get_confirmation_unsaved_changes', + return_value=False) +def test_on_action_new_scene_when_unsaved_changes_not_confirmed( + confirm_mock, view): + view.clear_scene = MagicMock() + view.on_action_new_scene() + confirm_mock.assert_called_once() + view.clear_scene.assert_not_called() + + +@patch('beeref.view.BeeGraphicsView.get_confirmation_unsaved_changes', + return_value=True) +def test_on_action_new_scene_when_unsaved_changes_confirmed( + confirm_mock, view): + view.clear_scene = MagicMock() + view.on_action_new_scene() + confirm_mock.assert_called_once() + view.clear_scene.assert_called_once_with() + + +@patch('beeref.view.BeeGraphicsView.get_confirmation_unsaved_changes', + return_value=False) +def test_on_action_open_recent_file_when_unsaved_changes_not_confirmed( + confirm_mock, view): + view.open_from_file = MagicMock() + view.on_action_open_recent_file('foo.bee') + confirm_mock.assert_called_once() + view.open_from_file.assert_not_called() + + +@patch('beeref.view.BeeGraphicsView.get_confirmation_unsaved_changes', + return_value=True) +def test_on_action_open_recent_file_when_unsaved_changes_confirmed( + confirm_mock, view): + view.open_from_file = MagicMock() + view.on_action_open_recent_file('foo.bee') + confirm_mock.assert_called_once() + view.open_from_file.assert_called_once_with('foo.bee') + + @patch('beeref.view.BeeGraphicsView.clear_scene') def test_open_from_file(clear_mock, view, qtbot): root = os.path.dirname(__file__) @@ -199,6 +281,19 @@ def test_on_action_open(dialog_mock, view, qtbot): view.cancel_active_modes.assert_called_with() +@patch('beeref.view.BeeGraphicsView.get_confirmation_unsaved_changes', + return_value=False) +@patch('PyQt6.QtWidgets.QFileDialog.getOpenFileName') +def test_on_action_open_when_unsaved_changes_not_confirmed( + dialog_mock, confirm_mock, view): + view.cancel_active_modes = MagicMock() + view.open_from_file = MagicMock() + view.on_action_open() + view.cancel_active_modes.assert_not_called() + dialog_mock.assert_not_called() + view.open_from_file.assert_not_called() + + @patch('PyQt6.QtWidgets.QFileDialog.getOpenFileName') @patch('beeref.view.BeeGraphicsView.open_from_file') def test_on_action_open_when_no_filename(open_mock, dialog_mock, view): @@ -464,6 +559,26 @@ def test_on_action_export_images_file_exists_canceled( imgfilename.read_text() == 'foo' +@patch('beeref.view.BeeGraphicsView.get_confirmation_unsaved_changes', + return_value=False) +@patch('beeref.__main__.BeeRefApplication.quit') +def test_on_action_quit_when_unsaved_changes_not_confirmed( + quit_mock, confirm_mock, view): + view.on_action_quit() + confirm_mock.assert_called_once() + quit_mock.assert_not_called() + + +@patch('beeref.view.BeeGraphicsView.get_confirmation_unsaved_changes', + return_value=True) +@patch('beeref.__main__.BeeRefApplication.quit') +def test_on_action_quit_when_unsaved_changes_confirmed( + quit_mock, confirm_mock, view): + view.on_action_quit() + confirm_mock.assert_called_once() + quit_mock.assert_called_once_with() + + @patch('beeref.widgets.settings.SettingsDialog.show') def test_on_action_settings(show_mock, view): view.on_action_settings() diff --git a/tests/widgets/test_settings.py b/tests/widgets/test_settings.py index 79c8ce6..dd84f8c 100644 --- a/tests/widgets/test_settings.py +++ b/tests/widgets/test_settings.py @@ -4,6 +4,7 @@ from beeref.widgets.settings import ( ArrangeGapWidget, + ConfirmCloseUnsavedWidget, ImageStorageFormatWidget, SettingsDialog, ) @@ -31,7 +32,7 @@ def test_image_storage_format_selects_radiobox(settings, view): def test_image_storage_format_saves_change(settings, view): settings.setValue('Items/image_storage_format', 'best') widget = ImageStorageFormatWidget() - widget.buttons['jpg'].setChecked(True) + widget.set_value('jpg') assert widget.buttons['best'].isChecked() is False assert widget.buttons['png'].isChecked() is False assert widget.buttons['jpg'].isChecked() is True @@ -41,7 +42,7 @@ def test_image_storage_format_saves_change(settings, view): def test_image_storage_format_on_restore_defaults(settings, view): widget = ImageStorageFormatWidget() - widget.buttons['jpg'].setChecked(True) + widget.set_value('jpg') settings.setValue('Items/image_storage_format', 'best') widget.on_restore_defaults() assert widget.buttons['best'].isChecked() is True @@ -70,27 +71,63 @@ def test_arrange_gap_sets_title_when_edited(settings, view): def test_arrange_gap_saves_change(settings, view): settings.setValue('Items/arrange_gap', 6) widget = ArrangeGapWidget() - widget.input.setValue(8) + widget.set_value(8) assert settings.valueOrDefault('Items/arrange_gap') == 8 assert widget.title() == 'Arrange Gap: ✎' def test_arrange_gap_on_restore_defaults(settings, view): widget = ArrangeGapWidget() - widget.input.setValue(7) + widget.set_value(7) settings.setValue('Items/arrange_gap', 0) widget.on_restore_defaults() assert widget.input.value() == 0 assert widget.title() == 'Arrange Gap:' +def test_confirm_closed_initialises_input_from_settings(settings, view): + settings.setValue('Save/confirm_close_unsaved', False) + widget = ConfirmCloseUnsavedWidget() + assert widget.input.isChecked() is False + + +def test_confirm_closed_sets_title_when_not_edited(settings, view): + widget = ConfirmCloseUnsavedWidget() + assert widget.title() == 'Confirm when closing an unsaved file:' + + +def test_confirm_closed_sets_title_when_edited(settings, view): + settings.setValue('Save/confirm_close_unsaved', False) + widget = ConfirmCloseUnsavedWidget() + assert widget.title() == 'Confirm when closing an unsaved file: ✎' + + +def test_confirm_closed_saves_change(settings, view): + settings.setValue('Save/confirm_close_unsaved', True) + widget = ConfirmCloseUnsavedWidget() + widget.set_value(False) + assert settings.valueOrDefault('Save/confirm_close_unsaved') is False + assert widget.title() == 'Confirm when closing an unsaved file: ✎' + + +def test_confirm_closed_on_restore_defaults(settings, view): + widget = ConfirmCloseUnsavedWidget() + widget.set_value(False) + settings.setValue('Save/confirm_close_unsaved', True) + widget.on_restore_defaults() + assert widget.input.isChecked() is True + assert widget.title() == 'Confirm when closing an unsaved file:' + + @patch('PyQt6.QtWidgets.QMessageBox.question', return_value=QtWidgets.QMessageBox.StandardButton.Yes) def test_settings_dialog_on_restore_defaults(msg_mock, settings, view): dialog = SettingsDialog(view) settings.setValue('Items/image_storage_format', 'jpg') settings.setValue('Items/arrange_gap', 10) + settings.setValue('Save/confirm_close_unsaved', False) dialog.on_restore_defaults() msg_mock.assert_called_once() assert settings.valueOrDefault('Items/image_storage_format') == 'best' assert settings.valueOrDefault('Items/arrange_gap') == 0 + assert settings.valueOrDefault('Save/confirm_close_unsaved') is True