openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #31363
[Merge] lp:~raoul-snyman/openlp/custom-shortcuts-2.4 into lp:openlp/2.4
Raoul Snyman has proposed merging lp:~raoul-snyman/openlp/custom-shortcuts-2.4 into lp:openlp/2.4.
Commit message:
Disable the shortcut controls when an action has not been selected
Requested reviews:
OpenLP Core (openlp-core)
For more details, see:
https://code.launchpad.net/~raoul-snyman/openlp/custom-shortcuts-2.4/+merge/321347
Disable the shortcut controls when an action has not been selected
Add this to your merge proposal:
--------------------------------
lp:~raoul-snyman/openlp/custom-shortcuts-2.4 (revision 2681)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1952/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1863/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1804/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1530/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/1120/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/1188/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/1056/
--
Your team OpenLP Core is requested to review the proposed merge of lp:~raoul-snyman/openlp/custom-shortcuts-2.4 into lp:openlp/2.4.
=== modified file 'CHANGELOG.rst'
--- CHANGELOG.rst 2017-03-27 23:25:12 +0000
+++ CHANGELOG.rst 2017-03-29 18:46:40 +0000
@@ -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
=== modified file 'openlp/core/ui/shortcutlistform.py'
--- openlp/core/ui/shortcutlistform.py 2016-12-31 11:05:48 +0000
+++ openlp/core/ui/shortcutlistform.py 2017-03-29 18:46:40 +0000
@@ -49,6 +49,8 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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:
=== modified file 'tests/interfaces/openlp_core_ui/test_shortcutlistform.py'
--- tests/interfaces/openlp_core_ui/test_shortcutlistform.py 2016-12-31 11:05:48 +0000
+++ tests/interfaces/openlp_core_ui/test_shortcutlistform.py 2017-03-29 18:46:40 +0000
@@ -52,6 +52,24 @@
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 @@
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
Follow ups
