Adds automatic functionality to the max-height for non-text items feature, and sets this as the default value.

Advanced tab spin box replaced with combo box.

Test coverage expanded on other functions in listpreviewwidget and pep8 errors fixed.
Fixed test naming.
Merged with trunk as of 2016-06-02_10:30.

lp:~knightrider0xd/openlp/non-text-item-height-default (revision 2674)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1584/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1...

bzr-revno: 2674

openlp/core/common/settings.py

@@ -129,7 +129,7 @@ class Settings(QtCore.QSettings):
         'advanced/recent file count': 4,
         'advanced/save current plugin': False,
         'advanced/slide limits': SlideLimits.End,
-        'advanced/slide max height': 0,
+        'advanced/slide max height': -4,
         'advanced/single click preview': False,
         'advanced/single click service preview': False,
         'advanced/x11 bypass wm': X11_BYPASS_DEFAULT,

openlp/core/ui/advancedtab.py

@@ -87,11 +87,14 @@ class AdvancedTab(SettingsTab):
         self.ui_layout.addRow(self.expand_service_item_check_box)
         self.slide_max_height_label = QtWidgets.QLabel(self.ui_group_box)
         self.slide_max_height_label.setObjectName('slide_max_height_label')
-        self.slide_max_height_spin_box = QtWidgets.QSpinBox(self.ui_group_box)
+        self.slide_max_height_combo_box = QtWidgets.QComboBox(self.ui_group_box)
-        self.slide_max_height_spin_box.setObjectName('slide_max_height_spin_box')
+        self.slide_max_height_combo_box.addItem('', userData=0)
-        self.slide_max_height_spin_box.setRange(0, 1000)
+        self.slide_max_height_combo_box.addItem('', userData=-4)
-        self.slide_max_height_spin_box.setSingleStep(20)
+        # Generate numeric values for combo box dynamically
-        self.ui_layout.addRow(self.slide_max_height_label, self.slide_max_height_spin_box)
+        for px in range(60, 801, 5):
+            self.slide_max_height_combo_box.addItem(str(px) + 'px', userData=px)
+        self.slide_max_height_combo_box.setObjectName('slide_max_height_combo_box')
+        self.ui_layout.addRow(self.slide_max_height_label, self.slide_max_height_combo_box)
         self.autoscroll_label = QtWidgets.QLabel(self.ui_group_box)
         self.autoscroll_label.setObjectName('autoscroll_label')
         self.autoscroll_combo_box = QtWidgets.QComboBox(self.ui_group_box)
@@ -265,7 +268,8 @@ class AdvancedTab(SettingsTab):
                                                              'Expand new service items on creation'))
         self.slide_max_height_label.setText(translate('OpenLP.AdvancedTab',
                                                       'Max height for non-text slides\nin slide controller:'))
-        self.slide_max_height_spin_box.setSpecialValueText(translate('OpenLP.AdvancedTab', 'Disabled'))
+        self.slide_max_height_combo_box.setItemText(0, translate('OpenLP.AdvancedTab', 'Disabled'))
+        self.slide_max_height_combo_box.setItemText(1, translate('OpenLP.AdvancedTab', 'Automatic'))
         self.autoscroll_label.setText(translate('OpenLP.AdvancedTab',
                                                 'When changing slides:'))
         self.autoscroll_combo_box.setItemText(0, translate('OpenLP.AdvancedTab', 'Do not auto-scroll'))
@@ -355,10 +359,13 @@ class AdvancedTab(SettingsTab):
         self.single_click_preview_check_box.setChecked(settings.value('single click preview'))
         self.single_click_service_preview_check_box.setChecked(settings.value('single click service preview'))
         self.expand_service_item_check_box.setChecked(settings.value('expand service item'))
-        self.slide_max_height_spin_box.setValue(settings.value('slide max height'))
+        slide_max_height_value = settings.value('slide max height')
+        for i in range(0, self.slide_max_height_combo_box.count()):
+            if self.slide_max_height_combo_box.itemData(i) == slide_max_height_value:
+                self.slide_max_height_combo_box.setCurrentIndex(i)
         autoscroll_value = settings.value('autoscrolling')
         for i in range(0, len(self.autoscroll_map)):
-            if self.autoscroll_map[i] == autoscroll_value:
+            if self.autoscroll_map[i] == autoscroll_value and i < self.autoscroll_combo_box.count():
                 self.autoscroll_combo_box.setCurrentIndex(i)
         self.enable_auto_close_check_box.setChecked(settings.value('enable exit confirmation'))
         self.hide_mouse_check_box.setChecked(settings.value('hide mouse'))
@@ -439,7 +446,9 @@ class AdvancedTab(SettingsTab):
         settings.setValue('single click preview', self.single_click_preview_check_box.isChecked())
         settings.setValue('single click service preview', self.single_click_service_preview_check_box.isChecked())
         settings.setValue('expand service item', self.expand_service_item_check_box.isChecked())
