openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #29724
[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:
OpenLP Core (openlp-core)
For more details, see:
https://code.launchpad.net/~knightrider0xd/openlp/non-text-item-height-default/+merge/295318
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.
lp:~knightrider0xd/openlp/non-text-item-height-default (revision 2669)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1565/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1476/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1414/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1194/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/784/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/852/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/720/
--
Your team OpenLP Core is requested to review the proposed merge of lp:~knightrider0xd/openlp/non-text-item-height-default into 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-05-20 13:56:40 +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-04-16 21:01:22 +0000
+++ openlp/core/ui/advancedtab.py 2016-05-20 13:56:40 +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-05-20 13:56:40 +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_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-05-20 13:56:40 +0000
@@ -227,6 +227,44 @@
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.resizeRowsToContents')
@patch(u'openlp.core.ui.lib.listpreviewwidget.ListPreviewWidget.setRowHeight')
+ def replace_recalculate_layout_test_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):
"""
@@ -331,6 +369,41 @@
# 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 row_resized_test_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')
Follow ups
