[RFC] Change button style
Needs ReviewPublic

Authored by ndavis on Oct 16 2019, 5:16 PM.

Details

Summary

This patch does a number of things.

  • Fixes a usability problem where it was hard to tell whether or not a focused and toggleable button was toggled because the focus color would always show unless another button took the focus.
  • Improves the look of hover effects on buttons
  • Removes the diagonal movement when buttons are sunken. Instead, the background color changes and the shadow disappears.
  • Removes the gradient on buttons. The breeze widget style rarely uses gradients and I didn't like how it messed with the button colors.

This patch should be landed alongside other patches that introduce a similar focus state style and a patch to the default colorschemes.

Test Plan

Diff Detail

Repository
R31 Breeze
Branch
ndavis/pushbutton-style (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 22971
Build 22989: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes

Wait no, I got it

  • Don't use HighlightedText if sunken
ndavis added a comment.EditedOct 16 2019, 5:43 PM

The hover color is still kind of an issue, but less of an issue. It looked really stiff and unnatural without a visible hover state though

ngraham added a comment.EditedOct 16 2019, 6:05 PM

I see what you're trying to do here, and I approve. But I think it might make more sense to more generally rethink how we indicate focus for buttons. IMO we should draw a highlight around the edges rather than changing the background color, for the following reasons:

  • It would avoid any confusion whatsoever regarding checkable buttons
  • It would make the currently focused button stop inappropriately looking like it's a dialog's default button
  • It would be more consistent with how focus is indicated for other UI elements in Breeze (e.g toolbuttons and line edits)
  • It would be more consistent with how other focus is indicated for other widget styles and platforms (e.g. macOS, Windows, and GNOME's Adwaita all show focus with a line around the edges)
ndavis added a subscriber: ngraham.Oct 16 2019, 7:35 PM

I see what you're trying to do here, and I approve. But I think it might make more sense to more generally rethink how we indicate focus for buttons. IMO we should draw a highlight around the edges rather than changing the background color, for the following reasons:

  • It would avoid any confusion whatsoever regarding checkable buttons
  • It would make the currently focused button stop inappropriately looking like it's a dialog's default button
  • It would be more consistent with how focus is indicated for other UI elements in Breeze (e.g toolbuttons and line edits)
  • It would be more consistent with how other focus is indicated for other widget styles and platforms (e.g. macOS, Windows, and GNOME's Adwaita all show focus with a line around the edges)

We already do that, it would just be a matter of removing the logic for using the focus color in the background. Here I did that to the focus color and the hover color:

Same thing, but with the toggle button focused and toggled.

Oh nice, lol. Could you do the same thing for pushbuttons and non-checked toggle buttons and comboboxes too? That way the focus behavior for checked toggle buttons wouldn't become inconsistent with the focus behavior for those other items.

I feel like the hover effect is not obvious enough though. In the future, I might want to make it so that the hover color is mixed with the current background color. That way it will be more visually related to whatever the toggled/untoggled state is. I might need to change the hover color to a shade of gray for that to look right.

Oh nice, lol. Could you do the same thing for pushbuttons and non-checked toggle buttons and comboboxes too? That way the focus behavior for checked toggle buttons wouldn't become inconsistent with the focus behavior for those other items.

They all get their looks from the same helper functions, so that's basically already done.

Hmm, I'm not sure it's working then. Focused pushbuttons still get a blue background.

Hmm, I'm not sure it's working then. Focused pushbuttons still get a blue background.

I didn't change the diff yet

This is starting to become more than just a usability fix for togglebuttons (I was hoping to make something that could be part of Plasma 5.17.1) and more of a general button highlight style patch.

Yeah I kind of think this should be a master-only re-think of Breeze's button focus indication in general rather than a targeted 5.17.1 patch.

ndavis retitled this revision from Don't use focus color on checkable button backgrounds to Change button hover and focus style.Oct 22 2019, 6:16 AM
ndavis edited the summary of this revision. (Show Details)
ndavis edited the test plan for this revision. (Show Details)
ndavis updated this revision to Diff 68495.Oct 22 2019, 6:16 AM
  • Change hover and focus style
ndavis edited the summary of this revision. (Show Details)Oct 22 2019, 6:17 AM
ndavis retitled this revision from Change button hover and focus style to [WIP] Change button hover and focus style.
ndavis retitled this revision from [WIP] Change button hover and focus style to [RFC] Change button hover and focus style.

There are certain bits that I don't feel are particularly elegant. Instead of having the logic for determining how the hover state looks only exist in the buttonBackgroundColor function, I should probably make a separate function. This would make the hover color from the colorscheme kind of useless though. I feel that hover should be adapted from the background color it is replacing instead of being one predefined color.

ndavis updated this revision to Diff 68496.Oct 22 2019, 6:25 AM
  • Use the lighten/darken values used in the video
ndavis retitled this revision from [RFC] Change button hover and focus style to [RFC] Change button style.Oct 22 2019, 6:34 AM
ndavis edited the summary of this revision. (Show Details)
ndavis edited the summary of this revision. (Show Details)Oct 22 2019, 7:04 AM
ndavis added a comment.EditedOct 22 2019, 7:28 AM

Currently, this makes text on light colorschemes that use a light Selection Text color difficult to read.

Arc colorscheme with focused button:

To fix that and keep this look, one of these things will need to happen:

-Change the colorschemes to use dark Selection Text on light themes. For colorschemes intended to be used with a Kvantum theme (Arc, Adapta), this isn't such a problem, but it's still less than ideal and it'll be really unpleasant for users of those colorschemes.

  • Adapt the colors created in the code to combinations of colors in the colorscheme. This could get messy and partially turns the colorscheme into a problem to solve instead of just a way to define a set of colors. Actually, I can just make it not use the Selection Text color.
  • Add More colorscheme colors. We already have a lot of these and we don't make good use of all of them.
  • Use existing colorscheme colors in ways that don't reflect their names/descriptions. For instance, using Focus for the background of the focus effect and hover for the outline or vice versa. This makes it less clear which colorscheme colors affect what things.

This is probably not everything, just what I've thought of.

I'm not a fan of changing the background for focused buttons. I think it just confuses things, and doesn't fix the current problem; it just changes it. IMO focus (not hover, focus) should be indicated exclusively with an outline around the control drawn in the highlight color.

ndavis updated this revision to Diff 68584.Oct 23 2019, 5:08 AM
  • Remove focus background

I'm not a fan of changing the background for focused buttons. I think it just confuses things, and doesn't fix the current problem; it just changes it. IMO focus (not hover, focus) should be indicated exclusively with an outline around the control drawn in the highlight color.

I like having an effect similar to list items in Plasmashell. Anyway, here's what no focus background looks like:

I think it might be a good idea to add an animation mode for the sunken state. Right now, the transition is just instant.

/me thinks that hover colors are more confusing than helpful, especially if they are similar / mixed with the "pressed" color. I have a mouse cursor to indicate the position of the mouse.

/me thinks that hover colors are more confusing than helpful, especially if they are similar / mixed with the "pressed" color. I have a mouse cursor to indicate the position of the mouse.

Why are they confusing for you? Are you talking about the current hover colors (one color for all button states) or the new hover colors (changes based on the background)? Have you tried the patch?

They're not meant to show where the mouse is, they're meant to show that you can interact with a control.

In general +1, I think this is an improvement. However something seems off about the text colors for focused buttons:

ndavis updated this revision to Diff 68632.Oct 23 2019, 8:37 PM
  • Don't use HightlightedText on PushButton labels

Should be fixed now. I forgot to add that to the diff here after I made the video.

Well that changed things, but it didn't fully fix the issue:

ndavis updated this revision to Diff 68637.Oct 23 2019, 8:49 PM
  • Don't use HighlightedText for focused non-flat toolbuttons

Looks like that's because those were ToolButtons and I only changed PushButtons

Combobox still affected:

Icons on focused buttons (ToolButtons as well as PushButtons) still have reversed colors:

Looks like the focused button background's color still changes a bit, too. In Breeze non-dark, it becomes a lighter shade of gray. I thought we were going to stop changing the background color for focused buttons and on;y use outlines?

Combobox still affected:

Icons on focused buttons (ToolButtons as well as PushButtons) still have reversed colors:

Looks like the focused button background's color still changes a bit, too. In Breeze non-dark, it becomes a lighter shade of gray. I thought we were going to stop changing the background color for focused buttons and on;y use outlines?

It has a different BG color because it's a default button, not just because it's focused. That has nothing to do with the patch (yet). I'll try to figure out why the icons are screwing up.

ndavis updated this revision to Diff 68645.Oct 23 2019, 10:07 PM
  • Don't use QIcon::Selected on non-flat focused buttons

Combobox still affected:

Icons on focused buttons (ToolButtons as well as PushButtons) still have reversed colors:

Looks like the focused button background's color still changes a bit, too. In Breeze non-dark, it becomes a lighter shade of gray. I thought we were going to stop changing the background color for focused buttons and on;y use outlines?

It has a different BG color because it's a default button, not just because it's focused. That has nothing to do with the patch (yet). I'll try to figure out why the icons are screwing up.

Actually, I might be wrong. For some reason, the focused background is still colored differently. I don't know where that's coming from.

Nice, this is looking better now. I'm gonna use it full time for a while.

I'm wondering if we might want to increase the thickness of the focus effect. It's maybe a bit too subtle now. In Adwaita, the focus outline around, say, spinboxes is quite chunky. But I think it works. macOS similarly has focus outlines that are quite large.

I still prefer having the focus background. It's a distinct feature of Breeze.

I do like how obvious it makes the selected button, but by doing this, it visually overwhelms and destroys the concept of the default button (which is supposed to be the most visually distinctive button). IMO it also interferes with the the checked and hover states IMO. For example, hovering over a focused button right now inverts its text color, which I don't like at all.

The confusing thing about focus / hover colors is having to move away the mouse in order to see weather it's focused or toggled.

I need to know: This button is pressed or not, that button will be interacted when I press space/enter. Highlighting the button under the mouse is an extra, it should not be too similar to the previously mentioned effects, and especially it should not be exactly like already having pressed the button. (If there is no toggle button, I need to have remembered the look of a toggled button.)

@ngraham: If I'll interact with the selected button (not sure about the correct words here), this button should be marked most obviously, since I'll NOT interact with the default button by pressing enter. Otherwise (e.g. while having tabbed to a different input element), the default button should be the most obvious one.

Maybe I'll describe it in HTML: <input type=text name=t><textarea name=ta></textarea><input type=button name=foo><input type=submit name=submit>
If I tab to t, the default button should be the most obvious, even if I hover the mouse above foo.
If I tab to ta, maybe the default button should be less obvious since enter will not press it?
If I tab to foo, foo should be the most obvious button, even if I mouse-hover submit, since enter/space will push foo.
If I tab to submit, it should have the same effect as foo just had.

Hover effects will just duplicate "mouse is in clickable area", and with decent designs, you should already know that without the effect:
<input type=togglebutton name=tb ><input type=submit name=submit> (I just invented a HTML toggle button)
I get distracted, come back to my desk and see the dialog. Disregarding the mouse cursor, I should see weather tb is pressed. When I move away the mouse, it should not optically un-toggle itself: "Ah that was the focus color, not the pressed-down-color!"

One more thing, I don't know weather it's a theme issue at all (probably not): Some applications will start an action on clicking a button. I remember these buttons to optically do nothing or just be pressed as long as the mouse button is, then … invisible background action ... then a modal dialog pops up. I'd prefer these buttons to remain pressed down until the modal dialog closes.

(Also you should not use toggle buttons outside toolbars, use checkboxes with clickable labels instead, ¢¢)

I do like how obvious it makes the selected button, but by doing this, it visually overwhelms and destroys the concept of the default button (which is supposed to be the most visually distinctive button). IMO it also interferes with the the checked and hover states IMO. For example, hovering over a focused button right now inverts its text color, which I don't like at all.

Any comment on the above?

I think we can change the background color on hover, but for focus, IMO we should try playing with outlines and see if we can make it work. When the background changes based on hover, focus, and also default button status, it's just too much, and it becomes hard to tell what's what IMO.

ndavis added a comment.EditedNov 15 2019, 8:32 AM

I do like how obvious it makes the selected button, but by doing this, it visually overwhelms and destroys the concept of the default button (which is supposed to be the most visually distinctive button). IMO it also interferes with the the checked and hover states IMO. For example, hovering over a focused button right now inverts its text color, which I don't like at all.

Any comment on the above?

I think we can change the background color on hover, but for focus, IMO we should try playing with outlines and see if we can make it work. When the background changes based on hover, focus, and also default button status, it's just too much, and it becomes hard to tell what's what IMO.

How is it for you compared to the git master focus decorations? I know it's not what you're asking, but I'd like to know.

I'm having a hard time letting go of what I wanted, but I know it has flaws. I've been exploring my options for making the complete experience of using buttons in Plasma more predictable, logical, accessible and attractive.

Things I've discovered about default buttons:

  • The default button stops being the default button if you focus a pushbutton or combobox. In that case, nothing happens when you press Enter. It may be unrelated to the Breeze QStyle, but I think this should change so that the default button is always activated when Enter is pressed.
  • GNOME's idea of a default button is effectively just the button which is focused by default.

On one hand, there's no way to tell that Enter and Space trigger different actions, partly based on what is focused in Qt applications.
On the other hand, knowing that you can press Enter to accept changes in a settings window without tabbing to the OK button (unless you focus a pushbutton or combobox) makes using settings windows feel a lot smoother for keyboard users.

  • GNOME's behavior would work perfectly with what I wanted and eliminate the need to come up with a better default button indicator
  • I think what we have is better in the long run for keyboard focused users if we can get it to work more consistently.

My primary objections to only using an outline to indicate focus on buttons are these:

  1. We don't normally do lines thicker than 1px
  2. A 1px focus outline could be a bit too thin if it doesn't contrast well enough with the background colors

A purely technical obstacle in the way of using just an outline is that the background still changes on focus even if the code for changing the background on focus is removed. I have no idea why this happens. Using the design I wanted and the git master design just masks the problem.

We do use thicker lines in some cases.

  • The Plasmashell tabs and Task Manager focus lines are 3px
  • the outlines around grid view items in SySe are 4px

Perhaps if we have a very limited set of line thicknesses it won't introduce more inconsistency, but anything larger than 2px looks really excessive and out of place for buttons.

How is it for you compared to the git master focus decorations? I know it's not what you're asking, but I'd like to know.

Almost perfect except for I'd like the focus outline to be 2px thick (1px is too hard to notice, as you point out) and default buttons still have a background color that makes them too hard to notice IMO.

Things I've discovered about default buttons:

  • The default button stops being the default button if you focus a pushbutton or combobox. In that case, nothing happens when you press Enter. It may be unrelated to the Breeze QStyle, but I think this should change so that the default button is always activated when Enter is pressed.
  • GNOME's idea of a default button is effectively just the button which is focused by default. On one hand, there's no way to tell that Enter and Space trigger different actions, partly based on what is focused in Qt applications. On the other hand, knowing that you can press Enter to accept changes in a settings window without tabbing to the OK button (unless you focus a pushbutton or combobox) makes using settings windows feel a lot smoother for keyboard users.
  • GNOME's behavior would work perfectly with what I wanted and eliminate the need to come up with a better default button indicator
  • I think what we have is better in the long run for keyboard focused users if we can get it to work more consistently.

I agree on both counts: that the GNOME approach is simpler and more immediately comprehensible, while our approach is more useful with real-world workflows, but slightly confusing in the subtlety of its behaviors. FWIW macOS does it largely the same way: enter triggers the default button, if there is one. Space triggers the focused thing, if it's different from the default button. Otherwise, both do the same thing. I guess it's not so weird to me since I got used to it over a lifetime of using macOS. The fact that the default button loses its default button status when a different pushbutton gets focus is the only difference between the Qt and macOS behaviors, and maybe something we could fix in Qt.

cblack added a subscriber: cblack.Nov 16 2019, 12:50 AM

I'm not a fan of dropping the diagonal movement- the transformation makes buttons feel much more button-y.

I'm not a fan of dropping the diagonal movement- the transformation makes buttons feel much more button-y.

How about vertical movement? I feel like the diagonal movement only works if your buttons have a diagonal light source (e.g., Windows 9x). Breeze widgets typically don't have any light direction and most widget styles only show the sunken state by changing the color, so I removed the movement all together.

How about doing what GNOME does with Adwaita where a small drop shadow is applied on the bottom of the button to create the illusion of a push-in-able button only for the shadow to then go when the button is being pushed down?

How about doing what GNOME does with Adwaita where a small drop shadow is applied on the bottom of the button to create the illusion of a push-in-able button only for the shadow to then go when the button is being pushed down?

That's a good idea. I think I'll do that.

+1, I like that effect too. I think they also move the drop shadow to the top to make it look like the button has been depressed and the top edge is casting a shadow on it, right?

+1, I like that effect too. I think they also move the drop shadow to the top to make it look like the button has been depressed and the top edge is casting a shadow on it, right?

Nope, they just remove the drop shadow and change the background color.

ndavis updated this revision to Diff 69901.EditedNov 18 2019, 4:07 AM
  • Don't use focus colors on sunken buttons
  • Don't horizontally offset shadows
  • If sunken, remove shadow
  • Use Button AlternateBackground color for sunken buttons

The effect of removing a shadow in Breeze is pretty subtle. I think it makes it at least a little better, but I wonder if the shadow should be bigger.

Would you mind rebasing this? It seems like we're moving forward with the idea of iterating on Breeze rather than making a whole new theme, and since Plasma 5.18 just branched, we have some space to hack on master now. I'd like to see if we can move forward with this.

There were some conflicts, so this isn't exactly how it was before.

ngraham added a comment.EditedJan 17 2020, 12:26 AM

Thanks Noah.

So you're the boss after all (as the de- facto Breeze maintainer now), and I think we should follow your lead design-wise. But it might also be interesting to have a discussion about what we want this to accomplish.

Personally here's my wishlist:

  1. Make it much more visually obvious which button is the default button
  2. Make the focused state look less like what most people would assume is the appearance for the default button
  3. Avoid making the focused state look too subtle
  4. Make the pressed state look more "pressed"

I think this patch does #3 and #4, but not #1 or #2. However it's possible that the focus style you've chosen will be just fine once the selection effects look like this everywhere for consistency, and if the default button gets a stronger look.

Thanks Noah.

So you're the boss after all (as the de- facto Breeze maintainer now), and I think we should follow your lead design-wise. But it might also be interesting to have a discussion about what we want this to accomplish.

Personally here's my wishlist:

  1. Make it much more visually obvious which button is the default button
  2. Make the focused state look less like what most people would assume is the appearance for the default button
  3. Avoid making the focused state look too subtle
  4. Make the pressed state look more "pressed"

    I think this patch does #3 and #4, but not #1 or #2. However it's possible that the focus style you've chosen will be just fine once the selection effects look like this everywhere for consistency, and if the default button gets a stronger look.

I can agree with all of these goals.

I think I'm going to need bigger, fancier shadows to get a good pressed movement (besides changing background color) with the diagonal movement removed.

TBH, the current way control shadows are rendered seems kind of wrong anyway. The shadow is rendered within the button, so when you rotate a button, the shadow gets rotated as well.

cfeck added a subscriber: cfeck.Jan 17 2020, 8:11 AM

If application developers need a rotated button, they need to rotate only the contents, not the frame.

If application developers need a rotated button, they need to rotate only the contents, not the frame.

Good to know. I remembered seeing an earlier version of the KScreen redesign patch that had rotated shadows. The buttons were made flat, which fixed that problem, so I assumed it was stuck like that because of the QStyle.

KScreen uses QML, so it may well be that the buttons were rotated using QML's rotation property, which does rotate the frame, since that's rendered by qqc2-desktop-style's StyleItem as a sub-part of the button.

KScreen uses QML, so it may well be that the buttons were rotated using QML's rotation property, which does rotate the frame, since that's rendered by qqc2-desktop-style's StyleItem as a sub-part of the button.

so would that need to be fixed in the QStyle or qqc2-desktop-style? Or is QML's rotation property not the right property to use?

Or is QML's rotation property not the right property to use?

Basically this. I would even go further and say that a normal Button/ToolButton is not the right control to use and instead you should use some sort of "VerticalTextButton" or so. Except that of course does no exist either.

I mostly think however that this is not a case to worry about, I do not really see a use case for buttons with vertical text in QML. Even in widgets they are often criticized.

ndavis edited the summary of this revision. (Show Details)Wed, Feb 26, 6:35 PM
ndavis edited the test plan for this revision. (Show Details)
ndavis edited the test plan for this revision. (Show Details)Wed, Feb 26, 6:44 PM