Transfer Indicator

Merge lp://staging/~charlesk/indicator-transfer/per-transfer-actions into lp://staging/indicator-transfer/14.10

  1. per-transfer-actions
  2. Merge into trunk.14.10
Proposed by Charles Kerr
Status: Merged
Approved by: Ted Gould
Approved revision: 12
Merged at revision: 7
Proposed branch: lp://staging/~charlesk/indicator-transfer/per-transfer-actions
Merge into: lp://staging/indicator-transfer/14.10
Diff against target: 693 lines (+285/-149)
8 files modified
MERGE-REVIEW (+40/-31)
README (+11/-10)
include/transfer/transfer.h (+9/-6)
src/CMakeLists.txt (+1/-0)
src/transfer.cpp (+72/-0)
src/view-gmenu.cpp (+102/-81)
src/world-dbus.cpp (+45/-17)
tests/test-view-gmenu.cpp (+5/-4)
To merge this branch: bzr merge lp://staging/~charlesk/indicator-transfer/per-transfer-actions
Reviewer Review Type Date Requested Status
Ted Gould (community) Approve
Review via email: mp+224537@code.staging.launchpad.net

Commit message

Create per-transfer actions for simpler state lookup by renderers. Previously this was done as a single action that contained a map of unique transfer ids to states.

Description of the change

== Description of the Change

1. Instead of having a single GAction "transfer-states" that gives *all* transfers' states inside a map, Have individual per-transfer actions so that it's easier to look up a transfer's state in the renderer.

2. Ensure that the Transfer's opaque unique id exposed in our GActions/GMenus really is opaque. Previously it leaked the object path of the corresponding com.canonical.applications.Download object, which is bad abstraction -- if for some reason we need to provide that path, we should do it explicitly rather than as a side-effect.

== MP Submission Checklist

> * Are there any related MPs required for this MP to build/function as expected? Please list.
Yes, three:
1. https://code.launchpad.net/~nick-dedekind/unity8/transfer-menu/+merge/224673
2. https://code.launchpad.net/~nick-dedekind/ubuntu-settings-components/transfer-menu/+merge/224672
3. https://code.launchpad.net/~tiheum/ubuntu-themes/suru-icons/+merge/217767

> * Is your branch in sync with latest trunk? (e.g. bzr pull lp:trunk -> no changes)
Yes

> * Did the code build without warnings?
Yes

> * Did the tests run successfully?
Yes

> * Did you perform an exploratory manual test run of your code change and any related functionality?

NO: suru icons MP could not be built into a deb, so I was not able to properly test the indicator's header icon changes at <http://bazaar.launchpad.net/~charlesk/indicator-transfer/per-transfer-actions/revision/14>.

YES: Other than the above exception, the exploratory manual test run was successful.

> * Has your component test plan been executed successfully on emulator or a physical device?
Yes, N4 mako

> * Please list which manual tests are germane for the reviewer in this MP.
indicator-transfer/simple-download-check

To post a comment you must log in.
Revision history for this message
Ted Gould (ted) wrote :

Couldn't find anything I wanted to complain about :-(

review: Approve
Revision history for this message
Ted Gould (ted) :
review: Approve
lp://staging/~charlesk/indicator-transfer/per-transfer-actions updated
13. By Charles Kerr

improve MERGE-REVIEW document based on indicator-datetime template

14. By Charles Kerr

add support for the new suru icons

15. By Charles Kerr

rename 'com.canonical.indicator.transfer-bulk-action' as 'com.canonical.indicator.button-section' as discussed with dednick and saviq

Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

* Have you checked that the submitter has accurately filled out the submitter checklist and has taken no shortcuts?

Yes.

* Did you run the manual tests listed by the submitter?

Yes.

* Did you do exploratory testing related to the component you own with the MP changeset included?

Yes.

* If new features have been added, are the manual tests sufficient to cover them?

Yes.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches