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

Better feedback for changes in the new settings widget #38799

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
6 changes: 3 additions & 3 deletions Framework/Properties/Mantid.properties.template
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ plots.y_min =
plots.y_max =

# The default width of the lines for the axes
plots.axesLineWidth = 1
plots.axesLineWidth = 1.0

# Whether to enable the grid by default
plots.enableGrid = Off
Expand Down Expand Up @@ -300,7 +300,7 @@ plots.line.Width = 1.5
plots.marker.Style = None

# Default maker size on 1d plots
plots.marker.Size = 6
plots.marker.Size = 6.0

# Default cap size on error bars in 1d plots
plots.errorbar.Capsize = 0.0
Expand All @@ -324,7 +324,7 @@ plots.errorbar.MarkerSize = 4
plots.legend.Location = best

# Default legend size
plots.legend.FontSize = 8
plots.legend.FontSize = 8.0

# Default colormap for image plots
plots.images.Colormap = viridis
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- The settings widget 'Apply' button is now disabled when there are no pending changes.
6 changes: 6 additions & 0 deletions qt/applications/workbench/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,16 @@ set(TEST_FILES
workbench/widgets/settings/test/test_settings_model.py
workbench/widgets/settings/test/test_settings_presenter.py
workbench/widgets/settings/test/test_settings_view.py
workbench/widgets/settings/base_classes/test/test_config_settings_changes_model.py
workbench/widgets/settings/base_classes/test/test_config_settings_presenter.py
workbench/widgets/settings/general/test/test_general_settings.py
workbench/widgets/settings/general/test/test_general_settings_model.py
workbench/widgets/settings/plots/test/test_plot_settings.py
workbench/widgets/settings/plots/test/test_plot_settings_model.py
workbench/widgets/settings/categories/test/test_categories_settings.py
workbench/widgets/settings/categories/test/test_categories_settings_model.py
workbench/widgets/settings/fitting/test/test_fitting_settings.py
workbench/widgets/settings/fitting/test/test_fitting_settings_model.py
workbench/projectrecovery/test/test_projectrecovery.py
workbench/projectrecovery/test/test_projectrecoveryloader.py
workbench/projectrecovery/test/test_projectrecoverysaver.py
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@
# NScD Oak Ridge National Laboratory, European Spallation Source,
# Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS
# SPDX - License - Identifier: GPL - 3.0 +
from abc import ABC
from typing import Dict, List

from mantid.kernel import ConfigService


class ConfigSettingsChangesModel:
class ConfigSettingsChangesModel(ABC):
def __init__(self):
self._changes: Dict[str, str] = {}

Expand All @@ -29,7 +30,7 @@ def apply_changes(self) -> None:

def add_change(self, property_string: str, value: str) -> None:
saved_value = self.get_saved_value(property_string)
if saved_value != value:
if saved_value.lower().rstrip() != value.lower().rstrip():
self._changes[property_string] = value
elif property_string in self._changes.keys():
self._changes.pop(property_string)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Mantid Repository : https://github.com/mantidproject/mantid
#
# Copyright © 2025 ISIS Rutherford Appleton Laboratory UKRI,
# NScD Oak Ridge National Laboratory, European Spallation Source,
# Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS
# SPDX - License - Identifier: GPL - 3.0 +
from abc import ABC

from workbench.widgets.settings.base_classes.config_settings_changes_model import ConfigSettingsChangesModel


class SettingsPresenterBase(ABC):
def __init__(self, model: ConfigSettingsChangesModel):
self._model: ConfigSettingsChangesModel = model
self._parent_presenter = None

def notify_changes(self):
if self._parent_presenter is not None:
self._parent_presenter.changes_updated(self._model.has_unsaved_changes())

def subscribe_parent_presenter(self, parent_presenter):
self._parent_presenter = parent_presenter
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
# NScD Oak Ridge National Laboratory, European Spallation Source,
# Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS
# SPDX - License - Identifier: GPL - 3.0 +
from unittest import TestCase
import unittest
from unittest.mock import patch, call

from workbench.widgets.settings.base_classes.config_settings_changes_model import ConfigSettingsChangesModel
from workbench.widgets.settings.test_utilities.mock_config_service import MockConfigService, BASE_CLASS_CONFIG_SERVICE_PATCH_PATH


@patch(BASE_CLASS_CONFIG_SERVICE_PATCH_PATH, new_callable=MockConfigService)
class ConfigSettingsChangesModelTest(TestCase):
class ConfigSettingsChangesModelTest(unittest.TestCase):
def setUp(self) -> None:
self.model = ConfigSettingsChangesModel()

Expand Down Expand Up @@ -88,3 +88,7 @@ def test_apply_changes(self, mock_config_service: MockConfigService):

mock_config_service.setString.assert_has_calls([call("property.1", "blue"), call("property.2", "apple")])
self.assertEqual(self.model.get_changes(), {})


if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Mantid Repository : https://github.com/mantidproject/mantid
#
# Copyright © 2025 ISIS Rutherford Appleton Laboratory UKRI,
# NScD Oak Ridge National Laboratory, European Spallation Source,
# Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS
# SPDX - License - Identifier: GPL - 3.0 +
import unittest
from unittest.mock import MagicMock

from workbench.widgets.settings.base_classes.config_settings_presenter import SettingsPresenterBase


class ConfigSettingsPresenterTest(unittest.TestCase):
def setUp(self) -> None:
self.mock_model = MagicMock()
self.mock_model.has_unsaved_changes = MagicMock()
self.presenter = SettingsPresenterBase(self.mock_model)
self._mock_parent_presenter = MagicMock()
self.presenter.subscribe_parent_presenter(self._mock_parent_presenter)

def test_notify_changes_with_changes(self):
self.mock_model.has_unsaved_changes.return_value = True
self.presenter.notify_changes()
self._mock_parent_presenter.changes_updated.assert_called_once_with(True)

def test_notify_changes_without_changes(self):
self.mock_model.has_unsaved_changes.return_value = False
self.presenter.notify_changes()
self._mock_parent_presenter.changes_updated.assert_called_once_with(False)


if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
# NScD Oak Ridge National Laboratory, European Spallation Source,
# Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS
# SPDX - License - Identifier: GPL - 3.0 +
from workbench.widgets.settings.base_classes.config_settings_presenter import SettingsPresenterBase
from workbench.widgets.settings.categories.categories_settings_model import CategoriesSettingsModel
from workbench.widgets.settings.categories.view import CategoriesSettingsView

from qtpy.QtCore import Qt


class CategoriesSettings(object):
class CategoriesSettings(SettingsPresenterBase):
"""
Presenter of the visible categories settings section. It handles all changes to options
within the section, and updates the ConfigService and workbench CONF accordingly.
Expand All @@ -18,10 +20,10 @@ class CategoriesSettings(object):
be handled here.
"""

def __init__(self, parent, model, view=None):
self._view = view if view else CategoriesSettingsView(parent, self)
def __init__(self, parent, model: CategoriesSettingsModel, view=None):
super().__init__(model)
self.parent = parent
self._model = model
self._view = view if view else CategoriesSettingsView(parent, self)
self._view.algorithm_tree_widget.setHeaderLabel("Show/Hide Algorithm Categories")
self._view.interface_tree_widget.setHeaderLabel("Show/Hide Interface Categories")
self.set_algorithm_tree_categories()
Expand All @@ -36,10 +38,12 @@ def get_view(self):
def set_hidden_algorithms_string(self, _):
categories_string = ";".join(self._create_hidden_categories_string(self._view.algorithm_tree_widget))
self._model.set_hidden_algorithms(categories_string)
self.notify_changes()

def set_hidden_interfaces_string(self, _):
categories_string = ";".join(self._create_hidden_categories_string(self._view.interface_tree_widget))
self._model.set_hidden_interfaces(categories_string)
self.notify_changes()

def nested_box_clicked(self, item_clicked, column):
new_state = item_clicked.checkState(column)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
# SPDX - License - Identifier: GPL - 3.0 +
import unittest

from unittest.mock import call, Mock, ANY, MagicMock
from unittest.mock import call, Mock, ANY, MagicMock, patch
from mantidqt.utils.qt.testing import start_qapplication
from mantidqt.utils.testing.mocks.mock_qt import MockQWidget
from mantidqt.utils.testing.strict_mock import StrictMock
Expand Down Expand Up @@ -117,7 +117,8 @@ def test_algorithm_categories_partial_states_change_correctly_when_bottom_level_
child_item1.setCheckState.assert_called_once_with(0, Qt.Checked)
parent_item.setCheckState.assert_called_once_with(0, Qt.PartiallyChecked)

def test_set_hidden_algorithms_string(self):
@patch("workbench.widgets.settings.categories.presenter.CategoriesSettings.notify_changes")
def test_set_hidden_algorithms_string(self, notify_changes_mock: MagicMock):
presenter = CategoriesSettings(None, view=self.mock_view, model=self.mock_model)
hidden_algorithim_string = [
i for i in sorted(self.mock_model.algorithm_and_states.keys()) if self.mock_model.algorithm_and_states[i] is True
Expand All @@ -126,6 +127,7 @@ def test_set_hidden_algorithms_string(self):

presenter.set_hidden_algorithms_string(None)
self.mock_model.set_hidden_algorithms.assert_called_once_with(";".join(hidden_algorithim_string))
notify_changes_mock.assert_called_once()

def test_interface_state_correct_when_created(self):
mock_main_window = MockMainWindow()
Expand All @@ -142,11 +144,17 @@ def test_interface_state_correct_when_created(self):

self.mock_view.add_checked_widget_item.assert_has_calls(expected_calls)

def test_set_hidden_interface_string(self):
@patch("workbench.widgets.settings.categories.presenter.CategoriesSettings.notify_changes")
def test_set_hidden_interface_string(self, notify_changes_mock: MagicMock):
self.mock_model.get_hidden_interfaces.return_value = ""
presenter = CategoriesSettings(None, view=self.mock_view, model=self.mock_model)
hidden_interface_string = "Indirect; Muon; Reflectometry"

presenter._create_hidden_categories_string = Mock(return_value=hidden_interface_string)
presenter.set_hidden_interfaces_string(None)
self.mock_model.set_hidden_interfaces.assert_called_once_with(";".join(hidden_interface_string))
notify_changes_mock.assert_called_once()


if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# NScD Oak Ridge National Laboratory, European Spallation Source,
# Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS
# SPDX - License - Identifier: GPL - 3.0 +
import unittest
from unittest.mock import patch, MagicMock, call

from workbench.widgets.settings.categories.categories_settings_model import CategoriesSettingsModel, CategoryProperties
Expand Down Expand Up @@ -54,3 +55,7 @@ def test_get_algorithm_factory_category_map(self):
[{"Basic": "Load"}, {"Arithmetic": "Mean"}],
call(),
)


if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,19 @@
# NScD Oak Ridge National Laboratory, European Spallation Source,
# Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS
# SPDX - License - Identifier: GPL - 3.0 +
from workbench.widgets.settings.base_classes.config_settings_presenter import SettingsPresenterBase
from workbench.widgets.settings.fitting.fitting_settings_model import FittingSettingsModel
from workbench.widgets.settings.fitting.view import FittingSettingsView
from workbench.widgets.settings.view_utilities.settings_view_utilities import filter_out_mousewheel_events_from_combo_or_spin_box

from qtpy.QtCore import Qt


class FittingSettings(object):
def __init__(self, parent, model, view=None):
self._view = view if view else FittingSettingsView(parent, self)
class FittingSettings(SettingsPresenterBase):
def __init__(self, parent, model: FittingSettingsModel, view=None):
super().__init__(model)
self.parent = parent
self._view = view if view else FittingSettingsView(parent, self)
self._model = model
self.add_filters()
self.add_items_to_combo_boxes()
Expand Down Expand Up @@ -77,25 +80,30 @@ def setup_signals(self):
def action_auto_background_changed(self, item_name):
if item_name == "None":
self._model.set_auto_background("")
return
background_string = item_name + " " + self._view.background_args.text()
self._model.set_auto_background(background_string)
else:
background_string = item_name + " " + self._view.background_args.text()
self._model.set_auto_background(background_string)
self.notify_changes()

def action_background_args_changed(self):
if self._view.auto_bkg.currentText() == "None":
self._model.set_auto_background("")
else:
background_string = self._view.auto_bkg.currentText() + " " + self._view.background_args.text()
self._model.set_auto_background(background_string)
self.notify_changes()

def action_default_peak_changed(self, item_name):
self._model.set_default_peak(item_name)
self.notify_changes()

def action_find_peaks_fwhm_changed(self, value):
self._model.set_fwhm(str(value))
self.notify_changes()

def action_find_peaks_tolerance_changed(self, value):
self._model.set_tolerance(str(value))
self.notify_changes()

def update_properties(self):
self.load_current_setting_values()
Expand Down
Loading