<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 15th, 2010, 9:03 p.m., <b>David Faure</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;">Hey don't let a short discussion discourage you.
If I understand correctly, this patch restores the KDE3 behavior: Show foo, Hide foo, no checkbox. This is what we wanted at the time; everything that happened since was unwanted side effects from Qt4. So I see no reason why we shouldn't apply this and get the behavior that we had actually decided for, at the time.</pre>
</blockquote>
<p>On October 15th, 2010, 9:37 p.m., <b>Parker Coates</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;">So where is it appropriate to use KToggleAction? Almost every KTA can be converted to a KDA by prepending two opposite verbs to the to it. (Ex: Show/Hide, Start/Stop, Lock/Unlock, Enable/Disable, Activate/Deactivate)
Does this change essentially deprecate KToggleAction?</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;">@David: thanks, I am going to reopen the request then.
@Parker: you may want to read this: http://techbase.kde.org/Projects/Usability/HIG/Toggle_Buttons . It shows valid uses of KToggleAction (for example: font formatting for a text processor). You will notice it lists "[x] Show" as a valid alternative to "Show/hide". I didn't want to recommend against those "[x] Show" actions when I first wrote this document because I was not sure it would be realist to fix them (KDualAction did not exist back then).
When Celeste reviewed the document she suggested to add some recommendations for developers who don't know which solution is best for their situation. I wanted to get through some applications fixes before revisiting this document with some practical recommendation. If this patch goes in I'll add the first recommendation: use "Show/hide" for UI elements.
</pre>
<br />
<p>- Aurélien</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>