[Kde-pim] Review Request 125128: Don't hardcode colors, use system theme instead (for some at least)

[Laurent Montel]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125128/#review85079
-----------------------------------------------------------



kmail/util.cpp (line 42)
<https://git.reviewboard.kde.org/r/125128/#comment58829>

    why add includes ? we remove code in this file;)



kmail/util.cpp (line 57)
<https://git.reviewboard.kde.org/r/125128/#comment58830>

    same :)



messageviewer/viewer/csshelperbase.cpp (line 528)
<https://git.reviewboard.kde.org/r/125128/#comment58831>

    Are you sure that it doesn't break unittest as you changed generated html ?



templateparser/src/CMakeLists.txt (line 87)
<https://git.reviewboard.kde.org/r/125128/#comment58832>

    this code is not related to this patch


- Laurent Montel


On sep. 9, 2015, 11:42 après-midi, Daniel Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125128/
> -----------------------------------------------------------
> 
> (Updated sep. 9, 2015, 11:42 après-midi)
> 
> 
> Review request for KDEPIM and Laurent Montel.
> 
> 
> Repository: kdepim
> 
> 
> Description
> -------
> 
> This patch does three things: firstly it makes sure that default message list colors always use KColorScheme instead of being some random hardcoded shades of blue, green and red. As a result the KMail default colors work on both light and dark widget color themes - currently when you use dark color theme you need to change at least the "Unread mail" color in the settings otherwise it's too dark and illegible.
> 
> Secondly it generates the 3 quotation color levels from KColorScheme (using QColor::darker()/QColor::lighter()), which again makes this usable by default with both light and dark widget themes without having to tweak the custom colors.
> 
> Finally it removes the color customization options for PGP signature/encryption and makes full use of KColorScheme colors again. The reason is that on dark theme (which has very light (white) text in most cases) users end up with white text on top of a light green/yellow background which again is very hard to lead due to low contrast. To fix this we would need to add options to specify not just background color but also foreground color for each of the signature types. However I don't think that there's anyone who ever changed the default KMail color schemes for the PGP boxes, for that reason I think that removing the customization option and always using background and foreground from the global color themes is better as it always ensures good contrast between text and background and simplifies the code. The only exception is the encryption box - there is no good color type in KColorScheme that would match the case so here I simply hardcoded the blue background + white text (which makes the text actually well legible on both dark and light color schemes, currently the light color scheme is hard to read due to low contrast between darkish blue background and black text). The other reason is that the encryption box is purely informative, so the color does not have any meaning on any color theme, while the PGP signature colors actually have an important meaning attached to them, so they should map to the KDE-wide color theme.
> 
> Implementation wise I tried to concentrate the default values into common classes removing some of the duplicated hardocded values.
> 
> 
> Diffs
> -----
> 
>   kmail/configuredialog/configureappearancepage.cpp 361b4e9 
>   kmail/editor/kmcomposereditorng.cpp afa18f7 
>   kmail/editor/widgets/cryptostateindicatorwidget.cpp 0164afa 
>   kmail/util.h f6a1c24 
>   kmail/util.cpp 31e0b03 
>   messagecore/CMakeLists.txt 281e683 
>   messagecore/messagecoreutil.h PRE-CREATION 
>   messagecore/messagecoreutil.cpp PRE-CREATION 
>   messagecore/settings/globalsettings_messagecore.kcfgc 945c4dc 
>   messagecore/settings/messagecore.kcfg b1e3d52 
>   messagelist/messagelistutil.cpp b48123a 
>   messageviewer/viewer/csshelperbase.h 5c15f24 
>   messageviewer/viewer/csshelperbase.cpp 70a9851 
>   templateparser/src/CMakeLists.txt 01e5640 
> 
> Diff: https://git.reviewboard.kde.org/r/125128/diff/
> 
> 
> Testing
> -------
> 
> Tested on both light and dark Breeze themes, in both cases the default colors work reasonably well.
> 
> 
> File Attachments
> ----------------
> 
> Encrypted + signed
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/09/09/3b00783d-b8cc-4f40-a61f-3bcd0a5d93ae__encrypted-light.png
> Signed with untrusted/unknown key
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/09/09/1138e1c5-91f9-412c-b2f4-e80821f91ffb__signed-untrusted-light.png
> Signed with bad key
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/09/09/4bd51482-c7d6-4ad3-9cb2-6f0d078312d6__signed-bad-light.png
> 
> 
> Thanks,
> 
> Daniel Vrátil
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/