Add confirmation dialog when discarding an unsaved file

.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

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

beeref/actions/actions.py

@@ -312,7 +312,7 @@ actions = ActionList([
         id='new_scene',
         text='&New Scene',
         shortcuts=['Ctrl+N'],
-        callback='clear_scene',
+        callback='on_action_new_scene',
     ),
     Action(
         id='fit_scene',

beeref/actions/mixin.py

@@ -123,7 +123,7 @@ class ActionsMixin:
                 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)

beeref/config/settings.py

@@ -109,6 +109,10 @@ settings_events = BeeSettingsEvents()
 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'),

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):

beeref/view.py

@@ -200,6 +200,26 @@ class BeeGraphicsView(MainControlsMixin,
         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 @@ class BeeGraphicsView(MainControlsMixin,
             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 @@ class BeeGraphicsView(MainControlsMixin,
         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 @@ class BeeGraphicsView(MainControlsMixin,
             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)

beeref/widgets/settings.py

@@ -17,6 +17,7 @@ from functools import partial
 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 @@ logger = logging.getLogger(__name__)
 
 
 class GroupBase(QtWidgets.QGroupBox):
+    TITLE = None
+    HELPTEXT = None
+    KEY = None
 
     def __init__(self):
         super().__init__()
@@ -50,16 +54,24 @@ class GroupBase(QtWidgets.QGroupBox):
         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 @@ class RadioGroup(GroupBase):
         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 @@ class IntegerGroup(GroupBase):
         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 @@ class SettingsDialog(QtWidgets.QDialog):
         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)

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",
 ]
 

setup.py

@@ -0,0 +1,3 @@
+from setuptools import setup
+
+setup()

tests/actions/test_mixin.py

@@ -20,7 +20,7 @@ class FooWidget(QtWidgets.QWidget, ActionsMixin):
     def on_bar(self):
         pass
 
-    def open_from_file(self):
+    def on_action_open_recent_file(self, filename):
         pass
 
 

tests/test_view.py

@@ -7,7 +7,7 @@ from unittest.mock import MagicMock, patch, mock_open
 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()

tests/widgets/test_settings.py

@@ -4,6 +4,7 @@ from PyQt6 import QtWidgets
 
 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