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

Proposed by Azaziah
Status: Superseded
Proposed branch: lp:~suutari-olli/openlp/escape-fixes-1294111-1497637
Merge into: lp:openlp
Diff against target: 128 lines (+44/-23)
2 files modified
openlp/core/ui/slidecontroller.py (+31/-18)
tests/functional/openlp_core_ui/test_slidecontroller.py (+13/-5)
To merge this branch: bzr merge lp:~suutari-olli/openlp/escape-fixes-1294111-1497637
Reviewer Review Type Date Requested Status
Tim Bentley Pending
Review via email: mp+274649@code.launchpad.net

This proposal supersedes a proposal from 2015-10-12.

Description of the change

- 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

To post a comment you must log in.
Revision history for this message
Tim Bentley (trb143) wrote : Posted in a previous version of this proposal

See below.
Should have a new test not just a fix to an existing one.

review: Needs Fixing
2566. By Azaziah

Added back end part of one comment that was previously deleted.

2567. By Azaziah

Merge to trunk on 1/7/2015.

I noticed this branch also seems to be fixing this bug:
https://bugs.launchpad.net/openlp/+bug/1531691

However, escape item still remains buggy with problems related to resuming
video and presentations.

2568. By Azaziah

Merge to trunk on 7.1.2015
I noticed this branch also seems to fix this bug:
https://bugs.launchpad.net/openlp/+bug/1531691

However, escape item still remains buggy with problems
related to resuming Live from video or presentations.

Unmerged revisions

2568. By Azaziah

Merge to trunk on 7.1.2015
I noticed this branch also seems to fix this bug:
https://bugs.launchpad.net/openlp/+bug/1531691

However, escape item still remains buggy with problems
related to resuming Live from video or presentations.

2567. By Azaziah

Merge to trunk on 1/7/2015.

I noticed this branch also seems to be fixing this bug:
https://bugs.launchpad.net/openlp/+bug/1531691

However, escape item still remains buggy with problems related to resuming
video and presentations.

2566. By Azaziah

Added back end part of one comment that was previously deleted.

2565. By Azaziah

Noticed I made one comment duplicate.

2564. By Azaziah

Made 1 line 3 chars shorter to fit 120.

2563. By Azaziah

Improved test for live_escape + Comments + Fixes

2562. By Azaziah

Edited out one comment, removed one empty line on test.

2561. By Azaziah

Reverted some useless changes I noticed.

2560. By Azaziah

Done some cleanup for proper merging part 7.

2559. By Azaziah

