[prev in list] [next in list] [prev in thread] [next in thread]
List: kfm-devel
Subject: Re: Review Request 119018: Do not tint the icon of the selected item in
From: "Commit Hook" <null () kde ! org>
Date: 2014-07-03 22:53:54
Message-ID: 20140703225354.25307.33176 () probe ! kde ! org
[Download RAW message or body]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119018/#review61584
-----------------------------------------------------------
This review has been submitted with commit 1f69714a23a222b5edabfdad9896e58a4818729e \
by Frank Reininghaus to branch master.
- Commit Hook
On June 29, 2014, 6:56 p.m., Frank Reininghaus wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119018/
> -----------------------------------------------------------
>
> (Updated June 29, 2014, 6:56 p.m.)
>
>
> Review request for Dolphin.
>
>
> Bugs: 304643
> http://bugs.kde.org/show_bug.cgi?id=304643
>
>
> Repository: kde-baseapps
>
>
> Description
> -------
>
> This is a modified version of Emmanuel's idea from \
> https://git.reviewboard.kde.org/r/106827/
> It removes the icon tinting for the selected item in Compact and Details View, and \
> extends the selection rectangle such that it includes the icon area as well. The \
> icon tinting can be disturbing, and having a selection rectangle that only includes \
> the text can look a bit strange, especially in the Places Panel.
> Ideally, we would also get rid of the tinting in Icons View, but the problem is \
> that
> 1. Highlighting only the text makes it difficult to see what the selected item is. \
> This was the Dolphin 2.0/KDE SC 4.8 approach, see \
> https://bugs.kde.org/show_bug.cgi?id=295515
> 2. Drawing a selection rectangle around both text and icon in Icons View leads to \
> inconsistencies with the clickable area, see \
> https://bugs.kde.org/show_bug.cgi?id=309722#c7
> 3. Having a non-rectangular area, which includes exactly the icon and the text, as \
> shown in http://bugsfiles.kde.org/attachment.cgi?id=83419 , might cause problems \
> with different styles, see http://lists.kde.org/?l=kfm-devel&m=140257867317630&w=2
>
> Diffs
> -----
>
> dolphin/src/kitemviews/kitemlistwidget.h a06bb5c
> dolphin/src/kitemviews/kitemlistwidget.cpp c261bf1
> dolphin/src/kitemviews/kstandarditemlistwidget.h 4f7a913
> dolphin/src/kitemviews/kstandarditemlistwidget.cpp e037548
>
> Diff: https://git.reviewboard.kde.org/r/119018/diff/
>
>
> Testing
> -------
>
> Screenshots show Places Panel, Compact and Details View with default settings \
> before/after applying this patch.
>
> File Attachments
> ----------------
>
> Before
> https://git.reviewboard.kde.org/media/uploaded/files/2014/06/29/46e94473-92d3-42fc-83b8-a140c358b70e__Selection-in-Details-and-Compact-View-original.png
> Selection-in-Details-and-Compact-View-patched.png
> https://git.reviewboard.kde.org/media/uploaded/files/2014/06/29/d3f6cb90-3d4b-45db-8fa1-57f42194e691__Selection-in-Details-and-Compact-View-patched.png
>
>
> Thanks,
>
> Frank Reininghaus
>
>
[Attachment #3 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119018/">https://git.reviewboard.kde.org/r/119018/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This review has been \
submitted with commit 1f69714a23a222b5edabfdad9896e58a4818729e by Frank Reininghaus \
to branch master.</pre> <br />
<p>- Commit Hook</p>
<br />
<p>On June 29th, 2014, 6:56 p.m. UTC, Frank Reininghaus wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for Dolphin.</div>
<div>By Frank Reininghaus.</div>
<p style="color: grey;"><i>Updated June 29, 2014, 6:56 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=304643">304643</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kde-baseapps
</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;">This is a modified version of Emmanuel's idea from \
https://git.reviewboard.kde.org/r/106827/
It removes the icon tinting for the selected item in Compact and Details View, and \
extends the selection rectangle such that it includes the icon area as well. The icon \
tinting can be disturbing, and having a selection rectangle that only includes the \
text can look a bit strange, especially in the Places Panel.
Ideally, we would also get rid of the tinting in Icons View, but the problem is that
1. Highlighting only the text makes it difficult to see what the selected item is. \
This was the Dolphin 2.0/KDE SC 4.8 approach, see \
https://bugs.kde.org/show_bug.cgi?id=295515
2. Drawing a selection rectangle around both text and icon in Icons View leads to \
inconsistencies with the clickable area, see \
https://bugs.kde.org/show_bug.cgi?id=309722#c7
3. Having a non-rectangular area, which includes exactly the icon and the text, as \
shown in http://bugsfiles.kde.org/attachment.cgi?id=83419 , might cause problems with \
different styles, see \
http://lists.kde.org/?l=kfm-devel&m=140257867317630&w=2</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;">Screenshots show Places Panel, Compact and Details View with default \
settings before/after applying this patch.</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>dolphin/src/kitemviews/kitemlistwidget.h <span style="color: \
grey">(a06bb5c)</span></li>
<li>dolphin/src/kitemviews/kitemlistwidget.cpp <span style="color: \
grey">(c261bf1)</span></li>
<li>dolphin/src/kitemviews/kstandarditemlistwidget.h <span style="color: \
grey">(4f7a913)</span></li>
<li>dolphin/src/kitemviews/kstandarditemlistwidget.cpp <span style="color: \
grey">(e037548)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119018/diff/" style="margin-left: \
3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments \
</h1>
<ul>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/06/29/46e94473 \
-92d3-42fc-83b8-a140c358b70e__Selection-in-Details-and-Compact-View-original.png">Before</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/06/29/d3f6cb90 \
-3d4b-45db-8fa1-57f42194e691__Selection-in-Details-and-Compact-View-patched.png">Selection-in-Details-and-Compact-View-patched.png</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic