diff --git a/client/web/src/end-to-end/code-intel/repository-component.test.ts b/client/web/src/end-to-end/code-intel/repository-component.test.ts index 00a7275bf837..5f21a35519f1 100644 --- a/client/web/src/end-to-end/code-intel/repository-component.test.ts +++ b/client/web/src/end-to-end/code-intel/repository-component.test.ts @@ -499,7 +499,7 @@ describe('Repository component', () => { await link.click() const selectedLine = await driver.page.waitForSelector( - `[data-testid="repo-blob"] .cm-line:nth-child(${line}).selected-line` + `[data-testid="repo-blob"] .cm-line:nth-child(${line})[data-testid="selected-line"]` ) expect(selectedLine).not.toBeNull() }) diff --git a/client/web/src/repo/blob/codemirror/blame-decorations.tsx b/client/web/src/repo/blob/codemirror/blame-decorations.tsx index 6d200ef5d6f1..1977081237b0 100644 --- a/client/web/src/repo/blob/codemirror/blame-decorations.tsx +++ b/client/web/src/repo/blob/codemirror/blame-decorations.tsx @@ -139,7 +139,6 @@ const blameDecorationTheme = EditorView.theme({ left: '0', height: '100%', display: 'inline-block', - background: 'var(--body-bg)', verticalAlign: 'bottom', width: 'var(--blame-decoration-width)', @@ -157,6 +156,15 @@ const blameDecorationTheme = EditorView.theme({ '.no-line-break-msg': { marginLeft: 'var(--blame-decoration-width)', }, + + // This ensures that line numbers and folding controls are reasonably aligned + // with the top of the line, even if line wrapping is enabled. + '.cm-lineNumbers .cm-gutterElement': { + lineHeight: '1.5rem', + }, + '.cm-foldGutter .cm-gutterElement': { + lineHeight: '1.5rem', + }, }) class BlameDecorationViewPlugin implements PluginValue { diff --git a/client/web/src/repo/blob/codemirror/linenumbers.ts b/client/web/src/repo/blob/codemirror/linenumbers.ts index 6da4be18b134..36f88659c78a 100644 --- a/client/web/src/repo/blob/codemirror/linenumbers.ts +++ b/client/web/src/repo/blob/codemirror/linenumbers.ts @@ -1,6 +1,5 @@ import { Annotation, - EditorSelection, type Extension, type Range, RangeSet, @@ -21,51 +20,8 @@ import { type ViewUpdate, } from '@codemirror/view' -import { isFirefox } from '@sourcegraph/common' - import { isValidLineRange, MOUSE_MAIN_BUTTON } from './utils' -const selectedLinesTheme = EditorView.theme({ - /** - * [RectangleMarker.forRange](https://sourcegraph.com/github.com/codemirror/view@a0a0b9ef5a4deaf58842422ac080030042d83065/-/blob/src/layer.ts?L60-75) - * returns absolutely positioned markers. Markers top position has extra 1px (5px in case blame decorations - * are visible) more in its `top` value breaking alignment wih the line. - * We compensate this spacing by setting negative margin-top. - * - * Line highlighting breaks (highlights two lines instead of 1) in firefox if minHeight is set, so we - * add a conditional to check for the current browser, and set the css accordingly. - * - * todo(fkling): Revisit this, styling is not correct for empty lines - */ - '.selected-lines-layer .selected-line': isFirefox() - ? { - marginTop: '-1px', - } - : { - marginTop: '-1px', - // Ensure selection marker height matches line height. - minHeight: '1.0rem', - }, - - '.selected-lines-layer .selected-line.blame-visible': { - marginTop: '-5px', - - // Ensure selection marker height matches the increased line height. - minHeight: 'calc(1.5rem + 1px)', - }, - - /** - * Rectangle markers `left` position matches the position of the character at the start of range - * (for selected lines it is first character of the first line in a range). When line content (`.cm-line`) - * has some padding to the left (e.g. to create extra space between gutters and code) there is a gap in - * highlight (background color) between the selected line gutters (decorated with {@link selectedLineGutterMarker}) and layer. - * To remove this gap we move padding from `.cm-line` to the last gutter. - */ - '.cm-gutter:last-child .cm-gutterElement': { - paddingRight: '0.2rem', - }, -}) - /** * Represents the currently selected line range. null means no lines are * selected. Line numbers are 1-based. @@ -74,7 +30,6 @@ const selectedLinesTheme = EditorView.theme({ export type SelectedLineRange = { line: number; character?: number; endLine?: number } | null const selectedLineDecoration = Decoration.line({ - class: 'selected-line', attributes: { tabIndex: '-1', 'data-line-focusable': '', @@ -135,12 +90,12 @@ export const selectedLines = StateField.define({ /** * We highlight selected lines using layer instead of line decorations. - * With this approach both selected lines and editor selection layers may be visible (with the latter taking precedence). - * It makes selected text highlighted even if it is on a selected line. + * With this approach both selected lines and editor selection layers are be visible (with the latter taking precedence). * - * We can't use line decorations for this because the editor selection layer is positioned behind the document content + * With line decorations the editor selection layer is positioned behind the document content * and thus the line background set by line decorations overrides the layer background making selected text - * not highlighted. + * not highlighted. An alternative would be to use reduce the opacity of the line decorations but this would + * change the text selection color. */ layer({ above: false, @@ -150,17 +105,70 @@ export const selectedLines = StateField.define({ return [] } - const endLineNumber = range.endLine ?? range.line - const startLine = view.state.doc.line(Math.min(range.line, endLineNumber)) - const endLine = view.state.doc.line( - Math.min(view.state.doc.lines, startLine.number === endLineNumber ? range.line : endLineNumber) - ) + // We can't use RectangleMarker.fromRange because this positions the marker exactly at the start/top of + // the actual text in the line, not at start/top what we consider to be the line. This is especially + // apparent when the blame column is visible because the line hight is increased to give the blame + // information more space. The following box illustrates the problem: + // + // ┌───┬────────────────────────────────────────────────┐ ──┐ + // │ │ ──┐ │ │ + // │ 1 │ Some text here ├── .fromRange gives us this │ ├─── we want this + // │ │ ──┘ │ │ + // └───┴────────────────────────────────────────────────┘ ──┘ + // + // The left and top positions, and width and height of the rectangle marker that corresponds to the + // selected line are computed from these values: + // + // rectangle.left + // ◄─► rectangle.width + // ◄──────────────────────► + // ▲ ┌─────────────────────────┐ ▲ + // │ │ │ │documentPadding.top + // │ ├─┬───────────────────────┤ ▲ ▲ ▼ + // rectangle.top│ │1│First line │ │ │ + // │ ├─┼───────────────────────┤ │ │topLineBlock.top + // │ │2│Second line │ │ │ + // ▲▼ ├─┼───────────────────────┤ │ ▼ + // rectangle.height│ │3│███████████████████████│ │topLineBlock.bottom + // ▼ ├─┼───────────────────────┤ ▼ + // │4│Fourth line │ + // ├─┼───────────────────────┤ + // │.│... │ + // └─┴───────────────────────┘ + // ◄───────────────────────► + // ▲ ▲ contentRect.width + // │ │ + // │ │ + // │ contentRect.left + // │ + // viewRect.left + + const viewRect = view.dom.getBoundingClientRect() + const contentRect = view.contentDOM.getBoundingClientRect() + + const topLine = view.state.doc.line(range.endLine ? Math.min(range.line, range.endLine) : range.line) + const topLineBlock = view.lineBlockAt(topLine.from) + + // Markers are positioned relative to the view DOM element, i.e. position 0 would be left of the gutter. + // This computes the left position of the content element relative to the view DOM element. + const left = contentRect.left - viewRect.left + + // block.top is relative to the document top, which is the top of the first line, _not_ including the + // content element's padding. So we have to add the padding to properly align the marker with the top + // of the line. + const top = topLineBlock.top + view.documentPadding.top + const width = contentRect.width + let height = topLineBlock.bottom - topLineBlock.top + + if (range.endLine !== undefined) { + const bottomLine = view.state.doc.line( + Math.min(view.state.doc.lines, Math.max(range.line, range.endLine)) + ) + const bottomLineBlock = view.lineBlockAt(bottomLine.from) + height = bottomLineBlock.bottom - topLineBlock.top + } - return RectangleMarker.forRange( - view, - 'selected-line', - EditorSelection.range(startLine.from, Math.min(endLine.to + 1, view.state.doc.length)) - ) + return [new RectangleMarker('selected-line', left, top, width, height)] }, update(update) { return ( @@ -175,8 +183,6 @@ export const selectedLines = StateField.define({ class: 'selected-lines-layer', }), - selectedLinesTheme, - gutterLineClass.compute([field], state => { const range = state.field(field) const marks: Range[] = [] @@ -261,13 +267,6 @@ const selectedLineNumberTheme = EditorView.theme({ cursor: 'pointer', color: 'var(--line-number-color)', - '& .cm-gutterElement': { - display: 'flex', - flexDirection: 'column', - alignItems: 'flex-end', - justifyContent: 'flex-end', - }, - '& .cm-gutterElement:hover': { textDecoration: 'underline', },