Merge lp:~chrismcginlay/ubuntuone-control-panel/fixes_715820 into lp:ubuntuone-control-panel

[Chris McGinlay]

- Implement tooltips for Connect/Disconnect and Account/Cloud/Devices (LP:715820).

Implement tooltips for Connect/Disconnect and Account/Cloud/Devices (LP:715820)
Modified _update_status(), adding tooltip-None argument after callback argument
Modified ManagementPanel.__init__(), using set_tooltip_text()
Set up corresponding tests, asserting existence and validity of tooltips.

[Natalia Bidart (nataliabidart)]

Chris,

Thanks for working on this! The branch is good to go except for a few things that need fixing:

* the adding of tooltip=None in _update_status should be the last argument, to maintain compatibility of the function API.

* for completeness sake, you should add tooltips (and the matching tests) for the rest of the buttons (ENABLE, RESTART, START, STOP). See below for proper strings to use as tooltips.

* tooltip strings should be (always start with an action since is a button tooltip):

    CONNECT_TOOLTIP = _('Connect the file sync service with your personal cloud.')
    DISCONNECT_TOOLTIP = _('Disconnect the file sync service from your personal cloud.')
    ENABLE_TOOLTIP = _('Enable the file sync service.')
    RESTART_TOOLTIP = _('Restart the file sync service.')
    START_TOOLTIP = _('Start the file sync service.')
    STOP_TOOLTIP = _('Stop the file sync service.')

    DASHBOARD_BUTTON_TOOLTIP = _('View your personal details and service summary.')
    DEVICES_BUTTON_TOOLTIP = _('Manage devices registered with your personal cloud.')
    VOLUMES_BUTTON_TOOLTIP = _('Manage your cloud folders.')

* fix a couple of docstrings to be PEP-257 compliant:

        """The file sync status is disconnected.
        * The correct connection status is displayed.
        * The button has a tooltip.
        * The correct connect tooltip is set.

        """

should be

        """The file sync status is disconnected.

        * The correct connection status is displayed.
        * The button has a tooltip.
        * The correct connect tooltip is set.

        """

(same for docstring in test_on_file_sync_status_syncing).

* Instead of duplicate the adding of the assertions

        self.assertTrue(self.ui.button.get_has_tooltip())
        self.assertEqual(self.ui.button.get_tooltip_text(),
                         self.ui.DISCONNECT_TOOLTIP)

modify assert_status_correct to receive the tooltip and assert over it in only one place.

One again, thanks a lot! This is pretty close to be landed.

review: Needs Fixing

[lp:~chrismcginlay/ubuntuone-control-panel/fixes_715820]

73. By Chris McGinlay on 2011-02-22

Added tooltip strings for enable/disable/start/stop sync states along with testcases
Moved tooltip argument in _update_status to end of argument list
Introduced tooltip=None argument in assert_status_correct to streamline assertion tests.
Fixed docstrings for tooltip testcases.

[Chris McGinlay (chrismcginlay)]

Hi Naty, thank you for suggestions and advice. I hope these are implemented correctly and have updated branch to be reviewed. On running my latest control panel tonight it seems a little slow when manually testing disconnect/reconnect. Probably is due to a local Internet connectivity issue here, although you might want to try it too. All test cases pass, except the 1 skipped.

[Natalia Bidart (nataliabidart)]

Very good! Thanks for working on this.

review: Approve

[Roberto Alsina (ralsina)]

+1, great work Chris!

review: Approve

=== modified file 'ubuntuone/controlpanel/gtk/gui.py'
--- ubuntuone/controlpanel/gtk/gui.py	2011-02-22 19:28:14 +0000
+++ ubuntuone/controlpanel/gtk/gui.py	2011-02-22 23:51:33 +0000
@@ -1132,6 +1132,15 @@
     START = _('Start')
     STOP = _('Stop')
 
