lineHighlight added to Find but not Search; low highlight contrast

[bgashler1]

• VSCode Version: Version 1.4.0-insider 01a3f86
• OS Version: macOS El Capitan

Steps to Reproduce:

1. "Edit" > "Find"
2. Type in something to find in the current editor
3. When it finds the string in your document a lineHighlight is added.

@alexandrudima, since this does not also occur in our Search viewlet, it feels inconsistent to me. Also, this line highlight reduces the contrast of the highlighted found text, making it harder to see what is found. Visual Studio IDE also does not use the lineHighlight for this.

After looking issue #2792, I can see the underlying problem that was trying to be solved with helping users find the active highlight, but perhaps it would not be needed if we had a stronger highlight color contrast. At a minimum we should increase that highlight contrast anyway since it would be beneficial with or without the line highlight.

Options:

• Keep the lineHighlight (also add it to Search for consistency) and increase contrast on the highlight for found text.
• Remove lineHighlight and increase contrast on the highlight for found text.
• Darken the lineHighlight color so it can be stronger in contrast (may be detrimental to the experience of "Go to line", making the found line be less obvious).

@stevencl do you have any thoughts on this?

[stevencl]

Visual Studio uses a line highlight for goto file and find in files/search but not for find. In fact, as soon as the user enters the find dialog, the current line highlight is removed hence reducing the potential for confusion when navigating find results.

It's definitely helpful in both VS and VS Code for goto line.

I think we should keep it for goto line, and either remove it for find or add it for search too.

[stevencl]

Since it was a suggestion by a user to add the line highlight for find, can we add it for search too. As soon as the user selects a search result, we highlight the result and the line, just like we do for find.

In addition, lets increase the contrast on the found text. @bgashler1 - can you suggest the right contrast to use?

[bgashler1]

@stevencl sure, I'll get that up here soon.

[alexdima]

I've added the current line for the find widget recently. I think the problems we have come from the fact that some colors are defined in a theme and some colors (decorations especially) are hard-coded in CSS with different values for dark vs light. IMHO we need to compute all decorations colors and derive them from the theme or support additional keys in the themes (a theme is an open-ended key-value pair in the end). IMHO This would allow for third-parties to customize all the colors. fyi @aeschli

[bgashler1]

@alexandrudima I've noticed that the Search viewlet leverages the current-line CSS class, whereas the Find/Replace widget uses the lineHighlight class.

After a lot of thought, I think we should actually just go with current-line. Here's some arguments for doing that

• If you tab out of the Find/Replace while you have some results to the editor, it's clear that the current-line was actually set at the line of whatever result you were on. Furthermore, the result is selected at this point and typing something will modify it. This implies that whatever result we're at while using the find widget is actually going to determine where the current-line is at.
• Even if you change focus away from the editor in other ways (e.g. focus on terminal, click on a viewlet, focus on a different editor…) editors always show you where your current-line is (this Find/Replace thing has been the only exception I could find, and I think we should make it consistent).
• The Search viewlet is already doing this.
• We could still keep the lineHighlight class and use it as an extension point to highlight

Thoughts?

In the mean time, I have created a pull request with better contrasting colors for the Find/Replace results and the lineHighlight. I would rather we go with the current-line as I'm suggesting here. If you agree, please close the PR and let's do that instead. Thanks :)

[alexdima]

@bgashler1

Sorry, I'm not sure I understand what you mean. Here are the current things we have:

current-line

• is built into the editor and nobody (search viewlet included) controls it.
• rendered only when the editor selection is collapsed [I WILL GET BACK TO THIS]
• rendered also when the editor is blurred
  

lineHighlight

• it is a decoration class name
• it is used outside the editor (by contributions such as the outline, go to line and recently the find widget to highlight a line)
  
  

The problem that I tried to solve by giving a lineHighlight from the find widget remains with the search viewlet. It is that it is unclear which result you have selected:

