openlp-core team mailing list archive

[Jonathan Springer <springermac@xxxxxxxxx>]

Jonathan Springer has proposed merging lp:~springermac/openlp/fix-ups into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)

For more details, see:
https://code.launchpad.net/~springermac/openlp/fix-ups/+merge/218827

Change how we define which menu items are included in the main menu on Mac OS
Change variable type to conform to expected type
Make vlcplayer stop() call asynchronous to avoid a deadlock with the UI thread
Add test for the main window function set_service_modified
-- 
https://code.launchpad.net/~springermac/openlp/fix-ups/+merge/218827
Your team OpenLP Core is requested to review the proposed merge of lp:~springermac/openlp/fix-ups into lp:openlp.

=== modified file 'openlp/core/ui/mainwindow.py'
--- openlp/core/ui/mainwindow.py	2014-05-03 15:01:43 +0000
+++ openlp/core/ui/mainwindow.py	2014-05-08 15:19:04 +0000
@@ -320,14 +320,14 @@
         # i18n add Language Actions
         add_actions(self.settings_language_menu, (self.auto_language_item, None))
         add_actions(self.settings_language_menu, self.language_group.actions())
-        # Order things differently in OS X so that Preferences menu item in the
-        # app menu is correct (this gets picked up automatically by Qt).
+        # Qt on OS X looks for keywords in the menu items title to determine which menu items get added to the main
+        # menu. If we are running on Mac OS X the menu items whose title contains those keywords but don't belong in the
+        # main menu need to be marked as such with QAction.NoRole.
         if sys.platform == 'darwin':
-            add_actions(self.settings_menu, (self.settings_plugin_list_item, self.settings_language_menu.menuAction(),
-                        None, self.settings_configure_item, self.settings_shortcuts_item, self.formatting_tag_item))
-        else:
-            add_actions(self.settings_menu, (self.settings_plugin_list_item, self.settings_language_menu.menuAction(),
-                        None, self.formatting_tag_item, self.settings_shortcuts_item, self.settings_configure_item))
+            self.settings_shortcuts_item.setMenuRole(QtGui.QAction.NoRole)
+            self.formatting_tag_item.setMenuRole(QtGui.QAction.NoRole)
+        add_actions(self.settings_menu, (self.settings_plugin_list_item, self.settings_language_menu.menuAction(),
+                    None, self.formatting_tag_item, self.settings_shortcuts_item, self.settings_configure_item))
         add_actions(self.tools_menu, (self.tools_add_tool_item, None))
         add_actions(self.tools_menu, (self.tools_open_data_folder, None))
         add_actions(self.tools_menu, (self.tools_first_time_wizard, None))

=== modified file 'openlp/core/ui/media/vlcplayer.py'
--- openlp/core/ui/media/vlcplayer.py	2014-03-20 19:10:31 +0000
+++ openlp/core/ui/media/vlcplayer.py	2014-05-08 15:19:04 +0000
@@ -34,6 +34,7 @@
 import logging
 import os
 import sys
+import threading
 
 from PyQt4 import QtGui
 
@@ -207,7 +208,7 @@
         start_time = 0
         if self.state != MediaState.Paused and controller.media_info.start_time > 0:
             start_time = controller.media_info.start_time
-        display.vlc_media_player.play()
+        threading.Thread(target=display.vlc_media_player.play).start()
         if not self.media_state_wait(display, vlc.State.Playing):
             return False
         self.volume(display, controller.media_info.volume)
@@ -233,7 +234,7 @@
         """
         Stop the current item
         """
-        display.vlc_media_player.stop()
+        threading.Thread(target=display.vlc_media_player.stop).start()
         self.state = MediaState.Stopped
 
     def volume(self, display, vol):

=== modified file 'openlp/plugins/images/lib/mediaitem.py'
--- openlp/plugins/images/lib/mediaitem.py	2014-04-20 12:06:02 +0000
+++ openlp/plugins/images/lib/mediaitem.py	2014-05-08 15:19:04 +0000
@@ -353,7 +353,7 @@
                     icon = build_icon(thumb)
                 else:
                     icon = create_thumb(imageFile.filename, thumb)
-            item_name = QtGui.QTreeWidgetItem(filename)
+            item_name = QtGui.QTreeWidgetItem([filename])
             item_name.setText(0, filename)
             item_name.setIcon(0, icon)
             item_name.setToolTip(0, imageFile.filename)

=== modified file 'tests/functional/openlp_core_ui/test_firsttimeform.py'
--- tests/functional/openlp_core_ui/test_firsttimeform.py	2014-04-15 18:44:33 +0000
+++ tests/functional/openlp_core_ui/test_firsttimeform.py	2014-05-08 15:19:04 +0000
@@ -31,11 +31,11 @@
 """
 from unittest import TestCase
 
+from openlp.core.common import Registry
+from openlp.core.ui.firsttimeform import FirstTimeForm
+
 from tests.functional import MagicMock
-
 from tests.helpers.testmixin import TestMixin
-from openlp.core.common import Registry
-from openlp.core.ui.firsttimeform import FirstTimeForm
 
 
 class TestFirstTimeForm(TestCase, TestMixin):

=== modified file 'tests/functional/openlp_core_ui/test_mainwindow.py'
--- tests/functional/openlp_core_ui/test_mainwindow.py	2014-04-08 20:45:40 +0000
+++ tests/functional/openlp_core_ui/test_mainwindow.py	2014-05-08 15:19:04 +0000
@@ -34,6 +34,7 @@
 from unittest import TestCase
 
 from openlp.core.ui.mainwindow import MainWindow
+from openlp.core.lib.ui import UiStrings
 from openlp.core.common.registry import Registry
 from tests.utils.constants import TEST_RESOURCES_PATH
 from tests.helpers.testmixin import TestMixin
@@ -95,3 +96,41 @@
 
             # THEN the file should not be opened
             assert not mocked_load_path.called, 'load_path should not have been called'
+
+    def main_window_title_test(self):
+        """
+        Test that running a new instance of OpenLP set the window title correctly
+        """
+        # GIVEN a newly opened OpenLP instance
+
+        # WHEN no changes are made to the service
+
+        # THEN the main window's title shoud be the same as the OLPV2x string in the UiStrings class
+        self.assertEqual(self.main_window.windowTitle(), UiStrings().OLPV2x,
+                         'The main window\'s title should be the same as the OLPV2x string in UiStrings class')
+
+    def set_service_modifed_test(self):
+        """
+        Test that when setting the service's title the main window's title is set correctly
+        """
+        # GIVEN a newly opened OpenLP instance
+
+        # WHEN set_service_modified is called with with the modified flag set true and a file name
+        self.main_window.set_service_modified(True, 'test.osz')
+
+        # THEN the main window's title should be set to the
+        self.assertEqual(self.main_window.windowTitle(), '%s - %s*' % (UiStrings().OLPV2x, 'test.osz'),
+                         'The main window\'s title should be set to "<the contents of UiStrings().OLPV2x> - test.osz*"')
+
+    def set_service_unmodified_test(self):
+        """
+        Test that when setting the service's title the main window's title is set correctly
+        """
+        # GIVEN a newly opened OpenLP instance
+
+        # WHEN set_service_modified is called with with the modified flag set False and a file name
+        self.main_window.set_service_modified(False, 'test.osz')
+
+        # THEN the main window's title should be set to the
+        self.assertEqual(self.main_window.windowTitle(), '%s - %s' % (UiStrings().OLPV2x, 'test.osz'),
+                         'The main window\'s title should be set to "<the contents of UiStrings().OLPV2x> - test.osz"')