From effc6d9d219233e0274ebf34bc58fa616579bd26 Mon Sep 17 00:00:00 2001
From: Antonio Scandurra <as-cii@github.com>
Date: Sat, 19 Aug 2017 15:39:32 +0200
Subject: [PATCH] Fix line number position when block decorations are at tile
 boundaries

When there are block decorations, we compute a `marginTop` style for
line numbers, so that they can be aligned correctly to the corresponding
line nodes.

This commit fixes a regression in the logic that calculated the
`marginTop` value, which was failing to correctly render line numbers
when block decorations were located right before the end of a tile, or
exactly at
the beginning of it.
---
 spec/text-editor-component-spec.js | 17 +++++++++++++++
 src/text-editor-component.js       | 34 +++++++++++++++++++++---------
 2 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/spec/text-editor-component-spec.js b/spec/text-editor-component-spec.js
index 6082d788f..e65e09e5d 100644
--- a/spec/text-editor-component-spec.js
+++ b/spec/text-editor-component-spec.js
@@ -2233,6 +2233,23 @@ describe('TextEditorComponent', () => {
       expect(item6.previousSibling).toBe(lineNodeForScreenRow(component, 12))
     })
 
+    it('correctly positions line numbers when block decorations are located at tile boundaries', async () => {
+      const {editor, component, element} = buildComponent({rowsPerTile: 3})
+      createBlockDecorationAtScreenRow(editor, 0, {height: 5, position: 'before'})
+      createBlockDecorationAtScreenRow(editor, 2, {height: 7, position: 'after'})
+      createBlockDecorationAtScreenRow(editor, 3, {height: 9, position: 'before'})
+      createBlockDecorationAtScreenRow(editor, 3, {height: 11, position: 'after'})
+      createBlockDecorationAtScreenRow(editor, 5, {height: 13, position: 'after'})
+
+      await component.getNextUpdatePromise()
+      assertLinesAreAlignedWithLineNumbers(component)
+      assertTilesAreSizedAndPositionedCorrectly(component, [
+        {tileStartRow: 0, height: 3 * component.getLineHeight() + 5 + 7},
+        {tileStartRow: 3, height: 3 * component.getLineHeight() + 9 + 11 + 13},
+        {tileStartRow: 6, height: 3 * component.getLineHeight()}
+      ])
+    })
+
     it('measures block decorations correctly when they are added before the component width has been updated', async () => {
       {
         const {editor, component, element} = buildComponent({autoHeight: false, width: 500, attach: false})
diff --git a/src/text-editor-component.js b/src/text-editor-component.js
index b02aea61d..25f879c38 100644
--- a/src/text-editor-component.js
+++ b/src/text-editor-component.js
@@ -2293,6 +2293,16 @@ class TextEditorComponent {
     return Math.max(0, this.lineTopIndex.rowForPixelPosition(pixelPosition))
   }
 
+  heightForBlockDecorationsBeforeRow (row) {
+    return this.pixelPositionAfterBlocksForRow(row) - this.pixelPositionBeforeBlocksForRow(row)
+  }
+
+  heightForBlockDecorationsAfterRow (row) {
+    const currentRowBottom = this.pixelPositionAfterBlocksForRow(row) + this.getLineHeight()
+    const nextRowTop = this.pixelPositionBeforeBlocksForRow(row + 1)
+    return nextRowTop - currentRowBottom
+  }
+
   pixelPositionBeforeBlocksForRow (row) {
     return this.lineTopIndex.pixelPositionBeforeBlocksForRow(row)
   }
@@ -3123,6 +3133,7 @@ class LineNumberGutterComponent {
         const tileEndRow = Math.min(endRow, tileStartRow + rowsPerTile)
         const tileChildren = new Array(tileEndRow - tileStartRow)
         for (let row = tileStartRow; row < tileEndRow; row++) {
+          const indexInTile = row - tileStartRow
           const j = row - startRow
           const key = keys[j]
           const softWrapped = softWrappedFlags[j]
@@ -3142,22 +3153,25 @@ class LineNumberGutterComponent {
             number = NBSP_CHARACTER.repeat(maxDigits - number.length) + number
           }
 
-          const lineNumberProps = {
+          // We need to adjust the line number position to account for block
+          // decorations preceding the current row and following the preceding
+          // row. Note that we ignore the latter when the line number starts at
+          // the beginning of the tile, because the tile will already be
+          // positioned to take into account block decorations added after the
+          // last row of the previous tile.
+          let marginTop = rootComponent.heightForBlockDecorationsBeforeRow(row)
+          if (indexInTile > 0) marginTop += rootComponent.heightForBlockDecorationsAfterRow(row - 1)
+
+          tileChildren[row - tileStartRow] = $(LineNumberComponent, {
             key,
             className,
             width,
             bufferRow,
             screenRow,
             number,
+            marginTop,
             nodePool: this.nodePool
-          }
-          const currentRowTop = rootComponent.pixelPositionAfterBlocksForRow(row)
-          const previousRowBottom = rootComponent.pixelPositionAfterBlocksForRow(row - 1) + lineHeight
-          if (currentRowTop > previousRowBottom) {
-            lineNumberProps.marginTop = currentRowTop - previousRowBottom
-          }
-
-          tileChildren[row - tileStartRow] = $(LineNumberComponent, lineNumberProps)
+          })
         }
 
         const tileTop = rootComponent.pixelPositionBeforeBlocksForRow(tileStartRow)
@@ -3264,7 +3278,7 @@ class LineNumberComponent {
     const {className, width, marginTop, bufferRow, screenRow, number, nodePool} = props
     this.props = props
     const style = {width: width + 'px'}
-    if (marginTop != null) style.marginTop = marginTop + 'px'
+    if (marginTop != null && marginTop > 0) style.marginTop = marginTop + 'px'
     this.element = nodePool.getElement('DIV', className, style)
     this.element.dataset.bufferRow = bufferRow
     this.element.dataset.screenRow = screenRow