Merge lp://qastaging/~charlesk/indicator-power/lp-1470080-missing-icon-when-apple-devices-connected into lp://qastaging/indicator-power

Proposed by Charles Kerr
Status: Merged
Approved by: Ted Gould
Approved revision: 302
Merged at revision: 296
Proposed branch: lp://qastaging/~charlesk/indicator-power/lp-1470080-missing-icon-when-apple-devices-connected
Merge into: lp://qastaging/indicator-power
Diff against target: 510 lines (+291/-112)
3 files modified
src/device.c (+2/-1)
src/service.c (+19/-1)
tests/test-device.cc (+270/-110)
To merge this branch: bzr merge lp://qastaging/~charlesk/indicator-power/lp-1470080-missing-icon-when-apple-devices-connected
Reviewer Review Type Date Requested Status
Ted Gould (community) Approve
PS Jenkins bot continuous-integration Pending
Review via email: mp+295752@code.qastaging.launchpad.net

Commit message

Fix bug that chose the wrong header icon if a connected device has a charge but its charging/discharging state is unknown.

Description of the change

1. If we know a upower device's charge level, but not its state (eg whether charging, discharging, etc), then use the same icons as when it’s discharging. (mpt in bug comment 10)

2. When choosing which upower device to show as the ``primary'' (ie the one used in the indicator icon), we shouldn’t choose a device in the menu title when we don’t know whether it’s charging or discharging, if for other things we *do* know whether they’re charging or discharging. (mpt in bug comment 10)

3. Clean up the device tests to be more readable. (ted)

4. If foo-charging icon isn't present, fall back to foo icon. (charles, seb128)

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

The code looks fine, but we might consider refactoring some of the magic numbers to enums at some point. The tables of data are getting a little hard to read.

review: Approve
Revision history for this message
Ted Gould (ted) wrote :

That's a crazy number of alternative icons. But works for me.

review: Approve

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