+    CONNECT_TOOLTIP = _('Connect the file sync service '
+                        'with your personal cloud')
+    DISCONNECT_TOOLTIP = _('Disconnect the file sync service '
+                           'from your personal cloud')
+    ENABLE_TOOLTIP = _('Enable the file sync service')
+    RESTART_TOOLTIP = _('Restart the file sync service')
+    START_TOOLTIP = _('Start the file sync service')
+    STOP_TOOLTIP = _('Stop the file sync service')
+
     def __init__(self):
         gtk.HBox.__init__(self)
         ControlPanelMixin.__init__(self)
@@ -1166,7 +1175,8 @@
                                       error_handler=error_handler)
         self.show_all()
 
-    def _update_status(self, msg, action, callback, icon=None, color=None):
+    def _update_status(self, msg, action, callback,
+                       icon=None, color=None, tooltip=None):
         """Update the status info."""
         if icon is not None:
             foreground = '' if color is None else 'foreground="%s"' % color
@@ -1178,6 +1188,8 @@
         self.button.set_uri(action)
         self.button.set_sensitive(True)
         self.button.set_data('callback', callback)
+        if tooltip is not None:
+            self.button.set_tooltip_text(tooltip)
 
     def _on_button_clicked(self, button):
         """Button was clicked, act accordingly the label."""
@@ -1190,48 +1202,49 @@
         """Backend notifies of file sync status being disabled."""
         self._update_status(self.FILE_SYNC_DISABLED,
                             self.ENABLE, self.on_enable_clicked,
-                            '✘', 'red')
+                            '✘', 'red', self.ENABLE_TOOLTIP)
 
     @log_call(logger.info)
     def on_file_sync_status_starting(self, msg):
         """Backend notifies of file sync status being starting."""
         self._update_status(self.FILE_SYNC_STARTING,
                             self.STOP, self.on_stop_clicked,
-                            '⇅', ORANGE)
+                            '⇅', ORANGE, self.STOP_TOOLTIP)
 
     @log_call(logger.info)
     def on_file_sync_status_stopped(self, msg):
         """Backend notifies of file sync being stopped."""
         self._update_status(self.FILE_SYNC_STOPPED,
                             self.START, self.on_start_clicked,
-                            '✘', 'red')
+                            '✘', 'red', self.START_TOOLTIP)
 
     @log_call(logger.info)
     def on_file_sync_status_disconnected(self, msg):
         """Backend notifies of file sync status being ready."""
         self._update_status(self.FILE_SYNC_DISCONNECTED,
                             self.CONNECT, self.on_connect_clicked,
-                            '✘', 'red')
+                            '✘', 'red', self.CONNECT_TOOLTIP,)
 
     @log_call(logger.info)
     def on_file_sync_status_syncing(self, msg):
         """Backend notifies of file sync status being syncing."""
         self._update_status(self.FILE_SYNC_SYNCING,
                             self.DISCONNECT, self.on_disconnect_clicked,
-                            '⇅', ORANGE)
+                            '⇅', ORANGE, self.DISCONNECT_TOOLTIP)
 
     @log_call(logger.info)
     def on_file_sync_status_idle(self, msg):
         """Backend notifies of file sync status being idle."""
         self._update_status(self.FILE_SYNC_IDLE,
                             self.DISCONNECT, self.on_disconnect_clicked,
-                            '✔', 'green')
+                            '✔', 'green', self.DISCONNECT_TOOLTIP)
 
     @log_call(logger.error)
     def on_file_sync_status_error(self, error_dict=None):
         """Backend notifies of an error when fetching file sync status."""
         self._update_status(WARNING_MARKUP % self.FILE_SYNC_ERROR,
-                            self.RESTART, self.on_restart_clicked)
+                            self.RESTART, self.on_restart_clicked,
+                            tooltip=self.RESTART_TOOLTIP)
 
     @log_call(logger.error)
     def on_files_start_error(self, error_dict=None):
@@ -1286,6 +1299,11 @@
     QUOTA_LABEL = _('Using %(used)s of %(total)s (%(percentage).1f%%)')
     DASHBOARD_BUTTON_NAME = 'Account'
     DEVICES_BUTTON_NAME = 'Devices'
+    DASHBOARD_BUTTON_TOOLTIP = _('View your personal details '
+                                 'and service summary.')
+    DEVICES_BUTTON_TOOLTIP = _('Manage devices registered '
+                               'with your personal cloud.')
+    VOLUMES_BUTTON_TOOLTIP = _('Manage your cloud folders.')
 
     def __init__(self, main_window=None):
         gtk.VBox.__init__(self)
@@ -1333,11 +1351,14 @@
             self.notebook.insert_page(getattr(self, tab), position=page_num)
 
         self.dashboard_button.set_name(self.DASHBOARD_BUTTON_NAME)
+        self.dashboard_button.set_tooltip_text(self.DASHBOARD_BUTTON_TOOLTIP)
 
         self.volumes_button.connect('clicked', lambda b: self.volumes.load())
         self.services_button.connect('clicked', lambda b: self.services.load())
+        self.volumes_button.set_tooltip_text(self.VOLUMES_BUTTON_TOOLTIP)
 
         self.devices_button.set_name(self.DEVICES_BUTTON_NAME)
+        self.devices_button.set_tooltip_text(self.DEVICES_BUTTON_TOOLTIP)
         self.devices_button.connect('clicked', lambda b: self.devices.load())
         self.devices.connect('local-device-removed',
                              lambda widget: self.emit('local-device-removed'))

=== modified file 'ubuntuone/controlpanel/gtk/tests/test_gui.py'
--- ubuntuone/controlpanel/gtk/tests/test_gui.py	2011-02-22 19:28:14 +0000
+++ ubuntuone/controlpanel/gtk/tests/test_gui.py	2011-02-22 23:51:33 +0000
@@ -2052,13 +2052,15 @@
 
     klass = gui.FileSyncStatus
 
-    def assert_status_correct(self, status, action=None, callback=None):
+    def assert_status_correct(self, status, action=None,
+                              callback=None, tooltip=None):
         """The shown status is correct.
 
         * The ui's label shows 'status'.
         * If action is not None, the ui's button shows that 'action' as label
         and when clicking it, 'self._set_called' gets executed.
         * If action is None, the ui's button should be hidden.
+        * If a tooltip is required, then it exists with correct text.
 
         """
         self.assertTrue(self.ui.label.get_visible())
@@ -2080,6 +2082,12 @@
         else:
             self.assertFalse(self.ui.button.get_visible())
 
+        if tooltip is not None:
+            self.assertTrue(self.ui.button.get_has_tooltip())
+            self.assertEqual(self.ui.button.get_tooltip_text(), tooltip)
+        else:
+            self.assertFalse(self.ui.button.get_has_tooltip())
+
     def test_is_a_box(self):
         """Inherits from gtk.Box."""
         self.assertIsInstance(self.ui, gui.gtk.Box)
@@ -2121,52 +2129,88 @@
         self.assert_backend_called('file_sync_status', ())
 
     def test_on_file_sync_status_disabled(self):
-        """The file sync is disabled."""
+        """The file sync is disabled.
+
+        * The correct connection status is displayed.
+        * The button has a tooltip with correct text.
+
+        """
         self.patch(self.ui, 'on_enable_clicked', self._set_called)
         self.ui.on_file_sync_status_disabled('msg')
 
         self.assert_status_correct(self.ui.FILE_SYNC_DISABLED,
-                                   action=self.ui.ENABLE)
+                                   action=self.ui.ENABLE,
+                                   tooltip=self.ui.ENABLE_TOOLTIP)
 
     def test_on_file_sync_status_starting(self):
-        """The file sync status is starting."""
+        """The file sync status is starting.
+
+        * The correct connection status is displayed.
+        * The button has a tooltip with correct text.
+
+        """
         self.patch(self.ui, 'on_stop_clicked', self._set_called)
         self.ui.on_file_sync_status_starting('msg')
 
         self.assert_status_correct(self.ui.FILE_SYNC_STARTING,
-                                   action=self.ui.STOP)
+                                   action=self.ui.STOP,
+                                   tooltip=self.ui.STOP_TOOLTIP)
 
     def test_on_file_sync_status_stopped(self):
