blob: Fix text selection visibility over selected lines and line number alignment

[fkling]

Closes #59096

It's been reported multiple times that text selection conflicts with line selection, i.e. the selected line background color would be rendered on top of text selection background color and thus it seemed that no text was selected.

A little history

I remembered that this was fixed at some point so I was curious why it didn't work anymore.

The original fix was made in #47378. The selected-line CSS class was removed from the selected line decoration (effectively removing the background color) and instead the selected line(s) background was drawn with CodeMirror's layer feature. Layers are drawn behind the text and therefore not conflicting with the text selection color.

#47378 added back the selected-line CSS class to the selected line decoration (I'm not quite sure why) but also set the .cm-line.selected-line's background color to transparent to keep the text selection visible. I.e. it was still the layer that would draw the selected line's background color.

#58675 removed background: transparent because due to how the layer's rectangle marker was set up, it would only cover the actual text of the line, not the whole line. This causes issues with block widgets that appeared in the line above the text. They would not appear to be part of the selected line:

By accident this seemingly innocent change brought back the text selection issue.

Rectangle marker computation problems

Reading the documentation for RectangleMarker.forRange it seems reasonable to assume that the computed rectangle would cover the area of the whole line. However, it seems that it really just covers the area of the text itself (the documentation does mention "content"). This was tried to counteract by adding margins and paddings but there where always situations where this didn't look right, especially with the blame view open, which changes the height of each line:

Solution

As can been seen in this PR it's possible to derive the real boundaries of a line, so that the rectangle marker can be drawn correctly. Removing the selected-line class from the selected line decoration doesn't appear to result in any visual differences, which, once again, fixes the text selection problem.
Because the rectangle marker is now drawn on line boundaries, block widgets are "covered" by it as well and there are no small shifts anymore.

sg-text-selection.mp4

Bonus: Line number and folding marker alignment

One thing I've noticed (and had noticed before) is that folding markers are not aligned well when the blame view is open. And I was surprised to see that line numbers seem wrongly aligned when line wrapping was turned on.

There is also a little bit of history here:

• Blame UI improvements #44287 always centered line numbers so that it looks properly aligned with the blame view enabled (blame view increases line height). But that didn't work well with line wrapping.
• Make line-number layout always align to top #46191 somewhat reverted that change, making line numbers top aligned again. That fixed line wrapping but "broke" the blame view again.
• OpenCodeGraph prototype #58675 on the other hand caused line numbers to be always bottom aligned. I assume that was done to make the line number align with the text and not with the block widget that appears before it. However this caused multiple problems:
  • With line wrapping enabled it appears like there is an unnumbered line.
    
  • The folding indicator is potentially completely "detached" from its line number if line wrapping is turned on or a block widget is shown.
    
  • Misalignment in the blame view
    

Solution

The simplest solution is to not mess with the line number alignment at all. That means in the opencodegraph use case the line number will be shown next to the line number, which I think is OK (there might also be a way to render the opencodegraph widget between two lines if that's more desirable).
For the blame view it appears to suffice to set the line number and folding gutter elements to the same line height as the code line.

Test plan

Manual testing, see video.

Opencodegraph block widget:

I also verified that https://github.com/sourcegraph/sourcegraph/issues/58007 isn't reintroduced.

[fkling]

This didn't seem to be necessary and removing it actually lets the selected line color "come through".

[peterguy]

I would approve this PR based on the comprehensive description alone, but the code changes look good also!