Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

Implement text editor DOM updates manually instead of via React #5624

Merged
merged 40 commits into from Feb 20, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
883af7a
Update cursor nodes manually
Feb 10, 2015
c294bb1
Update highlight nodes manually
Feb 10, 2015
b4ecb65
Make CursorsComponent a plain object instead of using React
Feb 10, 2015
b0a29bd
Make HighlightsComponent a plain object instead of using React
Feb 10, 2015
da793e8
Perform all LinesComponent DOM updates manually
Feb 11, 2015
c06e100
Make LinesComponent a normal object instead of a React component
Feb 11, 2015
679cada
Store maxLineNumberDigits in oldState
Feb 11, 2015
2d771ab
Perform GutterComponent DOM updates manually
Feb 11, 2015
168df98
Make GutterComponent a plain JS object instead of a React component
Feb 11, 2015
8e27d82
Store hidden input data in TextEditorPresenter::state
Feb 11, 2015
52a9a76
Use presenter state in InputComponent
Feb 11, 2015
0047e3b
Perform InputComponent DOM updates manually
Feb 11, 2015
ea49fc6
Construct LinesComponent with visibility param to avoid redundant update
Feb 11, 2015
8784d48
Use plain JS object for InputComponent instead of React
Feb 11, 2015
8552aca
:racehorse: Optimize line node updates
Feb 12, 2015
d9e100c
Perform ScrollbarComponent DOM updates manually
Feb 12, 2015
da6bd36
Use plain JS object for ScrollbarComponent instead of React
Feb 12, 2015
77a4482
Fix assignment of oldState values in ScrollbarComponent
Feb 13, 2015
6b3d29a
Manually update DOM in ScrollbarCornerComponent
Feb 13, 2015
5c7e0c3
Use plain JS object for ScrollbarCornerComponent instead of React
Feb 13, 2015
4654bad
Add .focused to presenter state
Feb 13, 2015
bf29a02
Use presenter for focused state in EditorComponent
Feb 14, 2015
fd603a0
Move new character measurement to end of full update to avoid reflow
Feb 14, 2015
156569f
Add TextEditorPresenter::state.gutter.visible
Feb 18, 2015
2fba497
Use presenter to determine gutter visibility
Feb 18, 2015
1845234
Remove unnecessary ::mini subscription on model in TextEditorComponent
Feb 18, 2015
5ecefe7
Update scoped config values in presenter when grammar changes
Feb 18, 2015
e244aae
Cache ::showIndentGuide config setting
Feb 18, 2015
d89ec25
Remove showIndentGuide subscription from view
Feb 18, 2015
ccdc4eb
Update TextEditorComponent DOM node manually
Feb 18, 2015
dae15ea
Remove unused prototype properties
Feb 18, 2015
0d109d6
Use CompositeDisposable instead of SubscriberMixin in editor view
Feb 18, 2015
1e8b0ac
Remove redundant update requests in editor view
Feb 18, 2015
c9a6c32
Replace cursor blink React props with normal properties
Feb 18, 2015
7033b27
Make EditorComponent a plain JS object rather than a React component
Feb 18, 2015
d337c88
Delete overlay node from hash before removing
Feb 18, 2015
de4d995
Add document coordination methods to ViewRegistry
Feb 19, 2015
1d84d74
Centralize text editor DOM interaction through atom.views
Feb 19, 2015
dc752ed
Update scrollHeight/Width before scrollTop/Left in dummy scrollbars
Feb 19, 2015
32d393d
Pause polling when updates are requested, but don’t start polling over
Feb 20, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions spec/spec-helper.coffee
Expand Up @@ -82,6 +82,7 @@ beforeEach ->
atom.keymaps.keyBindings = _.clone(keyBindingsToRestore)
atom.commands.restoreSnapshot(commandsToRestore)
atom.styles.restoreSnapshot(styleElementsToRestore)
atom.views.clearDocumentRequests()

atom.workspaceViewParentSelector = '#jasmine-content'

Expand Down
62 changes: 32 additions & 30 deletions spec/text-editor-component-spec.coffee
Expand Up @@ -46,7 +46,7 @@ describe "TextEditorComponent", ->

