OpenLP

Merge lp://qastaging/~sam92/openlp/bug-1695620 into lp://qastaging/openlp

  1. bug-1695620
  2. Merge into trunk
Proposed by Samuel Mehrbrodt
Status: Superseded
Proposed branch: lp://qastaging/~sam92/openlp/bug-1695620
Merge into: lp://qastaging/openlp
Diff against target: 724 lines (+284/-155)
6 files modified
openlp/core/lib/serviceitem.py (+5/-3)
openlp/core/ui/printserviceform.py (+5/-5)
openlp/plugins/songs/lib/mediaitem.py (+40/-36)
openlp/plugins/songs/lib/songstab.py (+72/-37)
openlp/plugins/songs/songsplugin.py (+55/-5)
tests/functional/openlp_plugins/songs/test_mediaitem.py (+107/-69)
To merge this branch: bzr merge lp://qastaging/~sam92/openlp/bug-1695620
Reviewer Review Type Date Requested Status
Tomas Groth Pending
Tim Bentley Pending
Raoul Snyman Pending
Review via email: mp+362915@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2017-06-13.

This proposal has been superseded by a proposal from 2019-02-21.

Commit message

Make use of Mako for footer generation being configurable in song settings

Description of the change

Make use of Mako for footer generation being configurable in song settings

- removed now obsolete and via template better configurable options to display "songbook", "written by" and "copyright" information in footer
- added explanation box for so far used settings as Mako placeholders
- added songs configuration setting for template including reset button
- added default template replacing currently existing configuration as best as possible (should be backwards compatible or at least be adaptable to correspond to former settings)
- write and adapt tests for new and removed functionality
- Added some more available fields in the footer: Alternate Title, CCLI Number, Topics, Authors (all music, all words)

To post a comment you must log in.
Revision history for this message
Tim Bentley (trb143) wrote : Posted in a previous version of this proposal

You need to run CI on this to show the success of tests.
Not sure why but this request did a full download when I did a check out. Also cannot run due to chord issues.
Cannot run this but there seem to be lots of line breaks in the footer display. As the footer area is small this will not be displayed and lead to forum comments.
You need some validation do defend against too long footers!

review: Needs Fixing
Revision history for this message
Raoul Snyman (raoul-snyman) wrote : Posted in a previous version of this proposal

We're moving away from Python generated HTML, and using a JS system based on Reveal.js. Come chat with tgc and myself in IRC.

review: Disapprove
Revision history for this message
Samuel Mehrbrodt (sam92) wrote : Posted in a previous version of this proposal

> Cannot run this but there seem to be lots of line breaks in the footer display. As the footer area is small this will not be displayed and lead to forum comments.

The footer has the same height as before, there should be no difference by default.

So I replaced Pystache with Mako. Should be safe to merge now I think.
As the new renderer is in a very early state atm, I can't see how I can adopt this for the new renderer. Maybe we can merge this now and adopt when the new renderer is ready?

Revision history for this message
Tim Bentley (trb143) wrote : Posted in a previous version of this proposal

Blank lines!
Will review tomorrow as I have bible study!

review: Needs Fixing
Revision history for this message
Samuel Mehrbrodt (sam92) wrote : Posted in a previous version of this proposal

So is there a chance this can be reviewed now?

> Blank lines!
What do you mean with that?

Revision history for this message
Raoul Snyman (raoul-snyman) wrote :

Linux tests passed!

Revision history for this message
Raoul Snyman (raoul-snyman) wrote :

Linting passed!

Revision history for this message
Raoul Snyman (raoul-snyman) wrote :

macOS tests passed!

lp://qastaging/~sam92/openlp/bug-1695620 updated
2762. By Samuel Mehrbrodt

Merge trunk

2763. By Samuel Mehrbrodt

Merge trunk

2764. By Samuel Mehrbrodt

Fix songstab

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.