-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Update styles for primary action button #376
Conversation
|
Aah, awesome! Do we also have a style for disabled buttons? |
I thought we did but it turns out we didn't! (or at least it wasn't getting set) Added in ac0edec |
|
The disabled style doesn't make it obvious at all that it's a disabled button... If you've never seen the button in its enabled state, it just looks like just a faded style. This kinda works in the website because the website 1) always has the buttons enabled from the start 2) they have a faux-3d style that becomes flat when they get disabled (which kinda works as a hint... kinda) I feel like green should only be there to signal that everything is green and ready to go. How about a monochromatic style for disabled buttons? |
|
I've been looking at buttons in different states in different themes and what they convey when they are initially disabled and there's no context for the user to compare (another button in an enabled state in the same area, for instance). Looking at the Commit button in the Changes pane, where there are no other buttons in the area and the Commit button starts disabled - the Light and the Dark theme make the background of the button the same as the background of the window, so only the outline and the gray text is showing. The blue theme turns the button background gray but the window background is white. I ran a completely random test with someone who is not used to these specific button styles in VS, to see if they could tell whether a button was in a disabled state or not. When in the Light or Dark theme, it was pretty obvious visually that the button is disabled. In the Blue theme, not so much - without an enabled button to compare, it was a bit of guess work and required clicking on the button to figure out if it would react or not. The disabled button theme just looks like a faded style without anything to compare it against. BTW, all the themes highlight enabled buttons on mouse hover (but that's learned behaviour, albeit pretty standard on windows). We should probably make use of that as an extra hint of state. Light themeDark themeBlue theme |
That makes sense. What if we treated our primary button the same: appear like a normal disabled button? I also took note of how the blue theme looks like a faded style and tried to make our disabled button's background closer to the window background color: (I purposefully removed the "Cancel" button so we have a better sense if it works as a disabled button) And then when enabled, we give it the full green button style:
💯 Having a hover state will help out with understanding if a button is disabled or not: By the way, these changes are only on the blue theme. If they look good, then we can go ahead and work the changes to the other themes. |
|
I've added the missing color definitions on the dark and light themes so that we can pull this in now. The colors are likely not correct and require validation, let's open another PR for that. |
|
This fixes #418 |











Ref: #356 (comment)
This PR updates the color for the primary action button to match the green specified in primer.
The text is also bolded for better readability / contrast.
EDIT: This only updates the color for the light and blue themes. The dark theme is unaffected aside from bolding the text.
cc: @shana