lineHeightInPixels = editor.getLineHeightInPixels()
charWidth = editor.getDefaultCharWidth()
componentNode = component.getDOMNode()
componentNode = component.domNode
verticalScrollbarNode = componentNode.querySelector('.vertical-scrollbar')
horizontalScrollbarNode = componentNode.querySelector('.horizontal-scrollbar')

Expand Down Expand Up @@ -193,7 +193,8 @@ describe "TextEditorComponent", ->
expect(linesNode.style.backgroundColor).toBe backgroundColor

wrapperNode.style.backgroundColor = 'rgb(255, 0, 0)'
advanceClock(component.domPollingInterval)

advanceClock(atom.views.documentPollingInterval)
nextAnimationFrame()
expect(linesNode.style.backgroundColor).toBe 'rgb(255, 0, 0)'

Expand Down Expand Up @@ -466,11 +467,6 @@ describe "TextEditorComponent", ->
expect(foldedLineNode.querySelector('.fold-marker')).toBeFalsy()

describe "gutter rendering", ->
[gutter] = []

beforeEach ->
{gutter} = component.refs

it "renders the currently-visible line numbers", ->
wrapperNode.style.height = 4.5 * lineHeightInPixels + 'px'
component.measureHeightAndWidth()
Expand Down Expand Up @@ -567,32 +563,32 @@ describe "TextEditorComponent", ->

# favor gutter color if it's assigned
gutterNode.style.backgroundColor = 'rgb(255, 0, 0)'
advanceClock(component.domPollingInterval)
advanceClock(atom.views.documentPollingInterval)
nextAnimationFrame()
expect(lineNumbersNode.style.backgroundColor).toBe 'rgb(255, 0, 0)'

it "hides or shows the gutter based on the '::isGutterVisible' property on the model and the global 'editor.showLineNumbers' config setting", ->
expect(component.refs.gutter?).toBe true
expect(component.gutterComponent?).toBe true

editor.setGutterVisible(false)
nextAnimationFrame()

expect(component.refs.gutter?).toBe false
expect(componentNode.querySelector('.gutter')).toBeNull()

atom.config.set("editor.showLineNumbers", false)
expect(nextAnimationFrame).toBe noAnimationFrame
nextAnimationFrame()

expect(component.refs.gutter?).toBe false
expect(componentNode.querySelector('.gutter')).toBeNull()

editor.setGutterVisible(true)
expect(nextAnimationFrame).toBe noAnimationFrame
nextAnimationFrame()

expect(component.refs.gutter?).toBe false
expect(componentNode.querySelector('.gutter')).toBeNull()

atom.config.set("editor.showLineNumbers", true)
nextAnimationFrame()

expect(component.refs.gutter?).toBe true
expect(componentNode.querySelector('.gutter')).toBeDefined()
expect(component.lineNumberNodeForScreenRow(3)?).toBe true

describe "fold decorations", ->
Expand Down Expand Up @@ -706,13 +702,13 @@ describe "TextEditorComponent", ->

cursorNodes = componentNode.querySelectorAll('.cursor')
expect(cursorNodes.length).toBe 2
expect(cursorNodes[0].style['-webkit-transform']).toBe "translate(#{11 * charWidth}px, #{8 * lineHeightInPixels}px)"
expect(cursorNodes[1].style['-webkit-transform']).toBe "translate(#{10 * charWidth}px, #{4 * lineHeightInPixels}px)"
expect(cursorNodes[0].style['-webkit-transform']).toBe "translate(#{10 * charWidth}px, #{4 * lineHeightInPixels}px)"
expect(cursorNodes[1].style['-webkit-transform']).toBe "translate(#{11 * charWidth}px, #{8 * lineHeightInPixels}px)"

wrapperView.on 'cursor:moved', cursorMovedListener = jasmine.createSpy('cursorMovedListener')
cursor3.setScreenPosition([4, 11], autoscroll: false)
nextAnimationFrame()
expect(cursorNodes[1].style['-webkit-transform']).toBe "translate(#{11 * charWidth}px, #{4 * lineHeightInPixels}px)"
expect(cursorNodes[0].style['-webkit-transform']).toBe "translate(#{11 * charWidth}px, #{4 * lineHeightInPixels}px)"
expect(cursorMovedListener).toHaveBeenCalled()

