Block cursor improvements

[timdown]

Why

The block cursor currently has a couple of issues.

Firstly, the cursor is taller on an empty line than on non-empty lines. To illustrate this, all of the following screenshots use line-height: 2.5.

Non-empty line:

Empty line:

Secondly, an unfocused cursor overlays a copy of the character such that both are visible, making the character look stronger:

What changed

For the first issue, I've added a fullHeightCoordsAtPos function, which acts like coordsAtPos except that the top and bottom coordinates represent the top and bottom of the whole visual line, not just the text. It also works with wrapped lines. However, in order to work with wrapped lines, it relies on the assumption that every visual line within the wrapped line has the same height, which is not necessarily the case if the line contains widgets or decorations that affect the height of the line.

For the second issue, I've made the colour of the content of an unfocused block cursor transparent:

Test plan

1. Add a line height of 2.5 to the editor in dev/index.ts: EditorView.baseTheme({ '.cm-line': { lineHeight: '2.5' } })
2. View the dev page in a browser. Place the cursor on an empty line and a non-empty line to observe the difference.
3. Place the cursor over a letter. Click outside the editor. Look closely at the letter inside the cursor.

[timdown]

I've simplified the implementation of fullHeightCoordsAtPos. However, I'm unsure about whether the whole thing is appropriate for everyone. We want it for our editor, where we've applied similar workarounds to get the selection, cursor and highlighter decorations to cover the full height of the line. However, it might look a bit odd in general. Example:

The inconsistency between the heights of the block cursor on empty and non-empty lines is still something I think should be fixed; I could try to find another way if you prefer.

[nightwing]

On which version of codemirror do you see cursor height changing on empty line, i don't see it with @codemirror/[email protected].

I am not completely sure, but I think changing the way character height is calculated only for block cursor is not a good approach. It adds more problems like not having good solution for widgets, your screenshot, the text cursor being one pixel higher than what it should be in some cases

If one wants both cursor and selection to be of defaultLineHeight, wouldn't it be simpler to override coordsAtPos method, like this?

if (!_view.__proto__.coordsAtPos0) _view.__proto__.coordsAtPos0 = _view.__proto__.coordsAtPos
_view.__proto__.coordsAtPos = function(pos, side) {

  const coords = this.coordsAtPos0(pos, side);
  if (!coords) {
    return null;
  }

  const halfLeading =
      (this.defaultLineHeight - (coords.bottom - coords.top)) / 2;

  return {
    left: coords.left,
    right: coords.right,
    top: coords.top - halfLeading,
    bottom: coords.bottom + halfLeading,
  };
}

The change for transparent color looks good.

[timdown]

On which version of codemirror do you see cursor height changing on empty line, i don't see it with @codemirror/[email protected].

I'm seeing it on @codemirror/[email protected]. It happens to the regular CM6 cursor too (try moving the cursor between empty and non-empty lines), so you could argue this is an issue for CM6 itself.

I am not completely sure, but I think changing the way character height is calculated only for block cursor is not a good approach. It adds more problems like not having good solution for widgets, your screenshot, the text cursor being one pixel higher than what it should be in some cases.

Yeah, I think I agree. I'm not sure what the answer is though. I'll give it some more thought.

If one wants both cursor and selection to be of defaultLineHeight, wouldn't it be simpler to override coordsAtPos method, like this?

if (!_view.__proto__.coordsAtPos0) _view.__proto__.coordsAtPos0 = _view.__proto__.coordsAtPos
_view.__proto__.coordsAtPos = function(pos, side) {

  const coords = this.coordsAtPos0(pos, side);
  if (!coords) {
    return null;
  }

  const halfLeading =
      (this.defaultLineHeight - (coords.bottom - coords.top)) / 2;

  return {
    left: coords.left,
    right: coords.right,
    top: coords.top - halfLeading,
    bottom: coords.bottom + halfLeading,
  };
}

coordsAtPos is used in lots of places, so I suspect it could have undesirable consequences. Also, that seems somewhat... brutal.

The change for transparent color looks good.

Great. I've made a new PR with just that part: #89