openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #29020
[Merge] lp:~knightrider0xd/openlp/better-slide-scaling into lp:openlp
Ian Knight has proposed merging lp:~knightrider0xd/openlp/better-slide-scaling into lp:openlp.
Requested reviews:
Tim Bentley (trb143)
Related bugs:
Bug #891860 in OpenLP: "switching GUI to live mode makes images and presentation slide thumbnails very big and very hard to use"
https://bugs.launchpad.net/openlp/+bug/891860
Bug #1550856 in OpenLP: "Max height for non-text slides in slide controller"
https://bugs.launchpad.net/openlp/+bug/1550856
For more details, see:
https://code.launchpad.net/~knightrider0xd/openlp/better-slide-scaling/+merge/290694
Adds the ability to choose a maximum height for non-text slides in the list-preview-widget in slide controllers. See branch description for further details. May require sanity check.
Changes since last proposal:
- Fixed copy-paste error in comments
- Shifted common test code into setup function as recommended.
- Testing max height doesn't exceed settings spin-box (as requested) not changed as any positive int is valid & spin-box values are arbitrary. (See reply to diff comments on r2630 for details)
lp:~knightrider0xd/openlp/better-slide-scaling (revision 2632)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1340/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1260/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1199/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1034/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/625/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/692/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/560/
--
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/core/common/settings.py'
--- openlp/core/common/settings.py 2016-03-15 21:34:58 +0000
+++ openlp/core/common/settings.py 2016-04-01 09:26:21 +0000
@@ -121,6 +121,7 @@
'advanced/double click live': False,
'advanced/enable exit confirmation': True,
'advanced/expand service item': False,
+ 'advanced/slide max height': 0,
'advanced/hide mouse': True,
'advanced/is portable': False,
'advanced/max recent files': 20,
=== modified file 'openlp/core/ui/advancedtab.py'
--- openlp/core/ui/advancedtab.py 2016-01-19 06:52:23 +0000
+++ openlp/core/ui/advancedtab.py 2016-04-01 09:26:21 +0000
@@ -83,6 +83,13 @@
self.expand_service_item_check_box = QtWidgets.QCheckBox(self.ui_group_box)
self.expand_service_item_check_box.setObjectName('expand_service_item_check_box')
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.search_as_type_check_box = QtWidgets.QCheckBox(self.ui_group_box)
self.search_as_type_check_box.setObjectName('SearchAsType_check_box')
self.ui_layout.addRow(self.search_as_type_check_box)
@@ -277,6 +284,9 @@
'Preview items when clicked in Service Manager'))
self.expand_service_item_check_box.setText(translate('OpenLP.AdvancedTab',
'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.enable_auto_close_check_box.setText(translate('OpenLP.AdvancedTab',
'Enable application exit confirmation'))
self.service_name_group_box.setTitle(translate('OpenLP.AdvancedTab', 'Default Service Name'))
@@ -346,6 +356,7 @@
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'))
self.enable_auto_close_check_box.setChecked(settings.value('enable exit confirmation'))
self.hide_mouse_check_box.setChecked(settings.value('hide mouse'))
self.service_name_day.setCurrentIndex(settings.value('default service day'))
@@ -428,6 +439,7 @@
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())
settings.setValue('enable exit confirmation', self.enable_auto_close_check_box.isChecked())
settings.setValue('hide mouse', self.hide_mouse_check_box.isChecked())
settings.setValue('alternate rows', self.alternate_rows_check_box.isChecked())
=== modified file 'openlp/core/ui/listpreviewwidget.py'
--- openlp/core/ui/listpreviewwidget.py 2015-12-31 22:46:06 +0000
+++ openlp/core/ui/listpreviewwidget.py 2016-04-01 09:26:21 +0000
@@ -26,7 +26,7 @@
from PyQt5 import QtCore, QtGui, QtWidgets
-from openlp.core.common import RegistryProperties
+from openlp.core.common import RegistryProperties, Settings
from openlp.core.lib import ImageSource, ServiceItem
@@ -63,6 +63,8 @@
# Initialize variables.
self.service_item = ServiceItem()
self.screen_ratio = screen_ratio
+ # Connect signals
+ self.verticalHeader().sectionResized.connect(self.row_resized)
def resizeEvent(self, event):
"""
@@ -80,12 +82,30 @@
# Sort out songs, bibles, etc.
if self.service_item.is_text():
self.resizeRowsToContents()
+ # Sort out image heights.
else:
- # Sort out image heights.
+ 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 max_img_row_height > 0 and height > max_img_row_height:
+ height = max_img_row_height
+ # Apply new height to slides
for frame_number in range(len(self.service_item.get_frames())):
- height = self.viewport().width() // self.screen_ratio
self.setRowHeight(frame_number, height)
+ def row_resized(self, row, old_height, new_height):
+ """
+ Will scale non-image slides.
+ """
+ # Only for non-text slides when row height cap in use
+ if self.service_item.is_text() or Settings().value('advanced/slide max height') <= 0:
+ return
+ # Get and validate label widget containing slide & adjust max width
+ try:
+ self.cellWidget(row, 0).children()[1].setMaximumWidth(new_height * self.screen_ratio)
+ except:
+ return
+
def screen_size_changed(self, screen_ratio):
"""
This method is called whenever the live screen size changes, which then makes a layout recalculation necessary
@@ -139,8 +159,26 @@
pixmap = QtGui.QPixmap.fromImage(image)
pixmap.setDevicePixelRatio(label.devicePixelRatio())
label.setPixmap(pixmap)
- self.setCellWidget(frame_number, 0, label)
slide_height = width // self.screen_ratio
+ # Setup row height cap if in use.
+ max_img_row_height = Settings().value('advanced/slide max height')
+ if max_img_row_height > 0:
+ if slide_height > max_img_row_height:
+ 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)
+ # Build widget with stretch padding
+ container = QtWidgets.QWidget()
+ hbox = QtWidgets.QHBoxLayout()
+ hbox.setContentsMargins(0, 0, 0, 0)
+ hbox.addWidget(label, stretch=1)
+ hbox.addStretch(0)
+ container.setLayout(hbox)
+ # Add to table
+ self.setCellWidget(frame_number, 0, container)
+ else:
+ # Add to table
+ self.setCellWidget(frame_number, 0, label)
row += 1
text.append(str(row))
self.setItem(frame_number, 0, item)
=== modified file 'tests/functional/openlp_core_ui/test_listpreviewwidget.py'
--- tests/functional/openlp_core_ui/test_listpreviewwidget.py 2015-12-31 22:46:06 +0000
+++ tests/functional/openlp_core_ui/test_listpreviewwidget.py 2016-04-01 09:26:21 +0000
@@ -23,9 +23,12 @@
Package to test the openlp.core.ui.listpreviewwidget package.
"""
from unittest import TestCase
+
+from openlp.core.common import Settings
from openlp.core.ui.listpreviewwidget import ListPreviewWidget
+from openlp.core.lib import ServiceItem
-from tests.functional import patch
+from tests.functional import MagicMock, patch, call
class TestListPreviewWidget(TestCase):
@@ -34,9 +37,27 @@
"""
Mock out stuff for all the tests
"""
- self.setup_patcher = patch('openlp.core.ui.listpreviewwidget.ListPreviewWidget._setup')
- self.mocked_setup = self.setup_patcher.start()
- self.addCleanup(self.setup_patcher.stop)
+ # Mock self.parent().width()
+ self.parent_patcher = patch('openlp.core.ui.listpreviewwidget.ListPreviewWidget.parent')
+ self.mocked_parent = self.parent_patcher.start()
+ self.mocked_parent.width.return_value = 100
+ self.addCleanup(self.parent_patcher.stop)
+
+ # Mock Settings().value()
+ self.Settings_patcher = patch('openlp.core.ui.listpreviewwidget.Settings')
+ self.mocked_Settings = self.Settings_patcher.start()
+ self.mocked_Settings_obj = MagicMock()
+ self.mocked_Settings_obj.value.return_value = None
+ self.mocked_Settings.return_value = self.mocked_Settings_obj
+ self.addCleanup(self.Settings_patcher.stop)
+
+ # Mock self.viewport().width()
+ self.viewport_patcher = patch('openlp.core.ui.listpreviewwidget.ListPreviewWidget.viewport')
+ self.mocked_viewport = self.viewport_patcher.start()
+ self.mocked_viewport_obj = MagicMock()
+ self.mocked_viewport_obj.width.return_value = 200
+ self.mocked_viewport.return_value = self.mocked_viewport_obj
+ self.addCleanup(self.viewport_patcher.stop)
def new_list_preview_widget_test(self):
"""
@@ -49,4 +70,206 @@
# THEN: The object is not None, and the _setup() method was called.
self.assertIsNotNone(list_preview_widget, 'The ListPreviewWidget object should not be None')
- self.mocked_setup.assert_called_with(1)
+ self.assertEquals(list_preview_widget.screen_ratio, 1, 'Should not be called')
+
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.resizeRowsToContents')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.setRowHeight')
+ def replace_recalculate_layout_test_text(self, mocked_setRowHeight, mocked_resizeRowsToContents):
+ """
+ Test if "Max height for non-text slides..." enabled, txt slides unchanged in replace_service_item & __recalc...
+ """
+ # GIVEN: A setting to adjust "Max height for non-text slides in slide controller",
+ # a text ServiceItem and a ListPreviewWidget.
+
+ # Mock Settings().value('advanced/slide max height')
+ self.mocked_Settings_obj.value.return_value = 100
+ # Mock self.viewport().width()
+ self.mocked_viewport_obj.width.return_value = 200
+ # Mock text service item
+ service_item = MagicMock()
+ service_item.is_text.return_value = True
+ service_item.get_frames.return_value = [{'title': None, 'text': None, 'verseTag': None},
+ {'title': None, 'text': None, 'verseTag': 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 resizeEvent)
+ list_preview_widget.resizeEvent(None)
+
+ # THEN: setRowHeight() should not be called, while resizeRowsToContents() should be called twice
+ # (once each in __recalculate_layout and replace_service_item)
+ self.assertEquals(mocked_resizeRowsToContents.call_count, 2, 'Should be called')
+ self.assertEquals(mocked_setRowHeight.call_count, 0, 'Should not be called')
+
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.resizeRowsToContents')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.setRowHeight')
+ def replace_recalculate_layout_test_img(self, mocked_setRowHeight, mocked_resizeRowsToContents):
+ """
+ Test if "Max height for non-text slides..." disabled, img slides unchanged 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 = 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.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 resizeEvent)
+ list_preview_widget.resizeEvent(None)
+
+ # 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, 4, 'Should be called twice for each slide')
+ calls = [call(0, 200), call(1, 200), call(0, 400), call(1, 400)]
+ mocked_setRowHeight.assert_has_calls(calls)
+
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.resizeRowsToContents')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.setRowHeight')
+ def replace_recalculate_layout_test_img_max(self, mocked_setRowHeight, mocked_resizeRowsToContents):
+ """
+ Test if "Max height for non-text slides..." enabled, 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 = 100
+ # 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.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 resizeEvent)
+ list_preview_widget.resizeEvent(None)
+
+ # 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, 4, 'Should be called twice for each slide')
+ calls = [call(0, 100), call(1, 100), call(0, 100), call(1, 100)]
+ mocked_setRowHeight.assert_has_calls(calls)
+
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.resizeRowsToContents')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.setRowHeight')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.cellWidget')
+ def row_resized_test_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.
+ """
+ # GIVEN: A setting to adjust "Max height for non-text slides in slide controller",
+ # a text ServiceItem and a ListPreviewWidget.
+
+ # Mock Settings().value('advanced/slide max height')
+ self.mocked_Settings_obj.value.return_value = 100
+ # Mock self.viewport().width()
+ self.mocked_viewport_obj.width.return_value = 200
+ # Mock text service item
+ service_item = MagicMock()
+ service_item.is_text.return_value = True
+ service_item.get_frames.return_value = [{'title': None, 'text': None, 'verseTag': None},
+ {'title': None, 'text': None, 'verseTag': None}]
+ # Mock self.cellWidget().children().setMaximumWidth()
+ mocked_cellWidget_child = MagicMock()
+ mocked_cellWidget_obj = MagicMock()
+ mocked_cellWidget_obj.children.return_value = [None, mocked_cellWidget_child]
+ 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)
+
+ # WHEN: row_resized() is called
+ list_preview_widget.row_resized(0, 100, 150)
+
+ # THEN: self.cellWidget(row, 0).children()[1].setMaximumWidth() should not be called
+ self.assertEquals(mocked_cellWidget_child.setMaximumWidth.call_count, 0, 'Should not be called')
+
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.resizeRowsToContents')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.setRowHeight')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.cellWidget')
+ def row_resized_test_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.
+ """
+ # 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.get_frames.return_value = [{'title': None, 'path': None, 'image': None},
+ {'title': None, 'path': None, 'image': None}]
+ # Mock self.cellWidget().children().setMaximumWidth()
+ mocked_cellWidget_child = MagicMock()
+ mocked_cellWidget_obj = MagicMock()
+ mocked_cellWidget_obj.children.return_value = [None, mocked_cellWidget_child]
+ 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)
+
+ # WHEN: row_resized() is called
+ list_preview_widget.row_resized(0, 100, 150)
+
+ # THEN: self.cellWidget(row, 0).children()[1].setMaximumWidth() should not be called
+ self.assertEquals(mocked_cellWidget_child.setMaximumWidth.call_count, 0, 'Should not be called')
+
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.resizeRowsToContents')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.setRowHeight')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.cellWidget')
+ def row_resized_test_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.
+ """
+ # 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 = 100
+ # 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.get_frames.return_value = [{'title': None, 'path': None, 'image': None},
+ {'title': None, 'path': None, 'image': None}]
+ # Mock self.cellWidget().children().setMaximumWidth()
+ mocked_cellWidget_child = MagicMock()
+ mocked_cellWidget_obj = MagicMock()
+ mocked_cellWidget_obj.children.return_value = [None, mocked_cellWidget_child]
+ 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)
+
+ # WHEN: row_resized() is called
+ list_preview_widget.row_resized(0, 100, 150)
+
+ # THEN: self.cellWidget(row, 0).children()[1].setMaximumWidth() should be called
+ mocked_cellWidget_child.setMaximumWidth.assert_called_once_with(150)
Follow ups
