<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="http://svn.reviewboard.kde.org/r/5622/">http://svn.reviewboard.kde.org/r/5622/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 13th, 2010, 10:59 p.m., <b>Markus Slopianka</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;">Hm..... I find the checkbox solution more intuitive.
If this patch gets accepted, one has to make sure, *all* KDE applications follow that method -- incl.Dolphin's "Show hidden file", KOffice's "Show grid", etc.</pre>
</blockquote>
<p>On October 13th, 2010, 11:03 p.m., <b>Hugo Pereira Da Costa</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;">... as well as 'lock toolbar positions'</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 have to differentiate between true actions and just configurable options. As I understand it right, the KDualAction class has been created for the cases where no "checked" or "down" state is applicable, such as Play/Pause, Reload/Stop, etc. These would be actions.
On the other hand there are options, such as whether certain UI elements should be visible. Since it is assumed that those things are saved between application invokations, they should keep the checkbox so that one quickly sees the state of the option, without even bothering to read the text.
Just the fact that invoking the option immediately causes an action (in particular the hiding or showing of a widget) doesn't make these actions per se. Try putting them in a configuration dialog with a checkbox, and you will see the difference.</pre>
<br />
<p>- Christoph</p>
<br />
<p>On October 13th, 2010, 10:19 p.m., Aurélien Gâteau wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs.</div>
<div>By Aurélien Gâteau.</div>
<p style="color: grey;"><i>Updated 2010-10-13 22:19:16</i></p>
<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;">The standard actions to toggle the visibility of the menubar and the statusbar are togglable "show" actions: their label is always "show {menu,status}bar" and they have a checkbox next to them. This means when the user wants to hide the statusbar, he must click an item which says "[x] Show Statusbar". I believe this is less intuitive than if the item label were "Hide Statusbar".
Attached patch implements this by introducing KStandardAction::showHideMenubar(), KStandardAction::showHideStatusbar() and using showHideStatusbar() in KXmlGuiWindow. It also deprecates KStandardAction::showMenubar() and KStandardAction::hideMenubar().
(I already have a patch for Konqueror to make use of KStandardAction::showHideMenubar() and will probably patch other applications if this request is approved)</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 with Konqueror, KWrite and other KDE applications (more patches to come to provide consistency among applications)</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>trunk/KDE/kdelibs/kdeui/actions/kstandardaction.h <span style="color: grey">(1185607)</span></li>
<li>trunk/KDE/kdelibs/kdeui/actions/kstandardaction.cpp <span style="color: grey">(1185607)</span></li>
<li>trunk/KDE/kdelibs/kdeui/xmlgui/kxmlguiwindow.cpp <span style="color: grey">(1185607)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5622/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>