-        settings.setValue('slide max height', self.slide_max_height_spin_box.value())
+        slide_max_height_index = self.slide_max_height_combo_box.currentIndex()
+        slide_max_height_value = self.slide_max_height_combo_box.itemData(slide_max_height_index)
+        settings.setValue('slide max height', slide_max_height_value)
         settings.setValue('autoscrolling', self.autoscroll_map[self.autoscroll_combo_box.currentIndex()])
         settings.setValue('enable exit confirmation', self.enable_auto_close_check_box.isChecked())
         settings.setValue('hide mouse', self.hide_mouse_check_box.isChecked())

openlp/core/ui/lib/listpreviewwidget.py

@@ -63,6 +63,7 @@ class ListPreviewWidget(QtWidgets.QTableWidget, RegistryProperties):
         # Initialize variables.
         self.service_item = ServiceItem()
         self.screen_ratio = screen_ratio
+        self.auto_row_height = 100
         # Connect signals
         self.verticalHeader().sectionResized.connect(self.row_resized)
 
@@ -87,8 +88,14 @@ class ListPreviewWidget(QtWidgets.QTableWidget, RegistryProperties):
                 height = self.viewport().width() // self.screen_ratio
                 max_img_row_height = Settings().value('advanced/slide max height')
                 # Adjust for row height cap if in use.
-                if isinstance(max_img_row_height, int) and max_img_row_height > 0 and height > max_img_row_height:
+                if isinstance(max_img_row_height, int):
+                    if max_img_row_height > 0 and height > max_img_row_height:
                         height = max_img_row_height
+                    elif max_img_row_height < 0:
+                        # If auto setting, show that number of slides, or if the resulting slides too small, 100px.
+                        # E.g. If setting is -4, 4 slides will be visible, unless those slides are < 100px high.
+                        self.auto_row_height = max(self.viewport().height() / (-1 * max_img_row_height), 100)
+                        height = min(height, self.auto_row_height)
                 # Apply new height to slides
                 for frame_number in range(len(self.service_item.get_frames())):
                     self.setRowHeight(frame_number, height)
