Merge pull request #15503 from atom/as-fix-invalidated-block-decoration-markers

Fix rendering of block decorations for invalid markers

spec/text-editor-component-spec.js

@@ -2284,6 +2284,93 @@ describe('TextEditorComponent', () => {
       ])
     })
 
+    it('removes block decorations whose markers are invalidated, and adds them back when they become valid again', async () => {
+      const editor = buildEditor({rowsPerTile: 3, autoHeight: false})
+      const {item, decoration, marker} = createBlockDecorationAtScreenRow(editor, 3, {height: 44, position: 'before', invalidate: 'touch'})
+      const {component, element} = buildComponent({editor, rowsPerTile: 3})
+
+      // Invalidating the marker removes the block decoration.
+      editor.getBuffer().deleteRows(2, 3)
+      await component.getNextUpdatePromise()
+      expect(item.parentElement).toBeNull()
+      assertLinesAreAlignedWithLineNumbers(component)
+      assertTilesAreSizedAndPositionedCorrectly(component, [
+        {tileStartRow: 0, height: 3 * component.getLineHeight()},
+        {tileStartRow: 3, height: 3 * component.getLineHeight()},
+        {tileStartRow: 6, height: 3 * component.getLineHeight()}
+      ])
+
+      // Moving invalid markers is ignored.
+      marker.setScreenRange([[2, 0], [2, 0]])
+      await component.getNextUpdatePromise()
+      expect(item.parentElement).toBeNull()
+      assertLinesAreAlignedWithLineNumbers(component)
+      assertTilesAreSizedAndPositionedCorrectly(component, [
+        {tileStartRow: 0, height: 3 * component.getLineHeight()},
+        {tileStartRow: 3, height: 3 * component.getLineHeight()},
+        {tileStartRow: 6, height: 3 * component.getLineHeight()}
+      ])
+
+      // Making the marker valid again adds back the block decoration.
+      marker.bufferMarker.valid = true
+      marker.setScreenRange([[3, 0], [3, 0]])
+      await component.getNextUpdatePromise()
+      expect(item.nextSibling).toBe(lineNodeForScreenRow(component, 3))
+      assertLinesAreAlignedWithLineNumbers(component)
+      assertTilesAreSizedAndPositionedCorrectly(component, [
+        {tileStartRow: 0, height: 3 * component.getLineHeight()},
+        {tileStartRow: 3, height: 3 * component.getLineHeight() + 44},
+        {tileStartRow: 6, height: 3 * component.getLineHeight()}
+      ])
+
+      // Destroying the decoration and invalidating the marker at the same time
+      // removes the block decoration correctly.
+      editor.getBuffer().deleteRows(2, 3)
+      decoration.destroy()
+      await component.getNextUpdatePromise()
+      expect(item.parentElement).toBeNull()
+      assertLinesAreAlignedWithLineNumbers(component)
+      assertTilesAreSizedAndPositionedCorrectly(component, [
+        {tileStartRow: 0, height: 3 * component.getLineHeight()},
+        {tileStartRow: 3, height: 3 * component.getLineHeight()},
+        {tileStartRow: 6, height: 3 * component.getLineHeight()}
+      ])
+    })
+
+    it('does not render block decorations when decorating invalid markers', async () => {
+      const editor = buildEditor({rowsPerTile: 3, autoHeight: false})
+      const {component, element} = buildComponent({editor, rowsPerTile: 3})
+
+      const marker = editor.markScreenPosition([3, 0], {invalidate: 'touch'})
+      const item = document.createElement('div')
+      item.style.height = 30 + 'px'
+      item.style.width = 30 + 'px'
+      editor.getBuffer().deleteRows(1, 4)
+
+      const decoration = editor.decorateMarker(marker, {type: 'block', item, position: 'before'})
+      await component.getNextUpdatePromise()
+      expect(item.parentElement).toBeNull()
+      assertLinesAreAlignedWithLineNumbers(component)
+      assertTilesAreSizedAndPositionedCorrectly(component, [
+        {tileStartRow: 0, height: 3 * component.getLineHeight()},
+        {tileStartRow: 3, height: 3 * component.getLineHeight()},
+        {tileStartRow: 6, height: 3 * component.getLineHeight()}
+      ])
+
+      // Making the marker valid again causes the corresponding block decoration
+      // to be added to the editor.
+      marker.bufferMarker.valid = true
+      marker.setScreenRange([[2, 0], [2, 0]])
+      await component.getNextUpdatePromise()
+      expect(item.nextSibling).toBe(lineNodeForScreenRow(component, 2))
+      assertLinesAreAlignedWithLineNumbers(component)
+      assertTilesAreSizedAndPositionedCorrectly(component, [
+        {tileStartRow: 0, height: 3 * component.getLineHeight() + 30},
+        {tileStartRow: 3, height: 3 * component.getLineHeight()},
+        {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})
@@ -2351,8 +2438,8 @@ describe('TextEditorComponent', () => {
       expect(editor.getCursorScreenPosition()).toEqual([0, 0])
     })
 
-    function createBlockDecorationAtScreenRow(editor, screenRow, {height, margin, marginTop, marginBottom, position}) {
-      const marker = editor.markScreenPosition([screenRow, 0], {invalidate: 'never'})
+    function createBlockDecorationAtScreenRow(editor, screenRow, {height, margin, marginTop, marginBottom, position, invalidate}) {
+      const marker = editor.markScreenPosition([screenRow, 0], {invalidate: invalidate || 'never'})
       const item = document.createElement('div')
       item.style.height = height + 'px'
       if (margin != null) item.style.margin = margin + 'px'
@@ -2360,7 +2447,7 @@ describe('TextEditorComponent', () => {
       if (marginBottom != null) item.style.marginBottom = marginBottom + 'px'
       item.style.width = 30 + 'px'
       const decoration = editor.decorateMarker(marker, {type: 'block', item, position})
-      return {item, decoration}
+      return {item, decoration, marker}
     }
 
     function assertTilesAreSizedAndPositionedCorrectly (component, tiles) {

src/text-editor-component.js

@@ -2444,37 +2444,61 @@ class TextEditorComponent {
     const {model} = this.props
     const decorations = model.getDecorations({type: 'block'})
     for (let i = 0; i < decorations.length; i++) {
-      this.didAddBlockDecoration(decorations[i])
+      this.addBlockDecoration(decorations[i])
     }
   }
 
-  didAddBlockDecoration (decoration) {
+  addBlockDecoration (decoration, subscribeToChanges = true) {
     const marker = decoration.getMarker()
     const {item, position} = decoration.getProperties()
     const element = TextEditor.viewForItem(item)
-    const row = marker.getHeadScreenPosition().row
-    this.lineTopIndex.insertBlock(decoration, row, 0, position === 'after')
 
-    this.blockDecorationsToMeasure.add(decoration)
-    this.blockDecorationsByElement.set(element, decoration)
-    this.blockDecorationResizeObserver.observe(element)
+    if (marker.isValid()) {
+      const row = marker.getHeadScreenPosition().row
+      this.lineTopIndex.insertBlock(decoration, row, 0, position === 'after')
+      this.blockDecorationsToMeasure.add(decoration)
+      this.blockDecorationsByElement.set(element, decoration)
+      this.blockDecorationResizeObserver.observe(element)
 
-    const didUpdateDisposable = marker.bufferMarker.onDidChange((e) => {
-      if (!e.textChanged) {
-        this.lineTopIndex.moveBlock(decoration, marker.getHeadScreenPosition().row)
-        this.scheduleUpdate()
-      }
-    })
-    const didDestroyDisposable = decoration.onDidDestroy(() => {
-      this.blockDecorationsToMeasure.delete(decoration)
-      this.heightsByBlockDecoration.delete(decoration)
-      this.blockDecorationsByElement.delete(element)
-      this.blockDecorationResizeObserver.unobserve(element)
-      this.lineTopIndex.removeBlock(decoration)
-      didUpdateDisposable.dispose()
-      didDestroyDisposable.dispose()
       this.scheduleUpdate()
-    })
+    }
+
+    if (subscribeToChanges) {
+      let wasValid = marker.isValid()
+
+      const didUpdateDisposable = marker.bufferMarker.onDidChange(({textChanged}) => {
+        const isValid = marker.isValid()
+        if (wasValid && !isValid) {
+          wasValid = false
+          this.blockDecorationsToMeasure.delete(decoration)
+          this.heightsByBlockDecoration.delete(decoration)
+          this.blockDecorationsByElement.delete(element)
+          this.blockDecorationResizeObserver.unobserve(element)
+          this.lineTopIndex.removeBlock(decoration)
+          this.scheduleUpdate()
+        } else if (!wasValid && isValid) {
+          wasValid = true
+          this.addBlockDecoration(decoration, false)
+        } else if (isValid && !textChanged) {
+          this.lineTopIndex.moveBlock(decoration, marker.getHeadScreenPosition().row)
+          this.scheduleUpdate()
+        }
+      })
+
+      const didDestroyDisposable = decoration.onDidDestroy(() => {
+        didUpdateDisposable.dispose()
+        didDestroyDisposable.dispose()
+
+        if (marker.isValid()) {
+          this.blockDecorationsToMeasure.delete(decoration)
+          this.heightsByBlockDecoration.delete(decoration)
+          this.blockDecorationsByElement.delete(element)
+          this.blockDecorationResizeObserver.unobserve(element)
+          this.lineTopIndex.removeBlock(decoration)
+          this.scheduleUpdate()
+        }
+      })
+    }
   }
 
   didResizeBlockDecorations (entries) {

src/text-editor.coffee

@@ -738,7 +738,7 @@ class TextEditor extends Model
   # Called by DecorationManager when a decoration is added.
   didAddDecoration: (decoration) ->
     if decoration.isType('block')
-      @component?.didAddBlockDecoration(decoration)
+      @component?.addBlockDecoration(decoration)
 
   # Extended: Calls your `callback` when the placeholder text is changed.
   #