Review: Needs Fixing
See line 189 for my main comment.
Others are just me being nit-picky!
Diff comments:
>
> === modified file 'openlp/plugins/presentations/lib/impresscontroller.py'
> --- openlp/plugins/presentations/lib/impresscontroller.py 2019-05-22
> 06:47:00 +0000
> +++ openlp/plugins/presentations/lib/impresscontroller.py 2019-05-29
> 19:24:56 +0000
> @@ -166,6 +187,8 @@
> Called at system exit to clean up any running presentations.
> """
> log.debug('Kill OpenOffice')
> + if self.presenter_screen_disabled_by_openlp:
> + self._toggle_presentation_screen(True)
is this correct with the leading underscore? or should it be
"self.toggle_presentation_screen(True)"
> while self.docs:
> self.docs[0].close_presentation()
> desktop = None
> @@ -195,6 +218,54 @@
> except Exception:
> log.warning('Failed to terminate OpenOffice')
>
> + def toggle_presentation_screen(self, target_value):
Would `set_visible` be more descriptive for `target_value`?
Can you document it? I.e. ":param bool {set_visible|target_value): .... "
> + """
> + Enable or disable the Presentation Screen/Console
> + """
> + # Create Instance of ConfigurationProvider
> + if not self.conf_provider:
> + if is_win():
> + self.conf_provider =
> self.manager.createInstance('com.sun.star.configuration.ConfigurationProvider')
> + else:
> + self.conf_provider = self.manager.createInstanceWithContext(
> + 'com.sun.star.configuration.ConfigurationProvider',
> uno.getComponentContext())
> + # Setup lookup properties to get Impress settings
> + properties = []
> + properties.append(self.create_property('nodepath',
> 'org.openoffice.Office.Impress'))
> + properties = tuple(properties)
> + try:
> + # Get an updateable configuration view
> + impress_conf_props =
> self.conf_provider.createInstanceWithArguments(
> + 'com.sun.star.configuration.ConfigurationUpdateAccess',
> properties)
> + # Get the specific setting for presentation screen
> + presenter_screen_enabled =
> impress_conf_props.getHierarchicalPropertyValue(
> + 'Misc/Start/EnablePresenterScreen')
> + # If the presentation screen is enabled we disable it
> + if presenter_screen_enabled != target_value:
> +
> impress_conf_props.setHierarchicalPropertyValue('Misc/Start/EnablePresenterScreen',
> target_value)
> + impress_conf_props.commitChanges()
> + # if target_value is False this is an attempt to disable the
> Presenter Screen
> + # so we make a note that it has been disabled, so it can be
> enabled again on close.
> + if target_value is False:
> + self.presenter_screen_disabled_by_openlp = True
> + except Exception as e:
> + log.exception(e)
> + trace_error_handler(log)
> + return
Not required
> +
> + def create_property(self, name, value):
> + """
> + Create an OOo style property object which are passed into some Uno
> methods.
> + """
can you document `name`, `value` and the return along with their types please?
> + log.debug('create property OpenOffice')
> + if is_win():
> + property_object =
> self.manager.Bridge_GetStruct('com.sun.star.beans.PropertyValue')
> + else:
> + property_object = PropertyValue()
> + property_object.Name = name
> + property_object.Value = value
> + return property_object
> +
>
> class ImpressDocument(PresentationDocument):
> """
> @@ -483,3 +563,97 @@
> note = ' '
> notes.append(note)
> self.save_titles_and_notes(titles, notes)
> +
> +
> +class SlideShowListener(SlideShowListenerImport):
> + """
> + Listener interface to receive global slide show events.
> + """
> +
> + def __init__(self, document):
> + """
> + Constructor
> +
> + :param document: The ImpressDocument being presented
> + """
> + self.document = document
> +
> + def paused(self):
> + """
> + Notify that the slide show is paused
> + """
> + log.debug('LibreOffice SlideShowListener event: paused')
> +
> + def resumed(self):
> + """
> + Notify that the slide show is resumed from a paused state
> + """
> + log.debug('LibreOffice SlideShowListener event: resumed')
> +
> + def slideTransitionStarted(self):
> + """
> + Notify that a new slide starts to become visible.
> + """
> + log.debug('LibreOffice SlideShowListener event:
> slideTransitionStarted')
> +
> + def slideTransitionEnded(self):
> + """
> + Notify that the slide transtion of the current slide ended.
> + """
> + log.debug('LibreOffice SlideShowListener event:
> slideTransitionEnded')
> +
> + def slideAnimationsEnded(self):
> + """
> + Notify that the last animation from the main sequence of the current
> slide has ended.
> + """
> + log.debug('LibreOffice SlideShowListener event:
> slideAnimationsEnded')
> + if not Registry().get('main_window').isActiveWindow():
> + log.debug('main window is not in focus - should update
> slidecontroller')
> + Registry().execute('slidecontroller_live_change',
> self.document.control.getCurrentSlideIndex() + 1)
> +
> + def slideEnded(self, reverse):
> + """
> + Notify that the current slide has ended, e.g. the user has clicked
> on the slide. Calling displaySlide()
> + twice will not issue this event.
Document `reverse` please with type info
> + """
> + log.debug('LibreOffice SlideShowListener event: slideEnded %d' %
> reverse)
> + if reverse:
> + self.document.slide_ended = False
> + self.document.slide_ended_reverse = True
> + else:
> + self.document.slide_ended = True
> + self.document.slide_ended_reverse = False
> +
> + def hyperLinkClicked(self, hyperLink):
> + """
> + Notifies that a hyperlink has been clicked.
> + """
> + log.debug('LibreOffice SlideShowListener event: hyperLinkClicked %s'
> % hyperLink)
> +
> + def disposing(self, source):
> + """
> + gets called when the broadcaster is about to be disposed.
> + :param source:
> + """
> + log.debug('LibreOffice SlideShowListener event: disposing')
> +
> + def beginEvent(self, node):
> + """
> + This event is raised when the element local timeline begins to play.
> + :param node:
> + """
> + log.debug('LibreOffice SlideShowListener event: beginEvent')
> +
> + def endEvent(self, node):
> + """
> + This event is raised at the active end of the element.
> + :param node:
> + """
> + log.debug('LibreOffice SlideShowListener event: endEvent')
> +
> + def repeat(self, node):
> + """
> + This event is raised when the element local timeline repeats.
> + :param node:
> + """
> + log.debug('LibreOffice SlideShowListener event: repeat')
>
> === modified file 'openlp/plugins/presentations/lib/presentationcontroller.py'
> --- openlp/plugins/presentations/lib/presentationcontroller.py
> 2019-05-22 06:47:00 +0000
> +++ openlp/plugins/presentations/lib/presentationcontroller.py
> 2019-05-29 19:24:56 +0000
> @@ -248,15 +248,17 @@
> def next_step(self):
> """
> Triggers the next effect of slide on the running presentation. This
> might be the next animation on the current
> - slide, or the next slide
> + slide, or the next slide.
> + Returns True if we stepped beyond the slides of the presentation
":returns bool: True if we stepped beyond the slides of the presentation"
would be better
> """
> - pass
> + return False
>
> def previous_step(self):
> """
> Triggers the previous slide on the running presentation
> + Returns True if we stepped beyond the slides of the presentation
Again: ":returns bool: ..."
> """
> - pass
> + return False
>
> def convert_thumbnail(self, image_path, index):
> """
--
https://code.launchpad.net/~tomasgroth/openlp/presentation-beyond-last/+merge/368091
Your team OpenLP Core is subscribed to branch lp:openlp.
_______________________________________________
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help : https://help.launchpad.net/ListHelp
