[Sugar-devel] [PATCH v2 sugar] Shutdown and Logout menu items should activate the buzzy cursor (SL#2151)

[James Cameron]

On 26/09/2010, at 7:29 AM, Anurag Chowdhury wrote:
> Shutdown (and Logout) menu items should activate the buzzy cursor (or provide some other visual feedback, perhaps dimming/locking the UI from use).

"buzzy" should be "busy".  The bug SL#2151 was already updated with the new summary "Shutdown and Logout menu items should activate the busy cursor (or provide some other visual feedback)".

Your text in the commit message was not wrapped within 80 column view, as can be seen when the patch is viewed here:

http://patchwork.sugarlabs.org/patch/281/

Please wrap this text next time you update the commit message.

> ---
> src/jarabe/view/buddymenu.py |   42 ++++++++++++++++++++++++++++++++++--------
> 1 files changed, 34 insertions(+), 8 deletions(-)
> 
> v1 was Reviewed-By: James Cameron <quozl at laptop.org>
> v1->v2: Set the cursor in its appropriate window 
> 
> diff --git a/src/jarabe/view/buddymenu.py b/src/jarabe/view/buddymenu.py
> index 0ba6cc1..7135d9e 100644
> --- a/src/jarabe/view/buddymenu.py
> +++ b/src/jarabe/view/buddymenu.py
> @@ -21,6 +21,8 @@ from gettext import gettext as _
> import gtk
> import gconf
> import dbus
> +import jarabe
> +import glib
> 
> from sugar.graphics.palette import Palette
> from sugar.graphics.menuitem import MenuItem
> @@ -98,16 +100,40 @@ class BuddyMenu(Palette):
>         item.show()
> 
>     def __logout_activate_cb(self, menu_item):
> -        session_manager = get_session_manager()
> -        session_manager.logout()
> +        def update_cur(window):
> +            window.get_window().set_cursor(gtk.gdk.Cursor(gtk.gdk.WATCH))
> +            return False     
> +        def shut(self, menu_item):
> +            session_manager = get_session_manager()
> +            session_manager.logout()
> +        window = jarabe.desktop.homewindow.get_instance()
> +        glib.timeout_add(3, update_cur, window)

Your solution defers the cursor change to several milliseconds in the future, and calls the session manager method immediately as part of a single-shot recursion into gtk.main().

How did you determine that three (3) milliseconds would be the appropriate delay?

> +        glib.idle_add(shut,self,menu_item)

Adding shut() function to the idle queue and then immediately calling gtk.main() would cause the shut() function to be called almost immediately.  Why not call the shut() function at this point instead?

> +        gtk.main()

Can you achieve the correct result without recursion back into gtk.main()?  Calling gtk.main() in a callback that gtk.main() has already called does not seem to be a good practice.

>     def __reboot_activate_cb(self, menu_item):
> -        session_manager = get_session_manager()
> -        session_manager.reboot()
> -
> -    def __shutdown_activate_cb(self, menu_item):
> -        session_manager = get_session_manager()
> -        session_manager.shutdown()
> +        def update_cur(window):
> +            window.get_window().set_cursor(gtk.gdk.Cursor(gtk.gdk.WATCH))
> +            return False     
> +        def shut(self, menu_item):
> +            session_manager = get_session_manager()
> +            session_manager.reboot()

These two functions are repetitive.  The same code with very little difference is used in two, no three, places.

> +        window = jarabe.desktop.homewindow.get_instance()
> +        glib.timeout_add(3, update_cur, window)              
> +        glib.idle_add(shut,self,menu_item)
> +        gtk.main()
> +    
> +    def __shutdown_activate_cb(self, menu_item):    
> +        def update_cur(window):
> +            window.get_window().set_cursor(gtk.gdk.Cursor(gtk.gdk.WATCH))
> +            return False     
> +        def shut(self, menu_item):
> +            session_manager = get_session_manager()
> +            session_manager.shutdown()
> +        window = jarabe.desktop.homewindow.get_instance()
> +        glib.timeout_add(3, update_cur, window)              
> +        glib.idle_add(shut,self,menu_item)
> +        gtk.main()
> 
>     def __controlpanel_activate_cb(self, menu_item):
>         panel = ControlPanel()
> -- 
> 1.7.2.2
> 
> _______________________________________________
> Sugar-devel mailing list
> Sugar-devel at lists.sugarlabs.org
> http://lists.sugarlabs.org/listinfo/sugar-devel

--
James Cameron
System Test Coordinator
One Laptop per Child