Merge branch 'fix_live_controls_toolbar' into 'master'

Fix live controls toolbar

See merge request openlp/openlp!255

openlp/core/ui/slidecontroller.py

@@ -49,7 +49,7 @@ from openlp.core.widgets.toolbar import OpenLPToolbar
 from openlp.core.widgets.views import ListPreviewWidget
 
 # Threshold which has to be trespassed to toggle.
-HIDE_MENU_THRESHOLD = 27
+HIDE_MENU_THRESHOLD = 80
 
 NARROW_MENU = [
     'hide_menu'
@@ -204,7 +204,7 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
         self.update_slide_limits()
         self.panel = QtWidgets.QWidget(self.main_window.control_splitter)
         self.slide_list = {}
-        self.controller_width = -1
+        self.ignore_toolbar_resize_events = False
         # Layout for holding panel
         self.panel_layout = QtWidgets.QVBoxLayout(self.panel)
         self.panel_layout.setSpacing(0)
@@ -474,6 +474,7 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
         self.preview_widget.clicked.connect(self.on_slide_selected)
         self.preview_widget.verticalHeader().sectionClicked.connect(self.on_slide_selected)
         if self.is_live:
+            self.preview_widget.resize_event.connect(self.on_controller_size_changed)
             # Need to use event as called across threads and UI is updated
             self.slidecontroller_toggle_display.connect(self.toggle_display)
             Registry().register_function('slidecontroller_live_spin_delay', self.receive_spin_delay)
@@ -676,6 +677,7 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
         self.ratio = self.screens.current.display_geometry.width() / self.screens.current.display_geometry.height()
         self.preview_display.resize(new_preview_size)
         self.slide_layout.set_aspect_ratio(self.ratio)
+        self.on_controller_size_changed()
 
     def __add_actions_to_widget(self, widget):
         """
@@ -696,39 +698,32 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
             self.theme_screen,
             self.blank_screen])
 
-    def on_controller_size_changed(self):
+    def on_controller_size_changed(self, event=None):
         """
-        Change layout of display control buttons on controller size change
+        Change layout of display control buttons when the controller size changes
         """
-        if self.is_live:
+        self.log_debug('on_controller_size_changed is_event:{}'.format(event is not None))
-            # The new width of the display
+        if self.is_live and self.ignore_toolbar_resize_events is False:
-            width = self.controller.width()
+            spare_space = self.controller.width() - self.toolbar.size().width()
-            # Space used by the toolbar.
+            if spare_space <= 0 and not self.hide_menu.isVisible():
-            used_space = self.toolbar.size().width() + self.hide_menu.size().width()
+                self.set_hide_mode_menu(narrow=True)
-            # Add the threshold to prevent flickering.
+            # Add threshold to prevent flickering.
-            if width > used_space + HIDE_MENU_THRESHOLD and self.hide_menu.isVisible():
+            elif spare_space > HIDE_MENU_THRESHOLD and self.hide_menu.isVisible() \
-                self.toolbar.set_widget_visible(NARROW_MENU, False)
+                    or spare_space > 0 and not self.hide_menu.isVisible():
-                self.set_blank_menu()
+                self.set_hide_mode_menu(narrow=False)
-            # Take away a threshold to prevent flickering.
-            elif width < used_space - HIDE_MENU_THRESHOLD and not self.hide_menu.isVisible():
-                self.set_blank_menu(False)
-                self.toolbar.set_widget_visible(NARROW_MENU)
-            # Fallback to the standard blank toolbar if the hide_menu is not visible.
-            elif not self.hide_menu.isVisible():
-                self.toolbar.set_widget_visible(NARROW_MENU, False)
-                self.set_blank_menu()
 
-    def set_blank_menu(self, visible=True):
+    def set_hide_mode_menu(self, narrow):
         """
-        Set the correct menu type dependent on the service item type
+        Set the wide or narrow hide mode menu
 
-        :param visible: Do I need to hide the menu?
+        :param compact: Use the narrow menu?
         """
+        self.toolbar.set_widget_visible(NARROW_MENU, narrow)
         self.toolbar.set_widget_visible(WIDE_MENU, False)
         if self.service_item and self.service_item.is_text():
-            self.toolbar.set_widget_visible(WIDE_MENU, visible)
+            self.toolbar.set_widget_visible(WIDE_MENU, not narrow)
         else:
-            self.toolbar.set_widget_visible(NON_TEXT_MENU, visible)
+            self.toolbar.set_widget_visible(NON_TEXT_MENU, not narrow)
 
     def receive_spin_delay(self):
         """
@@ -766,6 +761,8 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
         self.song_menu.hide()
         self.toolbar.set_widget_visible(LOOP_LIST, False)
         self.toolbar.set_widget_visible('song_menu', False)
+        # Set to narrow menu so we don't push the panel wider than it needs to be when adding new buttons
+        self.set_hide_mode_menu(narrow=True)
         # Reset the button
         self.play_slides_once.setChecked(False)
         self.play_slides_once.setIcon(UiIcons().play_slides)
@@ -782,10 +779,6 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
             self.mediabar.show()
         self.previous_item.setVisible(not item.is_media())
         self.next_item.setVisible(not item.is_media())
-        # The layout of the toolbar is size dependent, so make sure it fits. Reset stored controller_width.
-        if self.is_live:
-            self.controller_width = -1
-        self.on_controller_size_changed()
         # Work-around for OS X, hide and then show the toolbar
         # See bug #791050
         self.toolbar.show()
@@ -925,7 +918,9 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
         :param service_item: The current service item
         :param slide_no: The slide number to select
         """
+        self.log_debug('_process_item start')
         self.on_stop_loop()
+        self.ignore_toolbar_resize_events = True
         old_item = self.service_item
         # rest to allow the remote pick up verse 1 if large imaged
         self.selected_row = 0
@@ -1015,6 +1010,12 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
                 self.on_media_close()
         if self.is_live:
             Registry().execute('slidecontroller_{item}_started'.format(item=self.type_prefix), [self.service_item])
+            # Need to process events four times to get correct controller width
+            for _ in range(4):
+                self.application.process_events()
+            self.ignore_toolbar_resize_events = False
+            self.on_controller_size_changed()
+        self.log_debug('_process_item end')
 
     def on_slide_selected_index(self, message):
         """

openlp/core/widgets/views.py

@@ -70,6 +70,7 @@ class ListPreviewWidget(QtWidgets.QTableWidget, RegistryProperties):
     :param parent:
     :param screen_ratio:
     """
+    resize_event = QtCore.pyqtSignal()
 
     def __init__(self, parent, screen_ratio):
         """
@@ -105,6 +106,7 @@ class ListPreviewWidget(QtWidgets.QTableWidget, RegistryProperties):
         Overloaded method from QTableWidget. Will recalculate the layout.
         """
         self.__recalculate_layout()
+        self.resize_event.emit()
 
     def __recalculate_layout(self):
         """

tests/functional/openlp_core/ui/test_slidecontroller.py

@@ -31,8 +31,8 @@ from openlp.core.state import State
 from openlp.core.common.registry import Registry
 from openlp.core.lib import ServiceItemAction
 from openlp.core.ui import HideMode
-from openlp.core.ui.slidecontroller import NON_TEXT_MENU, WIDE_MENU, InfoLabel, LiveController, PreviewController, \
+from openlp.core.ui.slidecontroller import NON_TEXT_MENU, WIDE_MENU, NARROW_MENU, InfoLabel, LiveController, \
-    SlideController
+    PreviewController, SlideController
 
 
 def test_initial_slide_controller(registry):
@@ -84,9 +84,10 @@ def test_text_service_item_blank(settings):
 
     # WHEN: a text based service item is used
     slide_controller.service_item.is_text = MagicMock(return_value=True)
-    slide_controller.set_blank_menu()
+    slide_controller.set_hide_mode_menu(narrow=False)
 
     # THEN: the call to set the visible items on the toolbar should be correct
+    toolbar.set_widget_visible.assert_any_call(NARROW_MENU, False)
     toolbar.set_widget_visible.assert_called_with(WIDE_MENU, True)
 
 
@@ -104,12 +105,94 @@ def test_non_text_service_item_blank(settings):
 
     # WHEN a non text based service item is used
     slide_controller.service_item.is_text = MagicMock(return_value=False)
-    slide_controller.set_blank_menu()
+    slide_controller.set_hide_mode_menu(narrow=False)
 
     # THEN: then call set up the toolbar to blank the display screen.
+    toolbar.set_widget_visible.assert_any_call(NARROW_MENU, False)
     toolbar.set_widget_visible.assert_called_with(NON_TEXT_MENU, True)
 
 
+def test_text_service_item_blank_narrow(settings):
+    """
+    Test that loading a text-based service item into the slide controller sets the correct blank menu
+    """
+    # GIVEN: A new SlideController instance.
+    slide_controller = SlideController(None)
+    service_item = MagicMock()
+    toolbar = MagicMock()
+    toolbar.set_widget_visible = MagicMock()
+    slide_controller.toolbar = toolbar
+    slide_controller.service_item = service_item
+
+    # WHEN: a text based service item is used
+    slide_controller.service_item.is_text = MagicMock(return_value=True)
+    slide_controller.set_hide_mode_menu(narrow=True)
+
+    # THEN: the call to set the visible items on the toolbar should be correct
+    toolbar.set_widget_visible.assert_any_call(NARROW_MENU, True)
+    toolbar.set_widget_visible.assert_any_call(WIDE_MENU, False)
+
+
+def test_on_controller_size_changed_wide(settings):
+    """
+    Test that on_controller_size_changed
+    """
+    # GIVEN: A new SlideController instance where the toolbar has a lot of spare space.
+    slide_controller = SlideController(None)
+    slide_controller.is_live = True
+    slide_controller.ignore_toolbar_resize_events = False
+    slide_controller.controller = MagicMock(width=MagicMock(return_value=100))
+    slide_controller.toolbar = MagicMock(size=MagicMock(return_value=MagicMock(width=MagicMock(return_value=10))))
+    slide_controller.hide_menu = MagicMock(isVisible=MagicMock(return_value=False))
+    slide_controller.set_hide_mode_menu = MagicMock()
+
+    # WHEN: The on_controller_size_changed function is called
+    slide_controller.on_controller_size_changed()
+
+    # THEN: set_hide_mode_menu should have received the correct call
+    slide_controller.set_hide_mode_menu.assert_called_with(narrow=False)
+
+
+def test_on_controller_size_changed_narrow(settings):
+    """
+    Test that on_controller_size_changed
+    """
+    # GIVEN: A new SlideController instance where the toolbar has a lot of spare space.
+    slide_controller = SlideController(None)
+    slide_controller.is_live = True
+    slide_controller.ignore_toolbar_resize_events = False
+    slide_controller.controller = MagicMock(width=MagicMock(return_value=100))
+    slide_controller.toolbar = MagicMock(size=MagicMock(return_value=MagicMock(width=MagicMock(return_value=110))))
+    slide_controller.hide_menu = MagicMock(isVisible=MagicMock(return_value=False))
+    slide_controller.set_hide_mode_menu = MagicMock()
+
+    # WHEN: The on_controller_size_changed function is called
+    slide_controller.on_controller_size_changed()
+
+    # THEN: set_hide_mode_menu should have received the correct call
+    slide_controller.set_hide_mode_menu.assert_called_with(narrow=True)
+
+
+def test_on_controller_size_changed_can_not_expand(settings):
+    """
+    Test that on_controller_size_changed
+    """
+    # GIVEN: A new SlideController instance where the toolbar has a lot of spare space.
+    slide_controller = SlideController(None)
+    slide_controller.is_live = True
+    slide_controller.ignore_toolbar_resize_events = False
+    slide_controller.controller = MagicMock(width=MagicMock(return_value=100))
+    slide_controller.toolbar = MagicMock(size=MagicMock(return_value=MagicMock(width=MagicMock(return_value=95))))
+    slide_controller.hide_menu = MagicMock(isVisible=MagicMock(return_value=True))
+    slide_controller.set_hide_mode_menu = MagicMock()
+
+    # WHEN: The on_controller_size_changed function is called
+    slide_controller.on_controller_size_changed()
+
+    # THEN: set_hide_mode_menu should have received the correct call
+    slide_controller.set_hide_mode_menu.assert_not_called()
+
+
 def test_receive_spin_delay(mock_settings):
     """
     Test that the spin box is updated accordingly after a call to receive_spin_delay()