← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~marmyshev/openlp/full_background_audio into lp:openlp

 

Review: Needs Fixing

Follow up to my comments above.
The change to the servicemanager to place the song time in list makes the list unusable as by default it removes 50% of the text area.  Have a look at the hover code and the ability to mess with the icon (notes) for ideas how this can be made better.

editbackgroundaudioform.py is a core class but it imports a plugin class.  Core cannot reference plugin code so this needs to be refactored.

Also the code needs to be made more testable so have a look at https://code.launchpad.net/~trb143/openlp/ft as see what I have done with the controller class.  This moves the code from the UI to a simple class with is easier to run unit tests on.

ServiceItems with do associated songs( I do not use them) have interesting song times on the servicemanager list.
-- 
https://code.launchpad.net/~marmyshev/openlp/full_background_audio/+merge/185998
Your team OpenLP Core is subscribed to branch lp:openlp.


References