From kfm-devel Thu Oct 25 11:55:33 2012 From: "Emmanuel Pescosta" Date: Thu, 25 Oct 2012 11:55:33 +0000 To: kfm-devel Subject: Re: Review Request: Fix Bug 304643 - selected place looks ugly and incomplete Message-Id: <20121025115533.5126.12807 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=kfm-devel&m=135116614411937 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============6900847424905238485==" --===============6900847424905238485== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On Oct. 15, 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 KFilePla= cesView, i.e., the look of Dolphin's Places Panel in KDE <=3D 4.8. In parti= cular, it makes sure that the red color of the "Root" place is preserved (t= he user who reported this considered the color change distracting, and I ag= ree that it can be considered a bit odd). > > = > > However, when we change this, the behaviour of the Places Panel would b= e different from the one of the DolphinView. This is most obvious when usin= g Details View. I'm not really sure what the best solution is. There are se= veral options: > > = > > a) Apply the patch as it is. > > b) Only remove the color change, and do not extend the selection rectan= gle. > > 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 tha= t 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 h= ave any others to test here right now)? > > > = > 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. > = > Frank Reininghaus wrote: > Thanks for the new patch, Emmanuel! > = > I'm still not quite sure what look is best :-/ Should I push or discard my patch? ;) (What if we ask the other people on p= lanetkde or on the mailing-list?) - Emmanuel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106827/#review20387 ----------------------------------------------------------- On Oct. 17, 2012, 7:42 p.m., Emmanuel Pescosta wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106827/ > ----------------------------------------------------------- > = > (Updated Oct. 17, 2012, 7:42 p.m.) > = > = > Review request for Dolphin and Frank Reininghaus. > = > = > Description > ------- > = > Fix Bug 304643 - selected place looks ugly and incomplete > = > = > This addresses bug 304643. > http://bugs.kde.org/show_bug.cgi?id=3D304643 > = > = > Diffs > ----- > = > dolphin/src/kitemviews/kitemlistwidget.h 55181fa = > dolphin/src/kitemviews/kitemlistwidget.cpp 6a7111a = > dolphin/src/kitemviews/kstandarditemlistwidget.h 787722d = > dolphin/src/kitemviews/kstandarditemlistwidget.cpp 72d10cf = > dolphin/src/panels/folders/foldersitemlistwidget.cpp b4f9a5b = > dolphin/src/panels/places/placesitemlistwidget.h a2a88c1 = > dolphin/src/panels/places/placesitemlistwidget.cpp e33d1da = > = > Diff: http://git.reviewboard.kde.org/r/106827/diff/ > = > = > Testing > ------- > = > = > Screenshots > ----------- > = > Old and new Dolphin Places look > http://git.reviewboard.kde.org/r/106827/s/779/ > = > = > Thanks, > = > Emmanuel Pescosta > = > --===============6900847424905238485== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/106827/

On October 15th, 2012, 3:43 p.m., Frank Rei= ninghaus wrote:

First of =
all, thanks for the patch and the screenshots!

It should be added that the "new" look would match the look of KF=
ilePlacesView, i.e., the look of Dolphin's Places Panel in KDE <=3D =
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 chang=
e 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 di=
fferent from the one of the DolphinView. This is most obvious when using De=
tails View. I'm not really sure what the best solution is. There are se=
veral 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 conside=
ring 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 h=
ave any others to test here right now)?

On October 17th, 2012, 7:46 p.m., Emmanuel Pescosta wrote:

I impleme=
nted option d.) So you can decide what looks better, the old or the new Dol=
phin look. ;) =


I hope you and the other Dolphin users like it.

On October 19th, 2012, 11:50 a.m., Frank Reininghaus wrote:

Thanks fo=
r the new patch, Emmanuel!

I'm still not quite sure what look is best :-/
Should I pu=
sh or discard my patch? ;) (What if we ask the other people on planetkde or=
 on the mailing-list?)

- Emmanuel


On October 17th, 2012, 7:42 p.m., Emmanuel Pescosta wrote:

Review request for Dolphin and Frank Reininghaus.
By Emmanuel Pescosta.

Updated Oct. 17, 2012, 7:42 p.m.

Descripti= on 

Fix Bug 304643 - selected place looks ugly and incomplete
Bugs: 304643

Diffs=

  • dolphin/src/kitemviews/kitemlistwidget.h (= 55181fa)
  • dolphin/src/kitemviews/kitemlistwidget.cpp (6a7111a)
  • dolphin/src/kitemviews/kstandarditemlistwidget.h (787722d)
  • dolphin/src/kitemviews/kstandarditemlistwidget.cpp (72d10cf)
  • dolphin/src/panels/folders/foldersitemlistwidget.cpp (b4f9a5b)
  • dolphin/src/panels/places/placesitemlistwidget.h (a2a88c1)
  • dolphin/src/panels/places/placesitemlistwidget.cpp (e33d1da)

View Diff

Screensho= ts

3D"Old
--===============6900847424905238485==--