openlp-core team mailing list archive

[Tomas Groth <tomasgroth@xxxxxxxx>]

Review: Needs Fixing

Looks good now. As you mention in one of your source-comments the previous slide is shortly visible when display is unblanked. Don't know if it can be helped.
Added a comment to the code below.
Besides that, you need tests :)

Diff comments:

> 
> === modified file 'openlp/core/ui/slidecontroller.py'
> --- openlp/core/ui/slidecontroller.py	2016-02-28 20:33:19 +0000
> +++ openlp/core/ui/slidecontroller.py	2016-03-15 18:42:53 +0000
> @@ -789,11 +789,28 @@
>      def replace_service_manager_item(self, item):
>          """
>          Replacement item following a remote edit
> +        This action  also takes place when a song that is sent to live from Service Manager is edited.
> +        If display is blanked, this will update the song and then re-blank the display.
> +        As result, lyrics are flashed on screen for a very short time before re-blanking happens. (Bug)
> +        This happens only when Automatic unblanking is enabled when new item is sen to Live,
> +        if it's not enabled they won't flash.
>  
>          :param item: The current service item
>          """
>          if item == self.service_item:
> -            self._process_item(item, self.preview_widget.current_slide_number())
> +            if not self.hide_mode():
> +                self._process_item(item, self.preview_widget.current_slide_number())
> +            # "isChecked" method is required for checking blanks, on_xx_display(False) does not work.
> +            elif self.hide_mode():
> +                if self.blank_screen.isChecked():
> +                    self._process_item(item, self.preview_widget.current_slide_number())
> +                    self.on_blank_display(True)
> +                elif self.theme_screen.isChecked():
> +                    self._process_item(item, self.preview_widget.current_slide_number())
> +                    self.on_theme_display(True)
> +                elif self.desktop_screen.isChecked():
> +                    self._process_item(item, self.preview_widget.current_slide_number())
> +                    self.on_hide_display(True)

Why don't you just use the value returned by hide_mode() instead of isChecked()?
And why not move  self._process_item(item, self.preview_widget.current_slide_number()) to the outside of the if?

>  
>      def add_service_manager_item(self, item, slide_no):
>          """


-- 
https://code.launchpad.net/~suutari-olli/openlp/click-slide-to-go-live-from-blank/+merge/289026
Your team OpenLP Core is subscribed to branch lp:openlp.