openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #21892
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