From c366e5868334a240edda04fae4e4f97e4c6d1df3 Mon Sep 17 00:00:00 2001 From: Kyle Russell Date: Wed, 29 Aug 2018 23:11:30 -0400 Subject: [PATCH] Improve usability of image plugin choosegroupform In order to add an image to an existing group when no group was preselected, the user must currently choose the existing group name from the comboxbox and also select the Existing Group radio button. It should be assumed that by selecting a group name from the combobox, the user intendeds to add the image to an existing group, and the accompanying radio button should automatically be selected. This reduces the number of required clicks, and the likelihood of not actually adding the image to the correct group. Likewise, if a user enters text into the New Group field, the dialog should assume that the user's intent is to create a new group and auto select the appropriate radio button. Also removes some choosegroupdialog specific component logic from mediaitem, since it's now covered by the choosegroupdialog implementation. Better encapsulation, and improves testability. (Testing that the existing group radio button was selected when choosedialogform was initialized with a preselected group requires much more effort when the radio button selection logic spanned two components.) Adds simple test cases for the scenarios described above. --- .../plugins/images/forms/choosegroupdialog.py | 21 ++++ .../plugins/images/forms/choosegroupform.py | 1 + openlp/plugins/images/lib/mediaitem.py | 10 -- .../openlp_plugins/images/__init__.py | 21 ++++ .../images/forms/test_choosegroupform.py | 103 ++++++++++++++++++ 5 files changed, 146 insertions(+), 10 deletions(-) create mode 100644 tests/interfaces/openlp_plugins/images/__init__.py create mode 100644 tests/interfaces/openlp_plugins/images/forms/test_choosegroupform.py diff --git a/openlp/plugins/images/forms/choosegroupdialog.py b/openlp/plugins/images/forms/choosegroupdialog.py index 49e3f5f5b..62080fd7e 100644 --- a/openlp/plugins/images/forms/choosegroupdialog.py +++ b/openlp/plugins/images/forms/choosegroupdialog.py @@ -58,6 +58,7 @@ class Ui_ChooseGroupDialog(object): self.choose_group_layout.setWidget(3, QtWidgets.QFormLayout.LabelRole, self.existing_radio_button) self.group_combobox = QtWidgets.QComboBox(choose_group_dialog) self.group_combobox.setObjectName('group_combobox') + self.group_combobox.activated.connect(self.on_group_combobox_selected) self.choose_group_layout.setWidget(3, QtWidgets.QFormLayout.FieldRole, self.group_combobox) self.new_radio_button = QtWidgets.QRadioButton(choose_group_dialog) self.new_radio_button.setChecked(False) @@ -65,6 +66,7 @@ class Ui_ChooseGroupDialog(object): self.choose_group_layout.setWidget(4, QtWidgets.QFormLayout.LabelRole, self.new_radio_button) self.new_group_edit = QtWidgets.QLineEdit(choose_group_dialog) self.new_group_edit.setObjectName('new_group_edit') + self.new_group_edit.textEdited.connect(self.on_new_group_edit_changed) self.choose_group_layout.setWidget(4, QtWidgets.QFormLayout.FieldRole, self.new_group_edit) self.group_button_box = create_button_box(choose_group_dialog, 'buttonBox', ['ok']) self.choose_group_layout.setWidget(5, QtWidgets.QFormLayout.FieldRole, self.group_button_box) @@ -83,3 +85,22 @@ class Ui_ChooseGroupDialog(object): self.nogroup_radio_button.setText(translate('ImagePlugin.ChooseGroupForm', 'No group')) self.existing_radio_button.setText(translate('ImagePlugin.ChooseGroupForm', 'Existing group')) self.new_radio_button.setText(translate('ImagePlugin.ChooseGroupForm', 'New group')) + + def on_group_combobox_selected(self, index): + """ + Handles the activated signal from the existing group combobox when the + user makes a selection + + :param index: position of the selected item in the combobox + """ + self.existing_radio_button.setChecked(True) + self.group_combobox.setFocus() + + def on_new_group_edit_changed(self, new_group): + """ + Handles the textEdited signal from the new group text input field + when the user enters a new group name + + :param new_group: new text entered by the user + """ + self.new_radio_button.setChecked(True) diff --git a/openlp/plugins/images/forms/choosegroupform.py b/openlp/plugins/images/forms/choosegroupform.py index 2f61bf6db..643adb2ed 100644 --- a/openlp/plugins/images/forms/choosegroupform.py +++ b/openlp/plugins/images/forms/choosegroupform.py @@ -48,4 +48,5 @@ class ChooseGroupForm(QtWidgets.QDialog, Ui_ChooseGroupDialog): for index in range(self.group_combobox.count()): if self.group_combobox.itemData(index) == selected_group: self.group_combobox.setCurrentIndex(index) + self.existing_radio_button.setChecked(True) return QtWidgets.QDialog.exec(self) diff --git a/openlp/plugins/images/lib/mediaitem.py b/openlp/plugins/images/lib/mediaitem.py index 8d7701696..b82da4e26 100644 --- a/openlp/plugins/images/lib/mediaitem.py +++ b/openlp/plugins/images/lib/mediaitem.py @@ -430,16 +430,6 @@ class ImageMediaItem(MediaManagerItem): if isinstance(selected_item.data(0, QtCore.Qt.UserRole), ImageGroups): preselect_group = selected_item.data(0, QtCore.Qt.UserRole).id # Enable and disable parts of the 'choose group' form - if preselect_group is None: - self.choose_group_form.nogroup_radio_button.setChecked(True) - self.choose_group_form.nogroup_radio_button.setFocus() - self.choose_group_form.existing_radio_button.setChecked(False) - self.choose_group_form.new_radio_button.setChecked(False) - else: - self.choose_group_form.nogroup_radio_button.setChecked(False) - self.choose_group_form.existing_radio_button.setChecked(True) - self.choose_group_form.new_radio_button.setChecked(False) - self.choose_group_form.group_combobox.setFocus() if self.manager.get_object_count(ImageGroups) == 0: self.choose_group_form.existing_radio_button.setDisabled(True) self.choose_group_form.group_combobox.setDisabled(True) diff --git a/tests/interfaces/openlp_plugins/images/__init__.py b/tests/interfaces/openlp_plugins/images/__init__.py new file mode 100644 index 000000000..711ded4ae --- /dev/null +++ b/tests/interfaces/openlp_plugins/images/__init__.py @@ -0,0 +1,21 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2018 OpenLP Developers # +# --------------------------------------------------------------------------- # +# This program is free software; you can redistribute it and/or modify it # +# under the terms of the GNU General Public License as published by the Free # +# Software Foundation; version 2 of the License. # +# # +# This program is distributed in the hope that it will be useful, but WITHOUT # +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # +# more details. # +# # +# You should have received a copy of the GNU General Public License along # +# with this program; if not, write to the Free Software Foundation, Inc., 59 # +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # +############################################################################### diff --git a/tests/interfaces/openlp_plugins/images/forms/test_choosegroupform.py b/tests/interfaces/openlp_plugins/images/forms/test_choosegroupform.py new file mode 100644 index 000000000..c09982096 --- /dev/null +++ b/tests/interfaces/openlp_plugins/images/forms/test_choosegroupform.py @@ -0,0 +1,103 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2018 OpenLP Developers # +# --------------------------------------------------------------------------- # +# This program is free software; you can redistribute it and/or modify it # +# under the terms of the GNU General Public License as published by the Free # +# Software Foundation; version 2 of the License. # +# # +# This program is distributed in the hope that it will be useful, but WITHOUT # +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # +# more details. # +# # +# You should have received a copy of the GNU General Public License along # +# with this program; if not, write to the Free Software Foundation, Inc., 59 # +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # +############################################################################### +""" +Tests for choosegroupform from the openlp.plugins.images.forms package. +""" + +from unittest import TestCase +from unittest.mock import MagicMock + +from PyQt5 import QtWidgets + +from openlp.core.common.registry import Registry +from openlp.plugins.images.forms.choosegroupform import ChooseGroupForm +from tests.helpers.testmixin import TestMixin + +class TestImageChooseGroupForm(TestCase, TestMixin): + """ + Test the ChooseGroupForm class + """ + def setUp(self): + """ + Create the UI + """ + Registry.create() + self.setup_application() + self.main_window = QtWidgets.QMainWindow() + Registry().register('main_window', self.main_window) + self.form = ChooseGroupForm(self.main_window) + + def tearDown(self): + """ + Cleanup + """ + del self.form + del self.main_window + + def test_no_group_selected_by_default(self): + """ + Tests that the No Group option is the default selection + """ + assert self.form.nogroup_radio_button.isChecked() + + def test_provided_group_is_selected(self): + """ + Tests preselected group initialization + """ + # GIVEN: There are some existing groups + QtWidgets.QDialog.exec = MagicMock(return_value=QtWidgets.QDialog.Accepted) + self.form.group_combobox.addItem('Group 1', 0) + self.form.group_combobox.addItem('Group 2', 1) + + # WHEN: The form is displayed with preselected group index 1 + self.form.exec(1) + + # THEN: The Existing Group should be selected along with the radio button + assert self.form.group_combobox.currentIndex() == 1 + assert self.form.existing_radio_button.isChecked() + + + def test_auto_select_existing_group_on_combo_selection(self): + """ + Tests that the Existing Group option becomes selected when changing the combobox + """ + # GIVEN: No preselected group was provided during initialization + assert not self.form.existing_radio_button.isChecked() + + # WHEN: An existing group is selected from the combo box + self.form.on_group_combobox_selected(0) + + # THEN: The Existing Group radio button should also be selected + assert self.form.existing_radio_button.isChecked() + + def test_auto_select_new_group_on_edit(self): + """ + Tests that the New Group option becomes selected when changing the text field + """ + # GIVEN: The New Group option has not already been selected + assert not self.form.new_radio_button.isChecked() + + # WHEN: The user enters text into the new group name text field + self.form.on_new_group_edit_changed('Test Group') + + # THEN: The New Group radio button should also be selected + assert self.form.new_radio_button.isChecked()