<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/114513/">https://git.reviewboard.kde.org/r/114513/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 23rd, 2013, 4:50 a.m. UTC, <b>Alexander Schuch</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/114513/diff/1/?file=225786#file225786line125" style="color: black; font-weight: bold; text-decoration: underline;">src/window/mainwindow.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void Palapeli::MainWindow::setupActions()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">125</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="n">i18n</span><span class="p">(</span><span class="s">"Hide Preview"</span><span class="p">)</span> <span class="o">:</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Show Preview"</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I do not know the style guidelines, but isn't it better to a "toggle action" so that the button "show preview" is rendered as pushed? - http://qt-project.org/doc/qt-4.8/qaction.html#checkable-prop</pre>
</blockquote>
<p>On December 24th, 2013, 5:59 a.m. UTC, <b>Ian Wadham</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 style guidelines are a bit vague on this point, see http://techbase.kde.org/Projects/Usability/HIG/Toggle_Buttons and http://techbase.kde.org/Projects/Usability/HIG/Toolbar, which both warn against using toggle buttons in toolbars and also recommend not to mix toggle buttons with other types of button. I have used toggle buttons in a toolbar in Kubrick and in the game editor of KGoldrunner. Also KStars has a toolbar full of toggle buttons, which do such things as showing/hiding stars, planets, constellation names, etc.
In the current situation, I think a couple of words are better than a picture. The icon for "view-preview" (a hot-air balloon) is not very meaningful IMHO, so I went for explicit text that says what the button will do next. That way, a beginning user can easily see how to turn the preview off if it is getting in the way. Of course, a more advanced KDE jockey might right-click the toolbar and turn the text off, leaving him with just the hot-air balloon. So I'll settle for letting the button be "pushed in", but still having it say "Hide Preview", rather than "Show Preview" (confusingly), when it is pushed in.</pre>
</blockquote>
<p>On December 27th, 2013, 4:36 p.m. UTC, <b>Albert Astals Cid</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;">Agreed, i think it's a bit confusing when you have a toggle button that changes text when toggled because you never know what's happening. I think that a "Show Preview" toggle button that when pressed shows the preview and when depressed not is the "easiest" way to get it shown to the user.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This is what I actually meant: have a toggle button which always reads "Show Preview" but is rendered as pushed (active) or not pushed (not active). Thanks for the HIG links as well.</pre>
<br />
<p>- Alexander</p>
<br />
<p>On December 16th, 2013, 9:48 p.m. UTC, Ian Wadham 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 KDE Games.</div>
<div>By Ian Wadham.</div>
<p style="color: grey;"><i>Updated Dec. 16, 2013, 9:48 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
palapeli
</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;">Facility to preview a completed Palapeli jigsaw puzzle.
Original by Johannes Loehnert, 2010, modified by Ian Wadham to use current Palapeli interfaces.
The preview is controlled by a button in the toolbar, which is enabled when a puzzle is being solved. It appears in a floating tool-window and can be shown or hidden at will. The window can be moved, resized, maximized or closed (hidden, but not deleted). It is deleted when the user returns to the puzzle-collection view or terminates Palapeli when in solving mode. The window's title is derived from the name of the puzzle, e.g. "Citrus Fruits - Preview". Its visibility and geometry are saved in Settings between uses. If visibility is on in Settings, the puzzle preview appears immediately a puzzle is requested and can be viewed while the puzzle is loading.
When the mouse pointer is moved into the preview window, the view is automatically magnified to show an area of at least 3x3 pieces. As the pointer moves around, the view automatically scrolls, to allow the user to home in rapidly on any detail of the puzzle's image.</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;">Developed and tested on an Apple Macbook OS X platform, using KDE from Macports.
Tested all the features described above, including title bar buttons. Also tested cases where image and Palapeli metadata are unavailable, in which case the preview contains a text message and a generic title.
On Apple, the Minimize button is disabled and has not been tested. I am not sure if it will be enabled in Linux, nor what its effect should be.</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>src/window/mainwindow.cpp <span style="color: grey">(e3305fb)</span></li>
<li>src/window/mainwindow.h <span style="color: grey">(f94e52c)</span></li>
<li>src/palapeliui.rc <span style="color: grey">(d888c0d)</span></li>
<li>src/main.cpp <span style="color: grey">(f7fec86)</span></li>
<li>src/palapeli.kcfg <span style="color: grey">(b30d6b1)</span></li>
<li>src/engine/puzzlepreview.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/engine/puzzlepreview.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/CMakeLists.txt <span style="color: grey">(c648c91)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/114513/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>