I have a feeling that the root of all issues is that the current-line is rendered only when the selection is collapsed. We could simply render current-line all the time and then remove the lineHighlight usage from the find widget. It would help both the search viewlet and the find widget. If I remember correctly, we mimicked Visual Studio's behaviour in removing the current line highlight when the selection is not collapsed. Thoughts?

In parallel, @sandy081 starts to look into adding support for reading these colors from themes under custom Visual Studio Code theme keys (non standard TM keys), but that is a different discussion.

[bgashler1]

@alexandrudima thanks for the explanation. I'm actually seeing something a little different on the Search viewlet, though:

The above GIF might be too small to show it, but I'm seeing the current-line for each result I click on in the Search viewlet. It seems part of what you and I want is already working in the Insiders build.

It's true that hiding the current-line indication is how Visual Studio behaves when selections are not collapsed. However, we're already a little different because we are trying to emphasize the line of the current result (not used in Find/Replaces results within VS). On a side note, Sublime and Atom both show the current-line(s) even when selections are not collapsed, although their indicators are far more low-key by being in the left line-numbers margin only.

It seems we have 2 options

1. Continue using lineHighlight for Find/Replace and also start using it in the Search viewlet; also improve the color contrast of this highlight against the result highlight.
2. Use current-line for both Search and Find/Replace results (not the lineHighlight) and allow it to show for non-collapsed selections.

---

If we go with option 2, we could make it more beautiful by hiding the current-line emphasis whenever a selection spans multiple lines. If it is one big line that wraps to multiple sub-lines, then we could show the current-line on the top-most part of the selection (like this).

OR,

We could treat Find and Search result selections different from user selections so that they can receive a current-line on the top-most part of the result selection, but any time a user selects something then the current-line disappears.

What do you think?

[alexdima]

@bgashler1 I have narrowed down the difference in what we are seeing to a bug in the search viewlet. It appears the search viewlet does not select the result when the replace field is revealed. I believe this is a recent bug, from when the replace logic was added. Let's track that in #11024 . It is especially mean that it makes you think you have a selection there when in fact you don't (look at the status bar).

I am fine going either way, but I would prefer the lineHighlight. IMHO it should be used in the following way:

• when focus is outside the editor and someone wishes to emphasize something, then they should add a decoration of type lineHighlight to the range they wanna emphasize.
• when focus moves inside the editor, then that decoration should be removed by whomever added it.
• places that behave like this: quick outline, go to line, find widget
• places that do not behave like this: search viewlet with replace hidden, search viewlet with replace revealed.

In parallel, we should extract the color for lineHighlight and allow themes to overwrite it. @sandy081 is working on this for August.

Please also note the special case that works easily today of multiline emphasis:

[bgashler1]

@alexandrudima you convinced me. That's an excellent argument with the regex multiple result. I also like your rationale for lineHighlight being the go to decoration for pointing out something you are looking for, whether it be outline, go to line, find, search, etc.

Responding to theme colors that we don't yet respond to such as selection and lineHighlight are great plans along with a computed fallback in case none is provided so that contrast is always strong. Let me know if you need anything else for me on this or related issues.

@sandy081 I'd like to tweek the colors in my PR for lineHighlight once you have the lineHighlight theme colors working.

[sandy081]

@bgashler1 Implementation of reading VS Code specific (non standard) settings in TM themes is tracked here #11095. List of settings defined there are already extracted. Please feel free to add more if you think of any.

[sandy081]

Looks like this issue is tracking muliple issues

Color for Highlighting a line (Find, Go to line, Outline) is not good enough for all themes.

• With Extract the colors defined by editor as vs code custom settings in themes #11095 we exposed this as tm Theme setting rangeHighlight. So theme writers can provide an appropriate color for this.
• @bgashler1 working on default one in the PR: Better contrast for lineHighlight and find results #9631 #10924

Search not supporting highlighting the line.

Let's use this to track Color for line highlighting and use #11318 for other