@@ -99,7 +106,7 @@ class ListPreviewWidget(QtWidgets.QTableWidget, RegistryProperties):
         """
         # Only for non-text slides when row height cap in use
         max_img_row_height = Settings().value('advanced/slide max height')
-        if self.service_item.is_text() or not isinstance(max_img_row_height, int) or max_img_row_height <= 0:
+        if self.service_item.is_text() or not isinstance(max_img_row_height, int) or max_img_row_height == 0:
             return
         # Get and validate label widget containing slide & adjust max width
         try:
@@ -165,11 +172,15 @@ class ListPreviewWidget(QtWidgets.QTableWidget, RegistryProperties):
                 slide_height = width // self.screen_ratio
                 # Setup and validate row height cap if in use.
                 max_img_row_height = Settings().value('advanced/slide max height')
-                if isinstance(max_img_row_height, int) and max_img_row_height > 0:
+                if isinstance(max_img_row_height, int) and max_img_row_height != 0:
-                    if slide_height > max_img_row_height:
+                    if max_img_row_height > 0 and slide_height > max_img_row_height:
+                        # Manual Setting
                         slide_height = max_img_row_height
-                    label.setMaximumWidth(max_img_row_height * self.screen_ratio)
+                    elif max_img_row_height < 0 and slide_height > self.auto_row_height:
-                    label.resize(max_img_row_height * self.screen_ratio, max_img_row_height)
+                        # Auto Setting
+                        slide_height = self.auto_row_height
+                    label.setMaximumWidth(slide_height * self.screen_ratio)
+                    label.resize(slide_height * self.screen_ratio, slide_height)
                     # Build widget with stretch padding
                     container = QtWidgets.QWidget()
                     hbox = QtWidgets.QHBoxLayout()

tests/functional/openlp_core_common/test_registryproperties.py

@@ -75,4 +75,3 @@ class TestRegistryProperties(TestCase, RegistryProperties):
 
         # THEN the application should be none
         self.assertEqual(self.application, application, 'The application value should match')
-

tests/functional/openlp_core_ui_lib/test_listpreviewwidget.py

@@ -225,6 +225,44 @@ class TestListPreviewWidget(TestCase):
         calls = [call(0, 100), call(1, 100), call(0, 100), call(1, 100)]
         mocked_setRowHeight.assert_has_calls(calls)
 
+    @patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.resizeRowsToContents')
+    @patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.setRowHeight')
+    def test_replace_recalculate_layout_img_auto(self, mocked_setRowHeight, mocked_resizeRowsToContents):
+        """
+        Test if "Max height for non-text slides..." auto, img slides resized in replace_service_item & __recalc...
+        """
+        # GIVEN: A setting to adjust "Max height for non-text slides in slide controller",
+        #        an image ServiceItem and a ListPreviewWidget.
+
+        # Mock Settings().value('advanced/slide max height')
+        self.mocked_Settings_obj.value.return_value = -4
+        # Mock self.viewport().width()
+        self.mocked_viewport_obj.width.return_value = 200
+        self.mocked_viewport_obj.height.return_value = 600
+        # Mock image service item
+        service_item = MagicMock()
+        service_item.is_text.return_value = False
+        service_item.is_capable.return_value = False
+        service_item.get_frames.return_value = [{'title': None, 'path': None, 'image': None},
+                                                {'title': None, 'path': None, 'image': None}]
+        # init ListPreviewWidget and load service item
+        list_preview_widget = ListPreviewWidget(None, 1)
+        list_preview_widget.replace_service_item(service_item, 200, 0)
+        # Change viewport width before forcing a resize
+        self.mocked_viewport_obj.width.return_value = 400
+
+        # WHEN: __recalculate_layout() is called (via screen_size_changed)
+        list_preview_widget.screen_size_changed(1)
+        self.mocked_viewport_obj.height.return_value = 200
+        list_preview_widget.screen_size_changed(1)
+
+        # THEN: resizeRowsToContents() should not be called, while setRowHeight() should be called
+        #       twice for each slide.
+        self.assertEquals(mocked_resizeRowsToContents.call_count, 0, 'Should not be called')
+        self.assertEquals(mocked_setRowHeight.call_count, 6, 'Should be called 3 times for each slide')
+        calls = [call(0, 100), call(1, 100), call(0, 150), call(1, 150), call(0, 100), call(1, 100)]
+        mocked_setRowHeight.assert_has_calls(calls)
+
     @patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.resizeRowsToContents')
     @patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.setRowHeight')
     @patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.cellWidget')
@@ -331,6 +369,41 @@ class TestListPreviewWidget(TestCase):
         # THEN: self.cellWidget(row, 0).children()[1].setMaximumWidth() should be called
         mocked_cellWidget_child.setMaximumWidth.assert_called_once_with(150)
 
+    @patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.resizeRowsToContents')
+    @patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.setRowHeight')
+    @patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.cellWidget')
+    def test_row_resized_setting_changed(self, mocked_cellWidget, mocked_setRowHeight, mocked_resizeRowsToContents):
+        """
+        Test if "Max height for non-text slides..." enabled while item live, program doesn't crash on row_resized.
+        """
+        # GIVEN: A setting to adjust "Max height for non-text slides in slide controller",
+        #        an image ServiceItem and a ListPreviewWidget.
+
+        # Mock Settings().value('advanced/slide max height')
+        self.mocked_Settings_obj.value.return_value = 0
+        # Mock self.viewport().width()
+        self.mocked_viewport_obj.width.return_value = 200
+        # Mock image service item
+        service_item = MagicMock()
+        service_item.is_text.return_value = False
+        service_item.is_capable.return_value = False
+        service_item.get_frames.return_value = [{'title': None, 'path': None, 'image': None},
+                                                {'title': None, 'path': None, 'image': None}]
+        # Mock self.cellWidget().children()
+        mocked_cellWidget_obj = MagicMock()
+        mocked_cellWidget_obj.children.return_value = None
+        mocked_cellWidget.return_value = mocked_cellWidget_obj
+        # init ListPreviewWidget and load service item
+        list_preview_widget = ListPreviewWidget(None, 1)
+        list_preview_widget.replace_service_item(service_item, 200, 0)
+        self.mocked_Settings_obj.value.return_value = 100
+
+        # WHEN: row_resized() is called
+        list_preview_widget.row_resized(0, 100, 150)
+
+        # THEN: self.cellWidget(row, 0).children()[1].setMaximumWidth() should fail
+        self.assertRaises(Exception)
+
     @patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.selectRow')
     @patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.scrollToItem')
     @patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.item')

tests/functional/openlp_plugins/bibles/test_lib.py

@@ -43,6 +43,7 @@ class TestLib(TestCase):
         separators = {'sep_r': '\\s*(?:e)\\s*', 'sep_e_default': 'end', 'sep_v_display': 'w', 'sep_l_display': 'r',
                       'sep_v_default': ':|v|V|verse|verses', 'sep_l': '\\s*(?:r)\\s*', 'sep_l_default': ',|and',
                       'sep_e': '\\s*(?:t)\\s*', 'sep_v': '\\s*(?:w)\\s*', 'sep_r_display': 'e', 'sep_r_default': '-|to'}
+
         def _update_side_effect():
             """
             Update the references after mocking out the method

tests/functional/openlp_plugins/songs/test_opsproimport.py

@@ -171,4 +171,3 @@ class TestOpsProSongImport(TestCase):
         result_data = json.loads(result_file.read().decode())
         self.assertListEqual(importer.verses, _get_item(result_data, 'verses'))
         self.assertListEqual(importer.verse_order_list_generated, _get_item(result_data, 'verse_order_list'))
-