openlp-core team mailing list archive

[Phill <phill.ridout@xxxxxxxxx>]

Review: Needs Fixing

on_lock_button_toggled is such a small method, any changes of tests for that?

Also can you include the parameters in the doc string please. (See inline comments)

Diff comments:

> 
> === modified file 'openlp/plugins/bibles/lib/mediaitem.py'
> --- openlp/plugins/bibles/lib/mediaitem.py	2016-08-10 18:31:33 +0000
> +++ openlp/plugins/bibles/lib/mediaitem.py	2016-09-14 23:28:20 +0000
> @@ -548,19 +548,32 @@
>              self.advancedTab.setVisible(True)
>              self.advanced_book_combo_box.setFocus()
>  
> -    def on_clear_button(self):
> +    def on_clear_button_clicked(self):
>          # Clear the list, then set the "No search Results" message, then clear the text field and give it focus.
>          self.list_view.clear()
>          self.check_search_result()
>          self.quick_search_edit.clear()
>          self.quick_search_edit.setFocus()
>  
> +    def on_advanced_clear_button_clicked(self):
> +        # The same as the on_clear_button_clicked, but gives focus to Book name field in "Select" (advanced).
> +        self.list_view.clear()
> +        self.check_search_result()
> +        self.advanced_book_combo_box.setFocus()
> +
>      def on_lock_button_toggled(self, checked):
> -        self.quick_search_edit.setFocus()
> +        """
> +        Toggle the lock button, if Search tab is used, set focus to search field, if Select tab is used,
> +        give focus to Bible book name field.

:param checked: The state of the toggle button (qt checked type maybe? Or Boolean) 
:return: None

> +        """
>          if checked:
>              self.sender().setIcon(self.lock_icon)
>          else:
>              self.sender().setIcon(self.unlock_icon)
> +        if self.quickTab.isVisible():
> +            self.quick_search_edit.setFocus()
> +        else:
> +            self.advanced_book_combo_box.setFocus()
>  
>      def on_quick_style_combo_box_changed(self):
>          self.settings.layout_style = self.quickStyleComboBox.currentIndex()


-- 
https://code.launchpad.net/~suutari-olli/openlp/fix-advanced-bible-search-clear-button-giving-focus-to-quick/+merge/305770
Your team OpenLP Core is subscribed to branch lp:openlp.