D7537: KDE logout screen - change black icons & texts to white/greyish

[Nathaniel Graham]

ngraham added a comment.


  You can abandon the revision in favor of that other one using Phabricator's 
"Add Action..." menu above the comment field.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7537

To: pekkah, #plasma_workspaces, #plasma, #breeze, mart
Cc: davidedmundson, Pitel, mvourlakos, ngraham, mart, bshah, #breeze, pekkah, 
#plasma_workspaces, plasma-devel, #plasma, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D7537: KDE logout screen - change black icons & texts to white/greyish

[Radek Hušek]

Pitel added a comment.


  I finally found what was wrong here: the background should not be black when 
button background color is light but the calculation introduced by D5036 
 is wrong. My attempt to fix it is D11262 
.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7537

To: pekkah, #plasma_workspaces, #plasma, #breeze, mart
Cc: davidedmundson, Pitel, mvourlakos, ngraham, mart, bshah, #breeze, pekkah, 
#plasma_workspaces, plasma-devel, #plasma, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D7537: KDE logout screen - change black icons & texts to white/greyish

[David Edmundson]

davidedmundson added a comment.


  I'm a bit confused.
  
  Your "before" screenshot doesn't match what I see, and yet your bug report 
mentions "normal Breeze theme".
  
  In the current code, it should be loading the complementary colour set (which 
effectively is the "dark background colour set") which will theme icons 
appropriately.
  Clearly we have some issue there that needs fixing, but it's a bug in the 
colourscope/icon handling code, not in the frontend QML.
  
  I'll follow that up on your bug report.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7537

To: pekkah, #plasma_workspaces, #plasma, #breeze, mart
Cc: davidedmundson, Pitel, mvourlakos, ngraham, mart, bshah, #breeze, pekkah, 
#plasma_workspaces, plasma-devel, #plasma, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D7537: KDE logout screen - change black icons & texts to white/greyish

[Radek Hušek]

Pitel added a comment.


  I guess I am not that confused anymore -- with normal color group the color 
of text (outside of buttons) is determined by text color theme setting but with 
complementary group by button text color.
  
  I think correct solution would be to construct `ColorScope` with `textColor` 
set to default text color if it is not too dark and white otherwise (the 
background color is handled in similar way). The problem is I do not see any 
way to construct `ColorScope` with custom colors -- it always takes colors from 
some color group of current theme.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7537

To: pekkah, #plasma_workspaces, #plasma, #breeze, mart
Cc: Pitel, mvourlakos, ngraham, mart, bshah, #breeze, pekkah, 
#plasma_workspaces, plasma-devel, #plasma, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D7537: KDE logout screen - change black icons & texts to white/greyish

[Radek Hušek]

Pitel added a comment.


  @mvourlakos I did some more testing and I am really confused. The important 
color seems to be button text color in Plasma theme setting. Results:
  
  With `colorGroup: PlasmaCore.Theme.ComplementaryColorGroup` (which is 
currently hardcoded):
  
  - with white button text color: all text is white -- OK
  - with black button text color: all text is black -- not OK for text outside 
of buttons
  
  With `colorGroup: PlasmaCore.Theme.NormalColorGroup` (or just deleting 
`colorGroup` property):
  
  - with white button text color: all text is white -- OK
  - with black button text color: text on buttons is black, other text is white 
-- OK
  
  I have no idea why button text color affects color of text outside of buttons 
with Complementary color group but does not affect it with Normal.
  
  (By buttons I mean OK and Cancel who draw their backgrounds.)

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7537

To: pekkah, #plasma_workspaces, #plasma, #breeze, mart
Cc: Pitel, mvourlakos, ngraham, mart, bshah, #breeze, pekkah, 
#plasma_workspaces, plasma-devel, #plasma, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D7537: KDE logout screen - change black icons & texts to white/greyish

[Michail Vourlakos]

mvourlakos added a comment.


  @Pitel how did you manage to reproduce this, with what color theme? In my 
system logout screen always appears whitish in master branch

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7537

To: pekkah, #plasma_workspaces, #plasma, #breeze, mart
Cc: Pitel, mvourlakos, ngraham, mart, bshah, #breeze, pekkah, 
#plasma_workspaces, plasma-devel, #plasma, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D7537: KDE logout screen - change black icons & texts to white/greyish

[Radek Hušek]

Pitel added a comment.


  I have the same problem in 5.12.2 release. I think the root of it is assuming 
that foreground in `PlasmaCore.Theme.ComplementaryColorGroup` is white(ish) and 
background is dark (this is true only if normal color scheme uses dark text on 
light background, which is default) and forcing that background is really dark 
(since D5036 ). My personal hack is just to 
replace `ComplementaryColorGroup` with `NormalColorGroup` (which obviously 
breaks default setup). A bit less of a hack would be choosing either 
`NormalColorGroup` or `ComplementaryColorGroup` depending on which one has 
darker background.
  
  The same problem also hits lock screen and user switcher.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7537