cursor3.destroy()
Expand Down Expand Up @@ -800,11 +796,11 @@ describe "TextEditorComponent", ->

expect(cursorsNode.classList.contains('blink-off')).toBe false

advanceClock(component.props.cursorBlinkPeriod / 2)
advanceClock(component.cursorBlinkPeriod / 2)
nextAnimationFrame()
expect(cursorsNode.classList.contains('blink-off')).toBe true

advanceClock(component.props.cursorBlinkPeriod / 2)
advanceClock(component.cursorBlinkPeriod / 2)
nextAnimationFrame()
expect(cursorsNode.classList.contains('blink-off')).toBe false

Expand All @@ -813,8 +809,8 @@ describe "TextEditorComponent", ->
nextAnimationFrame()
expect(cursorsNode.classList.contains('blink-off')).toBe false

advanceClock(component.props.cursorBlinkResumeDelay)
advanceClock(component.props.cursorBlinkPeriod / 2)
advanceClock(component.cursorBlinkResumeDelay)
advanceClock(component.cursorBlinkPeriod / 2)
nextAnimationFrame()
expect(cursorsNode.classList.contains('blink-off')).toBe true

Expand Down Expand Up @@ -1501,12 +1497,14 @@ describe "TextEditorComponent", ->
expect(inputNode.offsetLeft).toBe 0

# In bounds and focused
inputNode.focus() # updates via state change
wrapperNode.focus() # updates via state change
nextAnimationFrame()
expect(inputNode.offsetTop).toBe (5 * lineHeightInPixels) - editor.getScrollTop()
expect(inputNode.offsetLeft).toBe (4 * charWidth) - editor.getScrollLeft()

# In bounds, not focused
inputNode.blur() # updates via state change
nextAnimationFrame()
expect(inputNode.offsetTop).toBe 0
expect(inputNode.offsetLeft).toBe 0

Expand All @@ -1518,6 +1516,7 @@ describe "TextEditorComponent", ->

# Out of bounds, focused
inputNode.focus() # updates via state change
nextAnimationFrame()
expect(inputNode.offsetTop).toBe 0
expect(inputNode.offsetLeft).toBe 0

Expand Down Expand Up @@ -1839,9 +1838,11 @@ describe "TextEditorComponent", ->
it "adds the 'is-focused' class to the editor when the hidden input is focused", ->
expect(document.activeElement).toBe document.body
inputNode.focus()
nextAnimationFrame()
expect(componentNode.classList.contains('is-focused')).toBe true
expect(wrapperView.hasClass('is-focused')).toBe true
inputNode.blur()
nextAnimationFrame()
expect(componentNode.classList.contains('is-focused')).toBe false
expect(wrapperView.hasClass('is-focused')).toBe false

Expand Down Expand Up @@ -2351,11 +2352,11 @@ describe "TextEditorComponent", ->
wrapperView.appendTo(hiddenParent)

{component} = wrapperView
componentNode = component.getDOMNode()
componentNode = component.domNode
expect(componentNode.querySelectorAll('.line').length).toBe 0

hiddenParent.style.display = 'block'
advanceClock(component.domPollingInterval)
advanceClock(atom.views.documentPollingInterval)

expect(componentNode.querySelectorAll('.line').length).toBeGreaterThan 0

Expand Down Expand Up @@ -2465,22 +2466,23 @@ describe "TextEditorComponent", ->
expect(parseInt(newHeight)).toBeLessThan wrapperNode.offsetHeight
wrapperNode.style.height = newHeight

advanceClock(component.domPollingInterval)
advanceClock(atom.views.documentPollingInterval)
nextAnimationFrame()
expect(componentNode.querySelectorAll('.line')).toHaveLength(4 + lineOverdrawMargin + 1)

gutterWidth = componentNode.querySelector('.gutter').offsetWidth
componentNode.style.width = gutterWidth + 14 * charWidth + editor.getVerticalScrollbarWidth() + 'px'
advanceClock(component.domPollingInterval)
nextAnimationFrame()
advanceClock(atom.views.documentPollingInterval)
nextAnimationFrame() # won't poll until cursor blinks
nextAnimationFrame() # handle update requested by poll
expect(componentNode.querySelector('.line').textContent).toBe "var quicksort "