Done some cleanup for proper merging part 6.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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:13:11 +0000
@@ -222,7 +222,16 @@
222 self.controller_type = DisplayControllerType.Preview222 self.controller_type = DisplayControllerType.Preview
223 if self.is_live:223 if self.is_live:
224 self.controller_type = DisplayControllerType.Live224 self.controller_type = DisplayControllerType.Live
225 # Hide Menu225 '''
226 Hide Menu
227 blank_screen = Blank display to black
228 theme_screen = Blank to theme
229 desktop_screen = Blank to desktop
230 escape_item = Escape item hides what is in live,
231 just like desktop screen but it does not require un-blanking
232 Default hotkey for escape item is "Esc"
233 The actual definitions on what these do are found further in code
234 '''
226 self.hide_menu = QtGui.QToolButton(self.toolbar)235 self.hide_menu = QtGui.QToolButton(self.toolbar)
227 self.hide_menu.setObjectName('hide_menu')236 self.hide_menu.setObjectName('hide_menu')
228 self.hide_menu.setText(translate('OpenLP.SlideController', 'Hide'))237 self.hide_menu.setText(translate('OpenLP.SlideController', 'Hide'))
@@ -244,10 +253,15 @@
244 icon=':/slides/slide_desktop.png',253 icon=':/slides/slide_desktop.png',
245 checked=False, can_shortcuts=True, category=self.category,254 checked=False, can_shortcuts=True, category=self.category,
246 triggers=self.on_hide_display)255 triggers=self.on_hide_display)
256 self.escape_item = create_action(self, 'escapeItem',
257 text=translate('OpenLP.SlideController', 'Escape Item'),
258 checked=False, can_shortcuts=True, category=self.category,
259 triggers=self.live_escape)
247 self.hide_menu.setDefaultAction(self.blank_screen)260 self.hide_menu.setDefaultAction(self.blank_screen)
248 self.hide_menu.menu().addAction(self.blank_screen)261 self.hide_menu.menu().addAction(self.blank_screen)
249 self.hide_menu.menu().addAction(self.theme_screen)262 self.hide_menu.menu().addAction(self.theme_screen)
250 self.hide_menu.menu().addAction(self.desktop_screen)263 self.hide_menu.menu().addAction(self.desktop_screen)
264 self.hide_menu.menu().addAction(self.escape_item)
251 # Wide menu of display control buttons.265 # Wide menu of display control buttons.
252 self.blank_screen_button = QtGui.QToolButton(self.toolbar)266 self.blank_screen_button = QtGui.QToolButton(self.toolbar)
253 self.blank_screen_button.setObjectName('blank_screen_button')267 self.blank_screen_button.setObjectName('blank_screen_button')
@@ -502,23 +516,6 @@
502 can_shortcuts=True, context=QtCore.Qt.WidgetWithChildrenShortcut,516 can_shortcuts=True, context=QtCore.Qt.WidgetWithChildrenShortcut,
503 category=self.category,517 category=self.category,
504 triggers=self.service_next)518 triggers=self.service_next)
505 self.escape_item = create_action(parent, 'escapeItem',
506 text=translate('OpenLP.SlideController', 'Escape Item'),
507 can_shortcuts=True, context=QtCore.Qt.WidgetWithChildrenShortcut,
508 category=self.category,
509 triggers=self.live_escape)
510
511 def live_escape(self, field=None):
512 """
513 If you press ESC on the live screen it should close the display temporarily.
514 """
515 self.display.setVisible(False)
516 self.media_controller.media_stop(self)
517 # Stop looping if active
518 if self.play_slides_loop.isChecked():
519 self.on_play_slides_loop(False)
520 elif self.play_slides_once.isChecked():
521 self.on_play_slides_once(False)
522519
523 def toggle_display(self, action):520 def toggle_display(self, action):
524 """521 """
@@ -1060,6 +1057,22 @@
1060 else:1057 else:
1061 Registry().execute('live_display_show')1058 Registry().execute('live_display_show')
10621059
1060 def live_escape(self, checked=None):
1061 """
1062 Pressing ESC (default) triggers live_escape and hides current Live item.
1063 """
1064 # This will hide songs, media etc.. but not Impress/PowerPoint.
1065 self.display.setVisible(False)
1066 self.media_controller.media_stop(self)
1067 # This will stop any Impress/PowerPoint in Live.
1068 if self.service_item is not None:
1069 Registry().execute('%s_hide' % self.service_item.name.lower(), [self.service_item, self.is_live])
1070 # Stop looping if active
1071 if self.play_slides_loop.isChecked():
1072 self.on_play_slides_loop(False)
1073 elif self.play_slides_once.isChecked():
1074 self.on_play_slides_once(False)
1075
1063 def on_slide_selected(self, field=None):1076 def on_slide_selected(self, field=None):
1064 """1077 """
1065 Slide selected in controller1078 Slide selected in controller
10661079
=== 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:13:11 +0000
@@ -212,7 +212,7 @@
212 """212 """
213 Test that when the live_escape() method is called, the display is set to invisible and any media is stopped213 Test that when the live_escape() method is called, the display is set to invisible and any media is stopped
214 """214 """
215 # GIVEN: A new SlideController instance and mocked out display and media_controller215 # GIVEN: A new SlideController instance, mocked out display, media_controller and service_item
216 mocked_display = MagicMock()216 mocked_display = MagicMock()
217 mocked_media_controller = MagicMock()217 mocked_media_controller = MagicMock()
218 Registry.create()218 Registry.create()
@@ -223,18 +223,26 @@
223 play_slides.isChecked.return_value = False223 play_slides.isChecked.return_value = False
224 slide_controller.play_slides_loop = play_slides224 slide_controller.play_slides_loop = play_slides
225 slide_controller.play_slides_once = play_slides225 slide_controller.play_slides_once = play_slides
226 service_item = MagicMock()
227 toolbar = MagicMock()
228 toolbar.set_widget_visible = MagicMock()
229 slide_controller.toolbar = toolbar
230 slide_controller.service_item = service_item
231 slide_controller.service_item.is_text = MagicMock(return_value=False)
226232
227 # WHEN: live_escape() is called233 # WHEN: live_escape() is called, set_blank simulates the part where Impress/PowerPoint is closed.
228 slide_controller.live_escape()234 slide_controller.live_escape()
235 slide_controller.set_blank_menu()
229236
230 # THEN: the display should be set to invisible and the media controller stopped237 # THEN: the display should be set to invisible, media controller stopped and any service_item on live hidden.
231 mocked_display.setVisible.assert_called_once_with(False)238 mocked_display.setVisible.assert_called_once_with(False)
232 mocked_media_controller.media_stop.assert_called_once_with(slide_controller)239 mocked_media_controller.media_stop.assert_called_once_with(slide_controller)
240 toolbar.set_widget_visible.assert_called_with(NON_TEXT_MENU, True)
233241
234 def on_go_live_live_controller_test(self):242 def on_go_live_live_controller_test(self):
235 """243 """
236 Test that when the on_go_live() method is called the message is sent to the live controller and focus is244 Test that when the on_go_live() method is called the message is
237 set correctly.245 sent to the live controller and focus is set correctly
238 """246 """
239 # GIVEN: A new SlideController instance and plugin preview then pressing go live should respond247 # GIVEN: A new SlideController instance and plugin preview then pressing go live should respond
240 mocked_display = MagicMock()248 mocked_display = MagicMock()