<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 June 2nd, 2014, 4:26 p.m. UTC, <b>Andrew Lake</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;">The group box HIG has been updated after discussion on the HIG mailing list. Using a group box with the 'flat' property set true is now recommended; it provides a consistent way to use spacing to group visual elements without the line-y, boxy noise of a visible frame. If you used group boxes here and followed this new guidance you should get the visual design you're aiming for with more consistent spacing.
Hope this helps!</pre>
</blockquote>
<p>On June 2nd, 2014, 4:39 p.m. UTC, <b>Sebastian Kügler</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;">It helps partly, but it still makes aligning the widgets in different groupboxes really hard. I'd really prefer just simple spacing. If we make it hard to align things properly, people just won't. In this specific case in powerdevil, using groupboxes really seems overkill as well, we can do just fine with section titles and a bit of spacing (as can be seen here).</pre>
</blockquote>
<p>On June 2nd, 2014, 5:07 p.m. UTC, <b>Andrew Lake</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;">Honestly, if in any one instance we can achieve the same consistency in spacing without using the groupbox I'm fine with deviating from the guidelines. They are, after all, guidelines. I understand the impact in terms alignment and there are times when we have to trade strict compliance with one guideline to reap the benefits of complying with the another.</pre>
</blockquote>
<p>On June 2nd, 2014, 5:17 p.m. UTC, <b>Sebastian Kügler</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;">Amen to that. :)</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;">We don't want to be telling devs to set the groupbox to flat, we want to adjust the style such that groupboxes look flat. Otherwise we'll still get a horribly inconsistent mess.
(there's a warning in QGroupBox that setFlat might have no effect depending on the style because even windows does that)
Worst case, we put it in the QStyle::polish() method to change flat. It's still better than every dev doing it.</pre>
<br />
<p>- David</p>
<br />
<p>On June 3rd, 2014, 3:32 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 June 3, 2014, 3:32 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>daemon/actions/bundled/brightnesscontrolconfig.cpp <span style="color: grey">(fea2a3e)</span></li>
<li>daemon/actions/bundled/dimdisplayconfig.cpp <span style="color: grey">(f683935)</span></li>
<li>daemon/actions/bundled/keyboardbrightnesscontrol.cpp <span style="color: grey">(44dbcd8)</span></li>
<li>daemon/actions/bundled/keyboardbrightnesscontrolconfig.cpp <span style="color: grey">(3177267)</span></li>
<li>daemon/actions/bundled/powerdevilbrightnesscontrolaction.desktop <span style="color: grey">(9f03d7f)</span></li>
<li>daemon/actions/bundled/powerdevildimdisplayaction.desktop <span style="color: grey">(df8d7e2)</span></li>
<li>daemon/actions/bundled/powerdevilhandlebuttoneventsaction.desktop <span style="color: grey">(58dccc0)</span></li>
<li>daemon/actions/bundled/powerdevilkeyboardbrightnesscontrolaction.desktop <span style="color: grey">(231c5d6)</span></li>
<li>daemon/actions/bundled/powerdevilrunscriptaction.desktop <span style="color: grey">(465768c)</span></li>
<li>daemon/actions/bundled/powerdevilsuspendsessionaction.desktop <span style="color: grey">(4bd8859)</span></li>
<li>daemon/actions/bundled/runscriptconfig.cpp <span style="color: grey">(c0d3adb)</span></li>
<li>daemon/actions/dpms/powerdevildpmsaction.desktop <span style="color: grey">(0492036)</span></li>
<li>daemon/actions/dpms/powerdevildpmsactionconfig.cpp <span style="color: grey">(4d96273)</span></li>
<li>daemon/actions/powerdevilaction.desktop <span style="color: grey">(5b74fea)</span></li>
<li>daemon/backends/hal/powerdevilhalbackend.desktop <span style="color: grey">(27afb55)</span></li>
<li>daemon/backends/upower/backlight_helper_actions.actions <span style="color: grey">(6ffa496)</span></li>
<li>daemon/backends/upower/powerdevilupowerbackend.desktop <span style="color: grey">(a963844)</span></li>
<li>daemon/powerdevil.desktop <span style="color: grey">(20d31c9)</span></li>
<li>kcmodule/activities/activitypage.cpp <span style="color: grey">(48e9c6c)</span></li>
<li>kcmodule/activities/powerdevilactivitiesconfig.desktop <span style="color: grey">(449ca0e)</span></li>
<li>kcmodule/common/actionconfigwidget.cpp <span style="color: grey">(2161c84)</span></li>
<li>kcmodule/common/actioneditwidget.cpp <span style="color: grey">(4c67b4f)</span></li>
<li>kcmodule/global/GeneralPage.cpp <span style="color: grey">(d025e42)</span></li>
<li>kcmodule/global/generalPage.ui <span style="color: grey">(2ce7cef)</span></li>
<li>kcmodule/global/powerdevilglobalconfig.desktop <span style="color: grey">(21d0212)</span></li>
<li>kcmodule/profiles/EditPage.cpp <span style="color: grey">(a674ccb)</span></li>
<li>kcmodule/profiles/powerdevilprofilesconfig.desktop <span style="color: grey">(9243a8f)</span></li>
<li>kcmodule/profiles/profileEditPage.ui <span style="color: grey">(dc26579)</span></li>
<li>powerdevil.notifyrc <span style="color: grey">(36acdb6)</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>