openlp-core team mailing list archive

[Andreas Preikschat <andreaspreikschat@xxxxxxxxxx>]

Andreas Preikschat has proposed merging lp:~googol/openlp/bug-1103906 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1103906 in OpenLP: ""Auto Play Slides"  menu should use radio boxes"
  https://bugs.launchpad.net/openlp/+bug/1103906

For more details, see:
https://code.launchpad.net/~googol/openlp/bug-1103906/+merge/156266

Hello,

-  use QActionGroup instead of Checkboxes (Bug #1103906 'Auto Play Slides menu should use radioboxes')

It looks like this:
http://img203.imageshack.us/img203/3468/bildschirmfoto300320131.png

Can you actually disable radio boxes? I don't remember that you can do so, and I am not sure if that is just me or if you actually cannot do this.

QUESTION:
What do you think about embedding the spinbox in the menu? Or is that too cluttered?
-- 
https://code.launchpad.net/~googol/openlp/bug-1103906/+merge/156266
Your team OpenLP Core is requested to review the proposed merge of lp:~googol/openlp/bug-1103906 into lp:openlp.

=== modified file 'openlp/core/ui/servicemanager.py'
--- openlp/core/ui/servicemanager.py	2013-03-29 12:53:07 +0000
+++ openlp/core/ui/servicemanager.py	2013-03-30 14:46:33 +0000
@@ -234,18 +234,22 @@
             icon=u':/general/general_edit.png', triggers=self.create_custom)
         self.menu.addSeparator()
         # Add AutoPlay menu actions