it "accounts for the scroll view's padding when determining the wrap location", ->
scrollViewNode = componentNode.querySelector('.scroll-view')
scrollViewNode.style.paddingLeft = 20 + 'px'
componentNode.style.width = 30 * charWidth + 'px'

advanceClock(component.domPollingInterval)
advanceClock(atom.views.documentPollingInterval)
nextAnimationFrame()

expect(component.lineNodeForScreenRow(0).textContent).toBe "var quicksort = "
Expand Down Expand Up @@ -2558,7 +2560,7 @@ describe "TextEditorComponent", ->
expect(wrapperNode.classList.contains('mini')).toBe true

it "does not have an opaque background on lines", ->
expect(component.refs.lines.getDOMNode().getAttribute('style')).not.toContain 'background-color'
expect(component.linesComponent.domNode.getAttribute('style')).not.toContain 'background-color'

it "does not render invisible characters", ->
atom.config.set('editor.invisibles', eol: 'E')
Expand Down
10 changes: 5 additions & 5 deletions spec/text-editor-element-spec.coffee
Expand Up @@ -46,12 +46,12 @@ describe "TextEditorElement", ->
jasmine.attachToDOM(element)

component = element.component
expect(component.isMounted()).toBe true
expect(component.mounted).toBe true
element.remove()
expect(component.isMounted()).toBe false
expect(component.mounted).toBe false

jasmine.attachToDOM(element)
expect(element.component.isMounted()).toBe true
expect(element.component.mounted).toBe true

describe "when the editor.useShadowDOM config option is false", ->
it "mounts the react component and unmounts when removed from the dom", ->
Expand All @@ -61,9 +61,9 @@ describe "TextEditorElement", ->
jasmine.attachToDOM(element)

component = element.component
expect(component.isMounted()).toBe true
expect(component.mounted).toBe true
element.getModel().destroy()
expect(component.isMounted()).toBe false
expect(component.mounted).toBe false

describe "focus and blur handling", ->
describe "when the editor.useShadowDOM config option is true", ->
Expand Down
108 changes: 108 additions & 0 deletions spec/text-editor-presenter-spec.coffee
Expand Up @@ -323,6 +323,69 @@ describe "TextEditorPresenter", ->
expectStateUpdate presenter, -> atom.config.set("editor.scrollPastEnd", false)
expect(presenter.state.verticalScrollbar.scrollTop).toBe presenter.contentHeight - presenter.clientHeight

describe ".hiddenInput", ->
describe ".top/.left", ->
it "is positioned over the last cursor it is in view and the editor is focused", ->
editor.setCursorBufferPosition([3, 6])
presenter = buildPresenter(focused: false, explicitHeight: 50, contentFrameWidth: 300, horizontalScrollbarHeight: 0, verticalScrollbarWidth: 0)
expectValues presenter.state.hiddenInput, {top: 0, left: 0}

expectStateUpdate presenter, -> presenter.setFocused(true)
expectValues presenter.state.hiddenInput, {top: 3 * 10, left: 6 * 10}

expectStateUpdate presenter, -> presenter.setScrollTop(15)
expectValues presenter.state.hiddenInput, {top: (3 * 10) - 15, left: 6 * 10}

expectStateUpdate presenter, -> presenter.setScrollLeft(35)
expectValues presenter.state.hiddenInput, {top: (3 * 10) - 15, left: (6 * 10) - 35}

expectStateUpdate presenter, -> presenter.setScrollTop(40)
expectValues presenter.state.hiddenInput, {top: 0, left: (6 * 10) - 35}

expectStateUpdate presenter, -> presenter.setScrollLeft(70)
expectValues presenter.state.hiddenInput, {top: 0, left: 0}

expectStateUpdate presenter, -> editor.setCursorBufferPosition([11, 43])
expectValues presenter.state.hiddenInput, {top: 50 - 10, left: 300 - 10}

newCursor = null
expectStateUpdate presenter, -> newCursor = editor.addCursorAtBufferPosition([6, 10])
expectValues presenter.state.hiddenInput, {top: (6 * 10) - 40, left: (10 * 10) - 70}

expectStateUpdate presenter, -> newCursor.destroy()
expectValues presenter.state.hiddenInput, {top: 50 - 10, left: 300 - 10}

expectStateUpdate presenter, -> presenter.setFocused(false)
expectValues presenter.state.hiddenInput, {top: 0, left: 0}

describe ".height", ->
it "is assigned based on the line height", ->
presenter = buildPresenter()
expect(presenter.state.hiddenInput.height).toBe 10

expectStateUpdate presenter, -> presenter.setLineHeight(20)
expect(presenter.state.hiddenInput.height).toBe 20

describe ".width", ->
it "is assigned based on the width of the character following the cursor", ->
waitsForPromise -> atom.packages.activatePackage('language-javascript')

runs ->
editor.setCursorBufferPosition([3, 6])
presenter = buildPresenter()
expect(presenter.state.hiddenInput.width).toBe 10

expectStateUpdate presenter, -> presenter.setBaseCharacterWidth(15)
expect(presenter.state.hiddenInput.width).toBe 15

expectStateUpdate presenter, -> presenter.setScopedCharacterWidth(['source.js', 'storage.modifier.js'], 'r', 20)
expect(presenter.state.hiddenInput.width).toBe 20

it "is 2px at the end of lines", ->
presenter = buildPresenter()
editor.setCursorBufferPosition([3, Infinity])
expect(presenter.state.hiddenInput.width).toBe 2

describe ".content", ->
describe ".scrollingVertically", ->
it "is true for ::stoppedScrollingDelay milliseconds following a changes to ::scrollTop", ->
Expand Down Expand Up @@ -1907,6 +1970,42 @@ describe "TextEditorPresenter", ->
editor.undo()
expect(lineNumberStateForScreenRow(presenter, 11).foldable).toBe false

describe ".visible", ->
it "is true iff the editor isn't mini, ::isGutterVisible is true on the editor, and 'editor.showLineNumbers' is enabled in config", ->
presenter = buildPresenter()

expect(editor.isGutterVisible()).toBe true
expect(presenter.state.gutter.visible).toBe true

expectStateUpdate presenter, -> editor.setMini(true)
expect(presenter.state.gutter.visible).toBe false

expectStateUpdate presenter, -> editor.setMini(false)
expect(presenter.state.gutter.visible).toBe true

expectStateUpdate presenter, -> editor.setGutterVisible(false)
expect(presenter.state.gutter.visible).toBe false

expectStateUpdate presenter, -> editor.setGutterVisible(true)
expect(presenter.state.gutter.visible).toBe true

expectStateUpdate presenter, -> atom.config.set('editor.showLineNumbers', false)
expect(presenter.state.gutter.visible).toBe false

it "updates when the editor's grammar changes", ->
presenter = buildPresenter()

atom.config.set('editor.showLineNumbers', false, scopeSelector: '.source.js')
expect(presenter.state.gutter.visible).toBe true
stateUpdated = false
presenter.onDidUpdateState -> stateUpdated = true

waitsForPromise -> atom.packages.activatePackage('language-javascript')

runs ->
expect(stateUpdated).toBe true
expect(presenter.state.gutter.visible).toBe false

describe ".height", ->
it "tracks the computed content height if ::autoHeight is true so the editor auto-expands vertically", ->
presenter = buildPresenter(explicitHeight: null, autoHeight: true)
Expand All @@ -1924,6 +2023,15 @@ describe "TextEditorPresenter", ->
expectStateUpdate presenter, -> editor.getBuffer().append("\n\n\n")
expect(presenter.state.height).toBe editor.getScreenLineCount() * 20

describe ".focused", ->
it "tracks the value of ::focused", ->
presenter = buildPresenter(focused: false)
expect(presenter.state.focused).toBe false
expectStateUpdate presenter, -> presenter.setFocused(true)
expect(presenter.state.focused).toBe true
expectStateUpdate presenter, -> presenter.setFocused(false)
expect(presenter.state.focused).toBe false

# disabled until we fix an issue with display buffer markers not updating when
# they are moved on screen but not in the buffer
xdescribe "when the model and view measurements are mutated randomly", ->
Expand Down