To: pekkah, #plasma_workspaces, #plasma, #breeze, mart
Cc: Pitel, mvourlakos, ngraham, mart, bshah, #breeze, pekkah, 
#plasma_workspaces, plasma-devel, #plasma, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D7537: KDE logout screen - change black icons & texts to white/greyish

[Michail Vourlakos]

mvourlakos added a comment.


  I dont know if I can help in this... I used the ColorOverlay approach lately 
with Latte in order to color properly the panel contents based on the 
underlying background.
  This approach could be also be used here I think.
  
  The luminosity of the textColor and the backgroundColor of the plasma theme 
could be calculated and the one that provides the most brightness is chosen ...

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7537

To: pekkah, #plasma_workspaces, #plasma, #breeze, mart
Cc: mvourlakos, ngraham, mart, bshah, #breeze, pekkah, #plasma_workspaces, 
plasma-devel, #plasma, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D7537: KDE logout screen - change black icons & texts to white/greyish

[Nathaniel Graham]

ngraham added a comment.


  @pekkah? Can you add the screenshots here and address the review comments?

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7537

To: pekkah, #plasma_workspaces, #plasma, #breeze, mart
Cc: ngraham, mart, bshah, #breeze, pekkah, #plasma_workspaces, plasma-devel, 
#plasma, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D7537: KDE logout screen - change black icons & texts to white/greyish

[Nathaniel Graham]

ngraham added a comment.


  Also, is this even still relevant?

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7537

To: pekkah, #plasma_workspaces, #plasma, #breeze, mart
Cc: ngraham, mart, bshah, #breeze, pekkah, #plasma_workspaces, plasma-devel, 
#plasma, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D7537: KDE logout screen - change black icons & texts to white/greyish

[Marco Martin]

mart added a comment.


  phabricator supports screenshots by itself, that should be used.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7537

To: pekkah, #plasma_workspaces, #plasma, #breeze, mart
Cc: mart, bshah, #breeze, pekkah, #plasma_workspaces, plasma-devel, #plasma, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, lukas

D7537: KDE logout screen - change black icons & texts to white/greyish

[Pekka Helenius]

pekkah added a comment.


  In https://phabricator.kde.org/D7537#139913, @bshah wrote:
  
  > Can you provide screenshot?
  
  
  Sure. I have opened a new bug issue on KDE Bugzilla. You can find it here: 
https://bugs.kde.org/show_bug.cgi?id=382264
  
  **Screenshots**
  
  I have submitted them as a part of the Bugzilla bug report. Direct links 
below.
  
  **Before:** https://bugs.kde.org/attachment.cgi?id=106572
  
  - Before the patch has been applied.,
  
  **After:** https://bugs.kde.org/attachment.cgi?id=106617
  
  - After the patch has been applied.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7537

To: pekkah, #plasma_workspaces, #plasma, #breeze, mart
Cc: mart, bshah, #breeze, pekkah, #plasma_workspaces, plasma-devel, #plasma, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, lukas

D7537: KDE logout screen - change black icons & texts to white/greyish

[Marco Martin]

mart requested changes to this revision.
mart added a comment.
This revision now requires changes to proceed.


  screenshots are necessary for visual changes

INLINE COMMENTS

> Logout.qml:140
>  name: kuser.fullName
> +/*ColorOverlay {
> +anchors.fill: parent

remove dead code

> Logout.qml:170
>  }
> +LogoutButton {
> +id: logoutButton

why is changing buttons order?

> Logout.qml:202
> +source: parent
> +color: "white"
>  }

no hardcoded colors, ever, also on any theme that has colored icons (so 
anything but breeze) this will break

> Logout.qml:216
>  }
> +color: "#c8c8c8"
>  text: {

no hardcoded colors

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7537

To: pekkah, #plasma_workspaces, #plasma, #breeze, mart
Cc: mart, bshah, #breeze, pekkah, #plasma_workspaces, plasma-devel, #plasma, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, lukas

D7537: KDE logout screen - change black icons & texts to white/greyish

[Bhushan Shah]

bshah added a comment.


  Can you provide screenshot?

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7537

To: pekkah, #plasma_workspaces, #plasma, #breeze
Cc: bshah, #breeze, pekkah, #plasma_workspaces, plasma-devel, #plasma, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, 
lukas

D7537: KDE logout screen - change black icons & texts to white/greyish

[Pekka Helenius]

pekkah added a reviewer: Breeze.
pekkah added a subscriber: Breeze.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7537

To: pekkah, #plasma_workspaces, #plasma, #breeze
Cc: #breeze, pekkah, #plasma_workspaces, plasma-devel, #plasma, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, 
lukas

D7537: KDE logout screen - change black icons & texts to white/greyish

[Pekka Helenius]

pekkah retitled this revision from "KDE logout window - change black icons & 
texts to white/greyish" to "KDE logout screen - change black icons & texts to 
white/greyish".

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7537

To: pekkah, #plasma_workspaces, #plasma
Cc: pekkah, #plasma_workspaces, plasma-devel, #plasma, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas