[PC3 ToolButton] Have the label take into account complementary color schemes
ClosedPublic

Authored by filipf on Dec 2 2019, 10:07 PM.

Details

Summary

We've ported the SDDM theme to PC3 and now have black labels in PC3 ToolButtons (keyboard and session button).

The SDDM theme uses a complementary color scheme, which is something the PC2 ToolButton respects and turns the labels white.

Therefore I just copy pasted PC2's label color code to its PC3 counterpart.

BUG: 414929
FIXED-IN: 5.66

Test Plan
import QtQuick 2.13
import org.kde.plasma.components 3.0 as PlasmaComponents
import org.kde.plasma.core 2.0 as PlasmaCore

PlasmaCore.ColorScope {
    colorGroup: PlasmaCore.Theme.ComplementaryColorGroup

    Rectangle {
        height: 80
        width: 100
        color: "red"

        PlasmaComponents.ToolButton{
            anchors.fill: parent
            text: "sup"
        }
    }
}

The label was white instead of black.

Diff Detail

Repository
R242 Plasma Framework (Library)
Branch
ok-text-colo (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19414
Build 19432: arc lint + arc unit
filipf created this revision.Dec 2 2019, 10:07 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptDec 2 2019, 10:07 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
filipf requested review of this revision.Dec 2 2019, 10:07 PM
filipf edited the test plan for this revision. (Show Details)Dec 2 2019, 10:07 PM
filipf added a reviewer: Plasma.
filipf edited the test plan for this revision. (Show Details)
filipf edited the test plan for this revision. (Show Details)
ngraham accepted this revision.Dec 2 2019, 11:16 PM
This revision is now accepted and ready to land.Dec 2 2019, 11:16 PM
filipf edited the summary of this revision. (Show Details)Dec 7 2019, 9:49 PM
fvogt added a subscriber: fvogt.Dec 7 2019, 10:13 PM

The check for the prefix was added in bf1d1cc6b2ad37cb586f44b56fa2438ed3a5dbfc, while the control.flat one got added much earlier.

The labels are visible again with just the control.flat condition, but the prefix one might be needed as well for non-breeze themes.

There's a part missing though, the triangle (visible on https://openqa.opensuse.org/tests/1105226#step/start_wayland_plasma5/21) is gone. That seems to be a feature lost with PC3 :-(

The check for the prefix was added in bf1d1cc6b2ad37cb586f44b56fa2438ed3a5dbfc, while the control.flat one got added much earlier.

The labels are visible again with just the control.flat condition, but the prefix one might be needed as well for non-breeze themes.

There's a part missing though, the triangle (visible on https://openqa.opensuse.org/tests/1105226#step/start_wayland_plasma5/21) is gone. That seems to be a feature lost with PC3 :-(

Thanks for looking into the patch.

Yeah, the triangle seems to be missing :/ possibly because PC2 has a menu property and PC3 doesn't... I will investigate further.

Yeah, PC2 ToolButton has this so it loads the arrow: https://github.com/KDE/plasma-framework/blob/master/src/declarativeimports/plasmastyle/ToolButtonStyle.qml#L110

For PC3 I guess we would have to hack it in in the sddm-theme code...

@mart and @davidedmundson are there plans for PC3 to have a menu property like PC2?

davidedmundson accepted this revision.Dec 11 2019, 9:45 AM

Please don't link external sites (GitHub) in the committed message.

RE: Menu
There is nothing in QQC2::Button to add a menu

If we want that it would have to be a custom button subclass, rather than something we support in the style.

Please don't link external sites (GitHub) in the committed message.

RE: Menu
There is nothing in QQC2::Button to add a menu

If we want that it would have to be a custom button subclass, rather than something we support in the style.

Or as a simple hack for this case, adding "▾" to the button's label.

filipf edited the summary of this revision. (Show Details)Dec 11 2019, 12:48 PM
filipf edited the test plan for this revision. (Show Details)
This revision was automatically updated to reflect the committed changes.
fvogt added a comment.Dec 12 2019, 7:09 PM

This fixed the button label, but the menu itself is unsuable due to a black text on dark background: https://openqa.opensuse.org/tests/1110939#step/start_wayland_plasma5/21

This fixed the button label, but the menu itself is unsuable due to a black text on dark background: https://openqa.opensuse.org/tests/1110939#step/start_wayland_plasma5/21

We've had several bugs due to the QQC2 and PC3 port and with the (LTS) release date being closer, we've reverted that port for now.

We'll have to figure this out at some point in the future though.