Activity Log Manager

Reviewer	Review Type	Date Requested	Status
Rico Tzschichholz		2013-07-09	Disapprove on 2013-07-09
Matthew Paul Thomas (community)	design	2013-07-09	Needs Fixing on 2013-07-09
Jeremy Bícha		2013-07-09	Pending
Review via email: mp+173678@code.staging.launchpad.net

Description of the change

Fixes LP: #1198364 where exclude list has smaller icons

Looks like: http://i.imgur.com/zPwGMPp.png

Revision history for this message

Matthew Paul Thomas (mpt) wrote on 2013-07-09:

This looks promising, but all icons should be the same size. Folder and document icons should be just as small as app icons.

review: Needs Fixing (design)

Revision history for this message

Rico Tzschichholz (ricotz) wrote on 2013-07-09:

I don't see the need in reducing the icon-sizes. The problem which you want to fix is not caused by them. The underlying widget structure is statically sized and therefore doesn't adapt to different font-sizes.

And yes, all icons should have the same sizes.

So this approach is not the right thing to do imo. (which makes the bug invalid as well)

review: Disapprove

Revision history for this message

Matthew Paul Thomas (mpt) wrote on 2013-07-09:

Ah, I didn't read the bug report, sorry.

I think that for consistency, icons in this list should be laid out exactly the same way as icons in menu items are. That does mean they should be smaller. However, Rico is right that that wouldn't actually fix the vertical alignment of the text. In the screenshot, the text for the previous-sized folder icons is too high, but the text for the newly-sized application icons is too low. It should be vertically centered, just like text in menu items.