[Kde-pim] Review Request 125128: Don't hardcode colors, use system theme instead (for some at least)
Daniel Vrátil
dvratil at kde.org
Thu Sep 10 13:59:31 BST 2015
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125128/
-----------------------------------------------------------
(Updated Sept. 10, 2015, 12:59 p.m.)
Status
------
This change has been marked as submitted.
Review request for KDEPIM and Laurent Montel.
Changes
-------
Submitted with commit f8e99b9d4c6cab36530bd1cb4a6d7009e0a92672 by Dan Vrátil to branch master.
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
-----
messagelist/messagelistutil.cpp b48123a
messagecore/settings/messagecore.kcfg b1e3d52
messagecore/settings/globalsettings_messagecore.kcfgc 945c4dc
messagecore/messagecoreutil.cpp PRE-CREATION
messagecore/messagecoreutil.h PRE-CREATION
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
messageviewer/autotests/data/encapsulated-with-attachment.mbox.html 39464dc
messageviewer/autotests/data/forward-openpgp-signed-encrypted.mbox.html 3b0b883
messageviewer/autotests/data/html.mbox.html 1d11778
messageviewer/autotests/data/htmlonly.mbox.html 1045018
messageviewer/autotests/data/inlinepgpencrypted-appendix.mbox.html 3abc158
messageviewer/autotests/data/inlinepgpencrypted.mbox.html 92f73f6
messageviewer/autotests/data/no-content-type.mbox.html f0036df
messageviewer/autotests/data/openpgp-encrypted-enigmail1.6.mbox.html 01ede4f
messageviewer/autotests/data/openpgp-encrypted-partially-signed-attachments.mbox.html 39dcf30
messageviewer/autotests/data/openpgp-encrypted-two-attachments.mbox.html 63a9be1
messageviewer/autotests/data/openpgp-encrypted.mbox.html 94849b4
messageviewer/autotests/data/openpgp-inline-charset-encrypted.mbox.html 8daf746
messageviewer/autotests/data/openpgp-inline-signed.mbox.html 1743b65
messageviewer/autotests/data/openpgp-signed-base64-mailman-footer.mbox.html bb92d8a
messageviewer/autotests/data/openpgp-signed-encrypted-two-attachments.mbox.html 479c422
messageviewer/autotests/data/openpgp-signed-encrypted.mbox.html e0858bb
messageviewer/autotests/data/openpgp-signed-mailinglist.mbox.html 34512fc
messageviewer/autotests/data/openpgp-signed-two-attachments.mbox.html 2c1eb6d
messageviewer/autotests/data/signed-forward-openpgp-signed-encrypted.mbox.html 4dffcd8
messageviewer/autotests/data/smime-encrypted-octet-stream.mbox.html 20b2dbf
messageviewer/autotests/data/smime-encrypted.mbox.html 20b2dbf
messageviewer/autotests/data/smime-signed-encrypted.mbox.html cf0e6fa
messageviewer/autotests/data/text+html-maillinglist.mbox.html b35368e
messageviewer/autotests/data/tnef-one-file.mbox.html 28acaf5
messageviewer/autotests/data/tnef-two-files.mbox.html 8fcf6b3
messageviewer/viewer/csshelperbase.h 5c15f24
messageviewer/viewer/csshelperbase.cpp 70a9851
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/
More information about the kde-pim
mailing list