-        """The file sync is stopped."""
+        """The file sync is stopped.
+
+        * The correct connection status is displayed.
+        * The button has a tooltip with correct text.
+
+        """
         self.patch(self.ui, 'on_start_clicked', self._set_called)
         self.ui.on_file_sync_status_stopped('msg')
 
         self.assert_status_correct(self.ui.FILE_SYNC_STOPPED,
-                                   action=self.ui.START)
+                                   action=self.ui.START,
+                                   tooltip=self.ui.START_TOOLTIP)
 
     def test_on_file_sync_status_disconnected(self):
-        """The file sync status is disconnected."""
+        """The file sync status is disconnected.
+
+        * The correct connection status is displayed.
+        * The button has a tooltip with correct text.
+
+        """
         self.patch(self.ui, 'on_connect_clicked', self._set_called)
         self.ui.on_file_sync_status_disconnected('msg')
 
         self.assert_status_correct(self.ui.FILE_SYNC_DISCONNECTED,
-                                   action=self.ui.CONNECT)
+                                   action=self.ui.CONNECT,
+                                   tooltip=self.ui.CONNECT_TOOLTIP)
 
     def test_on_file_sync_status_syncing(self):
-        """The file sync status is syncing."""
+        """The file sync status is syncing.
+
+        * The correct connection status is displayed.
+        * The button has a tooltip with correct text.
+
+        """
         self.patch(self.ui, 'on_disconnect_clicked', self._set_called)
         self.ui.on_file_sync_status_syncing('msg')
 
         self.assert_status_correct(self.ui.FILE_SYNC_SYNCING,
-                                   action=self.ui.DISCONNECT)
+                                   action=self.ui.DISCONNECT,
+                                   tooltip=self.ui.DISCONNECT_TOOLTIP)
 
     def test_on_file_sync_status_idle(self):
-        """The file sync status is idle."""
+        """The file sync status is idle.
+
+        * The correct connection status is displayed.
+        * The button has a tooltip with correct text.
+
+        """
         self.patch(self.ui, 'on_disconnect_clicked', self._set_called)
         self.ui.on_file_sync_status_idle('msg')
 
         self.assert_status_correct(self.ui.FILE_SYNC_IDLE,
-                                   action=self.ui.DISCONNECT)
+                                   action=self.ui.DISCONNECT,
+                                   tooltip=self.ui.DISCONNECT_TOOLTIP)
 
     def test_on_file_sync_status_error(self):
         """The file sync status couldn't be retrieved."""
@@ -2174,7 +2218,9 @@
         self.ui.on_file_sync_status_error({'error_msg': 'error msg'})
 
         msg = gui.WARNING_MARKUP % self.ui.FILE_SYNC_ERROR
-        self.assert_status_correct(msg, action=self.ui.RESTART)
+        self.assert_status_correct(msg,
+                                   action=self.ui.RESTART,
+                                   tooltip=self.ui.RESTART_TOOLTIP)
 
     def test_on_files_start_error(self):
         """The files service could not be started."""
@@ -2405,3 +2451,21 @@
         """The devices_button widget has the proper name."""
         self.assertEqual(self.ui.devices_button.get_name(),
                          self.ui.DEVICES_BUTTON_NAME)
+
+    def test_devices_button_tooltip(self):
+        """The devices button widget has the proper tooltip."""
+        self.assertTrue(self.ui.devices_button.get_has_tooltip())
+        self.assertEqual(self.ui.devices_button.get_tooltip_text(),
+                         self.ui.DEVICES_BUTTON_TOOLTIP)
+
+    def test_dashboard_button_tooltip(self):
+        """The dashboard button widget has the proper tooltip."""
+        self.assertTrue(self.ui.dashboard_button.get_has_tooltip())
+        self.assertEqual(self.ui.dashboard_button.get_tooltip_text(),
+                         self.ui.DASHBOARD_BUTTON_TOOLTIP)
+
+    def test_volumes_button_tooltip(self):
+        """The volumes button widget has the proper tooltip."""
+        self.assertTrue(self.ui.volumes_button.get_has_tooltip())
+        self.assertEqual(self.ui.volumes_button.get_tooltip_text(),
+                         self.ui.VOLUMES_BUTTON_TOOLTIP)