openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #29905
Re: [Merge] lp:~alisonken1/openlp/strings-templates into lp:openlp
Review: Needs Fixing
Can you use a template for the one bit I've highlighted below please? Also, you've written a number of "# NOTE" comments, which I feel are somewhat useless (especially considering the content of most of them). They don't get picked up when editing, and some poor soul is going to find them 10 years down the line have and have no idea what they mean.
Perhaps use "# TODO: " which generally gets picked up by linters like PyLint?
Diff comments:
>
> === modified file 'openlp/core/lib/renderer.py'
> --- openlp/core/lib/renderer.py 2016-05-15 17:33:42 +0000
> +++ openlp/core/lib/renderer.py 2016-06-04 08:15:27 +0000
> @@ -370,21 +370,22 @@
> self.web.resize(self.page_width, self.page_height)
> self.web_frame = self.web.page().mainFrame()
> # Adjust width and height to account for shadow. outline done in css.
> - # TODO: Verify before converting to python3 strings
> + # TODO: Tested at home
> html = """<!DOCTYPE html><html><head><script>
> - function show_text(newtext) {
> + function show_text(newtext) {{
> var main = document.getElementById('main');
> main.innerHTML = newtext;
> // We need to be sure that the page is loaded, that is why we
> // return the element's height (even though we do not use the
> // returned value).
> return main.offsetHeight;
> - }
> - </script><style>*{margin: 0; padding: 0; border: 0;}
> - #main {position: absolute; top: 0px; %s %s}</style></head><body>
> - <div id="main"></div></body></html>""" % \
> - (build_lyrics_format_css(theme_data, self.page_width, self.page_height),
> - build_lyrics_outline_css(theme_data))
> + }}
> + </script><style>*{{margin: 0; padding: 0; border: 0;}}
> + #main {{position: absolute; top: 0px; {format_css} {outline_css}}}</style></head><body>
> + <div id="main"></div></body></html>""".format(format_css=build_lyrics_format_css(theme_data,
Can't we use Template for this one too?
> + self.page_width,
> + self.page_height),
> + outline_css=build_lyrics_outline_css(theme_data))
> self.web.setHtml(html)
> self.empty_height = self.web_frame.contentsSize().height()
>
--
https://code.launchpad.net/~alisonken1/openlp/strings-templates/+merge/296486
Your team OpenLP Core is subscribed to branch lp:openlp.
References
