diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d6ff32b4b..ba847e1a7 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,11 +1,12 @@ OpenLP 2.4.6 ============ -* Fixed a bug where the author type upgrade was being ignore because it was looking at the wrong table -* Fixed a bug where the songs_songbooks table was not being created because the if expression was the wrong way round -* Changed the songs_songbooks migration SQL slightly to take into account a bug that has (hopefully) been fixed +* Fix a bug where the author type upgrade was being ignore because it was looking at the wrong table +* Fix a bug where the songs_songbooks table was not being created because the if expression was the wrong way round +* Change the songs_songbooks migration SQL slightly to take into account a bug that has (hopefully) been fixed * Sometimes the timer goes off as OpenLP is shutting down, and the application has already been deleted (reported via support system) * Fix opening the data folder (KDE thought the old way was an SMB share) * Fix a problem with the new QMediaPlayer not controlling the playlist anymore -* Added importing of author types to the OpenLP 2 song importer +* Add importing of author types to the OpenLP 2 song importer * Fix a problem with loading Qt's translation files, bug #1676163 +* Disable the controls in the shortcut dialog unless a shortcut is actually selected diff --git a/openlp/core/ui/shortcutlistform.py b/openlp/core/ui/shortcutlistform.py index 55097732f..cdceaf790 100644 --- a/openlp/core/ui/shortcutlistform.py +++ b/openlp/core/ui/shortcutlistform.py @@ -49,6 +49,8 @@ class ShortcutListForm(QtWidgets.QDialog, Ui_ShortcutListDialog, RegistryPropert self.changed_actions = {} self.action_list = ActionList.get_instance() self.dialog_was_shown = False + self.default_radio_button.setEnabled(False) + self.custom_radio_button.setEnabled(False) self.primary_push_button.toggled.connect(self.on_primary_push_button_clicked) self.alternate_push_button.toggled.connect(self.on_alternate_push_button_clicked) self.tree_widget.currentItemChanged.connect(self.on_current_item_changed) @@ -110,8 +112,89 @@ class ShortcutListForm(QtWidgets.QDialog, Ui_ShortcutListDialog, RegistryPropert self.reload_shortcut_list() self._adjust_button(self.primary_push_button, False, False, '') self._adjust_button(self.alternate_push_button, False, False, '') + self._set_controls_enabled(False) return QtWidgets.QDialog.exec(self) + def _validiate_shortcut(self, changing_action, key_sequence): + """ + Checks if the given ``changing_action `` can use the given ``key_sequence``. Returns ``True`` if the + ``key_sequence`` can be used by the action, otherwise displays a dialog and returns ``False``. + + :param changing_action: The action which wants to use the ``key_sequence``. + :param key_sequence: The key sequence which the action want so use. + """ + is_valid = True + for category in self.action_list.categories: + for action in category.actions: + shortcuts = self._action_shortcuts(action) + if key_sequence not in shortcuts: + continue + if action is changing_action: + if self.primary_push_button.isChecked() and shortcuts.index(key_sequence) == 0: + continue + if self.alternate_push_button.isChecked() and shortcuts.index(key_sequence) == 1: + continue + # Have the same parent, thus they cannot have the same shortcut. + if action.parent() is changing_action.parent(): + is_valid = False + # The new shortcut is already assigned, but if both shortcuts are only valid in a different widget the + # new shortcut is valid, because they will not interfere. + if action.shortcutContext() in [QtCore.Qt.WindowShortcut, QtCore.Qt.ApplicationShortcut]: + is_valid = False + if changing_action.shortcutContext() in [QtCore.Qt.WindowShortcut, QtCore.Qt.ApplicationShortcut]: + is_valid = False + if not is_valid: + self.main_window.warning_message(translate('OpenLP.ShortcutListDialog', 'Duplicate Shortcut'), + translate('OpenLP.ShortcutListDialog', + 'The shortcut "%s" is already assigned to another action, please' + ' use a different shortcut.') % + self.get_shortcut_string(key_sequence, for_display=True)) + self.dialog_was_shown = True + return is_valid + + def _action_shortcuts(self, action): + """ + This returns the shortcuts for the given ``action``, which also includes those shortcuts which are not saved + yet but already assigned (as changes are applied when closing the dialog). + """ + if action in self.changed_actions: + return self.changed_actions[action] + return action.shortcuts() + + def _current_item_action(self, item=None): + """ + Returns the action of the given ``item``. If no item is given, we return the action of the current item of + the ``tree_widget``. + """ + if item is None: + item = self.tree_widget.currentItem() + if item is None: + return + return item.data(0, QtCore.Qt.UserRole) + + def _adjust_button(self, button, checked=None, enabled=None, text=None): + """ + Can be called to adjust more properties of the given ``button`` at once. + """ + # Set the text before checking the button, because this emits a signal. + if text is not None: + button.setText(text) + if checked is not None: + button.setChecked(checked) + if enabled is not None: + button.setEnabled(enabled) + + def _set_controls_enabled(self, is_enabled=False): + """ + Enable or disable the shortcut controls + """ + self.default_radio_button.setEnabled(is_enabled) + self.custom_radio_button.setEnabled(is_enabled) + self.primary_push_button.setEnabled(is_enabled) + self.alternate_push_button.setEnabled(is_enabled) + self.clear_primary_button.setEnabled(is_enabled) + self.clear_alternate_button.setEnabled(is_enabled) + def reload_shortcut_list(self): """ Reload the ``tree_widget`` list to add new and remove old actions. @@ -229,8 +312,7 @@ class ShortcutListForm(QtWidgets.QDialog, Ui_ShortcutListDialog, RegistryPropert A item has been pressed. We adjust the button's text to the action's shortcut which is encapsulate in the item. """ action = self._current_item_action(item) - self.primary_push_button.setEnabled(action is not None) - self.alternate_push_button.setEnabled(action is not None) + self._set_controls_enabled(action is not None) primary_text = '' alternate_text = '' primary_label_text = '' @@ -244,7 +326,7 @@ class ShortcutListForm(QtWidgets.QDialog, Ui_ShortcutListDialog, RegistryPropert if len(action.default_shortcuts) == 2: alternate_label_text = self.get_shortcut_string(action.default_shortcuts[1], for_display=True) shortcuts = self._action_shortcuts(action) - # We do not want to loose pending changes, that is why we have to keep the text when, this function has not + # We do not want to loose pending changes, that is why we have to keep the text when this function has not # been triggered by a signal. if item is None: primary_text = self.primary_push_button.text() @@ -254,7 +336,7 @@ class ShortcutListForm(QtWidgets.QDialog, Ui_ShortcutListDialog, RegistryPropert elif len(shortcuts) == 2: primary_text = self.get_shortcut_string(shortcuts[0], for_display=True) alternate_text = self.get_shortcut_string(shortcuts[1], for_display=True) - # When we are capturing a new shortcut, we do not want, the buttons to display the current shortcut. + # When we are capturing a new shortcut, we do not want the buttons to display the current shortcut. if self.primary_push_button.isChecked(): primary_text = '' if self.alternate_push_button.isChecked(): @@ -396,75 +478,6 @@ class ShortcutListForm(QtWidgets.QDialog, Ui_ShortcutListDialog, RegistryPropert self.refresh_shortcut_list() self.on_current_item_changed(self.tree_widget.currentItem()) - def _validiate_shortcut(self, changing_action, key_sequence): - """ - Checks if the given ``changing_action `` can use the given ``key_sequence``. Returns ``True`` if the - ``key_sequence`` can be used by the action, otherwise displays a dialog and returns ``False``. - - :param changing_action: The action which wants to use the ``key_sequence``. - :param key_sequence: The key sequence which the action want so use. - """ - is_valid = True - for category in self.action_list.categories: - for action in category.actions: - shortcuts = self._action_shortcuts(action) - if key_sequence not in shortcuts: - continue - if action is changing_action: - if self.primary_push_button.isChecked() and shortcuts.index(key_sequence) == 0: - continue - if self.alternate_push_button.isChecked() and shortcuts.index(key_sequence) == 1: - continue - # Have the same parent, thus they cannot have the same shortcut. - if action.parent() is changing_action.parent(): - is_valid = False - # The new shortcut is already assigned, but if both shortcuts are only valid in a different widget the - # new shortcut is valid, because they will not interfere. - if action.shortcutContext() in [QtCore.Qt.WindowShortcut, QtCore.Qt.ApplicationShortcut]: - is_valid = False - if changing_action.shortcutContext() in [QtCore.Qt.WindowShortcut, QtCore.Qt.ApplicationShortcut]: - is_valid = False - if not is_valid: - self.main_window.warning_message(translate('OpenLP.ShortcutListDialog', 'Duplicate Shortcut'), - translate('OpenLP.ShortcutListDialog', - 'The shortcut "%s" is already assigned to another action, please' - ' use a different shortcut.') % - self.get_shortcut_string(key_sequence, for_display=True)) - self.dialog_was_shown = True - return is_valid - - def _action_shortcuts(self, action): - """ - This returns the shortcuts for the given ``action``, which also includes those shortcuts which are not saved - yet but already assigned (as changes are applied when closing the dialog). - """ - if action in self.changed_actions: - return self.changed_actions[action] - return action.shortcuts() - - def _current_item_action(self, item=None): - """ - Returns the action of the given ``item``. If no item is given, we return the action of the current item of - the ``tree_widget``. - """ - if item is None: - item = self.tree_widget.currentItem() - if item is None: - return - return item.data(0, QtCore.Qt.UserRole) - - def _adjust_button(self, button, checked=None, enabled=None, text=None): - """ - Can be called to adjust more properties of the given ``button`` at once. - """ - # Set the text before checking the button, because this emits a signal. - if text is not None: - button.setText(text) - if checked is not None: - button.setChecked(checked) - if enabled is not None: - button.setEnabled(enabled) - @staticmethod def get_shortcut_string(shortcut, for_display=False): if for_display: diff --git a/tests/interfaces/openlp_core_ui/test_shortcutlistform.py b/tests/interfaces/openlp_core_ui/test_shortcutlistform.py index 6e2d31955..2ddd3d2c8 100644 --- a/tests/interfaces/openlp_core_ui/test_shortcutlistform.py +++ b/tests/interfaces/openlp_core_ui/test_shortcutlistform.py @@ -52,6 +52,24 @@ class TestShortcutform(TestCase, TestMixin): del self.form del self.main_window + def test_set_controls_enabled(self): + """ + Test that the _set_controls_enabled() method works correctly + """ + # GIVEN: A shortcut form, and a value to set the controls + is_enabled = True + + # WHEN: _set_controls_enabled() is called + self.form._set_controls_enabled(is_enabled) + + # THEN: The controls should be enabled + assert self.form.default_radio_button.isEnabled() is True + assert self.form.custom_radio_button.isEnabled() is True + assert self.form.primary_push_button.isEnabled() is True + assert self.form.alternate_push_button.isEnabled() is True + assert self.form.clear_primary_button.isEnabled() is True + assert self.form.clear_alternate_button.isEnabled() is True + def adjust_button_test(self): """ Test the _adjust_button() method @@ -71,6 +89,27 @@ class TestShortcutform(TestCase, TestMixin): mocked_check_method.assert_called_once_with(True) self.assertEqual(button.isEnabled(), enabled, 'The button should be disabled.') + @patch('openlp.core.ui.shortcutlistform.QtWidgets.QDialog.exec') + def test_exec(self, mocked_exec): + """ + Test the exec method + """ + # GIVEN: A form and a mocked out base exec method + mocked_exec.return_value = True + + # WHEN: exec is called + result = self.form.exec() + + # THEN: The result should be True and the controls should be disabled + assert self.form.default_radio_button.isEnabled() is False + assert self.form.custom_radio_button.isEnabled() is False + assert self.form.primary_push_button.isEnabled() is False + assert self.form.alternate_push_button.isEnabled() is False + assert self.form.clear_primary_button.isEnabled() is False + assert self.form.clear_alternate_button.isEnabled() is False + mocked_exec.assert_called_once_with(self.form) + assert result is True + def space_key_press_event_test(self): """ Test the keyPressEvent when the spacebar was pressed