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

[theming] Make contrastActiveBorder around text optional/themeable #43761

Closed
mbenkmann opened this issue Feb 15, 2018 · 5 comments
Closed

[theming] Make contrastActiveBorder around text optional/themeable #43761

mbenkmann opened this issue Feb 15, 2018 · 5 comments
Assignees
Milestone

Comments

@mbenkmann
Copy link

  • VSCode Version: 1.20.1
  • OS Version: Linux

I like contrastActiveBorder and think it is very useful around UI elements such as buttons, list elements, active tabs etc.
However around text in the editor I think it is extremely counterproductive. It makes things harder to read. I find the borders around text so unbearable that I set contrastActiveBorder to transparent because I would rather lose all the benefits than deal with this (after all, I stare at text all day). It would be way more useful to expand the selectionForeground feature to more parts, so that e.g. Find locations (such as the one in the screenshot below) could be made more visible that way. A more general approach that would not require themeing would be to either invert the colors or swap background/foreground similar to what terminals do.

Anyway, until a better solution is implemented I would like to see an independent "contrastActiveTextBorder" color that is used around editor text. It can fall back to contrastActiveBorder so that backwards compatibility would be maintained (for whatever weird person finds this useful), but themes could independently set it to transparent to make it disappear while keeping the contrastActiveBorder around UI elements.

Image of contrastActiveBorder around a word

@aeschli aeschli changed the title Make contrastActiveBorder around text optional/themeable [themeing] Make contrastActiveBorder around text optional/themeable Feb 15, 2018
@aeschli aeschli changed the title [themeing] Make contrastActiveBorder around text optional/themeable [theming] Make contrastActiveBorder around text optional/themeable Feb 15, 2018
@aeschli
Copy link
Contributor

aeschli commented Feb 15, 2018

I added a new color id 'editor.rangeHighlightBorder' that controls the border color you mention.
No change of out-of-the box behaviour. It's only set in highContrast and it is the contrastActiveBorder color by default (you can change it...)

@mbenkmann
Copy link
Author

The name seems suboptimal as it sounds similar to editor.findRangeHighlightBackground and would suggest it only affects the same text. We're talking about at least 3 things here whose background colors are editor.findMatchBackground, editor.findMatchHighlightBackground and editor.findRangeHighlightBackground.
If individual border colors were created for all 3, then there would be an editor.findRangeHighlightBorder which sounds very similar to your name. If indeed your new color affects all 3, it should have a more generic name (such as editor.contrastActiveTexBorder).

@mbenkmann
Copy link
Author

Just to make sure we're on the same page. Aside from the findRange... there are other occurrences, such as peekViewResult.*
So prefixing the name with "editor." would not be wise. I think my original suggestion contrastActiveTextBorder would work really well as a generic name and it would be easy to notice in the suggestions box while editing the user settings, because it would pop up while you're entering "contrastActive..." to set the contrastActiveBorder.

@aeschli
Copy link
Contributor

aeschli commented Feb 16, 2018

Good point, there are more uses of contrastActiveBorder that I missed. I added more border settings as having border settings for the decorations was a request from others as well.

Agree that range highlight and 'findRange highlight' 'findMatchRange highlight' might be confusing, but I want to stick with the terminology that we started.

aeschli added a commit to microsoft/vscode-docs that referenced this issue Feb 16, 2018
aeschli added a commit to microsoft/vscode-docs that referenced this issue Feb 16, 2018
@mbenkmann
Copy link
Author

mbenkmann commented Mar 19, 2018

The border around the identifier name in "Peek Definition" is still not themeable.

Other than that things look good. I can activate contrastActiveBorder in my theme now without getting eye cancer. Thank you very much.

@vscodebot vscodebot bot locked and limited conversation to collaborators Apr 2, 2018
@aeschli aeschli added this to the March 2018 milestone Apr 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants