[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Review Request 119866: Thumbnails in Klipper
From:       Sebastian_Kügler <sebas () kde ! org>
Date:       2014-08-27 10:18:34
Message-ID: 20140827101834.11205.44961 () probe ! kde ! org
[Download RAW message or body]

--===============5308195345923110343==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119866/
-----------------------------------------------------------

(Updated Aug. 27, 2014, 10:18 a.m.)


Status
------

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-workspace


Description
-------

This patch paints previews of copied images, instead of displaying the urls in plain \
text.

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

It has a few rough edges (all fixed now):
- not all images get thumbnails -- need to investigate why
- vertical alignment throughout the list is quite bad, this is the case already, I \
                will fix that separately
- I want to add an indicator that it's more than 4 files (if applicable), working on \
                that still
- When no preview is loaded, it should show an icon, almost done, with some fixes

I'd like some feedback on the code, so that with the remaining issues fixed, it can \
get in.


Diffs
-----

  applets/clipboard/contents/ui/ClipboardItemDelegate.qml 8eecb80 
  klipper/org.kde.plasma.clipboard.operations 813aafa 
  klipper/clipboardjob.cpp d84d014 
  klipper/historyurlitem.cpp fb2a766 
  applets/clipboard/contents/ui/Menu.qml e6821c3 
  applets/clipboard/contents/ui/clipboard.qml ac784d2 
  klipper/CMakeLists.txt 660b0d1 
  klipper/clipboardjob.h b4f5284 

Diff: https://git.reviewboard.kde.org/r/119866/diff/


Testing
-------

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.


File Attachments
----------------

Before
  https://git.reviewboard.kde.org/media/uploaded/files/2014/08/20/ebdde483-136e-44da-8735-f98ba88485a9__klipper-before.png
 Klipper with thumbnails
  https://git.reviewboard.kde.org/media/uploaded/files/2014/08/20/65749cc3-3fed-4c9b-9a28-b9791b8e93d8__klipper-thumbnails.png
 Klipper with thumbnails 2nd version
  https://git.reviewboard.kde.org/media/uploaded/files/2014/08/22/4fb72495-0781-4535-8de8-5266652b467a__klipper-after.png



Thanks,

Sebastian Kügler


--===============5308195345923110343==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit




<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 />




<table bgcolor="#e0e0e0" width="100%" cellpadding="12" style="border: 1px gray solid; \
border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">  <tr>
  <td>
   <h1 style="margin: 0; padding: 0; font-size: 10pt;">This change has been marked as \
submitted.</h1>  </td>
 </tr>
</table>
<br />


<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. 27, 2014, 10:18 a.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 (all fixed now):<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">(8eecb80)</span></li>

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

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

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

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

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

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

 <li>klipper/clipboardjob.h <span style="color: grey">(b4f5284)</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>

 <li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/08/22/4fb72495-0781-4535-8de8-5266652b467a__klipper-after.png">Klipper \
with thumbnails 2nd version</a></li>

</ul>




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




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


--===============5308195345923110343==--



_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic