openlp-core team mailing list archive

[virbinarus <virbinarus@xxxxxxxxx>]

virbinarus has proposed merging lp:~virbinarus/openlp/expandserviceitem into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1238385 in OpenLP: "Cannot expand items in Service Manager via right arrow key"
  https://bugs.launchpad.net/openlp/+bug/1238385

For more details, see:
https://code.launchpad.net/~virbinarus/openlp/expandserviceitem/+merge/326248

You can now expand and collapse service items with right and left arrow keys.

lp:~virbinarus/openlp/expandserviceitem (revision 2757)
[?[1;32mSUCCESS?[1;m] https://ci.openlp.io/job/Branch-01-Pull/2095/
[?[1;32mSUCCESS?[1;m] https://ci.openlp.io/job/Branch-02-Functional-Tests/2005/
[?[1;32mSUCCESS?[1;m] https://ci.openlp.io/job/Branch-03-Interface-Tests/1918/
[?[1;32mSUCCESS?[1;m] https://ci.openlp.io/job/Branch-04a-Code_Analysis/1295/
[?[1;32mSUCCESS?[1;m] https://ci.openlp.io/job/Branch-04b-Test_Coverage/1141/
[?[1;32mSUCCESS?[1;m] https://ci.openlp.io/job/Branch-04c-Code_Analysis2/270/
[?[1;32mSUCCESS?[1;m] https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/115/

-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~virbinarus/openlp/expandserviceitem into lp:openlp.

=== modified file 'openlp/core/ui/servicemanager.py'
--- openlp/core/ui/servicemanager.py	2017-05-21 07:11:36 +0000
+++ openlp/core/ui/servicemanager.py	2017-06-23 20:33:34 +0000
@@ -66,6 +66,12 @@
             elif event.key() == QtCore.Qt.Key_Down:
                 self.service_manager.on_move_selection_down()
                 event.accept()
+            elif event.key() == QtCore.Qt.Key_Right:
+                self.service_manager.on_expand_selection()
+                event.accept()
+            elif event.key() == QtCore.Qt.Key_Left:
+                self.service_manager.on_collapse_selection()
+                event.accept()
             elif event.key() == QtCore.Qt.Key_Delete:
                 self.service_manager.on_delete_from_service()
                 event.accept()
@@ -1119,6 +1125,39 @@
             return
         self.service_manager_list.setCurrentItem(item_after)
 
+    def on_expand_selection(self):
+        """
+        Expands cursor selection on the window. Called by the right arrow
+        """
+        item = self.service_manager_list.currentItem()
+
+        # Since we only have 2 levels we find them by checking for children
+        if item.childCount():
+            if not self.service_manager_list.isExpanded(self.service_manager_list.currentIndex()):
+                self.service_manager_list.expandItem(item)
+                self.service_manager.expanded(item)
+                # If not expanded, Expand it
+
+            self.service_manager_list.setCurrentItem(self.service_manager_list.itemBelow(item))
+            # Then move selection down to child whether it needed to be expanded or not
+
+    def on_collapse_selection(self):
+        """
+        Collapses cursor selection on the window Called by the left arrow
+        """
+        item = self.service_manager_list.currentItem()
+
+        # Since we only have 2 levels we find them by checking for children
+        if item.childCount():
+            if self.service_manager_list.isExpanded(self.service_manager_list.currentIndex()):
+                self.service_manager_list.collapseItem(item)
+                self.service_manager.collapsed(item)
+
+        else:  # If selection is lower level
+            self.service_manager_list.collapseItem(item.parent())
+            self.service_manager.collapsed(item.parent())
+            self.service_manager_list.setCurrentItem(item.parent())
+
     def on_collapse_all(self, field=None):
         """
         Collapse all the service items.

=== modified file 'tests/interfaces/openlp_core_ui/test_servicemanager.py'
--- tests/interfaces/openlp_core_ui/test_servicemanager.py	2017-04-24 05:17:55 +0000
+++ tests/interfaces/openlp_core_ui/test_servicemanager.py	2017-06-23 20:33:34 +0000
@@ -28,9 +28,13 @@
 from openlp.core.common import Registry
 from openlp.core.lib import ScreenList, ServiceItem, ItemCapabilities
 from openlp.core.ui.mainwindow import MainWindow
+from openlp.core.ui.servicemanager import ServiceManagerList
+from openlp.core.lib.serviceitem import ServiceItem
 
 from tests.helpers.testmixin import TestMixin
 
+from PyQt5 import QtCore, QtGui, QtWidgets
+
 
 class TestServiceManager(TestCase, TestMixin):
 
@@ -351,3 +355,167 @@
         new_service.trigger()
 
         assert mocked_event.call_count == 1, 'The on_new_service_clicked method should have been called once'
+
+    def test_expand_selection_on_right_arrow(self):
+        """
+        Test that a right arrow key press event calls the on_expand_selection function
+        """
+        # GIVEN a mocked expand function
+        self.service_manager.on_expand_selection = MagicMock()
+
+        # WHEN the right arrow key event is called
+        self.service_manager.setup_ui(self.service_manager)
+        event = QtGui.QKeyEvent(QtCore.QEvent.KeyPress, QtCore.Qt.Key_Right, QtCore.Qt.NoModifier)
+        self.service_manager.service_manager_list.keyPressEvent(event)
+
+        # THEN the on_expand_selection function should have been called.
+        self.service_manager.on_expand_selection.assert_called_once_with()
+
+    def test_collapse_selection_on_left_arrow(self):
+        """
+        Test that a left arrow key press event calls the on_collapse_selection function
+        """
+        # GIVEN a mocked collapse function
+        self.service_manager.on_collapse_selection = MagicMock()
+
+        # WHEN the left arrow key event is called
+        self.service_manager.setup_ui(self.service_manager)
+        event = QtGui.QKeyEvent(QtCore.QEvent.KeyPress, QtCore.Qt.Key_Left, QtCore.Qt.NoModifier)
+        self.service_manager.service_manager_list.keyPressEvent(event)
+
+        # THEN the on_collapse_selection function should have been called.
+        self.service_manager.on_collapse_selection.assert_called_once_with()
+
+    def test_move_selection_down_on_down_arrow(self):
+        """
+        Test that a down arrow key press event calls the on_move_selection_down function
+        """
+        # GIVEN a mocked move down function
+        self.service_manager.on_move_selection_down = MagicMock()
+
+        # WHEN the down arrow key event is called
+        self.service_manager.setup_ui(self.service_manager)
+        event = QtGui.QKeyEvent(QtCore.QEvent.KeyPress, QtCore.Qt.Key_Down, QtCore.Qt.NoModifier)
+        self.service_manager.service_manager_list.keyPressEvent(event)
+
+        # THEN the on_move_selection_down function should have been called.
+        self.service_manager.on_move_selection_down.assert_called_once_with()
+
+    def test_move_selection_up_on_up_arrow(self):
+        """
+        Test that an up arrow key press event calls the on_move_selection_up function
+        """
+        # GIVEN a mocked move up function
+        self.service_manager.on_move_selection_up = MagicMock()
+
+        # WHEN the up arrow key event is called
+        self.service_manager.setup_ui(self.service_manager)
+        event = QtGui.QKeyEvent(QtCore.QEvent.KeyPress, QtCore.Qt.Key_Up, QtCore.Qt.NoModifier)
+        self.service_manager.service_manager_list.keyPressEvent(event)
+
+        # THEN the on_move_selection_up function should have been called.
+        self.service_manager.on_move_selection_up.assert_called_once_with()
+
+    def test_on_expand_selection(self):
+        """
+        Test that the on_expand_selection function successfully expands an item and moves to its first child
+        """
+        # GIVEN a mocked servicemanager list
+        self.service_manager.expanded = MagicMock()
+
+        verse_1 = QtWidgets.QTreeWidgetItem(0)
+        verse_2 = QtWidgets.QTreeWidgetItem(0)
+        song_item = QtWidgets.QTreeWidgetItem(0)
+        song_item.addChild(verse_1)
+        song_item.addChild(verse_2)
+
+        self.service_manager.setup_ui(self.service_manager)
+        self.service_manager.service_manager_list.addTopLevelItem(song_item)
+
+        self.service_manager.service_manager_list.setCurrentItem(song_item)
+
+        # Reset expanded function in case it has been called and/or changed in initialisation of the service manager.
+        self.service_manager.expanded = MagicMock()
+
+        # WHEN on_expand_selection is called
+        self.service_manager.on_expand_selection()
+
+        # THEN selection should be expanded
+
+        selected_index = self.service_manager.service_manager_list.currentIndex()
+        above_selected_index = self.service_manager.service_manager_list.indexAbove(selected_index)
+        self.assertTrue(self.service_manager.service_manager_list.isExpanded(above_selected_index),
+                        'Item should have been expanded')
+        self.service_manager.expanded.assert_called_once_with(song_item)
+
+    def test_on_collapse_selection_with_parent_selected(self):
+        """
+        Test that the on_collapse_selection function successfully collapses an item
+        """
+
+        # GIVEN a mocked servicemanager list
+        self.service_manager.expanded = MagicMock()
+        self.service_manager.collapsed = MagicMock()
+
+        verse_1 = QtWidgets.QTreeWidgetItem(0)
+        verse_2 = QtWidgets.QTreeWidgetItem(0)
+        song_item = QtWidgets.QTreeWidgetItem(0)
+        song_item.addChild(verse_1)
+        song_item.addChild(verse_2)
+
+        self.service_manager.setup_ui(self.service_manager)
+        self.service_manager.service_manager_list.addTopLevelItem(song_item)
+
+        self.service_manager.service_manager_list.setCurrentItem(song_item)
+        self.service_manager.service_manager_list.expandItem(song_item)
+
+        # Reset collapsed function in case it has been called and/or changed in initialisation of the service manager.
+        self.service_manager.collapsed = MagicMock()
+
+        # WHEN on_expand_selection is called
+        self.service_manager.on_collapse_selection()
+
+        # THEN selection should be expanded
+
+        selected_index = self.service_manager.service_manager_list.currentIndex()
+        self.assertFalse(self.service_manager.service_manager_list.isExpanded(selected_index),
+                         'Item should have been collapsed')
+        self.assertTrue(self.service_manager.service_manager_list.currentItem() == song_item,
+                        'Top item should have been selected')
+        self.service_manager.collapsed.assert_called_once_with(song_item)
+
+    def test_on_collapse_selection_with_child_selected(self):
+        """
+        Test that the on_collapse_selection function successfully collapses child's parent item
+        and moves selection to its parent.
+        """
+        # GIVEN a mocked servicemanager list
+        self.service_manager.expanded = MagicMock()
+        self.service_manager.collapsed = MagicMock()
+
+        verse_1 = QtWidgets.QTreeWidgetItem(0)
+        verse_2 = QtWidgets.QTreeWidgetItem(0)
+        song_item = QtWidgets.QTreeWidgetItem(0)
+        song_item.addChild(verse_1)
+        song_item.addChild(verse_2)
+
+        self.service_manager.setup_ui(self.service_manager)
+        self.service_manager.service_manager_list.addTopLevelItem(song_item)
+
+        self.service_manager.service_manager_list.setCurrentItem(verse_2)
+        self.service_manager.service_manager_list.expandItem(song_item)
+
+        # Reset collapsed function in case it has been called and/or changed in initialisation of the service manager.
+        self.service_manager.collapsed = MagicMock()
+
+        # WHEN on_expand_selection is called
+        self.service_manager.on_collapse_selection()
+
+        # THEN selection should be expanded
+
+        selected_index = self.service_manager.service_manager_list.currentIndex()
+        self.assertFalse(self.service_manager.service_manager_list.isExpanded(selected_index),
+                         'Item should have been collapsed')
+        self.assertTrue(self.service_manager.service_manager_list.currentItem() == song_item,
+                        'Top item should have been selected')
+        self.service_manager.collapsed.assert_called_once_with(song_item)