openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #24069
Re: [Merge] lp:~stewart-e/openlp/bug-1243881 into lp:openlp
Review: Needs Fixing
Inline comments
Diff comments:
> === modified file 'openlp/core/common/settings.py'
> --- openlp/core/common/settings.py 2014-05-02 06:42:17 +0000
> +++ openlp/core/common/settings.py 2014-07-23 20:56:11 +0000
> @@ -286,6 +286,7 @@
> 'themes/last directory export': '',
> 'themes/last directory import': '',
> 'themes/theme level': ThemeLevel.Song,
> + 'themes/wrap footer': False,
> 'user interface/live panel': True,
> 'user interface/live splitter geometry': QtCore.QByteArray(),
> 'user interface/lock panel': False,
>
> === modified file 'openlp/core/lib/htmlbuilder.py'
> --- openlp/core/lib/htmlbuilder.py 2014-03-20 19:10:31 +0000
> +++ openlp/core/lib/htmlbuilder.py 2014-07-23 20:56:11 +0000
> @@ -398,6 +398,7 @@
>
> from PyQt4 import QtWebKit
>
> +from openlp.core.common import Settings
> from openlp.core.lib.theme import BackgroundType, BackgroundGradientType, VerticalType, HorizontalType
>
> log = logging.getLogger(__name__)
> @@ -750,12 +751,13 @@
> font-size: %spt;
> color: %s;
> text-align: left;
> - white-space: nowrap;
> + white-space: %s;
> """
> theme = item.theme_data
> if not theme or not item.footer:
> return ''
> bottom = height - int(item.footer.y()) - int(item.footer.height())
> + whitespace = 'normal' if Settings().value('themes/wrap footer') else 'nowrap'
> lyrics_html = style % (item.footer.x(), bottom, item.footer.width(),
> - theme.font_footer_name, theme.font_footer_size, theme.font_footer_color)
> + theme.font_footer_name, theme.font_footer_size, theme.font_footer_color, whitespace)
> return lyrics_html
>
> === modified file 'openlp/core/ui/themestab.py'
> --- openlp/core/ui/themestab.py 2014-05-02 06:42:17 +0000
> +++ openlp/core/ui/themestab.py 2014-07-23 20:56:11 +0000
> @@ -69,6 +69,14 @@
> self.default_list_view.setObjectName('default_list_view')
> self.global_group_box_layout.addWidget(self.default_list_view)
> self.left_layout.addWidget(self.global_group_box)
> + self.universal_group_box = QtGui.QGroupBox(self.left_column)
> + self.universal_group_box.setObjectName('universal_group_box')
> + self.universal_group_box_layout = QtGui.QVBoxLayout(self.universal_group_box)
> + self.universal_group_box_layout.setObjectName('universal_group_box_layout')
> + self.wrap_footer_check_box = QtGui.QCheckBox(self.universal_group_box)
> + self.wrap_footer_check_box.setObjectName('wrap_footer_check_box')
> + self.universal_group_box_layout.addWidget(self.wrap_footer_check_box)
> + self.left_layout.addWidget(self.universal_group_box)
> self.left_layout.addStretch()
> self.level_group_box = QtGui.QGroupBox(self.right_column)
> self.level_group_box.setObjectName('level_group_box')
> @@ -112,6 +120,8 @@
> """
> self.tab_title_visible = UiStrings().Themes
> self.global_group_box.setTitle(translate('OpenLP.ThemesTab', 'Global Theme'))
> + self.universal_group_box.setTitle(translate('OpenLP.ThemesTab', 'Universal Settings'))
> + self.wrap_footer_check_box.setText(translate('OpenLP.ThemesTab', '&Wrap footer text'))
> self.level_group_box.setTitle(translate('OpenLP.ThemesTab', 'Theme Level'))
> self.song_level_radio_button.setText(translate('OpenLP.ThemesTab', 'S&ong Level'))
> self.song_level_label.setText(
> @@ -136,6 +146,7 @@
> settings.beginGroup(self.settings_section)
> self.theme_level = settings.value('theme level')
> self.global_theme = settings.value('global theme')
> + wrap_footer = settings.value('wrap footer')
Why the local variable.
> settings.endGroup()
> if self.theme_level == ThemeLevel.Global:
> self.global_level_radio_button.setChecked(True)
> @@ -143,15 +154,18 @@
> self.service_level_radio_button.setChecked(True)
> else:
> self.song_level_radio_button.setChecked(True)
> + self.wrap_footer_check_box.setChecked(wrap_footer)
>
> def save(self):
> """
> Save the settings
> """
> + wrap_footer = self.wrap_footer_check_box.isChecked()
Why the local variable.
> settings = Settings()
> settings.beginGroup(self.settings_section)
> settings.setValue('theme level', self.theme_level)
> settings.setValue('global theme', self.global_theme)
> + settings.setValue('wrap footer', wrap_footer)
> settings.endGroup()
> self.renderer.set_theme_level(self.theme_level)
> if self.tab_visited:
>
> === modified file 'openlp/plugins/songs/lib/__init__.py'
> --- openlp/plugins/songs/lib/__init__.py 2014-06-09 08:59:52 +0000
> +++ openlp/plugins/songs/lib/__init__.py 2014-07-23 20:56:11 +0000
> @@ -374,7 +374,7 @@
> :param manager: The song database manager object.
> :param song: The song object.
> """
> - from .xml import SongXML
> + from .openlyricsxml import SongXML
>
> if song.title:
> song.title = clean_title(song.title)
>
> === modified file 'tests/functional/openlp_core_lib/test_htmlbuilder.py'
> --- tests/functional/openlp_core_lib/test_htmlbuilder.py 2014-04-02 18:51:21 +0000
> +++ tests/functional/openlp_core_lib/test_htmlbuilder.py 2014-07-23 20:56:11 +0000
> @@ -6,11 +6,12 @@
>
> from PyQt4 import QtCore
>
> +from openlp.core.common import Settings
> from openlp.core.lib.htmlbuilder import build_html, build_background_css, build_lyrics_css, build_lyrics_outline_css, \
> build_lyrics_format_css, build_footer_css
> from openlp.core.lib.theme import HorizontalType, VerticalType
> from tests.functional import MagicMock, patch
> -
> +from tests.helpers.testmixin import TestMixin
>
> HTML = """
> <!DOCTYPE html>
> @@ -184,7 +185,7 @@
> LYRICS_FORMAT_CSS = ' word-wrap: break-word; text-align: justify; vertical-align: bottom; ' + \
> 'font-family: Arial; font-size: 40pt; color: #FFFFFF; line-height: 108%; margin: 0;padding: 0; ' + \
> 'padding-bottom: 0.5em; padding-left: 2px; width: 1580px; height: 810px; font-style:italic; font-weight:bold; '
> -FOOTER_CSS = """
> +FOOTER_CSS_BASE = """
> left: 10px;
> bottom: 0px;
> width: 1260px;
> @@ -192,11 +193,28 @@
> font-size: 12pt;
> color: #FFFFFF;
> text-align: left;
> - white-space: nowrap;
> - """
> -
> -
> -class Htmbuilder(TestCase):
> + white-space: %s;
> + """
> +FOOTER_CSS = FOOTER_CSS_BASE % ('nowrap')
> +FOOTER_CSS_WRAP = FOOTER_CSS_BASE % ('normal')
> +
> +
> +class Htmbuilder(TestCase, TestMixin):
> + """
> + Test the functions in the Htmlbuilder module
> + """
> + def setUp(self):
> + """
> + Create the UI
> + """
> + self.build_settings()
> +
> + def tearDown(self):
> + """
> + Delete all the C++ objects at the end so that we don't have a segfault
> + """
> + self.destroy_settings()
> +
> def build_html_test(self):
> """
> Test the build_html() function
> @@ -316,8 +334,27 @@
> item.theme_data.font_footer_color = '#FFFFFF'
> height = 1024
>
> - # WHEN: create the css.
> + # WHEN: create the css with default settings.
> css = build_footer_css(item, height)
>
> # THEN: THE css should be the same.
> assert FOOTER_CSS == css, 'The footer strings should be equal.'
> +
> + def build_footer_css_wrap_test(self):
> + """
> + Test the build_footer_css() function
> + """
> + # GIVEN: Create a theme.
> + item = MagicMock()
> + item.footer = QtCore.QRect(10, 921, 1260, 103)
> + item.theme_data.font_footer_name = 'Arial'
> + item.theme_data.font_footer_size = 12
> + item.theme_data.font_footer_color = '#FFFFFF'
> + height = 1024
> +
> + # WHEN: Settings say that footer should wrap
> + Settings().setValue('themes/wrap footer', True)
> + css = build_footer_css(item, height)
> +
> + # THEN: Footer should wrap
> + assert FOOTER_CSS_WRAP == css, 'The footer strings should be equal.'
self.assertEqual should be used.
>
> === modified file 'tests/functional/openlp_core_ui/test_slidecontroller.py'
> --- tests/functional/openlp_core_ui/test_slidecontroller.py 2014-07-13 11:04:37 +0000
> +++ tests/functional/openlp_core_ui/test_slidecontroller.py 2014-07-23 20:56:11 +0000
> @@ -504,21 +504,20 @@
> mocked_item = MagicMock()
> mocked_item.is_command.return_value = True
> mocked_item.name = 'Mocked Item'
> - mocked_execute = MagicMock()
> mocked_update_preview = MagicMock()
> mocked_preview_widget = MagicMock()
> mocked_slide_selected = MagicMock()
> - Registry.execute = mocked_execute
> - Registry.create()
> - slide_controller = SlideController(None)
> - slide_controller.service_item = mocked_item
> - slide_controller.update_preview = mocked_update_preview
> - slide_controller.preview_widget = mocked_preview_widget
> - slide_controller.slide_selected = mocked_slide_selected
> - slide_controller.is_live = True
> + with patch.object(Registry, 'execute') as mocked_execute:
> + Registry.create()
> + slide_controller = SlideController(None)
> + slide_controller.service_item = mocked_item
> + slide_controller.update_preview = mocked_update_preview
> + slide_controller.preview_widget = mocked_preview_widget
> + slide_controller.slide_selected = mocked_slide_selected
> + slide_controller.is_live = True
>
> - # WHEN: The method is called
> - slide_controller.on_slide_selected_index([9])
> + # WHEN: The method is called
> + slide_controller.on_slide_selected_index([9])
>
> # THEN: It should have sent a notification
> mocked_item.is_command.assert_called_once_with()
> @@ -535,20 +534,19 @@
> mocked_item = MagicMock()
> mocked_item.is_command.return_value = False
> mocked_item.name = 'Mocked Item'
> - mocked_execute = MagicMock()
> mocked_update_preview = MagicMock()
> mocked_preview_widget = MagicMock()
> mocked_slide_selected = MagicMock()
> - Registry.execute = mocked_execute
> - Registry.create()
> - slide_controller = SlideController(None)
> - slide_controller.service_item = mocked_item
> - slide_controller.update_preview = mocked_update_preview
> - slide_controller.preview_widget = mocked_preview_widget
> - slide_controller.slide_selected = mocked_slide_selected
> + with patch.object(Registry, 'execute') as mocked_execute:
> + Registry.create()
> + slide_controller = SlideController(None)
> + slide_controller.service_item = mocked_item
> + slide_controller.update_preview = mocked_update_preview
> + slide_controller.preview_widget = mocked_preview_widget
> + slide_controller.slide_selected = mocked_slide_selected
>
> - # WHEN: The method is called
> - slide_controller.on_slide_selected_index([7])
> + # WHEN: The method is called
> + slide_controller.on_slide_selected_index([7])
>
> # THEN: It should have sent a notification
> mocked_item.is_command.assert_called_once_with()
>
--
https://code.launchpad.net/~stewart-e/openlp/bug-1243881/+merge/227992
Your team OpenLP Core is subscribed to branch lp:openlp.
References
