Use a conditional lightness check to determine the color for the non-working day numbers, and switch between two grays that each connote "not a work day" and look readable against dark and light backgrounds.
BUG: 389075
FIXED-IN 5.47
cfeck |
Frameworks | |
VDG |
Use a conditional lightness check to determine the color for the non-working day numbers, and switch between two grays that each connote "not a work day" and look readable against dark and light backgrounds.
BUG: 389075
FIXED-IN 5.47
Breeze Light:
Breeze Dark:
No Linters Available |
No Unit Test Coverage |
We can't because KWidgetsAddons is a tier 1 framework and can't rely on any other frameworks, and the positive/negative colors come from KColorScheme rather than anywhere in Qt.
Maybe instead use the HighlightText QPalette color? Hardcoding red may work for the two styles you present, but I could just set the view background to red and the text becomes unusable.
That would change the color to something different (e.g. with the Breeze themes, it's blue). If we're okay with that I can do it, but I was trying to produce as minimal a patch as possible.
Let's try to fix the bug for real, instead of implementing half-baked workarounds that only work for the default configurations.
Alternative ideas: add setters for the two possible cell text colors, then somehow set the KColorScheme color from the outside.
We really should upstream more of KColorScheme to Qt/QPalette...
That would require changes to all clients, which doesn't seem desirable. "Fixing the bug for real" probably looks like making KWidgetsAddons not a tier 1 frameworks anymore so it can use KColorScheme.
FWIW, we ran into the same issue with the recent visual update to KMessageWidget: D12508. The only practical near-term options here are to hardcode colors, or start depending on KColorScheme.
So what's our path forward? I'd prefer to land this as-is since it does indeed fix the bug for now, and then we can have a larger discussion regarding what to do about the fact that KWidgetsAddons can't access all the colors from the active scheme, which is an issue for multiple things, not just this thing here.
I will not block this change, but cannot approve it either.
Reasons:
I understand, and agree. I'm also not sure that red is the right color here in the first place.
- We should not use hardcoded colors on varying backgrounds. While the chance that someone uses Qt::red as a background color is zero, the code _should_ handle the dark vs. bright case as stated in the bug report.
OK, I'll implement that part at least (even if for now we keep the colors identical, whatever we choose).
Use a conditional lightness check but the same colors, pending a better color option (TBD)
Hm, now Sundays look like they belong to the previous/next month. (Do we even need to change colour everywhere, or would it be enough to only highlight the weekend in the header? Edit: Hm, I think that's a feature, so should probably be kept…)
You could also look at the way KDE3 styled the header (see https://edu.kde.org/kstars/feature/SetTime.png).
I don't know. Perhaps it's time to involve more VDG people. VDG, how should we style weekend days and day names? Currently we draw them in dark red, which is problematic for a few reasons:
(This screenshot also exposes another bug: in my locale, Saturdays are inexplicably marked as work days, even though the "sat" header gets the correct color)
QColor c; c = QColor(Qt::gray); qDebug() << c.lightnessF(); c = QColor(Qt::darkGray); qDebug() << c.lightnessF();
I get:
0.635294
0.501961
Both colors are very similar and neither has enough contrast for background colors with lightness at 0.55.
Additionally, I would prefer computing the brightness of a color using qGray() or something similar to account for the different luminances of the primary colors.