openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #29815
[Merge] lp:~knightrider0xd/openlp/non-text-item-height-default into lp:openlp
Ian Knight has proposed merging lp:~knightrider0xd/openlp/non-text-item-height-default into lp:openlp.
Requested reviews:
Raoul Snyman (raoul-snyman)
For more details, see:
https://code.launchpad.net/~knightrider0xd/openlp/non-text-item-height-default/+merge/296274
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.
Merged with latest trunk.
Fixed test naming.
lp:~knightrider0xd/openlp/non-text-item-height-default (revision 2672)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1582/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1493/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1431/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1210/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/800/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/868/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/736/
--
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/core/common/settings.py'
--- openlp/core/common/settings.py 2016-05-15 17:32:04 +0000
+++ openlp/core/common/settings.py 2016-06-02 01:27:59 +0000
@@ -129,7 +129,7 @@
'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,
=== modified file 'openlp/core/ui/advancedtab.py'
--- openlp/core/ui/advancedtab.py 2016-05-20 16:22:06 +0000
+++ openlp/core/ui/advancedtab.py 2016-06-02 01:27:59 +0000
@@ -87,11 +87,14 @@
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_spin_box.setObjectName('slide_max_height_spin_box')
- self.slide_max_height_spin_box.setRange(0, 1000)
- self.slide_max_height_spin_box.setSingleStep(20)
- self.ui_layout.addRow(self.slide_max_height_label, self.slide_max_height_spin_box)
+ self.slide_max_height_combo_box = QtWidgets.QComboBox(self.ui_group_box)
+ self.slide_max_height_combo_box.addItem('', userData=0)
+ self.slide_max_height_combo_box.addItem('', userData=-4)
+ # Generate numeric values for combo box dynamically
+ 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 @@
'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 @@
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 @@
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())
=== modified file 'openlp/core/ui/lib/listpreviewwidget.py'
--- openlp/core/ui/lib/listpreviewwidget.py 2016-05-05 03:57:04 +0000
+++ openlp/core/ui/lib/listpreviewwidget.py 2016-06-02 01:27:59 +0000
@@ -63,6 +63,7 @@
# 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 @@
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:
- 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 @@
"""
# 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 @@
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 slide_height > max_img_row_height:
+ if isinstance(max_img_row_height, int) and max_img_row_height != 0:
+ 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)
- label.resize(max_img_row_height * self.screen_ratio, max_img_row_height)
+ elif max_img_row_height < 0 and slide_height > self.auto_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()
=== modified file 'tests/functional/openlp_core_common/test_registryproperties.py'
--- tests/functional/openlp_core_common/test_registryproperties.py 2016-05-31 16:10:31 +0000
+++ tests/functional/openlp_core_common/test_registryproperties.py 2016-06-02 01:27:59 +0000
@@ -75,4 +75,3 @@
# THEN the application should be none
self.assertEqual(self.application, application, 'The application value should match')
-
=== modified file 'tests/functional/openlp_core_ui_lib/test_listpreviewwidget.py'
--- tests/functional/openlp_core_ui_lib/test_listpreviewwidget.py 2016-05-06 01:46:49 +0000
+++ tests/functional/openlp_core_ui_lib/test_listpreviewwidget.py 2016-06-02 01:27:59 +0000
@@ -61,7 +61,7 @@
self.mocked_viewport.return_value = self.mocked_viewport_obj
self.addCleanup(self.viewport_patcher.stop)
- def new_list_preview_widget_test(self):
+ def test_new_list_preview_widget(self):
"""
Test that creating an instance of ListPreviewWidget works
"""
@@ -77,7 +77,7 @@
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.image_manager')
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.resizeRowsToContents')
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.setRowHeight')
- def replace_service_item_test_thumbs(self, mocked_setRowHeight, mocked_resizeRowsToContents,
+ def test_replace_service_item_thumbs(self, mocked_setRowHeight, mocked_resizeRowsToContents,
mocked_image_manager):
"""
Test that thubmails for different slides are loaded properly in replace_service_item.
@@ -123,7 +123,7 @@
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.resizeRowsToContents')
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.setRowHeight')
- def replace_recalculate_layout_test_text(self, mocked_setRowHeight, mocked_resizeRowsToContents):
+ def test_replace_recalculate_layout_text(self, mocked_setRowHeight, mocked_resizeRowsToContents):
"""
Test if "Max height for non-text slides..." enabled, txt slides unchanged in replace_service_item & __recalc...
"""
@@ -155,7 +155,7 @@
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.resizeRowsToContents')
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.setRowHeight')
- def replace_recalculate_layout_test_img(self, mocked_setRowHeight, mocked_resizeRowsToContents):
+ def test_replace_recalculate_layout_img(self, mocked_setRowHeight, mocked_resizeRowsToContents):
"""
Test if "Max height for non-text slides..." disabled, img slides unchanged in replace_service_item & __recalc...
"""
@@ -192,7 +192,7 @@
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.resizeRowsToContents')
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.setRowHeight')
- def replace_recalculate_layout_test_img_max(self, mocked_setRowHeight, mocked_resizeRowsToContents):
+ def test_replace_recalculate_layout_img_max(self, mocked_setRowHeight, mocked_resizeRowsToContents):
"""
Test if "Max height for non-text slides..." enabled, img slides resized in replace_service_item & __recalc...
"""
@@ -227,8 +227,46 @@
@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')
- def row_resized_test_text(self, mocked_cellWidget, mocked_setRowHeight, mocked_resizeRowsToContents):
+ def test_row_resized_text(self, mocked_cellWidget, mocked_setRowHeight, mocked_resizeRowsToContents):
"""
Test if "Max height for non-text slides..." enabled, text-based slides not affected in row_resized.
"""
@@ -262,7 +300,7 @@
@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 row_resized_test_img(self, mocked_cellWidget, mocked_setRowHeight, mocked_resizeRowsToContents):
+ def test_row_resized_img(self, mocked_cellWidget, mocked_setRowHeight, mocked_resizeRowsToContents):
"""
Test if "Max height for non-text slides..." disabled, image-based slides not affected in row_resized.
"""
@@ -299,7 +337,7 @@
@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 row_resized_test_img_max(self, mocked_cellWidget, mocked_setRowHeight, mocked_resizeRowsToContents):
+ def test_row_resized_img_max(self, mocked_cellWidget, mocked_setRowHeight, mocked_resizeRowsToContents):
"""
Test if "Max height for non-text slides..." enabled, image-based slides are scaled in row_resized.
"""
@@ -331,11 +369,46 @@
# 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')
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.slide_count')
- def autoscroll_test_setting_invalid(self, mocked_slide_count, mocked_item, mocked_scrollToItem, mocked_selectRow):
+ def test_autoscroll_setting_invalid(self, mocked_slide_count, mocked_item, mocked_scrollToItem, mocked_selectRow):
"""
Test if 'advanced/autoscrolling' setting None or invalid, that no autoscrolling occurs on change_slide().
"""
@@ -371,7 +444,7 @@
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.scrollToItem')
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.item')
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.slide_count')
- def autoscroll_test_dist_bounds(self, mocked_slide_count, mocked_item, mocked_scrollToItem, mocked_selectRow):
+ def test_autoscroll_dist_bounds(self, mocked_slide_count, mocked_item, mocked_scrollToItem, mocked_selectRow):
"""
Test if 'advanced/autoscrolling' setting asks to scroll beyond list bounds, that it does not beyond.
"""
@@ -401,7 +474,7 @@
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.scrollToItem')
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.item')
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.slide_count')
- def autoscroll_test_normal(self, mocked_slide_count, mocked_item, mocked_scrollToItem, mocked_selectRow):
+ def test_autoscroll_normal(self, mocked_slide_count, mocked_item, mocked_scrollToItem, mocked_selectRow):
"""
Test if 'advanced/autoscrolling' setting valid, autoscrolling called as expected.
"""
Follow ups
