Review Request: Improve KColorChooser dialog color preset selection implementing a custom ItemDelegate

[Michael Pyne]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3410/#review4726
-----------------------------------------------------------


Honestly I think the hollow circle from the beginning is the best idea because it makes it obvious which color is selected (given that the constrast is always chosen right).

Simply playing with the border looks garish for most of the color and is usually harder to determine which color is selected. Aurélien's idea #3 is the best looking of those IMO but still too hard to tell at a glance the selected color. To avoid turning this into a bikeshed to be painted I'd just say that if agree there is a positive change to be made here either way, to simply choose one of the alternatives and commit and see how it looks in practice.

- Michael


On 2010-03-28 22:35:17, Darío Andrés wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3410/
> -----------------------------------------------------------
> 
> (Updated 2010-03-28 22:35:17)
> 
> 
> Review request for kdelibs and usability.
> 
> 
> Summary
> -------
> 
> Do not use the QPalette::Highlight color to show the selected color preset in the KColorChooser dialog. 
> 
> Use a custom selection drawing, black or white (depending on the contrast with the original color (KColorUtils::constrastRatio > 3.5))
> This way you get a "this color is selected" hint, but you are still able to see which color it was/is
> 
> My last choice was a centered empty circle. Drawing a rectangle (or dotted rectangle) had too much glitches as the color preset buttons in the tableview are small (~ 13x13 pixels)
> Also, there is a bug (not caused by me) that reduces the color preset buttons height or width (by one) on the top/left borders (seems to be related to the QTableWidget without Headers..)
> 
> I tried to not use color shading as it is still confusing to the user...
> 
> As a gift, I added a small detail in the empty buttons (no color defined), drawing a cross on it (using the background color)
> 
> Attached screenshot: can you recognize the selected color in the first one ? (hint: the same color is selected in both screenies..)
> 
> 
> This addresses bug 230524.
>     https://bugs.kde.org/show_bug.cgi?id=230524
> 
> 
> Diffs
> -----
> 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdelibs/kdeui/colors/kcolordialog.cpp 1107102 
> 
> Diff: http://reviewboard.kde.org/r/3410/diff
> 
> 
> Testing
> -------
> 
> It works Ok and it looks good (ok, it could be prettier, any ideas? may be a gradient?)
> At least it doesn't block the real color.
> 
> 
> Screenshots
> -----------
> 
> Before and After my patch
>   http://reviewboard.kde.org/r/3410/s/340/
> With a radiobutton-like selection
>   http://reviewboard.kde.org/r/3410/s/341/
> Using sunken border on selected item
>   http://reviewboard.kde.org/r/3410/s/345/
> Raised and plain
>   http://reviewboard.kde.org/r/3410/s/346/
> Raised and sunken
>   http://reviewboard.kde.org/r/3410/s/347/
> Plain and raised
>   http://reviewboard.kde.org/r/3410/s/348/
> Aurélien's idea 1
>   http://reviewboard.kde.org/r/3410/s/349/
> Aurélien's idea 2
>   http://reviewboard.kde.org/r/3410/s/350/
> Aurélien's idea 3
>   http://reviewboard.kde.org/r/3410/s/351/
> 
> 
> Thanks,
> 
> Darío
> 
>