← Back to team overview

openlp-core team mailing list archive

[Merge] lp:~suutari-olli/openlp/escape-fixes-1294111-1497637 into lp:openlp

 

Azaziah has proposed merging lp:~suutari-olli/openlp/escape-fixes-1294111-1497637 into lp:openlp.

Requested reviews:
  Tim Bentley (trb143)
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-1294111-1497637/+merge/274652

- Escape item does not work unless “Live” has focus. https://bugs.launchpad.net/openlp/+bug/1294111
- Escape item does not work with Impress/PowerPoint. https://bugs.launchpad.net/openlp/+bug/1497637

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.

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

Whereas Blank to desktop, theme and black work everywhere 
within the program. The live_escape definition was moved to 
the same place, thus it works everywhere. Live_escape is not
a new feature, it is the default ESC-key behavior and part 
of OpenLP core features.

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.


New bug:
If PowerPoint/Impress presentation is shut with escape_item,clicking on it @ Live manager won’t
send it back to Live, it must be re-sent there.

This may be fixable by modifying:
def slide_selected(self, start=False):
#Starting from line 1073 of slidecontroller.py

Jenkins
https://ci.openlp.io/job/branch-01-pull/1135/console

https://ci.openlp.io/job/Branch-02-Functional-Tests/1058/console

https://ci.openlp.io/view/Branch/job/Branch-03-Interface-Tests/999/console

https://ci.openlp.io/view/Branch/job/Branch-04a-Windows_Functional_Tests/846/console

https://ci.openlp.io/view/Branch/job/Branch-04b-Windows_Interface_Tests/443/console

https://ci.openlp.io/view/Branch/job/Branch-05a-Code_Analysis/566/console

https://ci.openlp.io/view/Branch/job/Branch-05b-Test_Coverage/437/console
-- 
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/core/ui/slidecontroller.py'
--- openlp/core/ui/slidecontroller.py	2015-09-03 19:21:43 +0000
+++ openlp/core/ui/slidecontroller.py	2015-10-16 01:37:04 +0000
@@ -222,7 +222,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 = QtGui.QToolButton(self.toolbar)
             self.hide_menu.setObjectName('hide_menu')
             self.hide_menu.setText(translate('OpenLP.SlideController', 'Hide'))
@@ -244,10 +253,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 = QtGui.QToolButton(self.toolbar)
             self.blank_screen_button.setObjectName('blank_screen_button')
@@ -502,23 +516,6 @@
                                           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.
-        """
-        self.display.setVisible(False)
-        self.media_controller.media_stop(self)
-        # Stop looping if active
-        if self.play_slides_loop.isChecked():
-            self.on_play_slides_loop(False)
-        elif self.play_slides_once.isChecked():
-            self.on_play_slides_once(False)
 
     def toggle_display(self, action):
         """
@@ -1060,6 +1057,22 @@
             else:
                 Registry().execute('live_display_show')
 
+    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)
+        elif self.play_slides_once.isChecked():
+            self.on_play_slides_once(False)
+
     def on_slide_selected(self, field=None):
         """
         Slide selected in controller

=== modified file 'tests/functional/openlp_core_ui/test_slidecontroller.py'
--- tests/functional/openlp_core_ui/test_slidecontroller.py	2015-09-09 13:45:57 +0000
+++ tests/functional/openlp_core_ui/test_slidecontroller.py	2015-10-16 01:37:04 +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,18 +223,26 @@
         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):
         """
-        Test that when the on_go_live() method is called the message is sent to the live controller and focus is
-        set correctly.
+        Test that when the on_go_live() method is called the message is
+        sent to the live controller and focus is set correctly
         """
         # GIVEN: A new SlideController instance and plugin preview then pressing go live should respond
         mocked_display = MagicMock()


Follow ups