From a44f7116a2bf8c7299d3e96ff42ec9b561a7ada5 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 14 Sep 2015 11:34:13 +0200 Subject: [PATCH 01/20] Start building nodes via document.createElement --- src/lines-tile-component.coffee | 152 +++++++++++++++++--------------- 1 file changed, 81 insertions(+), 71 deletions(-) diff --git a/src/lines-tile-component.coffee b/src/lines-tile-component.coffee index f823ea8ac..aee13e4b4 100644 --- a/src/lines-tile-component.coffee +++ b/src/lines-tile-component.coffee @@ -118,24 +118,30 @@ class LinesTileComponent {width} = @newState {screenRow, tokens, text, top, lineEnding, fold, isSoftWrapped, indentLevel, decorationClasses} = @newTileState.lines[id] - classes = '' + lineNode = document.createElement("div") + lineNode.classList.add("line") if decorationClasses? for decorationClass in decorationClasses - classes += decorationClass + ' ' - classes += 'line' + lineNode.classList.add(decorationClass) - lineHTML = "
" + lineNode.style.position = "absolute" + lineNode.style.top = top + "px" + lineNode.style.width = width + "px" + lineNode.dataset.screenRow = screenRow if text is "" - lineHTML += @buildEmptyLineInnerHTML(id) + @appendEmptyLineInnerNodes(id, lineNode) else - lineHTML += @buildLineInnerHTML(id) + @appendLineInnerNodes(id, lineNode) - lineHTML += '' if fold - lineHTML += "
" - lineHTML + if fold + foldMarker = document.createElement("span") + foldMarker.classList.add("fold-marker") + lineNode.appendChild(foldMarker) - buildEmptyLineInnerHTML: (id) -> + lineNode.outerHTML + + appendEmptyLineInnerNodes: (id, lineNode) -> {indentGuidesVisible} = @newState {indentLevel, tabLength, endOfLineInvisibles} = @newTileState.lines[id] @@ -143,35 +149,46 @@ class LinesTileComponent invisibleIndex = 0 lineHTML = '' for i in [0...indentLevel] - lineHTML += "" + indentGuide = document.createElement("span") + indentGuide.classList.add("indent-guide") for j in [0...tabLength] if invisible = endOfLineInvisibles?[invisibleIndex++] - lineHTML += "#{invisible}" + invisibleCharacter = document.createElement("span") + invisibleCharacter.classList.add("invisible-character") + invisibleCharacter.textContent = invisible + indentGuide.appendChild(invisibleCharacter) else - lineHTML += ' ' - lineHTML += "" + indentGuide.insertAdjacentText("beforeend", " ") + lineNode.appendChild(indentGuide) while invisibleIndex < endOfLineInvisibles?.length - lineHTML += "#{endOfLineInvisibles[invisibleIndex++]}" - - lineHTML + invisibleCharacter = document.createElement("span") + invisibleCharacter.classList.add("invisible-character") + invisibleCharacter.textContent = endOfLineInvisibles[invisibleIndex++] + lineNode.appendChild(invisibleCharacter) else - @buildEndOfLineHTML(id) or ' ' + unless @appendEndOfLineNodes(id, lineNode) + lineNode.insertAdjacentHTML("beforeend", " ") - buildLineInnerHTML: (id) -> + appendLineInnerNodes: (id, lineNode) -> lineState = @newTileState.lines[id] {firstNonWhitespaceIndex, firstTrailingWhitespaceIndex, invisibles} = lineState lineIsWhitespaceOnly = firstTrailingWhitespaceIndex is 0 innerHTML = "" @tokenIterator.reset(lineState) + openScopeNode = lineNode while @tokenIterator.next() for scope in @tokenIterator.getScopeEnds() - innerHTML += "" + openScopeNode = openScopeNode.parentElement for scope in @tokenIterator.getScopeStarts() - innerHTML += "" + newScopeNode = document.createElement("span") + newScopeNode.className = scope.replace(/\.+/g, ' ') + openScopeNode.appendChild(newScopeNode) + + openScopeNode = newScopeNode tokenStart = @tokenIterator.getScreenStart() tokenEnd = @tokenIterator.getScreenEnd() @@ -196,87 +213,80 @@ class LinesTileComponent (invisibles?.tab and isHardTab) or (invisibles?.space and (hasLeadingWhitespace or hasTrailingWhitespace)) - innerHTML += @buildTokenHTML(tokenText, isHardTab, tokenFirstNonWhitespaceIndex, tokenFirstTrailingWhitespaceIndex, hasIndentGuide, hasInvisibleCharacters) + @appendToken(openScopeNode, tokenText, isHardTab, tokenFirstNonWhitespaceIndex, tokenFirstTrailingWhitespaceIndex, hasIndentGuide, hasInvisibleCharacters) - for scope in @tokenIterator.getScopeEnds() - innerHTML += "" + @appendEndOfLineNodes(id, lineNode) - for scope in @tokenIterator.getScopes() - innerHTML += "" - - innerHTML += @buildEndOfLineHTML(id) - innerHTML - - buildTokenHTML: (tokenText, isHardTab, firstNonWhitespaceIndex, firstTrailingWhitespaceIndex, hasIndentGuide, hasInvisibleCharacters) -> + appendToken: (scopeNode, tokenText, isHardTab, firstNonWhitespaceIndex, firstTrailingWhitespaceIndex, hasIndentGuide, hasInvisibleCharacters) -> if isHardTab - classes = 'hard-tab' - classes += ' leading-whitespace' if firstNonWhitespaceIndex? - classes += ' trailing-whitespace' if firstTrailingWhitespaceIndex? - classes += ' indent-guide' if hasIndentGuide - classes += ' invisible-character' if hasInvisibleCharacters - return "#{@escapeTokenText(tokenText)}" + hardTab = document.createElement("span") + hardTab.classList.add("hard-tab") + hardTab.classList.add("leading-whitespace") if firstNonWhitespaceIndex? + hardTab.classList.add("trailing-whitespace") if firstTrailingWhitespaceIndex? + hardTab.classList.add("indent-guide") if hasIndentGuide + hardTab.classList.add("invisible-character") if hasInvisibleCharacters + hardTab.textContent = tokenText + + scopeNode.appendChild(hardTab) else startIndex = 0 endIndex = tokenText.length - leadingHtml = '' - trailingHtml = '' + leadingWhitespaceNode = null + trailingWhitespaceNode = null if firstNonWhitespaceIndex? - leadingWhitespace = tokenText.substring(0, firstNonWhitespaceIndex) + leadingWhitespaceNode = document.createElement("span") + leadingWhitespaceNode.classList.add("leading-whitespace") + leadingWhitespaceNode.classList.add("indent-guide") if hasIndentGuide + leadingWhitespaceNode.classList.add("invisible-character") if hasInvisibleCharacters + leadingWhitespaceNode.textContent = tokenText.substring(0, firstNonWhitespaceIndex) - classes = 'leading-whitespace' - classes += ' indent-guide' if hasIndentGuide - classes += ' invisible-character' if hasInvisibleCharacters - - leadingHtml = "#{leadingWhitespace}" startIndex = firstNonWhitespaceIndex if firstTrailingWhitespaceIndex? tokenIsOnlyWhitespace = firstTrailingWhitespaceIndex is 0 - trailingWhitespace = tokenText.substring(firstTrailingWhitespaceIndex) - classes = 'trailing-whitespace' - classes += ' indent-guide' if hasIndentGuide and not firstNonWhitespaceIndex? and tokenIsOnlyWhitespace - classes += ' invisible-character' if hasInvisibleCharacters - - trailingHtml = "#{trailingWhitespace}" + trailingWhitespaceNode = document.createElement("span") + trailingWhitespaceNode.classList.add("trailing-whitespace") + trailingWhitespaceNode.classList.add("indent-guide") if hasIndentGuide and not firstNonWhitespaceIndex? and tokenIsOnlyWhitespace + trailingWhitespaceNode.classList.add("invisible-character") if hasInvisibleCharacters + trailingWhitespaceNode.textContent = tokenText.substring(firstTrailingWhitespaceIndex) endIndex = firstTrailingWhitespaceIndex - html = leadingHtml + scopeNode.appendChild(leadingWhitespaceNode) if leadingWhitespaceNode? + if tokenText.length > MaxTokenLength while startIndex < endIndex - html += "" + @escapeTokenText(tokenText, startIndex, startIndex + MaxTokenLength) + "" + tokenNode = document.createElement("span") + tokenNode.textContent = @sliceText(tokenText, startIndex, startIndex + MaxTokenLength) + scopeNode.appendChild(tokenNode) startIndex += MaxTokenLength else - html += @escapeTokenText(tokenText, startIndex, endIndex) + scopeNode.insertAdjacentText("beforeend", @sliceText(tokenText, startIndex, endIndex)) - html += trailingHtml - html + scopeNode.appendChild(trailingWhitespaceNode) if trailingWhitespaceNode? - escapeTokenText: (tokenText, startIndex, endIndex) -> + sliceText: (tokenText, startIndex, endIndex) -> if startIndex? and endIndex? and startIndex > 0 or endIndex < tokenText.length tokenText = tokenText.slice(startIndex, endIndex) - tokenText.replace(TokenTextEscapeRegex, @escapeTokenTextReplace) + tokenText - escapeTokenTextReplace: (match) -> - switch match - when '&' then '&' - when '"' then '"' - when "'" then ''' - when '<' then '<' - when '>' then '>' - else match - - buildEndOfLineHTML: (id) -> + appendEndOfLineNodes: (id, lineNode) -> {endOfLineInvisibles} = @newTileState.lines[id] - html = '' + hasInvisibles = false if endOfLineInvisibles? for invisible in endOfLineInvisibles - html += "#{invisible}" - html + hasInvisibles = true + + invisibleCharacter = document.createElement("span") + invisibleCharacter.classList.add("invisible-character") + invisibleCharacter.textContent = invisible + lineNode.appendChild(invisibleCharacter) + + hasInvisibles updateLineNode: (id) -> oldLineState = @oldTileState.lines[id] From 9af7795a7e26b57a0d4624769ada522e868f34f2 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 14 Sep 2015 11:45:29 +0200 Subject: [PATCH 02/20] Avoid skipping null bytes Now that we build DOM nodes via `document.createElement`, there's no need to skip null byte characters (nor to avoid to pair them) because the browser will keep them in the document (unlike `innerHTML`). --- spec/text-editor-component-spec.coffee | 7 ++++--- src/lines-tile-component.coffee | 14 +++++--------- src/text-utils.coffee | 2 -- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/spec/text-editor-component-spec.coffee b/spec/text-editor-component-spec.coffee index 2e914048e..2f0f6115a 100644 --- a/spec/text-editor-component-spec.coffee +++ b/spec/text-editor-component-spec.coffee @@ -1006,10 +1006,11 @@ describe "TextEditorComponent", -> cursor = componentNode.querySelector('.cursor') cursorRect = cursor.getBoundingClientRect() - cursorLocationTextNode = component.lineNodeForScreenRow(0).querySelector('.source.js').firstChild + cursorLocationTextNode = component.lineNodeForScreenRow(0).querySelector('.source.js').childNodes[2] + range = document.createRange() - range.setStart(cursorLocationTextNode, 3) - range.setEnd(cursorLocationTextNode, 4) + range.setStart(cursorLocationTextNode, 0) + range.setEnd(cursorLocationTextNode, 1) rangeRect = range.getBoundingClientRect() expect(cursorRect.left).toBe rangeRect.left diff --git a/src/lines-tile-component.coffee b/src/lines-tile-component.coffee index aee13e4b4..b9a54c1a9 100644 --- a/src/lines-tile-component.coffee +++ b/src/lines-tile-component.coffee @@ -89,24 +89,22 @@ class LinesTileComponent @removeLineNode(id) newLineIds = null - newLinesHTML = null + newLineNodes = null for id, lineState of @newTileState.lines if @oldTileState.lines.hasOwnProperty(id) @updateLineNode(id) else newLineIds ?= [] - newLinesHTML ?= "" + newLineNodes ?= [] newLineIds.push(id) - newLinesHTML += @buildLineHTML(id) + newLineNodes.push(@buildLineNode(id)) @screenRowsByLineId[id] = lineState.screenRow @lineIdsByScreenRow[lineState.screenRow] = id @oldTileState.lines[id] = cloneObject(lineState) return unless newLineIds? - WrapperDiv.innerHTML = newLinesHTML - newLineNodes = _.toArray(WrapperDiv.children) for id, i in newLineIds lineNode = newLineNodes[i] @lineNodesByLineId[id] = lineNode @@ -114,7 +112,7 @@ class LinesTileComponent return - buildLineHTML: (id) -> + buildLineNode: (id) -> {width} = @newState {screenRow, tokens, text, top, lineEnding, fold, isSoftWrapped, indentLevel, decorationClasses} = @newTileState.lines[id] @@ -139,7 +137,7 @@ class LinesTileComponent foldMarker.classList.add("fold-marker") lineNode.appendChild(foldMarker) - lineNode.outerHTML + lineNode appendEmptyLineInnerNodes: (id, lineNode) -> {indentGuidesVisible} = @newState @@ -353,8 +351,6 @@ class LinesTileComponent charLength = 1 textIndex++ - continue if char is '\0' - unless charWidths[char]? unless textNode? rangeForMeasurement ?= document.createRange() diff --git a/src/text-utils.coffee b/src/text-utils.coffee index ec3ca0c29..37955dcd6 100644 --- a/src/text-utils.coffee +++ b/src/text-utils.coffee @@ -30,7 +30,6 @@ isSurrogatePair = (charCodeA, charCodeB) -> # # Return a {Boolean}. isVariationSequence = (charCodeA, charCodeB) -> - return false if charCodeA is 0 not isVariationSelector(charCodeA) and isVariationSelector(charCodeB) # Are the given character codes a combined character pair? @@ -40,7 +39,6 @@ isVariationSequence = (charCodeA, charCodeB) -> # # Return a {Boolean}. isCombinedCharacter = (charCodeA, charCodeB) -> - return false if charCodeA is 0 not isCombiningCharacter(charCodeA) and isCombiningCharacter(charCodeB) # Is the character at the given index the start of high/low surrogate pair From ccb8623a88b4289a957644a4d498d766c9223b84 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 14 Sep 2015 12:00:55 +0200 Subject: [PATCH 03/20] :art: Extract a buildElement helper function --- src/lines-tile-component.coffee | 70 ++++++++++++++++----------------- 1 file changed, 33 insertions(+), 37 deletions(-) diff --git a/src/lines-tile-component.coffee b/src/lines-tile-component.coffee index b9a54c1a9..131db79f0 100644 --- a/src/lines-tile-component.coffee +++ b/src/lines-tile-component.coffee @@ -116,8 +116,7 @@ class LinesTileComponent {width} = @newState {screenRow, tokens, text, top, lineEnding, fold, isSoftWrapped, indentLevel, decorationClasses} = @newTileState.lines[id] - lineNode = document.createElement("div") - lineNode.classList.add("line") + lineNode = @buildElement("div", "line") if decorationClasses? for decorationClass in decorationClasses lineNode.classList.add(decorationClass) @@ -132,13 +131,15 @@ class LinesTileComponent else @appendLineInnerNodes(id, lineNode) - if fold - foldMarker = document.createElement("span") - foldMarker.classList.add("fold-marker") - lineNode.appendChild(foldMarker) - + lineNode.appendChild(@buildElement("span", "fold-marker")) if fold lineNode + buildElement: (name, className, textContent) -> + element = document.createElement(name) + element.className = className if className? + element.textContent = textContent if textContent? + element + appendEmptyLineInnerNodes: (id, lineNode) -> {indentGuidesVisible} = @newState {indentLevel, tabLength, endOfLineInvisibles} = @newTileState.lines[id] @@ -147,23 +148,21 @@ class LinesTileComponent invisibleIndex = 0 lineHTML = '' for i in [0...indentLevel] - indentGuide = document.createElement("span") - indentGuide.classList.add("indent-guide") + indentGuide = @buildElement("span", "indent-guide") for j in [0...tabLength] if invisible = endOfLineInvisibles?[invisibleIndex++] - invisibleCharacter = document.createElement("span") - invisibleCharacter.classList.add("invisible-character") - invisibleCharacter.textContent = invisible - indentGuide.appendChild(invisibleCharacter) + indentGuide.appendChild( + @buildElement("span", "invisible-character", invisible) + ) else indentGuide.insertAdjacentText("beforeend", " ") lineNode.appendChild(indentGuide) while invisibleIndex < endOfLineInvisibles?.length - invisibleCharacter = document.createElement("span") - invisibleCharacter.classList.add("invisible-character") - invisibleCharacter.textContent = endOfLineInvisibles[invisibleIndex++] - lineNode.appendChild(invisibleCharacter) + invisible = endOfLineInvisibles[invisibleIndex++] + lineNode.appendChild( + @buildElement("span", "invisible-character", invisible) + ) else unless @appendEndOfLineNodes(id, lineNode) lineNode.insertAdjacentHTML("beforeend", " ") @@ -182,10 +181,8 @@ class LinesTileComponent openScopeNode = openScopeNode.parentElement for scope in @tokenIterator.getScopeStarts() - newScopeNode = document.createElement("span") - newScopeNode.className = scope.replace(/\.+/g, ' ') + newScopeNode = @buildElement("span", scope.replace(/\.+/g, ' ')) openScopeNode.appendChild(newScopeNode) - openScopeNode = newScopeNode tokenStart = @tokenIterator.getScreenStart() @@ -217,13 +214,11 @@ class LinesTileComponent appendToken: (scopeNode, tokenText, isHardTab, firstNonWhitespaceIndex, firstTrailingWhitespaceIndex, hasIndentGuide, hasInvisibleCharacters) -> if isHardTab - hardTab = document.createElement("span") - hardTab.classList.add("hard-tab") + hardTab = @buildElement("span", "hard-tab", tokenText) hardTab.classList.add("leading-whitespace") if firstNonWhitespaceIndex? hardTab.classList.add("trailing-whitespace") if firstTrailingWhitespaceIndex? hardTab.classList.add("indent-guide") if hasIndentGuide hardTab.classList.add("invisible-character") if hasInvisibleCharacters - hardTab.textContent = tokenText scopeNode.appendChild(hardTab) else @@ -234,22 +229,26 @@ class LinesTileComponent trailingWhitespaceNode = null if firstNonWhitespaceIndex? - leadingWhitespaceNode = document.createElement("span") - leadingWhitespaceNode.classList.add("leading-whitespace") + leadingWhitespaceNode = @buildElement( + "span", + "leading-whitespace", + tokenText.substring(0, firstNonWhitespaceIndex) + ) leadingWhitespaceNode.classList.add("indent-guide") if hasIndentGuide leadingWhitespaceNode.classList.add("invisible-character") if hasInvisibleCharacters - leadingWhitespaceNode.textContent = tokenText.substring(0, firstNonWhitespaceIndex) startIndex = firstNonWhitespaceIndex if firstTrailingWhitespaceIndex? tokenIsOnlyWhitespace = firstTrailingWhitespaceIndex is 0 - trailingWhitespaceNode = document.createElement("span") - trailingWhitespaceNode.classList.add("trailing-whitespace") + trailingWhitespaceNode = @buildElement( + "span", + "trailing-whitespace", + tokenText.substring(firstTrailingWhitespaceIndex) + ) trailingWhitespaceNode.classList.add("indent-guide") if hasIndentGuide and not firstNonWhitespaceIndex? and tokenIsOnlyWhitespace trailingWhitespaceNode.classList.add("invisible-character") if hasInvisibleCharacters - trailingWhitespaceNode.textContent = tokenText.substring(firstTrailingWhitespaceIndex) endIndex = firstTrailingWhitespaceIndex @@ -257,9 +256,8 @@ class LinesTileComponent if tokenText.length > MaxTokenLength while startIndex < endIndex - tokenNode = document.createElement("span") - tokenNode.textContent = @sliceText(tokenText, startIndex, startIndex + MaxTokenLength) - scopeNode.appendChild(tokenNode) + text = @sliceText(tokenText, startIndex, startIndex + MaxTokenLength) + scopeNode.appendChild(@buildElement("span", null, text)) startIndex += MaxTokenLength else scopeNode.insertAdjacentText("beforeend", @sliceText(tokenText, startIndex, endIndex)) @@ -278,11 +276,9 @@ class LinesTileComponent if endOfLineInvisibles? for invisible in endOfLineInvisibles hasInvisibles = true - - invisibleCharacter = document.createElement("span") - invisibleCharacter.classList.add("invisible-character") - invisibleCharacter.textContent = invisible - lineNode.appendChild(invisibleCharacter) + lineNode.appendChild( + @buildElement("span", "invisible-character", invisible) + ) hasInvisibles From ece15b2a24acbbd9e68da239cdf54349793c266e Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 14 Sep 2015 14:48:30 +0200 Subject: [PATCH 04/20] Recycle tile nodes (and descendants) --- spec/dom-elements-pool-spec.coffee | 41 ++++++++++++++++++++++++++ src/dom-elements-pool.coffee | 25 ++++++++++++++++ src/line-numbers-tile-component.coffee | 3 ++ src/lines-component.coffee | 5 +++- src/lines-tile-component.coffee | 37 +++++++++++------------ src/tiled-component.coffee | 4 +-- 6 files changed, 91 insertions(+), 24 deletions(-) create mode 100644 spec/dom-elements-pool-spec.coffee create mode 100644 src/dom-elements-pool.coffee diff --git a/spec/dom-elements-pool-spec.coffee b/spec/dom-elements-pool-spec.coffee new file mode 100644 index 000000000..ee9c99062 --- /dev/null +++ b/spec/dom-elements-pool-spec.coffee @@ -0,0 +1,41 @@ +DomElementsPool = require '../src/dom-elements-pool' + +describe "DomElementsPool", -> + domElementsPool = null + + beforeEach -> + domElementsPool = new DomElementsPool + + it "creates new nodes until some of them are freed", -> + span1 = domElementsPool.build("span") + span2 = domElementsPool.build("span") + span3 = domElementsPool.build("span") + + expect(span1).not.toBe(span2) + expect(span2).not.toBe(span3) + + domElementsPool.free(span1) + domElementsPool.free(span2) + + expect(domElementsPool.build("span")).toBe(span2) + expect(domElementsPool.build("span")).toBe(span1) + + it "recursively frees a dom tree", -> + div = domElementsPool.build("div") + span1 = domElementsPool.build("span") + span2 = domElementsPool.build("span") + span3 = domElementsPool.build("span") + span4 = domElementsPool.build("span") + + div.appendChild(span1) + span1.appendChild(span2) + div.appendChild(span3) + span3.appendChild(span4) + + domElementsPool.freeElementAndDescendants(div) + + expect(domElementsPool.build("div")).toBe(div) + expect(domElementsPool.build("span")).toBe(span3) + expect(domElementsPool.build("span")).toBe(span4) + expect(domElementsPool.build("span")).toBe(span1) + expect(domElementsPool.build("span")).toBe(span2) diff --git a/src/dom-elements-pool.coffee b/src/dom-elements-pool.coffee new file mode 100644 index 000000000..482eb8417 --- /dev/null +++ b/src/dom-elements-pool.coffee @@ -0,0 +1,25 @@ +{toArray} = require 'underscore-plus' + +module.exports = +class DomElementsPool + constructor: -> + @freeElementsByTagName = {} + + build: (tagName, className, textContent) -> + element = @freeElementsByTagName[tagName]?.pop() + element ?= document.createElement(tagName) + element.className = className + element.textContent = textContent + element.removeAttribute("style") + element + + free: (element) -> + element.remove() + @freeElementsByTagName[element.tagName.toLowerCase()] ?= [] + @freeElementsByTagName[element.tagName.toLowerCase()].push(element) + + freeElementAndDescendants: (element) -> + for child in toArray(element.children) + @freeElementAndDescendants(child) + + @free(element) diff --git a/src/line-numbers-tile-component.coffee b/src/line-numbers-tile-component.coffee index 2ebccbc65..703054024 100644 --- a/src/line-numbers-tile-component.coffee +++ b/src/line-numbers-tile-component.coffee @@ -14,6 +14,9 @@ class LineNumbersTileComponent @domNode.style.display = "block" @domNode.style.top = 0 # Cover the space occupied by a dummy lineNumber + destroy: -> + @domNode.remove() + getDomNode: -> @domNode diff --git a/src/lines-component.coffee b/src/lines-component.coffee index 2b80235dc..d3d1f9b0a 100644 --- a/src/lines-component.coffee +++ b/src/lines-component.coffee @@ -3,6 +3,7 @@ CursorsComponent = require './cursors-component' LinesTileComponent = require './lines-tile-component' TiledComponent = require './tiled-component' +DomElementsPool = require './dom-elements-pool' DummyLineNode = $$(-> @div className: 'line', style: 'position: absolute; visibility: hidden;', => @span 'x')[0] @@ -23,6 +24,8 @@ class LinesComponent extends TiledComponent @cursorsComponent = new CursorsComponent @domNode.appendChild(@cursorsComponent.getDomNode()) + @elementsPool = new DomElementsPool + if @useShadowDOM insertionPoint = document.createElement('content') insertionPoint.setAttribute('select', '.overlayer') @@ -60,7 +63,7 @@ class LinesComponent extends TiledComponent @oldState.indentGuidesVisible = @newState.indentGuidesVisible - buildComponentForTile: (id) -> new LinesTileComponent({id, @presenter}) + buildComponentForTile: (id) -> new LinesTileComponent({id, @presenter, @elementsPool}) buildEmptyState: -> {tiles: {}} diff --git a/src/lines-tile-component.coffee b/src/lines-tile-component.coffee index 131db79f0..da653d279 100644 --- a/src/lines-tile-component.coffee +++ b/src/lines-tile-component.coffee @@ -14,13 +14,13 @@ cloneObject = (object) -> module.exports = class LinesTileComponent - constructor: ({@presenter, @id}) -> + constructor: ({@presenter, @id, @elementsPool}) -> @tokenIterator = new TokenIterator @measuredLines = new Set @lineNodesByLineId = {} @screenRowsByLineId = {} @lineIdsByScreenRow = {} - @domNode = document.createElement("div") + @domNode = @elementsPool.build("div", "tile") @domNode.classList.add("tile") @domNode.style.position = "absolute" @domNode.style.display = "block" @@ -28,6 +28,9 @@ class LinesTileComponent @highlightsComponent = new HighlightsComponent @domNode.appendChild(@highlightsComponent.getDomNode()) + destroy: -> + @elementsPool.freeElementAndDescendants(@domNode) + getDomNode: -> @domNode @@ -77,7 +80,7 @@ class LinesTileComponent return removeLineNode: (id) -> - @lineNodesByLineId[id].remove() + @elementsPool.freeElementAndDescendants(@lineNodesByLineId[id]) delete @lineNodesByLineId[id] delete @lineIdsByScreenRow[@screenRowsByLineId[id]] delete @screenRowsByLineId[id] @@ -116,7 +119,7 @@ class LinesTileComponent {width} = @newState {screenRow, tokens, text, top, lineEnding, fold, isSoftWrapped, indentLevel, decorationClasses} = @newTileState.lines[id] - lineNode = @buildElement("div", "line") + lineNode = @elementsPool.build("div", "line") if decorationClasses? for decorationClass in decorationClasses lineNode.classList.add(decorationClass) @@ -131,15 +134,9 @@ class LinesTileComponent else @appendLineInnerNodes(id, lineNode) - lineNode.appendChild(@buildElement("span", "fold-marker")) if fold + lineNode.appendChild(@elementsPool.build("span", "fold-marker")) if fold lineNode - buildElement: (name, className, textContent) -> - element = document.createElement(name) - element.className = className if className? - element.textContent = textContent if textContent? - element - appendEmptyLineInnerNodes: (id, lineNode) -> {indentGuidesVisible} = @newState {indentLevel, tabLength, endOfLineInvisibles} = @newTileState.lines[id] @@ -148,11 +145,11 @@ class LinesTileComponent invisibleIndex = 0 lineHTML = '' for i in [0...indentLevel] - indentGuide = @buildElement("span", "indent-guide") + indentGuide = @elementsPool.build("span", "indent-guide") for j in [0...tabLength] if invisible = endOfLineInvisibles?[invisibleIndex++] indentGuide.appendChild( - @buildElement("span", "invisible-character", invisible) + @elementsPool.build("span", "invisible-character", invisible) ) else indentGuide.insertAdjacentText("beforeend", " ") @@ -161,7 +158,7 @@ class LinesTileComponent while invisibleIndex < endOfLineInvisibles?.length invisible = endOfLineInvisibles[invisibleIndex++] lineNode.appendChild( - @buildElement("span", "invisible-character", invisible) + @elementsPool.build("span", "invisible-character", invisible) ) else unless @appendEndOfLineNodes(id, lineNode) @@ -181,7 +178,7 @@ class LinesTileComponent openScopeNode = openScopeNode.parentElement for scope in @tokenIterator.getScopeStarts() - newScopeNode = @buildElement("span", scope.replace(/\.+/g, ' ')) + newScopeNode = @elementsPool.build("span", scope.replace(/\.+/g, ' ')) openScopeNode.appendChild(newScopeNode) openScopeNode = newScopeNode @@ -214,7 +211,7 @@ class LinesTileComponent appendToken: (scopeNode, tokenText, isHardTab, firstNonWhitespaceIndex, firstTrailingWhitespaceIndex, hasIndentGuide, hasInvisibleCharacters) -> if isHardTab - hardTab = @buildElement("span", "hard-tab", tokenText) + hardTab = @elementsPool.build("span", "hard-tab", tokenText) hardTab.classList.add("leading-whitespace") if firstNonWhitespaceIndex? hardTab.classList.add("trailing-whitespace") if firstTrailingWhitespaceIndex? hardTab.classList.add("indent-guide") if hasIndentGuide @@ -229,7 +226,7 @@ class LinesTileComponent trailingWhitespaceNode = null if firstNonWhitespaceIndex? - leadingWhitespaceNode = @buildElement( + leadingWhitespaceNode = @elementsPool.build( "span", "leading-whitespace", tokenText.substring(0, firstNonWhitespaceIndex) @@ -242,7 +239,7 @@ class LinesTileComponent if firstTrailingWhitespaceIndex? tokenIsOnlyWhitespace = firstTrailingWhitespaceIndex is 0 - trailingWhitespaceNode = @buildElement( + trailingWhitespaceNode = @elementsPool.build( "span", "trailing-whitespace", tokenText.substring(firstTrailingWhitespaceIndex) @@ -257,7 +254,7 @@ class LinesTileComponent if tokenText.length > MaxTokenLength while startIndex < endIndex text = @sliceText(tokenText, startIndex, startIndex + MaxTokenLength) - scopeNode.appendChild(@buildElement("span", null, text)) + scopeNode.appendChild(@elementsPool.build("span", null, text)) startIndex += MaxTokenLength else scopeNode.insertAdjacentText("beforeend", @sliceText(tokenText, startIndex, endIndex)) @@ -277,7 +274,7 @@ class LinesTileComponent for invisible in endOfLineInvisibles hasInvisibles = true lineNode.appendChild( - @buildElement("span", "invisible-character", invisible) + @elementsPool.build("span", "invisible-character", invisible) ) hasInvisibles diff --git a/src/tiled-component.coffee b/src/tiled-component.coffee index 33719dda5..5fecb86f4 100644 --- a/src/tiled-component.coffee +++ b/src/tiled-component.coffee @@ -21,9 +21,7 @@ class TiledComponent return removeTileNode: (tileRow) -> - node = @componentsByTileId[tileRow].getDomNode() - - node.remove() + @componentsByTileId[tileRow].destroy() delete @componentsByTileId[tileRow] delete @oldState.tiles[tileRow] From 93ed0853a203a85e9521b43a7d2bc08252ae005f Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 14 Sep 2015 17:25:30 +0200 Subject: [PATCH 05/20] Recycle gutter nodes --- src/line-number-gutter-component.coffee | 18 ++++---- src/line-numbers-tile-component.coffee | 55 ++++++++++++++----------- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/src/line-number-gutter-component.coffee b/src/line-number-gutter-component.coffee index 8eef6bd27..390aba41b 100644 --- a/src/line-number-gutter-component.coffee +++ b/src/line-number-gutter-component.coffee @@ -1,7 +1,7 @@ TiledComponent = require './tiled-component' LineNumbersTileComponent = require './line-numbers-tile-component' WrapperDiv = document.createElement('div') -DummyLineNumberComponent = LineNumbersTileComponent.createDummy() +DomElementsPool = require './dom-elements-pool' module.exports = class LineNumberGutterComponent extends TiledComponent @@ -10,6 +10,10 @@ class LineNumberGutterComponent extends TiledComponent constructor: ({@onMouseDown, @editor, @gutter}) -> @visible = true + @elementsPool = new DomElementsPool + + @dummyLineNumberComponent = LineNumbersTileComponent.createDummy(@elementsPool) + @domNode = atom.views.getView(@gutter) @lineNumbersNode = @domNode.firstChild @lineNumbersNode.innerHTML = '' @@ -60,7 +64,7 @@ class LineNumberGutterComponent extends TiledComponent @oldState.styles = {} @oldState.maxLineNumberDigits = @newState.maxLineNumberDigits - buildComponentForTile: (id) -> new LineNumbersTileComponent({id}) + buildComponentForTile: (id) -> new LineNumbersTileComponent({id, @elementsPool}) ### Section: Private Methods @@ -69,14 +73,14 @@ class LineNumberGutterComponent extends TiledComponent # This dummy line number element holds the gutter to the appropriate width, # since the real line numbers are absolutely positioned for performance reasons. appendDummyLineNumber: -> - DummyLineNumberComponent.newState = @newState - WrapperDiv.innerHTML = DummyLineNumberComponent.buildLineNumberHTML({bufferRow: -1}) - @dummyLineNumberNode = WrapperDiv.children[0] + @dummyLineNumberComponent.newState = @newState + @dummyLineNumberNode = @dummyLineNumberComponent.buildLineNumberNode({bufferRow: -1}) @lineNumbersNode.appendChild(@dummyLineNumberNode) updateDummyLineNumber: -> - DummyLineNumberComponent.newState = @newState - @dummyLineNumberNode.innerHTML = DummyLineNumberComponent.buildLineNumberInnerHTML(0, false) + @dummyLineNumberComponent.newState = @newState + @dummyLineNumberNode.innerHTML = "" + @dummyLineNumberComponent.appendLineNumberInnerNodes(0, false, @dummyLineNumberNode) onMouseDown: (event) => {target} = event diff --git a/src/line-numbers-tile-component.coffee b/src/line-numbers-tile-component.coffee index 703054024..ed08e3727 100644 --- a/src/line-numbers-tile-component.coffee +++ b/src/line-numbers-tile-component.coffee @@ -3,19 +3,18 @@ WrapperDiv = document.createElement('div') module.exports = class LineNumbersTileComponent - @createDummy: -> - new LineNumbersTileComponent({id: -1}) + @createDummy: (elementsPool) -> + new LineNumbersTileComponent({id: -1, elementsPool}) - constructor: ({@id}) -> + constructor: ({@id, @elementsPool}) -> @lineNumberNodesById = {} - @domNode = document.createElement("div") - @domNode.classList.add("tile") + @domNode = @elementsPool.build("div", "tile") @domNode.style.position = "absolute" @domNode.style.display = "block" @domNode.style.top = 0 # Cover the space occupied by a dummy lineNumber destroy: -> - @domNode.remove() + @elementsPool.freeElementAndDescendants(@domNode) getDomNode: -> @domNode @@ -50,7 +49,9 @@ class LineNumbersTileComponent @oldTileState.zIndex = @newTileState.zIndex if @newState.maxLineNumberDigits isnt @oldState.maxLineNumberDigits - node.remove() for id, node of @lineNumberNodesById + for id, node of @lineNumberNodesById + @elementsPool.freeElementAndDescendants(node) + @oldState.tiles[@id] = {lineNumbers: {}} @oldTileState = @oldState.tiles[@id] @lineNumberNodesById = {} @@ -60,11 +61,11 @@ class LineNumbersTileComponent updateLineNumbers: -> newLineNumberIds = null - newLineNumbersHTML = null + newLineNumberNodes = null for id, lineNumberState of @oldTileState.lineNumbers unless @newTileState.lineNumbers.hasOwnProperty(id) - @lineNumberNodesById[id].remove() + @elementsPool.freeElementAndDescendants(@lineNumberNodesById[id]) delete @lineNumberNodesById[id] delete @oldTileState.lineNumbers[id] @@ -73,15 +74,12 @@ class LineNumbersTileComponent @updateLineNumberNode(id, lineNumberState) else newLineNumberIds ?= [] - newLineNumbersHTML ?= "" + newLineNumberNodes ?= [] newLineNumberIds.push(id) - newLineNumbersHTML += @buildLineNumberHTML(lineNumberState) + newLineNumberNodes.push(@buildLineNumberNode(lineNumberState)) @oldTileState.lineNumbers[id] = _.clone(lineNumberState) if newLineNumberIds? - WrapperDiv.innerHTML = newLineNumbersHTML - newLineNumberNodes = _.toArray(WrapperDiv.children) - node = @domNode for id, i in newLineNumberIds lineNumberNode = newLineNumberNodes[i] @@ -90,18 +88,25 @@ class LineNumbersTileComponent return - buildLineNumberHTML: (lineNumberState) -> + buildLineNumberNode: (lineNumberState) -> {screenRow, bufferRow, softWrapped, top, decorationClasses, zIndex} = lineNumberState - if screenRow? - style = "position: absolute; top: #{top}px; z-index: #{zIndex};" - else - style = "visibility: hidden;" + className = @buildLineNumberClassName(lineNumberState) - innerHTML = @buildLineNumberInnerHTML(bufferRow, softWrapped) + lineNumberNode = @elementsPool.build("div", className) + lineNumberNode.dataset.screenRow = screenRow + lineNumberNode.dataset.bufferRow = bufferRow - "
#{innerHTML}
" + if screenRow? + lineNumberNode.style.position = "absolute" + lineNumberNode.style.top = top + "px" + lineNumberNode.style.zIndex = zIndex + else + lineNumberNode.style.visibility = "hidden" - buildLineNumberInnerHTML: (bufferRow, softWrapped) -> + @appendLineNumberInnerNodes(bufferRow, softWrapped, lineNumberNode) + lineNumberNode + + appendLineNumberInnerNodes: (bufferRow, softWrapped, lineNumberNode) -> {maxLineNumberDigits} = @newState if softWrapped @@ -110,8 +115,10 @@ class LineNumbersTileComponent lineNumber = (bufferRow + 1).toString() padding = _.multiplyString(' ', maxLineNumberDigits - lineNumber.length) - iconHTML = '
' - padding + lineNumber + iconHTML + iconRight = @elementsPool.build("div", "icon-right") + + lineNumberNode.innerHTML = padding + lineNumber + lineNumberNode.appendChild(iconRight) updateLineNumberNode: (lineNumberId, newLineNumberState) -> oldLineNumberState = @oldTileState.lineNumbers[lineNumberId] From 6b2e7a67654bd77dd3f380a3778474a52bb4aa2a Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 14 Sep 2015 17:36:45 +0200 Subject: [PATCH 06/20] :art: --- src/dom-elements-pool.coffee | 4 ++-- src/line-numbers-tile-component.coffee | 4 ++-- src/lines-tile-component.coffee | 4 +--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/dom-elements-pool.coffee b/src/dom-elements-pool.coffee index 482eb8417..04a037ef4 100644 --- a/src/dom-elements-pool.coffee +++ b/src/dom-elements-pool.coffee @@ -19,7 +19,7 @@ class DomElementsPool @freeElementsByTagName[element.tagName.toLowerCase()].push(element) freeElementAndDescendants: (element) -> + @free(element) + for child in toArray(element.children) @freeElementAndDescendants(child) - - @free(element) diff --git a/src/line-numbers-tile-component.coffee b/src/line-numbers-tile-component.coffee index ed08e3727..1d8b37f19 100644 --- a/src/line-numbers-tile-component.coffee +++ b/src/line-numbers-tile-component.coffee @@ -114,10 +114,10 @@ class LineNumbersTileComponent else lineNumber = (bufferRow + 1).toString() - padding = _.multiplyString(' ', maxLineNumberDigits - lineNumber.length) + padding = _.multiplyString("\u00a0", maxLineNumberDigits - lineNumber.length) iconRight = @elementsPool.build("div", "icon-right") - lineNumberNode.innerHTML = padding + lineNumber + lineNumberNode.innerText = padding + lineNumber lineNumberNode.appendChild(iconRight) updateLineNumberNode: (lineNumberId, newLineNumberState) -> diff --git a/src/lines-tile-component.coffee b/src/lines-tile-component.coffee index da653d279..3492f9dac 100644 --- a/src/lines-tile-component.coffee +++ b/src/lines-tile-component.coffee @@ -143,7 +143,6 @@ class LinesTileComponent if indentGuidesVisible and indentLevel > 0 invisibleIndex = 0 - lineHTML = '' for i in [0...indentLevel] indentGuide = @elementsPool.build("span", "indent-guide") for j in [0...tabLength] @@ -162,14 +161,13 @@ class LinesTileComponent ) else unless @appendEndOfLineNodes(id, lineNode) - lineNode.insertAdjacentHTML("beforeend", " ") + lineNode.insertAdjacentText("beforeend", "\u00a0") appendLineInnerNodes: (id, lineNode) -> lineState = @newTileState.lines[id] {firstNonWhitespaceIndex, firstTrailingWhitespaceIndex, invisibles} = lineState lineIsWhitespaceOnly = firstTrailingWhitespaceIndex is 0 - innerHTML = "" @tokenIterator.reset(lineState) openScopeNode = lineNode From 536a5f4395f12b464d4c2c0b20ff4ca13bce59a1 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 14 Sep 2015 17:44:57 +0200 Subject: [PATCH 07/20] :racehorse: Save an allocation while removing nodes --- spec/dom-elements-pool-spec.coffee | 6 +++--- src/dom-elements-pool.coffee | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/spec/dom-elements-pool-spec.coffee b/spec/dom-elements-pool-spec.coffee index ee9c99062..2b7364a8c 100644 --- a/spec/dom-elements-pool-spec.coffee +++ b/spec/dom-elements-pool-spec.coffee @@ -35,7 +35,7 @@ describe "DomElementsPool", -> domElementsPool.freeElementAndDescendants(div) expect(domElementsPool.build("div")).toBe(div) - expect(domElementsPool.build("span")).toBe(span3) - expect(domElementsPool.build("span")).toBe(span4) - expect(domElementsPool.build("span")).toBe(span1) expect(domElementsPool.build("span")).toBe(span2) + expect(domElementsPool.build("span")).toBe(span1) + expect(domElementsPool.build("span")).toBe(span4) + expect(domElementsPool.build("span")).toBe(span3) diff --git a/src/dom-elements-pool.coffee b/src/dom-elements-pool.coffee index 04a037ef4..d91317539 100644 --- a/src/dom-elements-pool.coffee +++ b/src/dom-elements-pool.coffee @@ -21,5 +21,6 @@ class DomElementsPool freeElementAndDescendants: (element) -> @free(element) - for child in toArray(element.children) + for index in [element.children.length - 1..0] by -1 + child = element.children[index] @freeElementAndDescendants(child) From f52e000bec559e365b7188f0a860d1cab3e36bd5 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 14 Sep 2015 18:14:12 +0200 Subject: [PATCH 08/20] :fire: Remove unused code --- src/dom-elements-pool.coffee | 2 -- src/line-numbers-tile-component.coffee | 1 - src/lines-tile-component.coffee | 1 - 3 files changed, 4 deletions(-) diff --git a/src/dom-elements-pool.coffee b/src/dom-elements-pool.coffee index d91317539..d0c653b02 100644 --- a/src/dom-elements-pool.coffee +++ b/src/dom-elements-pool.coffee @@ -1,5 +1,3 @@ -{toArray} = require 'underscore-plus' - module.exports = class DomElementsPool constructor: -> diff --git a/src/line-numbers-tile-component.coffee b/src/line-numbers-tile-component.coffee index 1d8b37f19..0b4ca43b8 100644 --- a/src/line-numbers-tile-component.coffee +++ b/src/line-numbers-tile-component.coffee @@ -1,5 +1,4 @@ _ = require 'underscore-plus' -WrapperDiv = document.createElement('div') module.exports = class LineNumbersTileComponent diff --git a/src/lines-tile-component.coffee b/src/lines-tile-component.coffee index 3492f9dac..5baf661b8 100644 --- a/src/lines-tile-component.coffee +++ b/src/lines-tile-component.coffee @@ -3,7 +3,6 @@ _ = require 'underscore-plus' HighlightsComponent = require './highlights-component' TokenIterator = require './token-iterator' AcceptFilter = {acceptNode: -> NodeFilter.FILTER_ACCEPT} -WrapperDiv = document.createElement('div') TokenTextEscapeRegex = /[&"'<>]/g MaxTokenLength = 20000 From 4349b152d5de4de3f863cb823b0e06c29a7e6858 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 14 Sep 2015 18:18:00 +0200 Subject: [PATCH 09/20] :art: --- spec/text-utils-spec.coffee | 6 ------ src/lines-tile-component.coffee | 16 ++++++++-------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/spec/text-utils-spec.coffee b/spec/text-utils-spec.coffee index f5c2c87f9..6a03bb02f 100644 --- a/spec/text-utils-spec.coffee +++ b/spec/text-utils-spec.coffee @@ -17,9 +17,6 @@ describe 'text utilities', -> expect(textUtils.hasPairedCharacter('\uFE0E\uFE0E')).toBe false expect(textUtils.hasPairedCharacter('\u0301\u0301')).toBe false - expect(textUtils.hasPairedCharacter('\0\u0301')).toBe false - expect(textUtils.hasPairedCharacter('\0\uFE0E')).toBe false - describe '.isPairedCharacter(string, index)', -> it 'returns true when the index is the start of a high/low surrogate pair, variation sequence, or combined character', -> expect(textUtils.isPairedCharacter('a\uD835\uDF97b\uD835\uDF97c', 0)).toBe false @@ -47,6 +44,3 @@ describe 'text utilities', -> expect(textUtils.isPairedCharacter('ae\u0301c', 2)).toBe false expect(textUtils.isPairedCharacter('ae\u0301c', 3)).toBe false expect(textUtils.isPairedCharacter('ae\u0301c', 4)).toBe false - - expect(textUtils.isPairedCharacter('\0\u0301c', 0)).toBe false - expect(textUtils.isPairedCharacter('\0\uFE0E', 0)).toBe false diff --git a/src/lines-tile-component.coffee b/src/lines-tile-component.coffee index 5baf661b8..2b4762c76 100644 --- a/src/lines-tile-component.coffee +++ b/src/lines-tile-component.coffee @@ -202,19 +202,19 @@ class LinesTileComponent (invisibles?.tab and isHardTab) or (invisibles?.space and (hasLeadingWhitespace or hasTrailingWhitespace)) - @appendToken(openScopeNode, tokenText, isHardTab, tokenFirstNonWhitespaceIndex, tokenFirstTrailingWhitespaceIndex, hasIndentGuide, hasInvisibleCharacters) + @appendTokenNodes(tokenText, isHardTab, tokenFirstNonWhitespaceIndex, tokenFirstTrailingWhitespaceIndex, hasIndentGuide, hasInvisibleCharacters, openScopeNode) @appendEndOfLineNodes(id, lineNode) - appendToken: (scopeNode, tokenText, isHardTab, firstNonWhitespaceIndex, firstTrailingWhitespaceIndex, hasIndentGuide, hasInvisibleCharacters) -> + appendTokenNodes: (tokenText, isHardTab, firstNonWhitespaceIndex, firstTrailingWhitespaceIndex, hasIndentGuide, hasInvisibleCharacters, scopeNode) -> if isHardTab - hardTab = @elementsPool.build("span", "hard-tab", tokenText) - hardTab.classList.add("leading-whitespace") if firstNonWhitespaceIndex? - hardTab.classList.add("trailing-whitespace") if firstTrailingWhitespaceIndex? - hardTab.classList.add("indent-guide") if hasIndentGuide - hardTab.classList.add("invisible-character") if hasInvisibleCharacters + hardTabNode = @elementsPool.build("span", "hard-tab", tokenText) + hardTabNode.classList.add("leading-whitespace") if firstNonWhitespaceIndex? + hardTabNode.classList.add("trailing-whitespace") if firstTrailingWhitespaceIndex? + hardTabNode.classList.add("indent-guide") if hasIndentGuide + hardTabNode.classList.add("invisible-character") if hasInvisibleCharacters - scopeNode.appendChild(hardTab) + scopeNode.appendChild(hardTabNode) else startIndex = 0 endIndex = tokenText.length From e9bf04b50b9ed4763bd8f82f32dfb37d0dedf27e Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 14 Sep 2015 20:21:37 +0200 Subject: [PATCH 10/20] Guard against some edge cases --- spec/dom-elements-pool-spec.coffee | 49 +++++++++++++++--------------- src/dom-elements-pool.coffee | 20 ++++++++---- 2 files changed, 38 insertions(+), 31 deletions(-) diff --git a/spec/dom-elements-pool-spec.coffee b/spec/dom-elements-pool-spec.coffee index 2b7364a8c..56cf15761 100644 --- a/spec/dom-elements-pool-spec.coffee +++ b/spec/dom-elements-pool-spec.coffee @@ -6,26 +6,14 @@ describe "DomElementsPool", -> beforeEach -> domElementsPool = new DomElementsPool - it "creates new nodes until some of them are freed", -> - span1 = domElementsPool.build("span") - span2 = domElementsPool.build("span") - span3 = domElementsPool.build("span") - - expect(span1).not.toBe(span2) - expect(span2).not.toBe(span3) - - domElementsPool.free(span1) - domElementsPool.free(span2) - - expect(domElementsPool.build("span")).toBe(span2) - expect(domElementsPool.build("span")).toBe(span1) - - it "recursively frees a dom tree", -> - div = domElementsPool.build("div") - span1 = domElementsPool.build("span") - span2 = domElementsPool.build("span") - span3 = domElementsPool.build("span") - span4 = domElementsPool.build("span") + it "builds DOM nodes, recycling them when they are freed", -> + [div, span1, span2, span3, span4] = elements = [ + domElementsPool.build("div") + domElementsPool.build("span") + domElementsPool.build("span") + domElementsPool.build("span") + domElementsPool.build("span") + ] div.appendChild(span1) span1.appendChild(span2) @@ -34,8 +22,19 @@ describe "DomElementsPool", -> domElementsPool.freeElementAndDescendants(div) - expect(domElementsPool.build("div")).toBe(div) - expect(domElementsPool.build("span")).toBe(span2) - expect(domElementsPool.build("span")).toBe(span1) - expect(domElementsPool.build("span")).toBe(span4) - expect(domElementsPool.build("span")).toBe(span3) + expect(elements).toContain(domElementsPool.build("div")) + expect(elements).toContain(domElementsPool.build("span")) + expect(elements).toContain(domElementsPool.build("span")) + expect(elements).toContain(domElementsPool.build("span")) + expect(elements).toContain(domElementsPool.build("span")) + + expect(elements).not.toContain(domElementsPool.build("span")) + + it "throws an error when trying to free the same node twice", -> + div = domElementsPool.build("div") + domElementsPool.freeElementAndDescendants(div) + expect(-> domElementsPool.freeElementAndDescendants(div)).toThrow() + + it "throws an error when trying to free an invalid element", -> + expect(-> domElementsPool.freeElementAndDescendants(null)).toThrow() + expect(-> domElementsPool.freeElementAndDescendants(undefined)).toThrow() diff --git a/src/dom-elements-pool.coffee b/src/dom-elements-pool.coffee index d0c653b02..f75c904aa 100644 --- a/src/dom-elements-pool.coffee +++ b/src/dom-elements-pool.coffee @@ -2,6 +2,7 @@ module.exports = class DomElementsPool constructor: -> @freeElementsByTagName = {} + @freedElements = new Set build: (tagName, className, textContent) -> element = @freeElementsByTagName[tagName]?.pop() @@ -9,16 +10,23 @@ class DomElementsPool element.className = className element.textContent = textContent element.removeAttribute("style") - element - free: (element) -> - element.remove() - @freeElementsByTagName[element.tagName.toLowerCase()] ?= [] - @freeElementsByTagName[element.tagName.toLowerCase()].push(element) + @freedElements.delete(element) + + element freeElementAndDescendants: (element) -> @free(element) - for index in [element.children.length - 1..0] by -1 child = element.children[index] @freeElementAndDescendants(child) + + free: (element) -> + throw new Error("The element cannot be null or undefined.") unless element? + throw new Error("The element has already been freed!") if @freedElements.has(element) + + @freeElementsByTagName[element.tagName.toLowerCase()] ?= [] + @freeElementsByTagName[element.tagName.toLowerCase()].push(element) + @freedElements.add(element) + + element.remove() From f0bc6ca23a8a6d1ff2c911e8afa0e425e34449c1 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 14 Sep 2015 20:25:57 +0200 Subject: [PATCH 11/20] :art: Some renaming --- spec/dom-element-pool-spec.coffee | 44 +++++++++++++++++++ spec/dom-elements-pool-spec.coffee | 40 ----------------- ...ts-pool.coffee => dom-element-pool.coffee} | 7 +-- src/line-number-gutter-component.coffee | 8 ++-- src/line-numbers-tile-component.coffee | 18 ++++---- src/lines-component.coffee | 6 +-- src/lines-tile-component.coffee | 30 ++++++------- 7 files changed, 79 insertions(+), 74 deletions(-) create mode 100644 spec/dom-element-pool-spec.coffee delete mode 100644 spec/dom-elements-pool-spec.coffee rename src/{dom-elements-pool.coffee => dom-element-pool.coffee} (84%) diff --git a/spec/dom-element-pool-spec.coffee b/spec/dom-element-pool-spec.coffee new file mode 100644 index 000000000..f16185cca --- /dev/null +++ b/spec/dom-element-pool-spec.coffee @@ -0,0 +1,44 @@ +DOMElementPool = require '../src/dom-element-pool' + +describe "DOMElementPool", -> + domElementPool = null + + beforeEach -> + domElementPool = new DOMElementPool + + it "builds DOM nodes, recycling them when they are freed", -> + [div, span1, span2, span3, span4, span5] = elements = [ + domElementPool.build("div") + domElementPool.build("span") + domElementPool.build("span") + domElementPool.build("span") + domElementPool.build("span") + domElementPool.build("span") + ] + + div.appendChild(span1) + span1.appendChild(span2) + div.appendChild(span3) + span3.appendChild(span4) + + domElementPool.freeElementAndDescendants(div) + domElementPool.freeElementAndDescendants(span5) + + expect(elements).toContain(domElementPool.build("div")) + expect(elements).toContain(domElementPool.build("span")) + expect(elements).toContain(domElementPool.build("span")) + expect(elements).toContain(domElementPool.build("span")) + expect(elements).toContain(domElementPool.build("span")) + expect(elements).toContain(domElementPool.build("span")) + + expect(elements).not.toContain(domElementPool.build("div")) + expect(elements).not.toContain(domElementPool.build("span")) + + it "throws an error when trying to free the same node twice", -> + div = domElementPool.build("div") + domElementPool.freeElementAndDescendants(div) + expect(-> domElementPool.freeElementAndDescendants(div)).toThrow() + + it "throws an error when trying to free an invalid element", -> + expect(-> domElementPool.freeElementAndDescendants(null)).toThrow() + expect(-> domElementPool.freeElementAndDescendants(undefined)).toThrow() diff --git a/spec/dom-elements-pool-spec.coffee b/spec/dom-elements-pool-spec.coffee deleted file mode 100644 index 56cf15761..000000000 --- a/spec/dom-elements-pool-spec.coffee +++ /dev/null @@ -1,40 +0,0 @@ -DomElementsPool = require '../src/dom-elements-pool' - -describe "DomElementsPool", -> - domElementsPool = null - - beforeEach -> - domElementsPool = new DomElementsPool - - it "builds DOM nodes, recycling them when they are freed", -> - [div, span1, span2, span3, span4] = elements = [ - domElementsPool.build("div") - domElementsPool.build("span") - domElementsPool.build("span") - domElementsPool.build("span") - domElementsPool.build("span") - ] - - div.appendChild(span1) - span1.appendChild(span2) - div.appendChild(span3) - span3.appendChild(span4) - - domElementsPool.freeElementAndDescendants(div) - - expect(elements).toContain(domElementsPool.build("div")) - expect(elements).toContain(domElementsPool.build("span")) - expect(elements).toContain(domElementsPool.build("span")) - expect(elements).toContain(domElementsPool.build("span")) - expect(elements).toContain(domElementsPool.build("span")) - - expect(elements).not.toContain(domElementsPool.build("span")) - - it "throws an error when trying to free the same node twice", -> - div = domElementsPool.build("div") - domElementsPool.freeElementAndDescendants(div) - expect(-> domElementsPool.freeElementAndDescendants(div)).toThrow() - - it "throws an error when trying to free an invalid element", -> - expect(-> domElementsPool.freeElementAndDescendants(null)).toThrow() - expect(-> domElementsPool.freeElementAndDescendants(undefined)).toThrow() diff --git a/src/dom-elements-pool.coffee b/src/dom-element-pool.coffee similarity index 84% rename from src/dom-elements-pool.coffee rename to src/dom-element-pool.coffee index f75c904aa..4cea8b89e 100644 --- a/src/dom-elements-pool.coffee +++ b/src/dom-element-pool.coffee @@ -1,5 +1,5 @@ module.exports = -class DomElementsPool +class DOMElementPool constructor: -> @freeElementsByTagName = {} @freedElements = new Set @@ -25,8 +25,9 @@ class DomElementsPool throw new Error("The element cannot be null or undefined.") unless element? throw new Error("The element has already been freed!") if @freedElements.has(element) - @freeElementsByTagName[element.tagName.toLowerCase()] ?= [] - @freeElementsByTagName[element.tagName.toLowerCase()].push(element) + tagName = element.tagName.toLowerCase() + @freeElementsByTagName[tagName] ?= [] + @freeElementsByTagName[tagName].push(element) @freedElements.add(element) element.remove() diff --git a/src/line-number-gutter-component.coffee b/src/line-number-gutter-component.coffee index 390aba41b..c95d43641 100644 --- a/src/line-number-gutter-component.coffee +++ b/src/line-number-gutter-component.coffee @@ -1,7 +1,7 @@ TiledComponent = require './tiled-component' LineNumbersTileComponent = require './line-numbers-tile-component' WrapperDiv = document.createElement('div') -DomElementsPool = require './dom-elements-pool' +DOMElementPool = require './dom-element-pool' module.exports = class LineNumberGutterComponent extends TiledComponent @@ -10,9 +10,9 @@ class LineNumberGutterComponent extends TiledComponent constructor: ({@onMouseDown, @editor, @gutter}) -> @visible = true - @elementsPool = new DomElementsPool + @domElementPool = new DOMElementPool - @dummyLineNumberComponent = LineNumbersTileComponent.createDummy(@elementsPool) + @dummyLineNumberComponent = LineNumbersTileComponent.createDummy(@domElementPool) @domNode = atom.views.getView(@gutter) @lineNumbersNode = @domNode.firstChild @@ -64,7 +64,7 @@ class LineNumberGutterComponent extends TiledComponent @oldState.styles = {} @oldState.maxLineNumberDigits = @newState.maxLineNumberDigits - buildComponentForTile: (id) -> new LineNumbersTileComponent({id, @elementsPool}) + buildComponentForTile: (id) -> new LineNumbersTileComponent({id, @domElementPool}) ### Section: Private Methods diff --git a/src/line-numbers-tile-component.coffee b/src/line-numbers-tile-component.coffee index 0b4ca43b8..5a54b8208 100644 --- a/src/line-numbers-tile-component.coffee +++ b/src/line-numbers-tile-component.coffee @@ -2,18 +2,18 @@ _ = require 'underscore-plus' module.exports = class LineNumbersTileComponent - @createDummy: (elementsPool) -> - new LineNumbersTileComponent({id: -1, elementsPool}) + @createDummy: (domElementPool) -> + new LineNumbersTileComponent({id: -1, domElementPool}) - constructor: ({@id, @elementsPool}) -> + constructor: ({@id, @domElementPool}) -> @lineNumberNodesById = {} - @domNode = @elementsPool.build("div", "tile") + @domNode = @domElementPool.build("div", "tile") @domNode.style.position = "absolute" @domNode.style.display = "block" @domNode.style.top = 0 # Cover the space occupied by a dummy lineNumber destroy: -> - @elementsPool.freeElementAndDescendants(@domNode) + @domElementPool.freeElementAndDescendants(@domNode) getDomNode: -> @domNode @@ -49,7 +49,7 @@ class LineNumbersTileComponent if @newState.maxLineNumberDigits isnt @oldState.maxLineNumberDigits for id, node of @lineNumberNodesById - @elementsPool.freeElementAndDescendants(node) + @domElementPool.freeElementAndDescendants(node) @oldState.tiles[@id] = {lineNumbers: {}} @oldTileState = @oldState.tiles[@id] @@ -64,7 +64,7 @@ class LineNumbersTileComponent for id, lineNumberState of @oldTileState.lineNumbers unless @newTileState.lineNumbers.hasOwnProperty(id) - @elementsPool.freeElementAndDescendants(@lineNumberNodesById[id]) + @domElementPool.freeElementAndDescendants(@lineNumberNodesById[id]) delete @lineNumberNodesById[id] delete @oldTileState.lineNumbers[id] @@ -91,7 +91,7 @@ class LineNumbersTileComponent {screenRow, bufferRow, softWrapped, top, decorationClasses, zIndex} = lineNumberState className = @buildLineNumberClassName(lineNumberState) - lineNumberNode = @elementsPool.build("div", className) + lineNumberNode = @domElementPool.build("div", className) lineNumberNode.dataset.screenRow = screenRow lineNumberNode.dataset.bufferRow = bufferRow @@ -114,7 +114,7 @@ class LineNumbersTileComponent lineNumber = (bufferRow + 1).toString() padding = _.multiplyString("\u00a0", maxLineNumberDigits - lineNumber.length) - iconRight = @elementsPool.build("div", "icon-right") + iconRight = @domElementPool.build("div", "icon-right") lineNumberNode.innerText = padding + lineNumber lineNumberNode.appendChild(iconRight) diff --git a/src/lines-component.coffee b/src/lines-component.coffee index d3d1f9b0a..a96c4b91d 100644 --- a/src/lines-component.coffee +++ b/src/lines-component.coffee @@ -3,7 +3,7 @@ CursorsComponent = require './cursors-component' LinesTileComponent = require './lines-tile-component' TiledComponent = require './tiled-component' -DomElementsPool = require './dom-elements-pool' +DOMElementPool = require './dom-element-pool' DummyLineNode = $$(-> @div className: 'line', style: 'position: absolute; visibility: hidden;', => @span 'x')[0] @@ -24,7 +24,7 @@ class LinesComponent extends TiledComponent @cursorsComponent = new CursorsComponent @domNode.appendChild(@cursorsComponent.getDomNode()) - @elementsPool = new DomElementsPool + @domElementPool = new DOMElementPool if @useShadowDOM insertionPoint = document.createElement('content') @@ -63,7 +63,7 @@ class LinesComponent extends TiledComponent @oldState.indentGuidesVisible = @newState.indentGuidesVisible - buildComponentForTile: (id) -> new LinesTileComponent({id, @presenter, @elementsPool}) + buildComponentForTile: (id) -> new LinesTileComponent({id, @presenter, @domElementPool}) buildEmptyState: -> {tiles: {}} diff --git a/src/lines-tile-component.coffee b/src/lines-tile-component.coffee index 2b4762c76..c23c97055 100644 --- a/src/lines-tile-component.coffee +++ b/src/lines-tile-component.coffee @@ -13,13 +13,13 @@ cloneObject = (object) -> module.exports = class LinesTileComponent - constructor: ({@presenter, @id, @elementsPool}) -> + constructor: ({@presenter, @id, @domElementPool}) -> @tokenIterator = new TokenIterator @measuredLines = new Set @lineNodesByLineId = {} @screenRowsByLineId = {} @lineIdsByScreenRow = {} - @domNode = @elementsPool.build("div", "tile") + @domNode = @domElementPool.build("div", "tile") @domNode.classList.add("tile") @domNode.style.position = "absolute" @domNode.style.display = "block" @@ -28,7 +28,7 @@ class LinesTileComponent @domNode.appendChild(@highlightsComponent.getDomNode()) destroy: -> - @elementsPool.freeElementAndDescendants(@domNode) + @domElementPool.freeElementAndDescendants(@domNode) getDomNode: -> @domNode @@ -79,7 +79,7 @@ class LinesTileComponent return removeLineNode: (id) -> - @elementsPool.freeElementAndDescendants(@lineNodesByLineId[id]) + @domElementPool.freeElementAndDescendants(@lineNodesByLineId[id]) delete @lineNodesByLineId[id] delete @lineIdsByScreenRow[@screenRowsByLineId[id]] delete @screenRowsByLineId[id] @@ -118,7 +118,7 @@ class LinesTileComponent {width} = @newState {screenRow, tokens, text, top, lineEnding, fold, isSoftWrapped, indentLevel, decorationClasses} = @newTileState.lines[id] - lineNode = @elementsPool.build("div", "line") + lineNode = @domElementPool.build("div", "line") if decorationClasses? for decorationClass in decorationClasses lineNode.classList.add(decorationClass) @@ -133,7 +133,7 @@ class LinesTileComponent else @appendLineInnerNodes(id, lineNode) - lineNode.appendChild(@elementsPool.build("span", "fold-marker")) if fold + lineNode.appendChild(@domElementPool.build("span", "fold-marker")) if fold lineNode appendEmptyLineInnerNodes: (id, lineNode) -> @@ -143,11 +143,11 @@ class LinesTileComponent if indentGuidesVisible and indentLevel > 0 invisibleIndex = 0 for i in [0...indentLevel] - indentGuide = @elementsPool.build("span", "indent-guide") + indentGuide = @domElementPool.build("span", "indent-guide") for j in [0...tabLength] if invisible = endOfLineInvisibles?[invisibleIndex++] indentGuide.appendChild( - @elementsPool.build("span", "invisible-character", invisible) + @domElementPool.build("span", "invisible-character", invisible) ) else indentGuide.insertAdjacentText("beforeend", " ") @@ -156,7 +156,7 @@ class LinesTileComponent while invisibleIndex < endOfLineInvisibles?.length invisible = endOfLineInvisibles[invisibleIndex++] lineNode.appendChild( - @elementsPool.build("span", "invisible-character", invisible) + @domElementPool.build("span", "invisible-character", invisible) ) else unless @appendEndOfLineNodes(id, lineNode) @@ -175,7 +175,7 @@ class LinesTileComponent openScopeNode = openScopeNode.parentElement for scope in @tokenIterator.getScopeStarts() - newScopeNode = @elementsPool.build("span", scope.replace(/\.+/g, ' ')) + newScopeNode = @domElementPool.build("span", scope.replace(/\.+/g, ' ')) openScopeNode.appendChild(newScopeNode) openScopeNode = newScopeNode @@ -208,7 +208,7 @@ class LinesTileComponent appendTokenNodes: (tokenText, isHardTab, firstNonWhitespaceIndex, firstTrailingWhitespaceIndex, hasIndentGuide, hasInvisibleCharacters, scopeNode) -> if isHardTab - hardTabNode = @elementsPool.build("span", "hard-tab", tokenText) + hardTabNode = @domElementPool.build("span", "hard-tab", tokenText) hardTabNode.classList.add("leading-whitespace") if firstNonWhitespaceIndex? hardTabNode.classList.add("trailing-whitespace") if firstTrailingWhitespaceIndex? hardTabNode.classList.add("indent-guide") if hasIndentGuide @@ -223,7 +223,7 @@ class LinesTileComponent trailingWhitespaceNode = null if firstNonWhitespaceIndex? - leadingWhitespaceNode = @elementsPool.build( + leadingWhitespaceNode = @domElementPool.build( "span", "leading-whitespace", tokenText.substring(0, firstNonWhitespaceIndex) @@ -236,7 +236,7 @@ class LinesTileComponent if firstTrailingWhitespaceIndex? tokenIsOnlyWhitespace = firstTrailingWhitespaceIndex is 0 - trailingWhitespaceNode = @elementsPool.build( + trailingWhitespaceNode = @domElementPool.build( "span", "trailing-whitespace", tokenText.substring(firstTrailingWhitespaceIndex) @@ -251,7 +251,7 @@ class LinesTileComponent if tokenText.length > MaxTokenLength while startIndex < endIndex text = @sliceText(tokenText, startIndex, startIndex + MaxTokenLength) - scopeNode.appendChild(@elementsPool.build("span", null, text)) + scopeNode.appendChild(@domElementPool.build("span", null, text)) startIndex += MaxTokenLength else scopeNode.insertAdjacentText("beforeend", @sliceText(tokenText, startIndex, endIndex)) @@ -271,7 +271,7 @@ class LinesTileComponent for invisible in endOfLineInvisibles hasInvisibles = true lineNode.appendChild( - @elementsPool.build("span", "invisible-character", invisible) + @domElementPool.build("span", "invisible-character", invisible) ) hasInvisibles From 663de4c8d6c9ff2de4bf238513f3575eca92cb5a Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 14 Sep 2015 20:28:09 +0200 Subject: [PATCH 12/20] Use textContent instead of innerText --- src/line-numbers-tile-component.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/line-numbers-tile-component.coffee b/src/line-numbers-tile-component.coffee index 5a54b8208..1e568078d 100644 --- a/src/line-numbers-tile-component.coffee +++ b/src/line-numbers-tile-component.coffee @@ -116,7 +116,7 @@ class LineNumbersTileComponent padding = _.multiplyString("\u00a0", maxLineNumberDigits - lineNumber.length) iconRight = @domElementPool.build("div", "icon-right") - lineNumberNode.innerText = padding + lineNumber + lineNumberNode.textContent = padding + lineNumber lineNumberNode.appendChild(iconRight) updateLineNumberNode: (lineNumberId, newLineNumberState) -> From ce4281821d56380626db0f5d9ea042b358a9921b Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 14 Sep 2015 20:31:21 +0200 Subject: [PATCH 13/20] Replace insertAdjacentText with textContent --- src/lines-tile-component.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lines-tile-component.coffee b/src/lines-tile-component.coffee index c23c97055..f5e687572 100644 --- a/src/lines-tile-component.coffee +++ b/src/lines-tile-component.coffee @@ -160,7 +160,7 @@ class LinesTileComponent ) else unless @appendEndOfLineNodes(id, lineNode) - lineNode.insertAdjacentText("beforeend", "\u00a0") + lineNode.textContent = "\u00a0" appendLineInnerNodes: (id, lineNode) -> lineState = @newTileState.lines[id] From c8c69a99b913dc91e350bbcbb9c7367df3bc1c62 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 15 Sep 2015 10:36:47 +0200 Subject: [PATCH 14/20] Release free nodes after destroying TextEditor --- spec/dom-element-pool-spec.coffee | 11 +++++++++++ src/dom-element-pool.coffee | 5 +++++ src/line-number-gutter-component.coffee | 2 ++ src/lines-component.coffee | 3 +++ src/text-editor-component.coffee | 1 + 5 files changed, 22 insertions(+) diff --git a/spec/dom-element-pool-spec.coffee b/spec/dom-element-pool-spec.coffee index f16185cca..1399b17fc 100644 --- a/spec/dom-element-pool-spec.coffee +++ b/spec/dom-element-pool-spec.coffee @@ -34,6 +34,17 @@ describe "DOMElementPool", -> expect(elements).not.toContain(domElementPool.build("div")) expect(elements).not.toContain(domElementPool.build("span")) + it "forgets free nodes after being cleared", -> + span = domElementPool.build("span") + div = domElementPool.build("div") + domElementPool.freeElementAndDescendants(span) + domElementPool.freeElementAndDescendants(div) + + domElementPool.clear() + + expect(domElementPool.build("span")).not.toBe(span) + expect(domElementPool.build("div")).not.toBe(div) + it "throws an error when trying to free the same node twice", -> div = domElementPool.build("div") domElementPool.freeElementAndDescendants(div) diff --git a/src/dom-element-pool.coffee b/src/dom-element-pool.coffee index 4cea8b89e..93e798f93 100644 --- a/src/dom-element-pool.coffee +++ b/src/dom-element-pool.coffee @@ -4,6 +4,11 @@ class DOMElementPool @freeElementsByTagName = {} @freedElements = new Set + clear: -> + @freedElements.clear() + for tagName, freeElements of @freeElementsByTagName + freeElements.length = 0 + build: (tagName, className, textContent) -> element = @freeElementsByTagName[tagName]?.pop() element ?= document.createElement(tagName) diff --git a/src/line-number-gutter-component.coffee b/src/line-number-gutter-component.coffee index c95d43641..df8f8bd1b 100644 --- a/src/line-number-gutter-component.coffee +++ b/src/line-number-gutter-component.coffee @@ -22,6 +22,8 @@ class LineNumberGutterComponent extends TiledComponent @domNode.addEventListener 'mousedown', @onMouseDown destroy: -> + @domElementPool.clear() + @domNode.removeEventListener 'click', @onClick @domNode.removeEventListener 'mousedown', @onMouseDown diff --git a/src/lines-component.coffee b/src/lines-component.coffee index a96c4b91d..0fc9fea02 100644 --- a/src/lines-component.coffee +++ b/src/lines-component.coffee @@ -31,6 +31,9 @@ class LinesComponent extends TiledComponent insertionPoint.setAttribute('select', '.overlayer') @domNode.appendChild(insertionPoint) + destroy: -> + @domElementPool.clear() + getDomNode: -> @domNode diff --git a/src/text-editor-component.coffee b/src/text-editor-component.coffee index 51bc2c463..f15a15c3f 100644 --- a/src/text-editor-component.coffee +++ b/src/text-editor-component.coffee @@ -107,6 +107,7 @@ class TextEditorComponent @disposables.dispose() @presenter.destroy() @gutterContainerComponent?.destroy() + @linesComponent.destroy() getDomNode: -> @domNode From f86c9b2331dc06d102d903224a8c712f773f51b3 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 15 Sep 2015 10:58:24 +0200 Subject: [PATCH 15/20] Let TextEditorComponent manage DOMElementPool --- src/gutter-container-component.coffee | 4 ++-- src/line-number-gutter-component.coffee | 6 +----- src/lines-component.coffee | 8 +------- src/text-editor-component.coffee | 9 ++++++--- 4 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/gutter-container-component.coffee b/src/gutter-container-component.coffee index 09ab43f24..40e2c8c26 100644 --- a/src/gutter-container-component.coffee +++ b/src/gutter-container-component.coffee @@ -7,7 +7,7 @@ LineNumberGutterComponent = require './line-number-gutter-component' module.exports = class GutterContainerComponent - constructor: ({@onLineNumberGutterMouseDown, @editor}) -> + constructor: ({@onLineNumberGutterMouseDown, @editor, @domElementPool}) -> # An array of objects of the form: {name: {String}, component: {Object}} @gutterComponents = [] @gutterComponentsByGutterName = {} @@ -39,7 +39,7 @@ class GutterContainerComponent gutterComponent = @gutterComponentsByGutterName[gutter.name] if not gutterComponent if gutter.name is 'line-number' - gutterComponent = new LineNumberGutterComponent({onMouseDown: @onLineNumberGutterMouseDown, @editor, gutter}) + gutterComponent = new LineNumberGutterComponent({onMouseDown: @onLineNumberGutterMouseDown, @editor, gutter, @domElementPool}) @lineNumberGutterComponent = gutterComponent else gutterComponent = new CustomGutterComponent({gutter}) diff --git a/src/line-number-gutter-component.coffee b/src/line-number-gutter-component.coffee index df8f8bd1b..f0376b334 100644 --- a/src/line-number-gutter-component.coffee +++ b/src/line-number-gutter-component.coffee @@ -7,11 +7,9 @@ module.exports = class LineNumberGutterComponent extends TiledComponent dummyLineNumberNode: null - constructor: ({@onMouseDown, @editor, @gutter}) -> + constructor: ({@onMouseDown, @editor, @gutter, @domElementPool}) -> @visible = true - @domElementPool = new DOMElementPool - @dummyLineNumberComponent = LineNumbersTileComponent.createDummy(@domElementPool) @domNode = atom.views.getView(@gutter) @@ -22,8 +20,6 @@ class LineNumberGutterComponent extends TiledComponent @domNode.addEventListener 'mousedown', @onMouseDown destroy: -> - @domElementPool.clear() - @domNode.removeEventListener 'click', @onClick @domNode.removeEventListener 'mousedown', @onMouseDown diff --git a/src/lines-component.coffee b/src/lines-component.coffee index 0fc9fea02..b618563be 100644 --- a/src/lines-component.coffee +++ b/src/lines-component.coffee @@ -3,7 +3,6 @@ CursorsComponent = require './cursors-component' LinesTileComponent = require './lines-tile-component' TiledComponent = require './tiled-component' -DOMElementPool = require './dom-element-pool' DummyLineNode = $$(-> @div className: 'line', style: 'position: absolute; visibility: hidden;', => @span 'x')[0] @@ -11,7 +10,7 @@ module.exports = class LinesComponent extends TiledComponent placeholderTextDiv: null - constructor: ({@presenter, @hostElement, @useShadowDOM, visible}) -> + constructor: ({@presenter, @hostElement, @useShadowDOM, visible, @domElementPool}) -> @domNode = document.createElement('div') @domNode.classList.add('lines') @tilesNode = document.createElement("div") @@ -24,16 +23,11 @@ class LinesComponent extends TiledComponent @cursorsComponent = new CursorsComponent @domNode.appendChild(@cursorsComponent.getDomNode()) - @domElementPool = new DOMElementPool - if @useShadowDOM insertionPoint = document.createElement('content') insertionPoint.setAttribute('select', '.overlayer') @domNode.appendChild(insertionPoint) - destroy: -> - @domElementPool.clear() - getDomNode: -> @domNode diff --git a/src/text-editor-component.coffee b/src/text-editor-component.coffee index f15a15c3f..a8f610c0e 100644 --- a/src/text-editor-component.coffee +++ b/src/text-editor-component.coffee @@ -12,6 +12,7 @@ LinesComponent = require './lines-component' ScrollbarComponent = require './scrollbar-component' ScrollbarCornerComponent = require './scrollbar-corner-component' OverlayManager = require './overlay-manager' +DOMElementPool = require './dom-element-pool' module.exports = class TextEditorComponent @@ -54,6 +55,8 @@ class TextEditorComponent @presenter.onDidUpdateState(@requestUpdate) + @domElementPool = new DOMElementPool + @domNode = document.createElement('div') if @useShadowDOM @domNode.classList.add('editor-contents--private') @@ -75,7 +78,7 @@ class TextEditorComponent @hiddenInputComponent = new InputComponent @scrollViewNode.appendChild(@hiddenInputComponent.getDomNode()) - @linesComponent = new LinesComponent({@presenter, @hostElement, @useShadowDOM}) + @linesComponent = new LinesComponent({@presenter, @hostElement, @useShadowDOM, @domElementPool}) @scrollViewNode.appendChild(@linesComponent.getDomNode()) @horizontalScrollbarComponent = new ScrollbarComponent({orientation: 'horizontal', onScroll: @onHorizontalScroll}) @@ -107,7 +110,7 @@ class TextEditorComponent @disposables.dispose() @presenter.destroy() @gutterContainerComponent?.destroy() - @linesComponent.destroy() + @domElementPool.clear() getDomNode: -> @domNode @@ -166,7 +169,7 @@ class TextEditorComponent @overlayManager?.measureOverlays() mountGutterContainerComponent: -> - @gutterContainerComponent = new GutterContainerComponent({@editor, @onLineNumberGutterMouseDown}) + @gutterContainerComponent = new GutterContainerComponent({@editor, @onLineNumberGutterMouseDown, @domElementPool}) @domNode.insertBefore(@gutterContainerComponent.getDomNode(), @domNode.firstChild) becameVisible: -> From daf43169749cced33d5c97b0dee8aa457d829f2f Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 15 Sep 2015 10:59:32 +0200 Subject: [PATCH 16/20] Clear the pool when font or lineHeight change --- src/text-editor-component.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/src/text-editor-component.coffee b/src/text-editor-component.coffee index a8f610c0e..f0040038b 100644 --- a/src/text-editor-component.coffee +++ b/src/text-editor-component.coffee @@ -653,6 +653,7 @@ class TextEditorComponent {@fontSize, @fontFamily, @lineHeight} = getComputedStyle(@getTopmostDOMNode()) if @fontSize isnt oldFontSize or @fontFamily isnt oldFontFamily or @lineHeight isnt oldLineHeight + @domElementPool.clear() @measureLineHeightAndDefaultCharWidth() if (@fontSize isnt oldFontSize or @fontFamily isnt oldFontFamily) and @performedInitialMeasurement From 028f57aefb07de9c7d744559be8e136020816203 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 15 Sep 2015 11:35:30 +0200 Subject: [PATCH 17/20] :green_heart: --- spec/gutter-container-component-spec.coffee | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/gutter-container-component-spec.coffee b/spec/gutter-container-component-spec.coffee index 5d815fea8..595067de5 100644 --- a/spec/gutter-container-component-spec.coffee +++ b/spec/gutter-container-component-spec.coffee @@ -1,5 +1,6 @@ Gutter = require '../src/gutter' GutterContainerComponent = require '../src/gutter-container-component' +DOMElementPool = require '../src/dom-element-pool' describe "GutterContainerComponent", -> [gutterContainerComponent] = [] @@ -22,9 +23,10 @@ describe "GutterContainerComponent", -> mockTestState beforeEach -> + domElementPool = new DOMElementPool mockEditor = {} mockMouseDown = -> - gutterContainerComponent = new GutterContainerComponent({editor: mockEditor, onMouseDown: mockMouseDown}) + gutterContainerComponent = new GutterContainerComponent({editor: mockEditor, onMouseDown: mockMouseDown, domElementPool}) it "creates a DOM node with no child gutter nodes when it is initialized", -> expect(gutterContainerComponent.getDomNode() instanceof HTMLElement).toBe true From f8868ffcf2b60ac3bc69679e963dc2b4b6aa24be Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 15 Sep 2015 12:23:53 +0200 Subject: [PATCH 18/20] Clear dataset elements for pooled objects --- src/dom-element-pool.coffee | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/dom-element-pool.coffee b/src/dom-element-pool.coffee index 93e798f93..98049a9f6 100644 --- a/src/dom-element-pool.coffee +++ b/src/dom-element-pool.coffee @@ -9,12 +9,14 @@ class DOMElementPool for tagName, freeElements of @freeElementsByTagName freeElements.length = 0 - build: (tagName, className, textContent) -> + build: (tagName, className, textContent = "") -> element = @freeElementsByTagName[tagName]?.pop() element ?= document.createElement(tagName) - element.className = className - element.textContent = textContent + delete element.dataset[dataId] for dataId of element.dataset + element.removeAttribute("class") element.removeAttribute("style") + element.className = className if className? + element.textContent = textContent @freedElements.delete(element) From 8b52538213e847af920739b98dff82e586c57e92 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 17 Sep 2015 10:35:20 +0200 Subject: [PATCH 19/20] Pool highlight elements --- src/highlights-component.coffee | 11 ++++------- src/lines-tile-component.coffee | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/highlights-component.coffee b/src/highlights-component.coffee index 522dcb304..4c5e138d8 100644 --- a/src/highlights-component.coffee +++ b/src/highlights-component.coffee @@ -5,12 +5,11 @@ module.exports = class HighlightsComponent oldState: null - constructor: -> + constructor: (@domElementPool) -> @highlightNodesById = {} @regionNodesByHighlightId = {} - @domNode = document.createElement('div') - @domNode.classList.add('highlights') + @domNode = @domElementPool.build("div", "highlights") getDomNode: -> @domNode @@ -30,8 +29,7 @@ class HighlightsComponent # add or update highlights for id, highlightState of newState unless @oldState[id]? - highlightNode = document.createElement('div') - highlightNode.classList.add('highlight') + highlightNode = @domElementPool.build("div", "highlight") @highlightNodesById[id] = highlightNode @regionNodesByHighlightId[id] = {} @domNode.appendChild(highlightNode) @@ -75,12 +73,11 @@ class HighlightsComponent for newRegionState, i in newHighlightState.regions unless oldHighlightState.regions[i]? oldHighlightState.regions[i] = {} - regionNode = document.createElement('div') + regionNode = @domElementPool.build("div", "region") # This prevents highlights at the tiles boundaries to be hidden by the # subsequent tile. When this happens, subpixel anti-aliasing gets # disabled. regionNode.style.boxSizing = "border-box" - regionNode.classList.add('region') regionNode.classList.add(newHighlightState.deprecatedRegionClass) if newHighlightState.deprecatedRegionClass? @regionNodesByHighlightId[id][i] = regionNode highlightNode.appendChild(regionNode) diff --git a/src/lines-tile-component.coffee b/src/lines-tile-component.coffee index 6e9e1d890..627630e03 100644 --- a/src/lines-tile-component.coffee +++ b/src/lines-tile-component.coffee @@ -23,7 +23,7 @@ class LinesTileComponent @domNode.style.position = "absolute" @domNode.style.display = "block" - @highlightsComponent = new HighlightsComponent + @highlightsComponent = new HighlightsComponent(@domElementPool) @domNode.appendChild(@highlightsComponent.getDomNode()) destroy: -> From c6948b36dca943c621b0fdf0c9c75d31271957ff Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 17 Sep 2015 10:38:40 +0200 Subject: [PATCH 20/20] Clear pool after updating --- src/text-editor-component.coffee | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/text-editor-component.coffee b/src/text-editor-component.coffee index e3e816c0f..7ce212f34 100644 --- a/src/text-editor-component.coffee +++ b/src/text-editor-component.coffee @@ -156,6 +156,10 @@ class TextEditorComponent @overlayManager?.render(@newState) + if @clearPoolAfterUpdate + @domElementPool.clear() + @clearPoolAfterUpdate = false + if @editor.isAlive() @updateParentViewFocusedClassIfNeeded() @updateParentViewMiniClass() @@ -653,7 +657,7 @@ class TextEditorComponent {@fontSize, @fontFamily, @lineHeight} = getComputedStyle(@getTopmostDOMNode()) if @fontSize isnt oldFontSize or @fontFamily isnt oldFontFamily or @lineHeight isnt oldLineHeight - @domElementPool.clear() + @clearPoolAfterUpdate = true @measureLineHeightAndDefaultCharWidth() if (@fontSize isnt oldFontSize or @fontFamily isnt oldFontFamily) and @performedInitialMeasurement