[Kde-hardware-devel] Review Request 118390: Powerdevil KCM Updates

[Sebastian Kügler]

Hey,

First, thanks all for the reviews.

On Thursday, May 29, 2014 16:10:19 Andrew Lake wrote:
> > On May 29, 2014, 8:13 a.m., Thomas Pfeiffer wrote:
> > > Good improvements in general!
> > > There are two changes which are not HIG-compliant:
> > > - Control labels which are written next to the controls (checkboxes,
> > > spin boxes etc.) do not use Title Capitalization. See
> > > http://techbase.kde.org/Projects/Usability/HIG/Capitalization for
> > > details. Therefore most of the capitalization changes have to be
> > > reverted, I'm afraid. - For grouping controls (as in the case of
> > > Battery Levels and Events) the HIG recommends using group boxes (see
> > > http://techbase.kde.org/Projects/Usability/HIG/GroupBox ) instead of
> > > simply spacing Apart from those two things, the changes look fine to
> > > me!
> >  w
> > - For grouping controls (as in the case of Battery Levels and Events) the
> > HIG recommends using group boxes (see
> > http://techbase.kde.org/Projects/Usability/HIG/GroupBox ) instead of
> > simply spacing

> Hmm, may be time to update that portion of the HIG since, from a visual
> design standpoint, spacing can be quite effective at grouping visual
> elements while avoiding more visual "chrome". In this case, for the
> Advanced page, a bit more spacing between the Battery Levels group and the
> Events group should provide enough visual distinction between the groups.
> Also, the checkboxes in the Events group can probably be left-aligned under
> Events since they're unrelated to the widgets from the Battery Levels group
> and likely better match the alignment guidelines for making the checkboxes
> span two columns
> (http://techbase.kde.org/Projects/Usability/HIG/Alignment).

Would be nice to have that changed. I'm not a big fan of these groupboxes, for 
two reasons:

- They add visual clutter, and they're often over-used
- They break layouts (kind of the purpose), but aligning the widgets in two 
groupboxes requires internal layout poking, which sucks on the one hand and 
usually means that the widgets stay unaligned (Aurelien's hack mentioned in 
the HIG is really that -- a hack and not available for Qt5 yet).

In some cases, groupboxes might be warranted, inside FormLayout, they usually 
do more harm than good, and there's a semantic grouping mechanism already: The 
titles.

I'll not change to groupboxes in this review, but anticipate the HIG catching 
up. :)

Other changes are coming up, of course.

> Otherwise, the changes look quite nice to me!

Thanks!
-- 
sebas

http://www.kde.org | http://vizZzion.org | GPG Key ID: 9119 0EF9