diff --git a/spec/text-editor-component-spec.js b/spec/text-editor-component-spec.js index 3b2f2072a..2c13b48af 100644 --- a/spec/text-editor-component-spec.js +++ b/spec/text-editor-component-spec.js @@ -1524,6 +1524,27 @@ describe('TextEditorComponent', () => { await setScrollTop(component, component.getLineHeight() * 3) expect(element.querySelectorAll('.highlight.a').length).toBe(0) }) + + it('does not move existing highlights when adding or removing other highlight decorations (regression)', async () => { + const {component, element, editor} = buildComponent() + + const marker1 = editor.markScreenRange([[1, 6], [1, 10]]) + editor.decorateMarker(marker1, {type: 'highlight', class: 'a'}) + await component.getNextUpdatePromise() + const marker1Region = element.querySelector('.highlight.a') + expect(Array.from(marker1Region.parentElement.children).indexOf(marker1Region)).toBe(0) + + const marker2 = editor.markScreenRange([[1, 2], [1, 4]]) + editor.decorateMarker(marker2, {type: 'highlight', class: 'b'}) + await component.getNextUpdatePromise() + const marker2Region = element.querySelector('.highlight.b') + expect(Array.from(marker1Region.parentElement.children).indexOf(marker1Region)).toBe(0) + expect(Array.from(marker2Region.parentElement.children).indexOf(marker2Region)).toBe(1) + + marker2.destroy() + await component.getNextUpdatePromise() + expect(Array.from(marker1Region.parentElement.children).indexOf(marker1Region)).toBe(0) + }) }) describe('overlay decorations', () => { diff --git a/src/text-editor-component.js b/src/text-editor-component.js index 881aaad81..970ee29c1 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -3417,8 +3417,10 @@ class CursorsAndInputComponent { class LinesTileComponent { constructor (props) { + this.highlightComponentsByKey = new Map() this.props = props etch.initialize(this) + this.updateHighlights() this.createLines() this.updateBlockDecorations({}, props) } @@ -3432,13 +3434,22 @@ class LinesTileComponent { this.updateLines(oldProps, newProps) this.updateBlockDecorations(oldProps, newProps) } + this.updateHighlights() } } destroy () { + this.highlightComponentsByKey.forEach((highlightComponent) => { + highlightComponent.destroy() + }) + this.highlightComponentsByKey.clear() + for (let i = 0; i < this.lineComponents.length; i++) { this.lineComponents[i].destroy() } + this.lineComponents.length = 0 + + return etch.destroy(this) } render () { @@ -3456,34 +3467,12 @@ class LinesTileComponent { backgroundColor: 'inherit' } }, - this.renderHighlights() - // Lines and block decorations will be manually inserted here for efficiency - ) - } - - renderHighlights () { - const {top, lineHeight, highlightDecorations} = this.props - - let children = null - if (highlightDecorations) { - const decorationCount = highlightDecorations.length - children = new Array(decorationCount) - for (let i = 0; i < decorationCount; i++) { - const highlightProps = Object.assign( - {parentTileTop: top, lineHeight}, - highlightDecorations[i] - ) - children[i] = $(HighlightComponent, highlightProps) - highlightDecorations[i].flashRequested = false - } - } - - return $.div( - { + $.div({ + ref: 'highlights', className: 'highlights', - style: {contain: 'layout'} - }, - children + style: {layout: 'contain'} + }) + // Lines and block decorations will be manually inserted here for efficiency ) } @@ -3676,6 +3665,40 @@ class LinesTileComponent { } } + updateHighlights () { + const {top, lineHeight, highlightDecorations} = this.props + + const visibleHighlightDecorations = new Set() + if (highlightDecorations) { + for (let i = 0; i < highlightDecorations.length; i++) { + const highlightDecoration = highlightDecorations[i] + + const highlightProps = Object.assign( + {parentTileTop: top, lineHeight}, + highlightDecorations[i] + ) + let highlightComponent = this.highlightComponentsByKey.get(highlightDecoration.key) + if (highlightComponent) { + highlightComponent.update(highlightProps) + } else { + highlightComponent = new HighlightComponent(highlightProps) + this.refs.highlights.appendChild(highlightComponent.element) + this.highlightComponentsByKey.set(highlightDecoration.key, highlightComponent) + } + + highlightDecorations[i].flashRequested = false + visibleHighlightDecorations.add(highlightDecoration.key) + } + } + + this.highlightComponentsByKey.forEach((highlightComponent, key) => { + if (!visibleHighlightDecorations.has(key)) { + highlightComponent.destroy() + this.highlightComponentsByKey.delete(key) + } + }) + } + shouldUpdate (newProps) { const oldProps = this.props if (oldProps.top !== newProps.top) return true @@ -3876,6 +3899,17 @@ class HighlightComponent { if (this.props.flashRequested) this.performFlash() } + destroy () { + if (this.timeoutsByClassName) { + this.timeoutsByClassName.forEach((timeout) => { + window.clearTimeout(timeout) + }) + this.timeoutsByClassName.clear() + } + + return etch.destroy(this) + } + update (newProps) { this.props = newProps etch.updateSync(this)