openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #29240
[Merge] lp:~knightrider0xd/openlp/auto-scroll-choice into lp:openlp
Ian Knight has proposed merging lp:~knightrider0xd/openlp/auto-scroll-choice into lp:openlp.
Requested reviews:
Raoul Snyman (raoul-snyman)
Related bugs:
Bug #1550858 in OpenLP: "Choose behaviour for auto-scroll of slides in slide-controller"
https://bugs.launchpad.net/openlp/+bug/1550858
For more details, see:
https://code.launchpad.net/~knightrider0xd/openlp/auto-scroll-choice/+merge/292068
Allows users to change the automatic scrolling behaviour for when a new slide is selected.
Currently on selecting a new slide, the next slide will be scrolled into view.
The changes provide a combo-box in the advanced settings that allow users to select additional options (see branch description for details).
Additional bounds checking and unit tests have been added to support this.
This latest proposal also contains additional input validation and testing thereof for values from Settings() which are added across several extra functions not otherwise affected by this feature.
--
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/core/common/settings.py'
--- openlp/core/common/settings.py 2016-03-20 15:00:15 +0000
+++ openlp/core/common/settings.py 2016-04-16 16:39:13 +0000
@@ -107,6 +107,7 @@
__default_settings__ = {
'advanced/add page break': False,
'advanced/alternate rows': not is_win(),
+ 'advanced/autoscrolling': {'dist': 1, 'pos': 0},
'advanced/current media plugin': -1,
'advanced/data path': '',
'advanced/default color': '#ffffff',
@@ -121,7 +122,6 @@
'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,
@@ -131,6 +131,7 @@
'advanced/recent file count': 4,
'advanced/save current plugin': False,
'advanced/slide limits': SlideLimits.End,
+ 'advanced/slide max height': 0,
'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-05 17:30:20 +0000
+++ openlp/core/ui/advancedtab.py 2016-04-16 16:39:13 +0000
@@ -49,6 +49,10 @@
self.default_color = '#ffffff'
self.data_exists = False
self.icon_path = ':/system/system_settings.png'
+ self.autoscroll_map = [None, {'dist': -1, 'pos': 0}, {'dist': -1, 'pos': 1}, {'dist': -1, 'pos': 2},
+ {'dist': 0, 'pos': 0}, {'dist': 0, 'pos': 1}, {'dist': 0, 'pos': 2},
+ {'dist': 0, 'pos': 3}, {'dist': 1, 'pos': 0}, {'dist': 1, 'pos': 1},
+ {'dist': 1, 'pos': 2}, {'dist': 1, 'pos': 3}]
advanced_translated = translate('OpenLP.AdvancedTab', 'Advanced')
super(AdvancedTab, self).__init__(parent, 'Advanced', advanced_translated)
@@ -90,6 +94,13 @@
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.autoscroll_label = QtWidgets.QLabel(self.ui_group_box)
+ self.autoscroll_label.setObjectName('autoscroll_label')
+ self.autoscroll_combo_box = QtWidgets.QComboBox(self.ui_group_box)
+ self.autoscroll_combo_box.addItems(['', '', '', '', '', '', '', '', '', '', '', ''])
+ self.autoscroll_combo_box.setObjectName('autoscroll_combo_box')
+ self.ui_layout.addRow(self.autoscroll_label)
+ self.ui_layout.addRow(self.autoscroll_combo_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)
@@ -287,6 +298,31 @@
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.autoscroll_label.setText(translate('OpenLP.AdvancedTab',
+ 'When changing slides:'))
+ self.autoscroll_combo_box.setItemText(0, translate('OpenLP.AdvancedTab', 'Do not auto-scroll'))
+ self.autoscroll_combo_box.setItemText(1, translate('OpenLP.AdvancedTab',
+ 'Auto-scroll the previous slide into view'))
+ self.autoscroll_combo_box.setItemText(2, translate('OpenLP.AdvancedTab',
+ 'Auto-scroll the previous slide to top'))
+ self.autoscroll_combo_box.setItemText(3, translate('OpenLP.AdvancedTab',
+ 'Auto-scroll the previous slide to middle'))
+ self.autoscroll_combo_box.setItemText(4, translate('OpenLP.AdvancedTab',
+ 'Auto-scroll the current slide into view'))
+ self.autoscroll_combo_box.setItemText(5, translate('OpenLP.AdvancedTab',
+ 'Auto-scroll the current slide to top'))
+ self.autoscroll_combo_box.setItemText(6, translate('OpenLP.AdvancedTab',
+ 'Auto-scroll the current slide to middle'))
+ self.autoscroll_combo_box.setItemText(7, translate('OpenLP.AdvancedTab',
+ 'Auto-scroll the current slide to bottom'))
+ self.autoscroll_combo_box.setItemText(8, translate('OpenLP.AdvancedTab',
+ 'Auto-scroll the next slide into view'))
+ self.autoscroll_combo_box.setItemText(9, translate('OpenLP.AdvancedTab',
+ 'Auto-scroll the next slide to top'))
+ self.autoscroll_combo_box.setItemText(10, translate('OpenLP.AdvancedTab',
+ 'Auto-scroll the next slide to middle'))
+ self.autoscroll_combo_box.setItemText(11, translate('OpenLP.AdvancedTab',
+ 'Auto-scroll the next slide to bottom'))
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'))
@@ -357,6 +393,10 @@
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'))
+ autoscroll_value = settings.value('autoscrolling')
+ for i in range(0, len(self.autoscroll_map)):
+ if self.autoscroll_map[i] == autoscroll_value:
+ 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'))
self.service_name_day.setCurrentIndex(settings.value('default service day'))
@@ -440,6 +480,7 @@
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('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())
settings.setValue('alternate rows', self.alternate_rows_check_box.isChecked())
=== modified file 'openlp/core/ui/listpreviewwidget.py'
--- openlp/core/ui/listpreviewwidget.py 2016-03-05 16:41:32 +0000
+++ openlp/core/ui/listpreviewwidget.py 2016-04-16 16:39:13 +0000
@@ -87,7 +87,7 @@
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:
+ if isinstance(max_img_row_height, int) and 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())):
@@ -98,7 +98,8 @@
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:
+ 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:
return
# Get and validate label widget containing slide & adjust max width
try:
@@ -160,9 +161,9 @@
pixmap.setDevicePixelRatio(label.devicePixelRatio())
label.setPixmap(pixmap)
slide_height = width // self.screen_ratio
- # Setup row height cap if in use.
+ # Setup and validate row height cap if in use.
max_img_row_height = Settings().value('advanced/slide max height')
- if 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:
slide_height = max_img_row_height
label.setMaximumWidth(max_img_row_height * self.screen_ratio)
@@ -194,11 +195,22 @@
"""
Switches to the given row.
"""
- if slide >= self.slide_count():
- slide = self.slide_count() - 1
- # Scroll to next item if possible.
- if slide + 1 < self.slide_count():
- self.scrollToItem(self.item(slide + 1, 0))
+ # Retrieve setting
+ autoscrolling = Settings().value('advanced/autoscrolling')
+ # Check if auto-scroll disabled (None) and validate value as dict containing 'dist' and 'pos'
+ # 'dist' represents the slide to scroll to relative to the new slide (-1 = previous, 0 = current, 1 = next)
+ # 'pos' represents the vert position of of the slide (0 = in view, 1 = top, 2 = middle, 3 = bottom)
+ if not (isinstance(autoscrolling, dict) and 'dist' in autoscrolling and 'pos' in autoscrolling and
+ isinstance(autoscrolling['dist'], int) and isinstance(autoscrolling['pos'], int)):
+ return
+ # prevent scrolling past list bounds
+ scroll_to_slide = slide + autoscrolling['dist']
+ if scroll_to_slide < 0:
+ scroll_to_slide = 0
+ if scroll_to_slide >= self.slide_count():
+ scroll_to_slide = self.slide_count() - 1
+ # Scroll to item if possible.
+ self.scrollToItem(self.item(scroll_to_slide, 0), autoscrolling['pos'])
self.selectRow(slide)
def current_slide_number(self):
=== modified file 'tests/functional/openlp_core_ui/test_listpreviewwidget.py'
--- tests/functional/openlp_core_ui/test_listpreviewwidget.py 2016-03-23 14:14:37 +0000
+++ tests/functional/openlp_core_ui/test_listpreviewwidget.py 2016-04-16 16:39:13 +0000
@@ -130,12 +130,14 @@
# WHEN: __recalculate_layout() is called (via resizeEvent)
list_preview_widget.resizeEvent(None)
+ self.mocked_Settings_obj.value.return_value = None
+ 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)]
+ self.assertEquals(mocked_setRowHeight.call_count, 6, 'Should be called 3 times for each slide')
+ calls = [call(0, 200), call(1, 200), call(0, 400), call(1, 400), call(0, 400), call(1, 400)]
mocked_setRowHeight.assert_has_calls(calls)
@patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.resizeRowsToContents')
@@ -236,6 +238,8 @@
# WHEN: row_resized() is called
list_preview_widget.row_resized(0, 100, 150)
+ self.mocked_Settings_obj.value.return_value = None
+ 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')
@@ -273,3 +277,101 @@
# 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.listpreviewwidget.ListPreviewWidget.selectRow')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.scrollToItem')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.item')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.slide_count')
+ def autoscroll_test_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().
+ """
+ # GIVEN: A setting for autoscrolling and a ListPreviewWidget.
+ # Mock Settings().value('advanced/autoscrolling')
+ self.mocked_Settings_obj.value.return_value = None
+ # Mocked returns
+ mocked_slide_count.return_value = 1
+ mocked_item.return_value = None
+ # init ListPreviewWidget and load service item
+ list_preview_widget = ListPreviewWidget(None, 1)
+
+ # WHEN: change_slide() is called
+ list_preview_widget.change_slide(0)
+ self.mocked_Settings_obj.value.return_value = 1
+ list_preview_widget.change_slide(0)
+ self.mocked_Settings_obj.value.return_value = {'fail': 1}
+ list_preview_widget.change_slide(0)
+ self.mocked_Settings_obj.value.return_value = {'dist': 1, 'fail': 1}
+ list_preview_widget.change_slide(0)
+ self.mocked_Settings_obj.value.return_value = {'dist': 'fail', 'pos': 1}
+ list_preview_widget.change_slide(0)
+ self.mocked_Settings_obj.value.return_value = {'dist': 1, 'pos': 'fail'}
+ list_preview_widget.change_slide(0)
+
+ # THEN: no further functions should be called
+ self.assertEquals(mocked_slide_count.call_count, 0, 'Should not be called')
+ self.assertEquals(mocked_scrollToItem.call_count, 0, 'Should not be called')
+ self.assertEquals(mocked_selectRow.call_count, 0, 'Should not be called')
+ self.assertEquals(mocked_item.call_count, 0, 'Should not be called')
+
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.selectRow')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.scrollToItem')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.item')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.slide_count')
+ def autoscroll_test_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.
+ """
+ # GIVEN: A setting for autoscrolling and a ListPreviewWidget.
+ # Mock Settings().value('advanced/autoscrolling')
+ self.mocked_Settings_obj.value.return_value = {'dist': -1, 'pos': 1}
+ # Mocked returns
+ mocked_slide_count.return_value = 1
+ mocked_item.return_value = None
+ # init ListPreviewWidget and load service item
+ list_preview_widget = ListPreviewWidget(None, 1)
+
+ # WHEN: change_slide() is called
+ list_preview_widget.change_slide(0)
+ self.mocked_Settings_obj.value.return_value = {'dist': 1, 'pos': 1}
+ list_preview_widget.change_slide(0)
+
+ # THEN: no further functions should be called
+ self.assertEquals(mocked_slide_count.call_count, 3, 'Should be called')
+ self.assertEquals(mocked_scrollToItem.call_count, 2, 'Should be called')
+ self.assertEquals(mocked_selectRow.call_count, 2, 'Should be called')
+ self.assertEquals(mocked_item.call_count, 2, 'Should be called')
+ calls = [call(0, 0), call(0, 0)]
+ mocked_item.assert_has_calls(calls)
+
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.selectRow')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.scrollToItem')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.item')
+ @patch(u'openlp.core.ui.listpreviewwidget.ListPreviewWidget.slide_count')
+ def autoscroll_test_normal(self, mocked_slide_count, mocked_item, mocked_scrollToItem, mocked_selectRow):
+ """
+ Test if 'advanced/autoscrolling' setting valid, autoscrolling called as expected.
+ """
+ # GIVEN: A setting for autoscrolling and a ListPreviewWidget.
+ # Mock Settings().value('advanced/autoscrolling')
+ self.mocked_Settings_obj.value.return_value = {'dist': -1, 'pos': 1}
+ # Mocked returns
+ mocked_slide_count.return_value = 3
+ mocked_item.return_value = None
+ # init ListPreviewWidget and load service item
+ list_preview_widget = ListPreviewWidget(None, 1)
+
+ # WHEN: change_slide() is called
+ list_preview_widget.change_slide(1)
+ self.mocked_Settings_obj.value.return_value = {'dist': 0, 'pos': 1}
+ list_preview_widget.change_slide(1)
+ self.mocked_Settings_obj.value.return_value = {'dist': 1, 'pos': 1}
+ list_preview_widget.change_slide(1)
+
+ # THEN: no further functions should be called
+ self.assertEquals(mocked_slide_count.call_count, 3, 'Should be called')
+ self.assertEquals(mocked_scrollToItem.call_count, 3, 'Should be called')
+ self.assertEquals(mocked_selectRow.call_count, 3, 'Should be called')
+ self.assertEquals(mocked_item.call_count, 3, 'Should be called')
+ calls = [call(0, 0), call(1, 0), call(2, 0)]
+ mocked_item.assert_has_calls(calls)
Follow ups
