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

add option for input validation foreground color #57734

Merged

Conversation

ParkourKarthik
Copy link
Contributor

@ParkourKarthik ParkourKarthik commented Sep 1, 2018

This closes #57536

@bpasero I've made changes only related to "find Input control".
Review and let me know if similar changes could be applied to all other input validations.

@bpasero bpasero self-assigned this Sep 1, 2018
@bpasero bpasero added this to the September 2018 milestone Sep 1, 2018
Copy link
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

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

@ParkourKarthik unless I am mistaken, these colors should not have any default set, otherwise we change the semantics, aren't we? Today I do not think we apply any particular foreground color by default and I think we should only do so if the theme defines this color explicitly.

Also, we need to support this color everywhere where the correlating background color is passed through, so please find all occurrences of where input background colors are used and pass them through.

@ParkourKarthik
Copy link
Contributor Author

ParkourKarthik commented Sep 5, 2018

@bpasero I've set the default values to null. I believe that is meant to be done when no actual defaults are to be set.

Note: I've not added any Validation Foreground colors to the below as there should not be defaults for Foreground.
https://github.com/Microsoft/vscode/blob/8539f48172f1ba5acfe34bb5633eb684d59634ba/src/vs/base/browser/ui/inputbox/inputBox.ts#L72-L81

Copy link
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

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

@ParkourKarthik much better. There is one remaining usage of inputValidationInfoBackground in messageController that can also have a foreground color from the theme.

@bpasero
Copy link
Member

bpasero commented Sep 5, 2018

Thanks 🍺

@bpasero bpasero merged commit 3053bc6 into microsoft:master Sep 5, 2018
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 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.

Add inputValidation.foreground as a theme token
2 participants