diff --git a/openlp/core/display/render.py b/openlp/core/display/render.py
index 69b0f9f28..5efd7577e 100644
--- a/openlp/core/display/render.py
+++ b/openlp/core/display/render.py
@@ -616,8 +616,8 @@ class ThemePreviewRenderer(DisplayWindow, LogMixin):
text = text.replace(' [---]', '[---]')
while '[---] ' in text:
text = text.replace('[---] ', '[---]')
- # Grab the number of occurrences of "[---]" in the text to use as a max number of loops
- count = text.count('[---]')
+ # Grab the number of lines in the text to use as a max number of loops
+ count = text.count('\n') + 1
for _ in range(count):
slides = text.split('\n[---]\n', 2)
# If there are (at least) two occurrences of [---] we use the first two slides (and neglect the last
@@ -664,7 +664,6 @@ class ThemePreviewRenderer(DisplayWindow, LogMixin):
lines = text.strip('\n').split('\n')
pages.extend(self._paginate_slide(lines, line_end))
break
- count += 1
else:
# Clean up line endings.
pages = self._paginate_slide(text.split('\n'), line_end)
@@ -694,11 +693,13 @@ class ThemePreviewRenderer(DisplayWindow, LogMixin):
html_lines = list(map(render_tags, lines))
# Text too long so go to next page.
if not self._text_fits_on_slide(separator.join(html_lines)):
- html_text, previous_raw = self._binary_chop(
+ previous_html, previous_raw = self._binary_chop(
formatted, previous_html, previous_raw, html_lines, lines, separator, '')
else:
previous_raw = separator.join(lines)
- formatted.append(previous_raw)
+ previous_html = True
+ if previous_html:
+ formatted.append(previous_raw)
return formatted
def _paginate_slide_words(self, lines, line_end):
@@ -739,7 +740,8 @@ class ThemePreviewRenderer(DisplayWindow, LogMixin):
else:
previous_html += html_line + line_end
previous_raw += line + line_end
- formatted.append(previous_raw)
+ if previous_html:
+ formatted.append(previous_raw)
return formatted
def _binary_chop(self, formatted, previous_html, previous_raw, html_list, raw_list, separator, line_end):
@@ -751,7 +753,7 @@ class ThemePreviewRenderer(DisplayWindow, LogMixin):
:param formatted: The list to append any slides.
:param previous_html: The html text which is know to fit on a slide, but is not yet added to the list of
slides. (unicode string)
- :param previous_raw: The raw text (with formatting tags) which is know to fit on a slide, but is not yet added
+ :param previous_raw: The raw text (with formatting tags) which is known to fit on a slide, but is not yet added
to the list of slides. (unicode string)
:param html_list: The elements which do not fit on a slide and needs to be processed using the binary chop.
The text contains html.
@@ -772,7 +774,7 @@ class ThemePreviewRenderer(DisplayWindow, LogMixin):
else:
smallest_index = index
index = index + (highest_index - index) // 2
- # We found the number of words which will fit.
+ # We found the number of elements which will fit.
if smallest_index == index or highest_index == index:
index = smallest_index
text = previous_raw.rstrip('
') + separator.join(raw_list[:index + 1])
@@ -787,9 +789,10 @@ class ThemePreviewRenderer(DisplayWindow, LogMixin):
else:
continue
# Check if the remaining elements fit on the slide.
- if self._text_fits_on_slide(html_tags + separator.join(html_list[index + 1:]).strip()):
- previous_html = html_tags + separator.join(html_list[index + 1:]).strip() + line_end
- previous_raw = raw_tags + separator.join(raw_list[index + 1:]).strip() + line_end
+ check_string = separator.join(html_list[index + 1:]).strip()
+ if self._text_fits_on_slide(html_tags + check_string):
+ previous_html = html_tags + check_string + line_end
+ previous_raw = raw_tags + check_string + line_end
break
else:
# The remaining elements do not fit, thus reset the indexes, create a new list and continue.
@@ -806,16 +809,19 @@ class ThemePreviewRenderer(DisplayWindow, LogMixin):
def _text_fits_on_slide(self, text):
"""
Checks if the given ``text`` fits on a slide. If it does ``True`` is returned, otherwise ``False``.
+ Text should always "fit" for empty strings
:param text: The text to check. It may contain HTML tags.
"""
+ if text == '':
+ return True
self.clear_slides()
self.log_debug('_text_fits_on_slide: 1\n{text}'.format(text=text))
self.run_javascript('Display.setTextSlide("{text}");'
.format(text=text.replace('"', '\\"')), is_sync=True)
self.log_debug('_text_fits_on_slide: 2')
- does_text_fits = self.run_javascript('Display.doesContentFit();', is_sync=True)
- return does_text_fits
+ does_text_fit = self.run_javascript('Display.doesContentFit();', is_sync=True)
+ return does_text_fit
def save_screenshot(self, fname=None):
"""
diff --git a/tests/functional/openlp_core/display/test_render.py b/tests/functional/openlp_core/display/test_render.py
index 8c82cc3b5..25b3aa624 100644
--- a/tests/functional/openlp_core/display/test_render.py
+++ b/tests/functional/openlp_core/display/test_render.py
@@ -21,10 +21,10 @@
"""
Test the :mod:`~openlp.core.display.render` package.
"""
-from unittest.mock import patch
+from unittest.mock import MagicMock, patch
from openlp.core.display.render import compare_chord_lyric_width, find_formatting_tags, remove_tags, render_chords, \
- render_chords_for_printing, render_tags
+ render_chords_for_printing, render_tags, ThemePreviewRenderer
from openlp.core.lib.formattingtags import FormattingTags
@@ -211,3 +211,45 @@ def test_find_formatting_tags(settings):
# THEN: The list of active tags should contain only 'st'
assert active_tags == ['st'], 'The list of active tags should contain only "st"'
+
+
+def test_format_slide(settings):
+ """
+ Test that the format_slide function works as expected
+ """
+ # GIVEN: Renderer where no text fits on screen (force one line per slide)
+ with patch('openlp.core.display.render.ThemePreviewRenderer.__init__') as init_fn:
+ init_fn.return_value = None
+ preview_renderer = ThemePreviewRenderer()
+ lyrics = 'hello {st}test{/st}\nline two\n[---]\nline after optional split'
+ preview_renderer._is_initialised = True
+ preview_renderer.log_debug = MagicMock()
+ preview_renderer._text_fits_on_slide = MagicMock(side_effect=lambda a: a == '')
+ preview_renderer.force_page = False
+
+ # WHEN: format_slide is run
+ formatted_slides = preview_renderer.format_slide(lyrics, None)
+
+ # THEN: The formatted slides should have all the text and no blank slides
+ assert formatted_slides == ['hello {st}test{/st}', 'line two', 'line after optional split']
+
+
+def test_format_slide_no_split(settings):
+ """
+ Test that the format_slide function doesn't split a slide that fits
+ """
+ # GIVEN: Renderer where no text fits on screen (force one line per slide)
+ with patch('openlp.core.display.render.ThemePreviewRenderer.__init__') as init_fn:
+ init_fn.return_value = None
+ preview_renderer = ThemePreviewRenderer()
+ lyrics = 'line one\n[---]\nline two'
+ preview_renderer._is_initialised = True
+ preview_renderer.log_debug = MagicMock()
+ preview_renderer._text_fits_on_slide = MagicMock(return_value=True)
+ preview_renderer.force_page = False
+
+ # WHEN: format_slide is run
+ formatted_slides = preview_renderer.format_slide(lyrics, None)
+
+ # THEN: The formatted slides should have all the text and no blank slides
+ assert formatted_slides == ['line one
line two']