Review: Needs Information Some questions, mainly - they may all have perfectly sensible answers, but I wasn't sure, so I asked!
Not sure that having a "toolbar" type for the service items is that intuitive a name - if the toolbar in charge of rendering as well? Or maybe I've misunderstood the point? The docstring in add_using_toolbar() doesn't match the name/function of the function anymore, but I'm not sure what it would be better to say! Do the "toolbar" types of render/toolbar have to have filenames associated? I'm wondering if there might be other sources of data in future for which that doesn't fit well (but I can't hink of one at the moment, so maybe it doesn't matter!) In the Impress toolbar, how come the live toolbar gets "to first" and "to last" buttons, but the preview one doesn't? In the "slightly bigger niggle" category: If the CloseScreen button is going to be on all toolbars (as I assume), should that be provided along with any other "commonly" used code by a suitable base class (like we did with the Media manager) so that the common stuff doesn't have to be written by hand, and stays in the same place all the time? The OnSlideSelected* functions look like candidates for that as well. Sorry to be a pain! Martin -- https://code.launchpad.net/~trb143/openlp/plugins/+merge/8635 Your team openlp.org Core is subscribed to branch lp:openlp. _______________________________________________ Mailing list: https://launchpad.net/~openlp-core Post to : [email protected] Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
