<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/129648/">https://git.reviewboard.kde.org/r/129648/</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 21st, 2016, 5 p.m. CET, <b>Christoph Feck</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/129648/diff/5/?file=487886#file487886line72" style="color: black; font-weight: bold; text-decoration: underline;">src/ktooltipwidget.h</a>
<span style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<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">72</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="nf">showUnder</span><span class="p">(</span><span class="k">const</span> <span class="n">QRect</span> <span class="o">&</span><span class="n">rect</span><span class="p">,</span> <span class="n">QWidget</span> <span class="o">*</span><span class="n">content</span><span class="p">,</span> <span class="n">QWindow</span> <span class="o">*</span><span class="n">transientParent</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 am not a native english speaker, but I think 'showBelow' sounds better.
Needs a comment from a native speaker maybe.
Additionally, how about adding a 'showAbove' too?</pre>
</blockquote>
<p>On December 21st, 2016, 6:02 p.m. CET, <b>Elvis Angelaccio</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;">Pheraps we need a different naming approach here: <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">showBelow</code> is actually "show below if there is enough space on screen", otherwise it will be shown above the rectangle. That's why I think a <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">showAbove</code> wouldn't make much sense.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What about something like <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">showNear</code>?</p></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;">Well, I was thinking about making it possible for the application to choose the default position (top or bottom). But you are right, the default should really be below, because the content starts at the top, and I think the name should reflect that. So unless someone corrects me, go for 'showBelow'.</pre>
<br />
<p>- Christoph</p>
<br />
<p>On December 21st, 2016, 6:19 p.m. CET, Elvis Angelaccio 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 KDE Frameworks, Ben Cooksley, Christoph Feck, and Martin Gräßlin.</div>
<div>By Elvis Angelaccio.</div>
<p style="color: grey;"><i>Updated Dec. 21, 2016, 6:19 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kwidgetsaddons
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This new widget is based on the KToolTip code that is duplicated across multiple products: at least Dolphin, systemsettings, kinfocenter, ktp-contact-list.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Rationale: with a single class in frameworks, it will be possible to apply features/fixes only once. See for example the comments in https://phabricator.kde.org/D3112</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">A new feature that the old code doesn't have is the delayed hide: this makes it possible to actually use the widget shown in the tooltip.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Manual test works both in X11 and Wayland. Unit tests pass.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Ported Dolphin locally to this new class, everything seems to work (and this will fix bug <a href="https://bugs.kde.org/show_bug.cgi?id=352276" style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">#352276</a> and <a href="https://bugs.kde.org/show_bug.cgi?id=371223" style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">#371223</a>).</p></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>autotests/CMakeLists.txt <span style="color: grey">(ffcce3a046ec98b07c4677578f6bc997de1ef16b)</span></li>
<li>autotests/ktooltipwidgettest.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>autotests/ktooltipwidgettest.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/CMakeLists.txt <span style="color: grey">(de0a8d965f1541d5ffeec93d1aa06600b0b9c138)</span></li>
<li>src/ktooltipwidget.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/ktooltipwidget.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/CMakeLists.txt <span style="color: grey">(fcb348b1ae9d4270468c3f9003c5ba5f3903db84)</span></li>
<li>tests/ktooltipwidget_test.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/ktooltipwidget_test.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/129648/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>