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(...

bzr-revno: 2644
Fixes: https://launchpad.net/bugs/1550858
This commit is contained in:
ian@knightly.xyz 2016-04-16 22:01:22 +01:00 committed by Tim Bentley
commit dadff3c4de
4 changed files with 168 additions and 12 deletions

View File

@ -107,6 +107,7 @@ class Settings(QtCore.QSettings):
__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': '',
# 7 stands for now, 0 to 6 is Monday to Sunday.
@ -119,7 +120,6 @@ class Settings(QtCore.QSettings):
'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,
@ -129,6 +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/single click preview': False,
'advanced/single click service preview': False,
'advanced/x11 bypass wm': X11_BYPASS_DEFAULT,

View File

@ -47,6 +47,10 @@ class AdvancedTab(SettingsTab):
"""
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)
@ -88,6 +92,13 @@ class AdvancedTab(SettingsTab):
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)
@ -255,6 +266,31 @@ class AdvancedTab(SettingsTab):
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'))
@ -320,6 +356,10 @@ class AdvancedTab(SettingsTab):
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'))
@ -400,6 +440,7 @@ class AdvancedTab(SettingsTab):
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())

View File

@ -87,7 +87,7 @@ 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 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 @@ class ListPreviewWidget(QtWidgets.QTableWidget, RegistryProperties):
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 @@ class ListPreviewWidget(QtWidgets.QTableWidget, RegistryProperties):
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 @@ class ListPreviewWidget(QtWidgets.QTableWidget, RegistryProperties):
"""
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):

View File

@ -130,12 +130,14 @@ class TestListPreviewWidget(TestCase):
# 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 @@ class TestListPreviewWidget(TestCase):
# 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 @@ 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.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)