Fix toggle button colours
Needs ReviewPublic

Authored by cblack on Sat, Sep 21, 1:45 AM.

Details

Reviewers
ndavis
ngraham
Group Reviewers
Breeze
Summary

Toggle button colors are now correct.

Test Plan

before:


after:

Diff Detail

Repository
R98 Breeze for Gtk
Branch
fix-toggle-state (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 16938
Build 16956: arc lint + arc unit
cblack created this revision.Sat, Sep 21, 1:45 AM
Restricted Application added a project: Plasma. · View Herald TranscriptSat, Sep 21, 1:45 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
cblack requested review of this revision.Sat, Sep 21, 1:45 AM
cblack edited the test plan for this revision. (Show Details)Sat, Sep 21, 1:46 AM
ndavis accepted this revision.Sat, Sep 21, 1:51 AM
This revision is now accepted and ready to land.Sat, Sep 21, 1:51 AM
ngraham requested changes to this revision.Sat, Sep 21, 1:51 AM
ngraham added a subscriber: ngraham.

I was actually just about to report this, hah. The text color for the checked state looks too light for Breeze and Breeze light though:

The Breeze Widget theme has this using a dark text color instead of a light one. The color appears to be correct for Breeze Dark though.

This revision now requires changes to proceed.Sat, Sep 21, 1:51 AM

Ah, I only tested Breeze Dark, which uses the same white text color for everything.

@cblack Can you change the focus state to only have a blue outline? This behavior is really confusing, even though it seems consistent with Breeze:

ndavis requested changes to this revision.EditedSat, Sep 21, 1:56 AM

In the beginning of the video where it looks like I'm just hovering, I'm actually clicking once every half second.

This behavior is really confusing, even though it seems consistent with Breeze

I really dislike this about Breeze itself and would welcome a fix there too. Then maybe we can make the default button actually be blue, rather than an almost-indistinguishable shade of gray (See https://bugs.kde.org/show_bug.cgi?id=386158).

ndavis added a comment.EditedSat, Sep 21, 2:01 AM

This behavior is really confusing, even though it seems consistent with Breeze

I really dislike this about Breeze itself and would welcome a fix there too. Then maybe we can make the default button actually be blue, rather than an almost-indistinguishable shade of gray (See https://bugs.kde.org/show_bug.cgi?id=386158).

This is because the default button uses the View Background color. On Breeze Dark, the default button is much more obvious:


I still agree that blue would be better for the default button.

Yeah, it's definitely more obvious in Breeze dark, but I've always felt that a more colorful color was appropriate for the default button. Maybe it's just my history in macOS, where it's been like that for 20 years, but I've always appreciated being able to visually pick out in an instant what button the programmers wanted to highlight as the obvious or safe choice. GNOME does this too in some apps, using a bright blue button color to help guide you through a UI. See SimpleScan for example.

cblack added a comment.EditedSun, Sep 22, 1:41 AM

@cblack Can you change the focus state to only have a blue outline? This behavior is really confusing, even though it seems consistent with Breeze: -snip-

I'd rather not diverge from the Breeze here for a few reasons:

  • I don't have a guarantee that Breeze is going to have the exact same change
  • Breeze GTK is supposed to look like the current state of Breeze, with adaptations where necessary due to differences between GTK and Qt

If Breeze gets the change, I'll add it.

In D24127#535551, @ngraham shared an image:

That doesn't match the color of what I have on my machine.

Weird, this happens with the regular old breeze color scheme for me.

ndavis added a comment.EditedSun, Sep 22, 5:59 AM

@cblack Can you change the focus state to only have a blue outline? This behavior is really confusing, even though it seems consistent with Breeze: -snip-

I'd rather not diverge from the Breeze here for a few reasons:

  • I don't have a guarantee that Breeze is going to have the exact same change
  • Breeze GTK is supposed to look like the current state of Breeze, with adaptations where necessary due to differences between GTK and Qt

    If Breeze gets the change, I'll add it.

I will do the same thing to Breeze if I ever find an example of a situation where a focusable and toggleable button exists with the same usability issue. We do have a real example where this usability issue happens with GTK, so I think it's best to do this now instead of later.

cblack updated this revision to Diff 66712.Mon, Sep 23, 8:21 PM

Substitute border decoration for fill decoration

ngraham added a comment.EditedMon, Sep 23, 8:33 PM

@cblack if we're going to use this patch to do that, then the same treatment needs to be done for other buttonlike controls like comboboxes (with and without text entry), font buttons, file picker buttons, etc. Also I feel like maybe the border thickness of the highlight could stand to be increased so it's more visible.

@ndavis, if we're going to do this, can you make the same change in Breeze itself so the themes are consistent?

ndavis added a comment.EditedMon, Sep 23, 11:02 PM

@cblack if we're going to use this patch to do that, then the same treatment needs to be done for other buttonlike controls like comboboxes (with and without text entry), font buttons, file picker buttons, etc. Also I feel like maybe the border thickness of the highlight could stand to be increased so it's more visible.

@ndavis, if we're going to do this, can you make the same change in Breeze itself so the themes are consistent?

Yes. I just tested a focusable and toggleable button in Qt Designer and the Breeze QStyle does have the same usability problem.