<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/124687/">https://git.reviewboard.kde.org/r/124687/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 17th, 2015, 6:16 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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Is it me or the Ctrl+1,2,etc only works if the toolbar is shown?</p></pre>
</blockquote>
<p>On August 18th, 2015, 7:06 a.m. UTC, <b>Tobias Koenig</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Yes, because widget shortcuts are only active if the associated widget is visible and enabled.</p></pre>
</blockquote>
<p>On August 19th, 2015, 10:40 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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So this is the same problem than with the painting, why are you using a single action with buttons inside instead of generating multiple actions and just pluggin them in?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This way we don't need the custom painting code (which doesn't work as produces a totally different hover code) and the shortcuts may also work (though need to be sure they don't conflict with the other stuff) but at least we would get the correct rendering without the need to emulate it, no?</p></pre>
</blockquote>
<p>On August 20th, 2015, 7:16 a.m. UTC, <b>Tobias Koenig</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Hmm, I cannot follow you here... so you'd prefer to use separate QActions for each drawing tool and plug them into the toolbar? The problem I see here is how to synchronize them (uncheck all other actions if one is checked), because we wouldn't have a central component then which keeps the pointer to all of them all... And how would it avoid the custom painting code? We still want to show buttons that preview the color of the drawing tool. I don't see how to achieve that with pure QActions... or would we generate the icon property of the QActions on-the-fly?</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Just tested by using QToolButton with a colored icon... the problem is that with the default style of KDE, you cannot see at the first glance if a button is down or not... so not really a good solution IMHO. I explicitly added the check mark for this purpose, which requires a custom painting code anyway.</p></pre>
<br />
<p>- Tobias</p>
<br />
<p>On August 18th, 2015, 7:38 a.m. UTC, Tobias Koenig wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Okular, Albert Astals Cid and Laurent Montel.</div>
<div>By Tobias Koenig.</div>
<p style="color: grey;"><i>Updated Aug. 18, 2015, 7:38 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
okular
</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;">Extend the configuration dialog to allow defining multiple pens,
that can be used for drawing on pages in presentation mode.
Replace the pen toggle button in presentation mode top bar with
a select action, to quickly switch between the various pens.
FEATURE: 345269</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 manually with Okular</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>conf/dlgpresentation.cpp <span style="color: grey">(6c68f1e2e16f72a4723a48e331f161d27d068b00)</span></li>
<li>conf/editannottooldialog.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>conf/dlgpresentationbase.ui <span style="color: grey">(78f47f7a8c8e4d74c78b562d7bfd0732f8fa5803)</span></li>
<li>conf/autotests/editdrawingtooldialogtest.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>conf/autotests/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>CMakeLists.txt <span style="color: grey">(27a88ddfec18d9259e669eb2ab7b60ef03e2af75)</span></li>
<li>conf/autotests/editdrawingtooldialogtest.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>conf/editannottooldialog.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>conf/editdrawingtooldialog.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>conf/editdrawingtooldialog.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>conf/okular.kcfg <span style="color: grey">(332eafb7da83f778501ef63c4a607f7568ed7713)</span></li>
<li>conf/widgetannottools.h <span style="color: grey">(f15aa1c4f02293b45c30c309ceeb78b54b27674f)</span></li>
<li>conf/widgetannottools.cpp <span style="color: grey">(6d05eed8bbd4a6f49f54b406ad98f9a10905b7b7)</span></li>
<li>conf/widgetconfigurationtoolsbase.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>conf/widgetconfigurationtoolsbase.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>conf/widgetdrawingtools.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>conf/widgetdrawingtools.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>part.cpp <span style="color: grey">(d461ebefe9533eefdcf918eb153826479e034476)</span></li>
<li>ui/annotationtools.h <span style="color: grey">(dc36dc7c08778e427636bb9d0454c27ca59f6877)</span></li>
<li>ui/annotationtools.cpp <span style="color: grey">(afc71764fd8f82d67fa7f56ce99cf8f4f8c3a64b)</span></li>
<li>ui/data/CMakeLists.txt <span style="color: grey">(491b9935760d0fa81d46b6007f7f44e5e679156b)</span></li>
<li>ui/data/drawingtools.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>ui/drawingtoolselectaction.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>ui/drawingtoolselectaction.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>ui/presentationwidget.h <span style="color: grey">(81beb6c53b9232b409efc3a1f2bd8fb5575b492b)</span></li>
<li>ui/presentationwidget.cpp <span style="color: grey">(0d9a55dc236a016f6808261bc95f8286f9cfa2ec)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124687/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>