openlp-core team mailing list archive

[Azaziah <suutari.olli@xxxxxxxxx>]

Azaziah has proposed merging lp:~suutari-olli/openlp/escape-fixes-jan-2016-qt5 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1294111 in OpenLP: ""Escape Item" needs focus on the live controller verse list"
  https://bugs.launchpad.net/openlp/+bug/1294111
  Bug #1338033 in OpenLP: "Esc doesn't close the presentation when started from Servicemanager"
  https://bugs.launchpad.net/openlp/+bug/1338033
  Bug #1497637 in OpenLP: "Escape item does not work with .ppt or .odp"
  https://bugs.launchpad.net/openlp/+bug/1497637

For more details, see:
https://code.launchpad.net/~suutari-olli/openlp/escape-fixes-jan-2016-qt5/+merge/282239

This branch fixes the following issues:
- Escape item does not work unless Live has focus
- Escape item does not work with PowerPoint or Impress

Focus issue was fixed by moving the escape item
definitions away from set_live_hot_keys to the same
section where definitions for blank to … are located.

Old definition was under set_live_hot_keys and thus,
it only worked when "Live" had focus.
For an example using escape item won’t work while
Service manager or Library has focus.

Impress/PowerPoint issue was fixed by
using script to hide them.

test_slidecontroller.py was modified so it is compatible
with the service_item which is used to determine if
Impress/PowerPoint presentations are running.

Bugs that this branch does not fix:
https://bugs.launchpad.net/openlp/+bug/1531691
Song gets sent back to live if edited if it is blanked with escape item.
https://bugs.launchpad.net/openlp/+bug/1514155
Videos only play audio if resumed after stopping the playback with Escape item
New bug: Clicking on a PPT/Impress slide on Live won't resume 
playback after Escape item, the presentation has to be re-loaded
by sending it back to live again.

[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1224/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1168/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1107/
[FAILURE] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/994/
Do note that this test was broken at the time being.
-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~suutari-olli/openlp/escape-fixes-jan-2016-qt5 into lp:openlp.

=== modified file 'openlp/core/ui/slidecontroller.py'
--- openlp/core/ui/slidecontroller.py	2015-12-31 22:46:06 +0000
+++ openlp/core/ui/slidecontroller.py	2016-01-11 23:34:40 +0000
@@ -223,7 +223,16 @@
         self.controller_type = DisplayControllerType.Preview
         if self.is_live:
             self.controller_type = DisplayControllerType.Live
-            # Hide Menu
+            '''
+            Hide Menu
+            blank_screen = Blank display to black
+            theme_screen = Blank to theme
+            desktop_screen = Blank to desktop
+            escape_item = Escape item hides what is in live,
+            just like desktop screen but it does not require un-blanking
+            Default hotkey for escape item is "Esc"
+            The actual definitions on what these do are found further in code
+            '''
             self.hide_menu = QtWidgets.QToolButton(self.toolbar)
             self.hide_menu.setObjectName('hide_menu')
             self.hide_menu.setText(translate('OpenLP.SlideController', 'Hide'))
@@ -245,10 +254,15 @@
                                                 icon=':/slides/slide_desktop.png',
                                                 checked=False, can_shortcuts=True, category=self.category,
                                                 triggers=self.on_hide_display)
+            self.escape_item = create_action(self, 'escapeItem',
+                                             text=translate('OpenLP.SlideController', 'Escape Item'),
+                                             checked=False, can_shortcuts=True, category=self.category,
+                                             triggers=self.live_escape)
             self.hide_menu.setDefaultAction(self.blank_screen)
             self.hide_menu.menu().addAction(self.blank_screen)
             self.hide_menu.menu().addAction(self.theme_screen)
             self.hide_menu.menu().addAction(self.desktop_screen)
+            self.hide_menu.menu().addAction(self.escape_item)
             # Wide menu of display control buttons.
             self.blank_screen_button = QtWidgets.QToolButton(self.toolbar)
             self.blank_screen_button.setObjectName('blank_screen_button')
@@ -502,18 +516,17 @@
                                           can_shortcuts=True, context=QtCore.Qt.WidgetWithChildrenShortcut,
                                           category=self.category,
                                           triggers=self.service_next)
-        self.escape_item = create_action(parent, 'escapeItem',
-                                         text=translate('OpenLP.SlideController', 'Escape Item'),
-                                         can_shortcuts=True, context=QtCore.Qt.WidgetWithChildrenShortcut,
-                                         category=self.category,
-                                         triggers=self.live_escape)
 
-    def live_escape(self, field=None):
-        """
-        If you press ESC on the live screen it should close the display temporarily.
-        """
+    def live_escape(self, checked=None):
+        """
+        Pressing ESC (default) triggers live_escape and hides current Live item.
+        """
+        # This will hide songs, media etc.. but not Impress/PowerPoint.
         self.display.setVisible(False)
         self.media_controller.media_stop(self)
+        # This will stop any Impress/PowerPoint in Live.
+        if self.service_item is not None:
+            Registry().execute('%s_hide' % self.service_item.name.lower(), [self.service_item, self.is_live])
         # Stop looping if active
         if self.play_slides_loop.isChecked():
             self.on_play_slides_loop(False)

=== modified file 'tests/functional/openlp_core_ui/test_slidecontroller.py'
--- tests/functional/openlp_core_ui/test_slidecontroller.py	2015-12-31 22:46:06 +0000
+++ tests/functional/openlp_core_ui/test_slidecontroller.py	2016-01-11 23:34:40 +0000
@@ -212,7 +212,7 @@
         """
         Test that when the live_escape() method is called, the display is set to invisible and any media is stopped
         """
-        # GIVEN: A new SlideController instance and mocked out display and media_controller
+        # GIVEN: A new SlideController instance, mocked out display, media_controller and service_item
         mocked_display = MagicMock()
         mocked_media_controller = MagicMock()
         Registry.create()
@@ -223,13 +223,21 @@
         play_slides.isChecked.return_value = False
         slide_controller.play_slides_loop = play_slides
         slide_controller.play_slides_once = play_slides
+        service_item = MagicMock()
+        toolbar = MagicMock()
+        toolbar.set_widget_visible = MagicMock()
+        slide_controller.toolbar = toolbar
+        slide_controller.service_item = service_item
+        slide_controller.service_item.is_text = MagicMock(return_value=False)
 
-        # WHEN: live_escape() is called
+        # WHEN: live_escape() is called, set_blank simulates the part where Impress/PowerPoint is closed.
         slide_controller.live_escape()
+        slide_controller.set_blank_menu()
 
-        # THEN: the display should be set to invisible and the media controller stopped
+        # THEN: the display should be set to invisible, media controller stopped and any service_item on live hidden.
         mocked_display.setVisible.assert_called_once_with(False)
         mocked_media_controller.media_stop.assert_called_once_with(slide_controller)
+        toolbar.set_widget_visible.assert_called_with(NON_TEXT_MENU, True)
 
     def on_go_live_live_controller_test(self):
         """