[prev in list] [next in list] [prev in thread] [next in thread]
List: kfm-devel
Subject: Re: Review Request 129918: Apply the file preview shadow frame to most previews instead of only imag
From: Emmanuel Pescosta <emmanuelpescosta099 () gmail ! com>
Date: 2017-02-18 21:42:00
Message-ID: 20170218214200.13662.21231 () mimi ! kde ! org
[Download RAW message or body]
--===============8934291173892728223==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129918/#review102547
-----------------------------------------------------------
Ship it!
Ship It!
- Emmanuel Pescosta
On Feb. 4, 2017, 10:56 p.m., Diego Soenens wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129918/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2017, 10:56 p.m.)
>
>
> Review request for Dolphin.
>
>
> Repository: dolphin
>
>
> Description
> -------
>
> This is basically the same patch as https://git.reviewboard.kde.org/r/118961/ which \
> I discarded as the diff was blank and updating it didn't work (probably because it \
> was old pre-Qt/Frameworks 5 ?).
> Anyways, currently image file previews have a nice shadow around the thumbnail.
> It looks nice and modern but the same shadow doesn't get applied to anything else \
> such as video previews. This makes non-image previews look out of place (especially \
> in mixed content folders) and, IMHO, ugly. Especially with the new modern look \
> Plasma 5 brought. The patch applies the shadow to all previews except for folders, \
> fonts and Win32 exe/dll previews, similar to how Windows and MacOS handles it.
>
> Diffs
> -----
>
> src/kitemviews/kfileitemmodelrolesupdater.cpp \
> 6e1c3d3ddd08a62d423665b2db6b2d1a446348f7
> Diff: https://git.reviewboard.kde.org/r/129918/diff/
>
>
> Testing
> -------
>
> Enabled all previews and created a folder with common file types (see screenshots) \
> - works as intended.
> Note that most of the thumbnail generators inside kio-extras/thumbnail currently \
> add a 1px black and white border directly into the cached thumbnail image to give \
> them a 3d-depth look. This causes the border of the preview to look slightly \
> blurry/unsharp with the shadow around it (see the text previews in the \
> screenshots). The fix for this is a simple patch to make thier flags() function \
> return ThumbCreator::None instead of ThumbCreator::DrawFrame.
> Edit: fix https://git.reviewboard.kde.org/r/129921/
>
>
> File Attachments
> ----------------
>
> before.png
> https://git.reviewboard.kde.org/media/uploaded/files/2017/02/04/874945d7-80a5-4365-8052-6a35a4f9ee58__before.png
> after.png
> https://git.reviewboard.kde.org/media/uploaded/files/2017/02/04/b70cf854-bdaa-4cea-b27c-ec05ef4324e2__after.png
>
>
> Thanks,
>
> Diego Soenens
>
>
--===============8934291173892728223==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 7bit
<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/129918/">https://git.reviewboard.kde.org/r/129918/</a>
</td>
</tr>
</table>
<br />
<p>
Ship it!
</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ship It!</pre> <br />
<p>- Emmanuel Pescosta</p>
<br />
<p>On February 4th, 2017, 10:56 p.m. CET, Diego Soenens 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 Dolphin.</div>
<div>By Diego Soenens.</div>
<p style="color: grey;"><i>Updated Feb. 4, 2017, 10:56 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
dolphin
</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 is basically the same patch as \
https://git.reviewboard.kde.org/r/118961/ which I discarded as the diff was blank and \
updating it didn't work (probably because it was old pre-Qt/Frameworks 5 ?).</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Anyways, currently image file previews have a nice shadow around the \
thumbnail. It looks nice and modern but the same shadow doesn't get applied to \
anything else such as video previews. This makes non-image previews look out of place \
(especially in mixed content folders) and, IMHO, ugly. Especially with the new modern \
look Plasma 5 brought. The patch applies the shadow to all previews except for \
folders, fonts and Win32 exe/dll previews, similar to how Windows and MacOS handles \
it.</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;">Enabled all previews and created a folder with common \
file types (see screenshots) - works as intended.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Note \
that most of the thumbnail generators inside kio-extras/thumbnail currently add a 1px \
black and white border directly into the cached thumbnail image to give them a \
3d-depth look. This causes the border of the preview to look slightly blurry/unsharp \
with the shadow around it (see the text previews in the screenshots). The fix for \
this is a simple patch to make thier flags() function return ThumbCreator::None \
instead of ThumbCreator::DrawFrame.
Edit: fix https://git.reviewboard.kde.org/r/129921/</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>src/kitemviews/kfileitemmodelrolesupdater.cpp <span style="color: \
grey">(6e1c3d3ddd08a62d423665b2db6b2d1a446348f7)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/129918/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/2017/02/04/874945d7-80a5-4365-8052-6a35a4f9ee58__before.png">before.png</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2017/02/04/b70cf854-bdaa-4cea-b27c-ec05ef4324e2__after.png">after.png</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============8934291173892728223==--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic