[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 10:15:13 BST 2015


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

(Updated Sept. 10, 2015, 11:15 a.m.)


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 (updated)
-----

  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