Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use background color for input toggle active state #77481

Merged
merged 4 commits into from
Jul 19, 2019

Conversation

miguelsolorio
Copy link
Contributor

Fixes #36439 as discussed in #36439 (comment)

Dark

image

Light

image

This adds inputOption.activeBackground on active toggle states (search, find widget, etc.). This also sets inputOption.activeBorder to transparent as will only use outline on toggles for focus, as discussed in #36439 (comment). The new color is re-using focusBorder so it works well with other themes. Sample below:

ezgif com-gif-maker(47)

cc @Tyriar @sandy081 @roblourens @bpasero @aeschli

@@ -203,7 +203,8 @@ export const widgetShadow = registerColor('widget.shadow', { dark: '#000000', li
export const inputBackground = registerColor('input.background', { dark: '#3C3C3C', light: Color.white, hc: Color.black }, nls.localize('inputBoxBackground', "Input box background."));
export const inputForeground = registerColor('input.foreground', { dark: foreground, light: foreground, hc: foreground }, nls.localize('inputBoxForeground', "Input box foreground."));
export const inputBorder = registerColor('input.border', { dark: null, light: null, hc: contrastBorder }, nls.localize('inputBoxBorder', "Input box border."));
export const inputActiveOptionBorder = registerColor('inputOption.activeBorder', { dark: '#007ACC', light: '#007ACC', hc: activeContrastBorder }, nls.localize('inputBoxActiveOptionBorder', "Border color of activated options in input fields."));
export const inputActiveOptionBorder = registerColor('inputOption.activeBorder', { dark: '#007ACC00', light: '#007ACC00', hc: '#007ACC00' }, nls.localize('inputBoxActiveOptionBorder', "Border color of activated options in input fields."));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isidorn Also, not sure if you can help me understand why setting inputOption.activeBorder to null results in the colors not updating when being toggled via workbench.colorCustomizations. Ideally this should be null but can only get it to work when setting it to a transparent color.

@miguelsolorio
Copy link
Contributor Author

miguelsolorio commented Jul 16, 2019

Updated the colors for high contrast mode:

Copy link
Member

@roblourens roblourens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Does this affect other checkboxes or just the ones in the find input? Does it look good with other popular themes?

@miguelsolorio
Copy link
Contributor Author

@roblourens doesn't affect other checkboxes, just the toggle ones like exclude and search widget.

Here's what it looks like in other themes, some explicitly set inputOption.activeBorder so it doesn't look like much of a change for them:

gif

@roblourens
Copy link
Member

Ok cool, how about the checkboxes in the settings editor? (sorry I could build it myself but my repo is in the middle of something else)

@miguelsolorio
Copy link
Contributor Author

@roblourens doesn't affect checkboxes in the settings editor as it's not a toggle:

image

@miguelsolorio miguelsolorio merged commit 5c4a2e1 into master Jul 19, 2019
@miguelsolorio miguelsolorio deleted the misolori/button-toggle-style branch July 19, 2019 22:05
@isidorn
Copy link
Contributor

isidorn commented Jul 29, 2019

@misolori I was out on vacation, sorry for not responding.
This looks good, I will provide more feedback once I selfhost on it for a couple of days.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Toggle button focus is difficult to determine
3 participants