Merge into trunk : fixes_715820 : Code : Ubuntu One Control Panel

Status:

Merged

Approved by:

Natalia Bidart on 2011-02-23

Approved revision:

73

Merged at revision:

80

Proposed branch:

lp://staging/~chrismcginlay/ubuntuone-control-panel/fixes_715820

Merge into:

lp://staging/ubuntuone-control-panel

Diff against target:

290 lines (+107/-22)

2 files modified

ubuntuone/controlpanel/gtk/gui.py (+29/-8)
ubuntuone/controlpanel/gtk/tests/test_gui.py (+78/-14)

To merge this branch:

bzr merge lp://staging/~chrismcginlay/ubuntuone-control-panel/fixes_715820

Medium

Fix Released

Link a bug report

Reviewer	Date Requested	Status
Roberto Alsina (community)		Approve on 2011-02-23
Natalia Bidart (community)	2011-02-19	Approve on 2011-02-23
Review via email: mp+50417@code.staging.launchpad.net

Commit message

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

Description of the change

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.

Revision history for this message

Natalia Bidart (nataliabidart) wrote on 2011-02-21:

#

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.

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://staging/~chrismcginlay/ubuntuone-control-panel/fixes_715820 updated on 2011-02-22

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.

Revision history for this message

Chris McGinlay (chrismcginlay) wrote on 2011-02-22:

#

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.

Revision history for this message

Natalia Bidart (nataliabidart) wrote on 2011-02-23:

#

Very good! Thanks for working on this.

review: Approve

Revision history for this message

Roberto Alsina (ralsina) wrote on 2011-02-23:

#

+1, great work Chris!

review: Approve

Ubuntu One Control Panel

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

Commit message

Description of the change

Preview Diff

Subscribers