Review Request 108570: This patch add support for bulk operations in systemtray applet settings.

[Sandro Andrade]

> On Jan. 24, 2013, 10:15 a.m., Sebastian Kügler wrote:
> > plasma/generic/applets/systemtray/ui/applet.cpp, line 111
> > <http://git.reviewboard.kde.org/r/108570/diff/1/?file=109069#file109069line111>
> >
> >     Where do the magic numbers come from? Try to avoid them, otherwise add a comment.

These are basically to horizontally align header's checkbox to each item's checkbox. Fixed by getting the checkbox rectangle from current app style and painting the checkbox vertically centralized in header section. A fixed left margin is used but that is font/style-independent. That matches checkbox position for items.


> On Jan. 24, 2013, 10:15 a.m., Sebastian Kügler wrote:
> > plasma/generic/applets/systemtray/ui/applet.cpp, line 569
> > <http://git.reviewboard.kde.org/r/108570/diff/1/?file=109069#file109069line569>
> >
> >     Again, magic numbers? Especially with geometry properties, this blows up as soon as font size changes.

Fixed again by setting size hint to QRect's size acquired from app-styled checkbox.


> On Jan. 24, 2013, 10:15 a.m., Sebastian Kügler wrote:
> > plasma/generic/applets/systemtray/ui/applet.cpp, line 587
> > <http://git.reviewboard.kde.org/r/108570/diff/1/?file=109069#file109069line587>
> >
> >     hardcoded height, try using fontmetrics or KIconLoader sizes here, whichever fits

Fixed to use headerview's height + appropriate padding.


> On Jan. 24, 2013, 10:15 a.m., Sebastian Kügler wrote:
> > plasma/generic/applets/systemtray/ui/autohide.ui, line 9
> > <http://git.reviewboard.kde.org/r/108570/diff/1/?file=109070#file109070line9>
> >
> >     necessary to add width and height here?

Reverted


- Sandro


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108570/#review26127
-----------------------------------------------------------


On Jan. 24, 2013, 2:34 a.m., Sandro Andrade wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108570/
> -----------------------------------------------------------
> 
> (Updated Jan. 24, 2013, 2:34 a.m.)
> 
> 
> Review request for kde-workspace.
> 
> 
> Description
> -------
> 
> In page 'entries', user can select/deselect a specific or all entries and then change visibility and/or reset shortcut for all of them with a single combo selection/clear button click. Individually selecting all entries causes header's checkbox to automatically be checked. After selecting all entries (individually or by clicking in header's checkbox), a single entry deselection causes header's checkbox to automatically be unchecked. General suggestions and, in particular about UI usability, are appreciated.
> 
> 
> Diffs
> -----
> 
>   plasma/generic/applets/systemtray/ui/applet.h 0b4a869 
>   plasma/generic/applets/systemtray/ui/applet.cpp 09482d7 
>   plasma/generic/applets/systemtray/ui/autohide.ui 3b6efff 
> 
> Diff: http://git.reviewboard.kde.org/r/108570/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sandro Andrade
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130124/42bc862f/attachment.htm>