Merge lp:~raoul-snyman/openlp/custom-shortcuts-2.4 into lp:openlp/2.4

Proposed by Raoul Snyman
Status: Merged
Merged at revision: 2681
Proposed branch: lp:~raoul-snyman/openlp/custom-shortcuts-2.4
Merge into: lp:openlp/2.4
Diff against target: 284 lines (+130/-77)
3 files modified
CHANGELOG.rst (+5/-4)
openlp/core/ui/shortcutlistform.py (+86/-73)
tests/interfaces/openlp_core_ui/test_shortcutlistform.py (+39/-0)
To merge this branch: bzr merge lp:~raoul-snyman/openlp/custom-shortcuts-2.4
Reviewer Review Type Date Requested Status
Tim Bentley Approve
Review via email: mp+321347@code.launchpad.net

Commit message

Disable the shortcut controls when an action has not been selected

To post a comment you must log in.
Revision history for this message
Tim Bentley (trb143) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CHANGELOG.rst'
2--- CHANGELOG.rst 2017-03-27 23:25:12 +0000
3+++ CHANGELOG.rst 2017-03-29 18:46:40 +0000
4@@ -1,11 +1,12 @@
5 OpenLP 2.4.6
6 ============
7
8-* Fixed a bug where the author type upgrade was being ignore because it was looking at the wrong table
9-* Fixed a bug where the songs_songbooks table was not being created because the if expression was the wrong way round
10-* Changed the songs_songbooks migration SQL slightly to take into account a bug that has (hopefully) been fixed
11+* Fix a bug where the author type upgrade was being ignore because it was looking at the wrong table
12+* Fix a bug where the songs_songbooks table was not being created because the if expression was the wrong way round
13+* Change the songs_songbooks migration SQL slightly to take into account a bug that has (hopefully) been fixed
14 * Sometimes the timer goes off as OpenLP is shutting down, and the application has already been deleted (reported via support system)
15 * Fix opening the data folder (KDE thought the old way was an SMB share)
16 * Fix a problem with the new QMediaPlayer not controlling the playlist anymore
17-* Added importing of author types to the OpenLP 2 song importer
18+* Add importing of author types to the OpenLP 2 song importer
19 * Fix a problem with loading Qt's translation files, bug #1676163
20+* Disable the controls in the shortcut dialog unless a shortcut is actually selected
21
22=== modified file 'openlp/core/ui/shortcutlistform.py'
23--- openlp/core/ui/shortcutlistform.py 2016-12-31 11:05:48 +0000
24+++ openlp/core/ui/shortcutlistform.py 2017-03-29 18:46:40 +0000
25@@ -49,6 +49,8 @@
26 self.changed_actions = {}
27 self.action_list = ActionList.get_instance()
28 self.dialog_was_shown = False
29+ self.default_radio_button.setEnabled(False)
30+ self.custom_radio_button.setEnabled(False)
31 self.primary_push_button.toggled.connect(self.on_primary_push_button_clicked)
32 self.alternate_push_button.toggled.connect(self.on_alternate_push_button_clicked)
33 self.tree_widget.currentItemChanged.connect(self.on_current_item_changed)
34@@ -110,8 +112,89 @@
35 self.reload_shortcut_list()
36 self._adjust_button(self.primary_push_button, False, False, '')
37 self._adjust_button(self.alternate_push_button, False, False, '')
38+ self._set_controls_enabled(False)
39 return QtWidgets.QDialog.exec(self)
40
41+ def _validiate_shortcut(self, changing_action, key_sequence):
42+ """
43+ Checks if the given ``changing_action `` can use the given ``key_sequence``. Returns ``True`` if the
44+ ``key_sequence`` can be used by the action, otherwise displays a dialog and returns ``False``.
45+
46+ :param changing_action: The action which wants to use the ``key_sequence``.
47+ :param key_sequence: The key sequence which the action want so use.
48+ """
49+ is_valid = True
50+ for category in self.action_list.categories:
51+ for action in category.actions:
52+ shortcuts = self._action_shortcuts(action)
53+ if key_sequence not in shortcuts:
54+ continue
55+ if action is changing_action:
56+ if self.primary_push_button.isChecked() and shortcuts.index(key_sequence) == 0:
57+ continue
58+ if self.alternate_push_button.isChecked() and shortcuts.index(key_sequence) == 1:
59+ continue
60+ # Have the same parent, thus they cannot have the same shortcut.
61+ if action.parent() is changing_action.parent():
62+ is_valid = False
63+ # The new shortcut is already assigned, but if both shortcuts are only valid in a different widget the
64+ # new shortcut is valid, because they will not interfere.
65+ if action.shortcutContext() in [QtCore.Qt.WindowShortcut, QtCore.Qt.ApplicationShortcut]:
66+ is_valid = False
67+ if changing_action.shortcutContext() in [QtCore.Qt.WindowShortcut, QtCore.Qt.ApplicationShortcut]:
68+ is_valid = False
69+ if not is_valid:
70+ self.main_window.warning_message(translate('OpenLP.ShortcutListDialog', 'Duplicate Shortcut'),
71+ translate('OpenLP.ShortcutListDialog',
72+ 'The shortcut "%s" is already assigned to another action, please'
73+ ' use a different shortcut.') %
74+ self.get_shortcut_string(key_sequence, for_display=True))
75+ self.dialog_was_shown = True
76+ return is_valid
77+
78+ def _action_shortcuts(self, action):
79+ """
80+ This returns the shortcuts for the given ``action``, which also includes those shortcuts which are not saved
81+ yet but already assigned (as changes are applied when closing the dialog).
82+ """
83+ if action in self.changed_actions:
84+ return self.changed_actions[action]
85+ return action.shortcuts()
86+
87+ def _current_item_action(self, item=None):
88+ """
89+ Returns the action of the given ``item``. If no item is given, we return the action of the current item of
90+ the ``tree_widget``.
91+ """
92+ if item is None:
93+ item = self.tree_widget.currentItem()
94+ if item is None:
95+ return
96+ return item.data(0, QtCore.Qt.UserRole)
97+
98+ def _adjust_button(self, button, checked=None, enabled=None, text=None):
99+ """
100+ Can be called to adjust more properties of the given ``button`` at once.
101+ """
102+ # Set the text before checking the button, because this emits a signal.
103+ if text is not None:
104+ button.setText(text)
105+ if checked is not None:
106+ button.setChecked(checked)
107+ if enabled is not None:
108+ button.setEnabled(enabled)
109+
110+ def _set_controls_enabled(self, is_enabled=False):
111+ """
112+ Enable or disable the shortcut controls
113+ """
114+ self.default_radio_button.setEnabled(is_enabled)
115+ self.custom_radio_button.setEnabled(is_enabled)
116+ self.primary_push_button.setEnabled(is_enabled)
117+ self.alternate_push_button.setEnabled(is_enabled)
118+ self.clear_primary_button.setEnabled(is_enabled)
119+ self.clear_alternate_button.setEnabled(is_enabled)
120+
121 def reload_shortcut_list(self):
122 """
123 Reload the ``tree_widget`` list to add new and remove old actions.
124@@ -229,8 +312,7 @@
125 A item has been pressed. We adjust the button's text to the action's shortcut which is encapsulate in the item.
126 """
127 action = self._current_item_action(item)
128- self.primary_push_button.setEnabled(action is not None)
129- self.alternate_push_button.setEnabled(action is not None)
130+ self._set_controls_enabled(action is not None)
131 primary_text = ''
132 alternate_text = ''
133 primary_label_text = ''
134@@ -244,7 +326,7 @@
135 if len(action.default_shortcuts) == 2:
136 alternate_label_text = self.get_shortcut_string(action.default_shortcuts[1], for_display=True)
137 shortcuts = self._action_shortcuts(action)
138- # We do not want to loose pending changes, that is why we have to keep the text when, this function has not
139+ # We do not want to loose pending changes, that is why we have to keep the text when this function has not
140 # been triggered by a signal.
141 if item is None:
142 primary_text = self.primary_push_button.text()
143@@ -254,7 +336,7 @@
144 elif len(shortcuts) == 2:
145 primary_text = self.get_shortcut_string(shortcuts[0], for_display=True)
146 alternate_text = self.get_shortcut_string(shortcuts[1], for_display=True)
147- # When we are capturing a new shortcut, we do not want, the buttons to display the current shortcut.
148+ # When we are capturing a new shortcut, we do not want the buttons to display the current shortcut.
149 if self.primary_push_button.isChecked():
150 primary_text = ''
151 if self.alternate_push_button.isChecked():
152@@ -396,75 +478,6 @@
153 self.refresh_shortcut_list()
154 self.on_current_item_changed(self.tree_widget.currentItem())
155
156- def _validiate_shortcut(self, changing_action, key_sequence):
157- """
158- Checks if the given ``changing_action `` can use the given ``key_sequence``. Returns ``True`` if the
159- ``key_sequence`` can be used by the action, otherwise displays a dialog and returns ``False``.
160-
161- :param changing_action: The action which wants to use the ``key_sequence``.
162- :param key_sequence: The key sequence which the action want so use.
163- """
164- is_valid = True
165- for category in self.action_list.categories:
166- for action in category.actions:
167- shortcuts = self._action_shortcuts(action)
168- if key_sequence not in shortcuts:
169- continue
170- if action is changing_action:
171- if self.primary_push_button.isChecked() and shortcuts.index(key_sequence) == 0:
172- continue
173- if self.alternate_push_button.isChecked() and shortcuts.index(key_sequence) == 1:
174- continue
175- # Have the same parent, thus they cannot have the same shortcut.
176- if action.parent() is changing_action.parent():
177- is_valid = False
178- # The new shortcut is already assigned, but if both shortcuts are only valid in a different widget the
179- # new shortcut is valid, because they will not interfere.
180- if action.shortcutContext() in [QtCore.Qt.WindowShortcut, QtCore.Qt.ApplicationShortcut]:
181- is_valid = False
182- if changing_action.shortcutContext() in [QtCore.Qt.WindowShortcut, QtCore.Qt.ApplicationShortcut]:
183- is_valid = False
184- if not is_valid:
185- self.main_window.warning_message(translate('OpenLP.ShortcutListDialog', 'Duplicate Shortcut'),
186- translate('OpenLP.ShortcutListDialog',
187- 'The shortcut "%s" is already assigned to another action, please'
188- ' use a different shortcut.') %
189- self.get_shortcut_string(key_sequence, for_display=True))
190- self.dialog_was_shown = True
191- return is_valid
192-
193- def _action_shortcuts(self, action):
194- """
195- This returns the shortcuts for the given ``action``, which also includes those shortcuts which are not saved
196- yet but already assigned (as changes are applied when closing the dialog).
197- """
198- if action in self.changed_actions:
199- return self.changed_actions[action]
200- return action.shortcuts()
201-
202- def _current_item_action(self, item=None):
203- """
204- Returns the action of the given ``item``. If no item is given, we return the action of the current item of
205- the ``tree_widget``.
206- """
207- if item is None:
208- item = self.tree_widget.currentItem()
209- if item is None:
210- return
211- return item.data(0, QtCore.Qt.UserRole)
212-
213- def _adjust_button(self, button, checked=None, enabled=None, text=None):
214- """
215- Can be called to adjust more properties of the given ``button`` at once.
216- """
217- # Set the text before checking the button, because this emits a signal.
218- if text is not None:
219- button.setText(text)
220- if checked is not None:
221- button.setChecked(checked)
222- if enabled is not None:
223- button.setEnabled(enabled)
224-
225 @staticmethod
226 def get_shortcut_string(shortcut, for_display=False):
227 if for_display:
228
229=== modified file 'tests/interfaces/openlp_core_ui/test_shortcutlistform.py'
230--- tests/interfaces/openlp_core_ui/test_shortcutlistform.py 2016-12-31 11:05:48 +0000
231+++ tests/interfaces/openlp_core_ui/test_shortcutlistform.py 2017-03-29 18:46:40 +0000
232@@ -52,6 +52,24 @@
233 del self.form
234 del self.main_window
235
236+ def test_set_controls_enabled(self):
237+ """
238+ Test that the _set_controls_enabled() method works correctly
239+ """
240+ # GIVEN: A shortcut form, and a value to set the controls
241+ is_enabled = True
242+
243+ # WHEN: _set_controls_enabled() is called
244+ self.form._set_controls_enabled(is_enabled)
245+
246+ # THEN: The controls should be enabled
247+ assert self.form.default_radio_button.isEnabled() is True
248+ assert self.form.custom_radio_button.isEnabled() is True
249+ assert self.form.primary_push_button.isEnabled() is True
250+ assert self.form.alternate_push_button.isEnabled() is True
251+ assert self.form.clear_primary_button.isEnabled() is True
252+ assert self.form.clear_alternate_button.isEnabled() is True
253+
254 def adjust_button_test(self):
255 """
256 Test the _adjust_button() method
257@@ -71,6 +89,27 @@
258 mocked_check_method.assert_called_once_with(True)
259 self.assertEqual(button.isEnabled(), enabled, 'The button should be disabled.')
260
261+ @patch('openlp.core.ui.shortcutlistform.QtWidgets.QDialog.exec')
262+ def test_exec(self, mocked_exec):
263+ """
264+ Test the exec method
265+ """
266+ # GIVEN: A form and a mocked out base exec method
267+ mocked_exec.return_value = True
268+
269+ # WHEN: exec is called
270+ result = self.form.exec()
271+
272+ # THEN: The result should be True and the controls should be disabled
273+ assert self.form.default_radio_button.isEnabled() is False
274+ assert self.form.custom_radio_button.isEnabled() is False
275+ assert self.form.primary_push_button.isEnabled() is False
276+ assert self.form.alternate_push_button.isEnabled() is False
277+ assert self.form.clear_primary_button.isEnabled() is False
278+ assert self.form.clear_alternate_button.isEnabled() is False
279+ mocked_exec.assert_called_once_with(self.form)
280+ assert result is True
281+
282 def space_key_press_event_test(self):
283 """
284 Test the keyPressEvent when the spacebar was pressed

Subscribers

People subscribed via source and target branches

to all changes: