<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/119866/">https://git.reviewboard.kde.org/r/119866/</a>
     </td>
    </tr>
   </table>
   <br />






<blockquote style="margin: 1em 0 0 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On August 20th, 2014, 6:38 p.m. UTC, <b>Kai Uwe Broulik</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding: 0 0 0 1em;">
  
  <br style="display: none;" />

  <table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
   <tr>
    <td>
     <a href="https://git.reviewboard.kde.org/r/119866/file/1508/" style="color: black; font-weight: bold; font-size: 9pt;">klipper-thumbnails.png</a>

     <p>Klipper with thumbnails</p>




    </td>
   </tr>
  </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;">A tooltip might be useful, there's not much room for the filename.</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I thought about this as well, but it has a few downsides:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The tooltip would either<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
- have to carry all the filenames, or a selection, resulting in a huge tooltip<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
- be per preview, which means we'll leave some of them out, and the delegate becomes very "popuppy", i.e. lots of changes on mouse over, tooltips covering up other interesting bits, etc</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As I've done the delegate now, the previews are distinguishable and provide "just enough" information to get a good idea of what's in the buffer. I'd like to avoid this, as I think showing a tooltip is more of sign of trying to be overly correct, instead of making an educated choice what to display. I'll drop this issues, if you're OK with that. If not, we can work from here and try to find a better solution. Let's try it first, and experience how it fares (quite well in my testing, so maybe this is a non-issue).</p></pre>
<br />








<p>- Sebastian</p>


<br />
<p>On August 20th, 2014, 6:29 p.m. UTC, Sebastian Kügler 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 Plasma.</div>
<div>By Sebastian Kügler.</div>


<p style="color: grey;"><i>Updated Aug. 20, 2014, 6:29 p.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-workspace
</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 patch paints previews of copied images, instead of displaying the urls in plain text.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The display is limited to 4 images right now. The painting of the delegates is in line with the recommendations from the HIG at https://techbase.kde.org/Projects/Usability/HIG/Layout/Image</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It has a few rough edges:<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
- not all images get thumbnails -- need to investigate why<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
- vertical alignment throughout the list is quite bad, this is the case already, I will fix that separately<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
- I want to add an indicator that it's more than 4 files (if applicable), working on that still<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
- When no preview is loaded, it should show an icon, almost done, with some fixes</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'd like some feedback on the code, so that with the remaining issues fixed, it can get in.</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;">Copied images and files in Dolphin, see them showing up in Klipper. Made sure that the previews are only loaded when needed, so no additional memory taken when the systray / klipper popup is not open.</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>applets/clipboard/contents/ui/ClipboardItemDelegate.qml <span style="color: grey">(8eecb80cff1b5535b43242fe114fdaeae5ad510a)</span></li>

 <li>applets/clipboard/contents/ui/clipboard.qml <span style="color: grey">(ac784d2a46961083aad24eb142b9befa74ec61bd)</span></li>

 <li>klipper/CMakeLists.txt <span style="color: grey">(660b0d174f9d41eec095d83d640111d1c340fb17)</span></li>

 <li>klipper/clipboardjob.cpp <span style="color: grey">(d84d01471440eed670c459e775867eda25fdbc58)</span></li>

 <li>klipper/historyurlitem.cpp <span style="color: grey">(fb2a766b39abe2040555ee694c55337481003bf9)</span></li>

 <li>klipper/org.kde.plasma.clipboard.operations <span style="color: grey">(813aafa96440304b918ef06c3863bd7c4527d62a)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/119866/diff/" style="margin-left: 3em;">View Diff</a></p>



<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>


 <li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/08/20/ebdde483-136e-44da-8735-f98ba88485a9__klipper-before.png">Before</a></li>

 <li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/08/20/65749cc3-3fed-4c9b-9a28-b9791b8e93d8__klipper-thumbnails.png">Klipper with thumbnails</a></li>

</ul>




  </td>
 </tr>
</table>








  </div>
 </body>
</html>