-        self.auto_play_slides_group = QtGui.QMenu(translate('OpenLP.ServiceManager', '&Auto play slides'))
-        self.menu.addMenu(self.auto_play_slides_group)
-        self.auto_play_slides_loop = create_widget_action(self.auto_play_slides_group,
+        self.auto_play_slides_menu = QtGui.QMenu(translate('OpenLP.ServiceManager', '&Auto play slides'))
+        self.menu.addMenu(self.auto_play_slides_menu)
+        auto_play_slides_group = QtGui.QActionGroup(self.auto_play_slides_menu)
+        auto_play_slides_group.setExclusive(True)
+        self.auto_play_slides_loop = create_widget_action(self.auto_play_slides_menu,
             text=translate('OpenLP.ServiceManager', 'Auto play slides &Loop'),
             checked=False, triggers=self.toggle_auto_play_slides_loop)
-        self.auto_play_slides_once = create_widget_action(self.auto_play_slides_group,
+        auto_play_slides_group.addAction(self.auto_play_slides_loop)
+        self.auto_play_slides_once = create_widget_action(self.auto_play_slides_menu,
             text=translate('OpenLP.ServiceManager', 'Auto play slides &Once'),
             checked=False, triggers=self.toggle_auto_play_slides_once)
-        self.auto_play_slides_group.addSeparator()
-        self.timed_slide_interval = create_widget_action(self.auto_play_slides_group,
+        auto_play_slides_group.addAction(self.auto_play_slides_once)
+        self.auto_play_slides_menu.addSeparator()
+        self.timed_slide_interval = create_widget_action(self.auto_play_slides_menu,
             text=translate('OpenLP.ServiceManager', '&Delay between slides'),
-            checked=False, triggers=self.on_timed_slide_interval)
+            triggers=self.on_timed_slide_interval)
         self.menu.addSeparator()
         self.preview_action = create_widget_action(self.menu, text=translate('OpenLP.ServiceManager', 'Show &Preview'),
             icon=u':/general/general_preview.png', triggers=self.make_preview)
@@ -786,7 +790,7 @@
             self.notes_action.setVisible(True)
         if service_item[u'service_item'].is_capable(ItemCapabilities.CanLoop) and  \
                 len(service_item[u'service_item'].get_frames()) > 1:
-            self.auto_play_slides_group.menuAction().setVisible(True)
+            self.auto_play_slides_menu.menuAction().setVisible(True)
             self.auto_play_slides_once.setChecked(service_item[u'service_item'].auto_play_slides_once)
             self.auto_play_slides_loop.setChecked(service_item[u'service_item'].auto_play_slides_loop)
             self.timed_slide_interval.setChecked(service_item[u'service_item'].timed_slide_interval > 0)
@@ -798,7 +802,7 @@
                 translate('OpenLP.ServiceManager', '&Delay between slides') + delay_suffix)
             # TODO for future: make group explains itself more visually
         else:
-            self.auto_play_slides_group.menuAction().setVisible(False)
+            self.auto_play_slides_menu.menuAction().setVisible(False)
         if service_item[u'service_item'].is_capable(ItemCapabilities.HasVariableStartTime):
             self.time_action.setVisible(True)
         if service_item[u'service_item'].is_capable(ItemCapabilities.CanAutoStartForLive):

=== modified file 'tests/interfaces/openlp_core_ui/test_servicemanager.py'
--- tests/interfaces/openlp_core_ui/test_servicemanager.py	2013-03-14 22:22:18 +0000
+++ tests/interfaces/openlp_core_ui/test_servicemanager.py	2013-03-30 14:46:33 +0000
@@ -3,11 +3,11 @@
 """
 
 from unittest import TestCase
-from mock import MagicMock, patch
+from mock import MagicMock, Mock, patch
 
 from PyQt4 import QtGui
 
-from openlp.core.lib import Registry, ScreenList
+from openlp.core.lib import Registry, ScreenList, ServiceItem
 from openlp.core.ui.mainwindow import MainWindow
 
 
@@ -42,3 +42,44 @@
         # THEN the count of items should be zero
         self.assertEqual(self.service_manager.service_manager_list.topLevelItemCount(), 0,
             u'The service manager list should be empty ')
+
+    def context_menu_test(self):
+        """
+        Test the context_menu() method.
+        """
+        # GIVEN: A service item added
+        with patch(u'PyQt4.QtGui.QTreeWidget.itemAt') as mocked_item_at_method, \
+                patch(u'PyQt4.QtGui.QWidget.mapToGlobal') as mocked_map_to_global, \
+                patch(u'PyQt4.QtGui.QMenu.exec_') as mocked_exec:
+            mocked_item = MagicMock()
+            mocked_item.parent.return_value = None
+            mocked_item_at_method.return_value = mocked_item
+            # We want 1 to be returned for the position
+            mocked_item.data.return_value = 1
+            # A service item without capabilities.
+            service_item = ServiceItem()
+            self.service_manager.service_items = [{u'service_item': service_item}]
+            q_point = None
+            # Mocked actions.
+            self.service_manager.edit_action.setVisible = Mock()
+            self.service_manager.create_custom_action.setVisible = Mock()
+            self.service_manager.maintain_action.setVisible = Mock()
+            self.service_manager.notes_action.setVisible = Mock()
+            self.service_manager.time_action.setVisible = Mock()
+            self.service_manager.auto_start_action.setVisible = Mock()
+
+            # WHEN: Show the context menu.
+            self.service_manager.context_menu(q_point)
+
+            # THEN: The following actions should be not visible.
+            self.service_manager.edit_action.setVisible.assert_called_once_with(False), \
+                u'The action should be set invisible.'
+            self.service_manager.create_custom_action.setVisible.assert_called_with(False), \
+                u'The action should be set invisible.'
+            self.service_manager.maintain_action.setVisible.assert_called_with(False), \
+                u'The action should be set invisible.'
+            self.service_manager.notes_action.setVisible.assert_called_with(True), u'The action should be set visible.'
+            self.service_manager.time_action.setVisible.assert_called_with(False), \
+                u'The action should be set invisible.'
+            self.service_manager.auto_start_action.setVisible.assert_called_with(False), \
+                u'The action should be set invisible.'