<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/118390/">https://git.reviewboard.kde.org/r/118390/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 29th, 2014, 8:13 a.m. UTC, <b>Thomas Pfeiffer</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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!</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> - 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).
Otherwise, the changes look quite nice to me!</pre>
<br />
<p>- Andrew</p>
<br />
<p>On May 28th, 2014, 11:33 p.m. UTC, Sebastian Kügler wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma, Solid and KDE Usability.</div>
<div>By Sebastian Kügler.</div>
<p style="color: grey;"><i>Updated May 28, 2014, 11:33 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
powerdevil
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This is a series of UI updates I've applied to the powerdevil KCMs. You can find them as individual patch series in the sebas/kcmupdates branch.
General:
- Less icon usage, especially in the form layouts
- Title casing throughout
- Better HIG compliance (not 100%, but improved)
- Energy Saving / Actions UI now scales with dialog
- Proper usage of FormLayouts
- A bunch of cleanups of dead code
- Parenting fixes
In Detail:
* Improve Advanced Settings page
- Use a QFormLayout, and do it properly
- Fix up spacing and alignment
- Remove icons before titles
- Use Title Case for Labels
- Shorter labels for better readability
* Clean up brightness-OSD-related dead code
* Make powerdevil actions layout stretch out horizontally
* Compile-time connections in actionconfigwidget
* widget and layout parenting fixes
* No bold font, increased spacing instead
This removes the bold fonts from the checkboxes, as that is non-standard
in the HIG. In order to make it look a bit more structured between the
sections, a bit of spacing is added.
* Remove icons from actions
The icons are really small and add more visual noise than being useful.
* Streamline Comments of KCMs
* Clean up dead code</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Tested in kcmshell5 and systemsettings, still fully functional.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kcmodule/profiles/profileEditPage.ui <span style="color: grey">(dc2657943a2bb63f137ad11b77b254b3782f0407)</span></li>
<li>kcmodule/profiles/powerdevilprofilesconfig.desktop <span style="color: grey">(e95b908dfea30a4a15ccd8c89d4ceb36256e2185)</span></li>
<li>kcmodule/profiles/EditPage.cpp <span style="color: grey">(a674ccbdd76ba682c7b7111f2f9d0b9123ffe8b8)</span></li>
<li>kcmodule/global/powerdevilglobalconfig.desktop <span style="color: grey">(1d4aa2238b2fa02dfa984f9163f133c941ff509e)</span></li>
<li>kcmodule/global/generalPage.ui <span style="color: grey">(2ce7cef98e30f872ea9233edc7882deea134fdd3)</span></li>
<li>kcmodule/global/GeneralPage.cpp <span style="color: grey">(d025e429df7ca191bf56443b2bd30353a74993a9)</span></li>
<li>kcmodule/common/actioneditwidget.cpp <span style="color: grey">(4c67b4feeb2cff0df2e397f44ffba2926bbb7276)</span></li>
<li>kcmodule/common/actionconfigwidget.cpp <span style="color: grey">(2161c84b68e5fc3b56240a482ed77825ce4a5f03)</span></li>
<li>kcmodule/activities/powerdevilactivitiesconfig.desktop <span style="color: grey">(9ee81b55ec7f4685adb5d102cb21df81f17cf3e3)</span></li>
<li>kcmodule/activities/activitypage.cpp <span style="color: grey">(48e9c6c75f2ab95ce52c96b586475dbb4d6022fd)</span></li>
<li>daemon/actions/dpms/powerdevildpmsactionconfig.cpp <span style="color: grey">(4d96273697f11864716dae0204009d52264be99f)</span></li>
<li>daemon/actions/bundled/runscriptconfig.cpp <span style="color: grey">(c0d3adb6651320702bb6503e505b6ebdc06d6004)</span></li>
<li>daemon/actions/bundled/keyboardbrightnesscontrolconfig.cpp <span style="color: grey">(31772679560a256dcf9f66aefbb4cf02351bd5d3)</span></li>
<li>daemon/actions/bundled/keyboardbrightnesscontrol.cpp <span style="color: grey">(44dbcd87c06f5dce6d1ff10697837a60cdcc5f29)</span></li>
<li>daemon/actions/bundled/dimdisplayconfig.cpp <span style="color: grey">(f683935994e4a5a08fd7da6ceb95ba7b726ac708)</span></li>
<li>daemon/actions/bundled/brightnesscontrolconfig.cpp <span style="color: grey">(fea2a3ea7cb44c27496ea81f4fe35a8ccfde3acc)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/118390/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<ul>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/05/28/680a38ab-18d6-4343-86e8-6d6aeaf63032__powerdevil-kcm-profiles-before.png">Energy Saving page before</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/05/28/30357866-df50-4c1a-afc2-63e3e565f55a__powerdevil-kcm-profiles-after.png">Energy Saving page after</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/05/28/36401d04-da7a-4d30-8b6d-a64ccf3a7865__powerdevil-kcm-advanced-after.png">Advanced page after</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/05/28/e06c29ef-c926-4ae9-b01b-f502c3cfd0a0__powerdevil-kcm-advanced-before.png">Advanced page before</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>