[prev in list] [next in list] [prev in thread] [next in thread]
List: kfm-devel
Subject: Re: Review Request: Fix Bug 304643 - selected place looks ugly and incomplete
From: šumski <hrvoje.senjan () gmail ! com>
Date: 2012-10-19 20:06:45
Message-ID: 2141296.qhgcbRMIUd () shumarija
[Download RAW message or body]
On Friday 19 of October 2012 11:50:56 Frank Reininghaus wrote:
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106827/
On October 15th, 2012, 3:43 p.m., Frank Reininghaus wrote:
First of all, thanks for the patch and the screenshots!
It should be added that the "new" look would match the look of
KFilePlacesView, i.e., the look of Dolphin's Places Panel in KDE <= 4.8. In
particular, it makes sure that the red color of the "Root" place is preserved
(the user who reported this considered the color change distracting, and I
agree that it can be considered a bit odd).
However, when we change this, the behaviour of the Places Panel would be
different from the one of the DolphinView. This is most obvious when using
Details View. I'm not really sure what the best solution is. There are several
options:
a) Apply the patch as it is.
b) Only remove the color change, and do not extend the selection rectangle.
c) Don't change anything.
d) Like a), but do the same change in the DolphinView.
e) Like b), but do the same change in the DolphinView.
I'm not entirely sure what the 'right' approach is, but considering that the
wish report did not get much feedback, it's not clear if it's really worth
adding extra complexity to implement this.
Any other opinions? Do we know how other file managers do it (I don't have any
others to test here right now)?
On October 17th, 2012, 7:46 p.m., Emmanuel Pescosta wrote:
I implemented option d.) So you can decide what looks better, the old or the
new Dolphin look. ;)
I hope you and the other Dolphin users like it.
Thanks for the new patch, Emmanuel!
I'm still not quite sure what look is best :-/
I've tried Emmanuel's patch #2 and it looks fine to me. However, it took me
some time to actually notice the difference between the current situation and
patched dolphin. It probably does look a little better, but i don't use Places
panel (and i'm not really convinced it looks better in the DolphinView ), so
can't really say
[Attachment #3 (unknown)]
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN" \
"http://www.w3.org/TR/REC-html40/strict.dtd"> <html><head><meta name="qrichtext" \
content="1" /><style type="text/css"> p, li { white-space: pre-wrap; }
</style></head><body style=" font-family:'Lato'; font-size:9.5pt; font-weight:400; \
font-style:normal;"> <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">On Friday \
19 of October 2012 11:50:56 Frank Reininghaus wrote:<br /></p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; "> </p> <table border="0" \
style=" margin-top:0px; margin-bottom:0px; margin-left:40px; margin-right:40px;" \
width="100%" cellspacing="2" cellpadding="8" bgcolor="#f9f3c9"> <tr>
<td>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><span style=" \
font-family:'Verdana,Arial,Helvetica,Sans-Serif';">This is an automatically generated \
e-mail. To reply, visit: </span><a \
href="http://git.reviewboard.kde.org/r/106827/"><span style=" \
font-family:'Verdana,Arial,Helvetica,Sans-Serif'; text-decoration: underline; \
color:#0057ae;">http://git.reviewboard.kde.org/r/106827/</span></a><span style=" \
font-family:'Verdana,Arial,Helvetica,Sans-Serif';"> </span></p></td></tr></table> <p \
style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><span style=" \
font-family:'Verdana,Arial,Helvetica,Sans-Serif';"><br /></span></p> <p style=" \
margin-top:12px; margin-bottom:12px; margin-left:52px; margin-right:80px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><span style=" \
font-family:'Verdana,Arial,Helvetica,Sans-Serif';">On October 15th, 2012, 3:43 p.m., \
</span><span style=" font-family:'Verdana,Arial,Helvetica,Sans-Serif'; \
font-weight:600;">Frank Reininghaus</span><span style=" \
font-family:'Verdana,Arial,Helvetica,Sans-Serif';"> wrote: </span></p> <p style=" \
margin-top:12px; margin-bottom:0px; margin-left:64px; margin-right:120px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><span style=" \
font-family:'Courier New,courier';">First of all, thanks for the patch and the \
screenshots!</span></p> <p style="-qt-paragraph-type:empty; margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; "> </p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:64px; margin-right:120px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;"><span style=" font-family:'Courier New,courier';">It should be \
added that the "new" look would match the look of KFilePlacesView, i.e., \
the look of Dolphin's Places Panel in KDE <= 4.8. In particular, it makes sure \
that the red color of the "Root" place is preserved (the user who reported \
this considered the color change distracting, and I agree that it can be considered a \
bit odd).</span></p> <p style="-qt-paragraph-type:empty; margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; "> </p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:64px; margin-right:120px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;"><span style=" font-family:'Courier New,courier';">However, when we \
change this, the behaviour of the Places Panel would be different from the one of the \
DolphinView. This is most obvious when using Details View. I'm not really sure what \
the best solution is. There are several options:</span></p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; "> </p> <p style=" \
margin-top:0px; margin-bottom:0px; margin-left:64px; margin-right:120px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><span style=" \
font-family:'Courier New,courier';">a) Apply the patch as it is.</span></p> <p \
style=" margin-top:0px; margin-bottom:0px; margin-left:64px; margin-right:120px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><span style=" \
font-family:'Courier New,courier';">b) Only remove the color change, and do not \
extend the selection rectangle.</span></p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:64px; margin-right:120px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;"><span style=" font-family:'Courier \
New,courier';">c) Don't change anything.</span></p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:64px; margin-right:120px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;"><span style=" font-family:'Courier \
New,courier';">d) Like a), but do the same change in the DolphinView.</span></p> <p \
style=" margin-top:0px; margin-bottom:0px; margin-left:64px; margin-right:120px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><span style=" \
font-family:'Courier New,courier';">e) Like b), but do the same change in the \
DolphinView.</span></p> <p style="-qt-paragraph-type:empty; margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; "> </p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:64px; margin-right:120px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;"><span style=" font-family:'Courier New,courier';">I'm not entirely \
sure what the 'right' approach is, but considering that the wish report did not get \
much feedback, it's not clear if it's really worth adding extra complexity to \
implement this.</span></p> <p style="-qt-paragraph-type:empty; margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; "> </p> <p style=" margin-top:0px; margin-bottom:12px; \
margin-left:64px; margin-right:120px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;"><span style=" font-family:'Courier New,courier';">Any other \
opinions? Do we know how other file managers do it (I don't have any others to test \
here right now)?</span><span style=" \
font-family:'Verdana,Arial,Helvetica,Sans-Serif';"> </span></p> <p style=" \
margin-top:12px; margin-bottom:12px; margin-left:52px; margin-right:80px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><span style=" \
font-family:'Verdana,Arial,Helvetica,Sans-Serif';">On October 17th, 2012, 7:46 p.m., \
</span><span style=" font-family:'Verdana,Arial,Helvetica,Sans-Serif'; \
font-weight:600;">Emmanuel Pescosta</span><span style=" \
font-family:'Verdana,Arial,Helvetica,Sans-Serif';"> wrote: </span></p> <p style=" \
margin-top:12px; margin-bottom:0px; margin-left:64px; margin-right:120px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><span style=" \
font-family:'Courier New,courier';">I implemented option d.) So you can decide what \
looks better, the old or the new Dolphin look. ;) </span></p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; "> </p> <p style=" \
margin-top:0px; margin-bottom:12px; margin-left:64px; margin-right:120px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><span style=" \
font-family:'Courier New,courier';">I hope you and the other Dolphin users like \
it.</span><span style=" font-family:'Verdana,Arial,Helvetica,Sans-Serif';"> \
</span></p> <p style=" margin-top:12px; margin-bottom:0px; margin-left:40px; \
margin-right:40px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><span \
style=" font-family:'Courier New,courier';">Thanks for the new patch, \
Emmanuel!</span></p> <p style="-qt-paragraph-type:empty; margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; "> </p> <p style=" margin-top:0px; margin-bottom:12px; \
margin-left:40px; margin-right:40px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;"><span style=" font-family:'Courier New,courier';">I'm still not \
quite sure what look is best :-/</span></p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;"><span style=" \
font-family:'Verdana,Arial,Helvetica,Sans-Serif';"><br />I've tried Emmanuel's patch \
#2 and it looks fine to me. However, it took me some time to actually notice the \
difference between the current situation and patched dolphin. It probably does look a \
little better, but i don't use Places panel (and i'm not really convinced it looks \
better in the DolphinView ), so can't really say </span></p> <p style=" \
margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><br /><br \
/></p></body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic