openlp-core team mailing list archive

[Tim Bentley <tim.bentley@xxxxxxxxx>]

Review: Needs Fixing

See below.
Should have a new test not just a fix to an existing one.

Diff comments:

> === modified file 'openlp/core/ui/slidecontroller.py'
> --- openlp/core/ui/slidecontroller.py	2015-09-03 19:21:43 +0000
> +++ openlp/core/ui/slidecontroller.py	2015-10-12 17:20:41 +0000
> @@ -244,10 +244,15 @@
>                                                  icon=':/slides/slide_desktop.png',
>                                                  checked=False, can_shortcuts=True, category=self.category,
>                                                  triggers=self.on_hide_display)
> +            self.escape_item = create_action(self, 'escapeItem',
> +                                             text=translate('OpenLP.SlideController', 'Escape Item'),

What does Escape Item mean.  Need a more meaning full description.  
Is this only for Portable / single screen.  Is it always needed.

> +                                             checked=False, can_shortcuts=True, category=self.category,
> +                                             triggers=self.live_escape)
>              self.hide_menu.setDefaultAction(self.blank_screen)
>              self.hide_menu.menu().addAction(self.blank_screen)
>              self.hide_menu.menu().addAction(self.theme_screen)
>              self.hide_menu.menu().addAction(self.desktop_screen)
> +            self.hide_menu.menu().addAction(self.escape_item)
>              # Wide menu of display control buttons.
>              self.blank_screen_button = QtGui.QToolButton(self.toolbar)
>              self.blank_screen_button.setObjectName('blank_screen_button')
> @@ -1060,6 +1048,23 @@
>              else:
>                  Registry().execute('live_display_show')
>  
> +    def live_escape(self):

Need field = None for debug injection.
Why have you moved the method.

> +        """
> +        Pressing ESC (default) triggers live_escape and hides current Live item.
> +        """
> +        self.display.setVisible(False)
> +        self.media_controller.media_stop(self)
> +

No Need for spaces.

> +        # This will stop any odp/ppt presentations in progress.
> +        if self.service_item is not None:
> +            Registry().execute('%s_hide' % self.service_item.name.lower(), [self.service_item, self.is_live])
> +
> +        # Stop looping if active
> +        if self.play_slides_loop.isChecked():
> +            self.on_play_slides_loop(False)
> +        elif self.play_slides_once.isChecked():
> +            self.on_play_slides_once(False)
> +
>      def on_slide_selected(self, field=None):
>          """
>          Slide selected in controller


-- 
https://code.launchpad.net/~suutari-olli/openlp/escape-fixes-1294111-1497637/+merge/274175
Your team OpenLP Core is subscribed to branch lp:openlp.