From 025370b9f88085162f2d61a625cf09b24e34f0bf Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Thu, 12 Jun 2014 16:17:18 -0700 Subject: [PATCH 01/11] Add editor component resize spec --- spec/editor-component-spec.coffee | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/editor-component-spec.coffee b/spec/editor-component-spec.coffee index 8baea2607..dbc0ed296 100644 --- a/spec/editor-component-spec.coffee +++ b/spec/editor-component-spec.coffee @@ -1265,6 +1265,14 @@ describe "EditorComponent", -> wrapperView.show() expect(node.querySelector('.cursor').style['-webkit-transform']).toBe "translate3d(#{9 * charWidth}px, 0px, 0px)" + describe "when the editor component is resized", -> + it "updates the size in the editor model", -> + originalHeight = editor.getHeight() + fourLinesHeight = 4 * editor.getLineHeightInPixels() + node.style.height = "#{originalHeight - fourLinesHeight}px" + component.forceUpdate() + expect(editor.getHeight()).not.toBe originalHeight + buildMouseEvent = (type, properties...) -> properties = extend({bubbles: true, cancelable: true}, properties...) event = new MouseEvent(type, properties) From e96d2dbd17bef92cd0d18bb728ad318b3314d600 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Thu, 12 Jun 2014 16:17:45 -0700 Subject: [PATCH 02/11] Add the simplest resize fix that will work. --- src/editor-component.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/editor-component.coffee b/src/editor-component.coffee index d278905ac..4e1e2ab34 100644 --- a/src/editor-component.coffee +++ b/src/editor-component.coffee @@ -176,6 +176,7 @@ EditorComponent = React.createClass @updateParentViewFocusedClassIfNeeded(prevState) @measureScrollbars() if @measuringScrollbars @measureLineHeightAndCharWidthsIfNeeded(prevState) + @requestScrollViewMeasurement() @props.parentView.trigger 'editor:display-updated' requestUpdate: -> @@ -266,7 +267,6 @@ EditorComponent = React.createClass node.addEventListener 'focus', @onFocus # For some reason, React's built in focus events seem to bubble scrollViewNode = @refs.scrollView.getDOMNode() - scrollViewNode.addEventListener 'overflowchanged', @onScrollViewOverflowChanged scrollViewNode.addEventListener 'scroll', @onScrollViewScroll window.addEventListener('resize', @onWindowResize) From 8d84a97b2bca406bc87cba2af40d0b9d25788a5d Mon Sep 17 00:00:00 2001 From: probablycorey Date: Fri, 13 Jun 2014 11:36:40 -0700 Subject: [PATCH 03/11] Move scroll view measurements to componentWillUpdate --- src/editor-component.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/editor-component.coffee b/src/editor-component.coffee index 4e1e2ab34..7ac0e8c57 100644 --- a/src/editor-component.coffee +++ b/src/editor-component.coffee @@ -167,6 +167,7 @@ EditorComponent = React.createClass componentWillUpdate: -> if @props.editor.isAlive() + @requestScrollViewMeasurement() @props.parentView.trigger 'cursor:moved' if @cursorsMoved @props.parentView.trigger 'selection:changed' if @selectionChanged @@ -176,7 +177,6 @@ EditorComponent = React.createClass @updateParentViewFocusedClassIfNeeded(prevState) @measureScrollbars() if @measuringScrollbars @measureLineHeightAndCharWidthsIfNeeded(prevState) - @requestScrollViewMeasurement() @props.parentView.trigger 'editor:display-updated' requestUpdate: -> From 9e6756ed6d387d7cc94f3a61b2c9a40b92de1cb9 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Fri, 13 Jun 2014 17:06:18 -0600 Subject: [PATCH 04/11] Don't perform an update in response scroll view dimension changes We always measure the scroll view in the ::componentWillUpdate hook, so performing *another* update in response to the measurement causes an invariant violation in react. Whenever we are measuring, we are always already updating. --- spec/editor-component-spec.coffee | 62 ++++++++++++++++--------------- src/editor-component.coffee | 30 +++++++++------ 2 files changed, 51 insertions(+), 41 deletions(-) diff --git a/spec/editor-component-spec.coffee b/spec/editor-component-spec.coffee index dbc0ed296..4d1b346da 100644 --- a/spec/editor-component-spec.coffee +++ b/spec/editor-component-spec.coffee @@ -47,15 +47,19 @@ describe "EditorComponent", -> node.style.height = editor.getLineCount() * lineHeightInPixels + 'px' node.style.width = '1000px' - component.measureScrollView() + measureScrollView() afterEach -> contentNode.style.width = '' + measureScrollView = -> + component.measureScrollView() + component.forceUpdate() + describe "line rendering", -> it "renders the currently-visible lines plus the overdraw margin", -> node.style.height = 4.5 * lineHeightInPixels + 'px' - component.measureScrollView() + measureScrollView() linesNode = node.querySelector('.lines') expect(linesNode.style['-webkit-transform']).toBe "translate3d(0px, 0px, 0px)" @@ -122,7 +126,7 @@ describe "EditorComponent", -> it "renders the .lines div at the full height of the editor if there aren't enough lines to scroll vertically", -> editor.setText('') node.style.height = '300px' - component.measureScrollView() + measureScrollView() linesNode = node.querySelector('.lines') expect(linesNode.offsetHeight).toBe 300 @@ -166,7 +170,7 @@ describe "EditorComponent", -> editor.setText "a line that wraps " editor.setSoftWrap(true) node.style.width = 16 * charWidth + 'px' - component.measureScrollView() + measureScrollView() it "doesn't show end of line invisibles at the end of wrapped lines", -> expect(component.lineNodeForScreenRow(0).textContent).toBe "a line that " @@ -242,7 +246,7 @@ describe "EditorComponent", -> it "renders the currently-visible line numbers", -> node.style.height = 4.5 * lineHeightInPixels + 'px' - component.measureScrollView() + measureScrollView() expect(node.querySelectorAll('.line-number').length).toBe 6 + 2 + 1 # line overdraw margin below + dummy line number expect(component.lineNumberNodeForScreenRow(0).textContent).toBe "#{nbsp}1" @@ -282,7 +286,7 @@ describe "EditorComponent", -> editor.setSoftWrap(true) node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 30 * charWidth + 'px' - component.measureScrollView() + measureScrollView() expect(node.querySelectorAll('.line-number').length).toBe 6 + lineOverdrawMargin + 1 # 1 dummy line node expect(component.lineNumberNodeForScreenRow(0).textContent).toBe "#{nbsp}1" @@ -428,7 +432,7 @@ describe "EditorComponent", -> describe "when decorations are applied to buffer rows", -> it "renders line number classes based on the decorations on their buffer row", -> node.style.height = 4.5 * lineHeightInPixels + 'px' - component.measureScrollView() + measureScrollView() expect(component.lineNumberNodeForScreenRow(9)).not.toBeDefined() @@ -465,7 +469,7 @@ describe "EditorComponent", -> editor.setSoftWrap(true) node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 30 * charWidth + 'px' - component.measureScrollView() + measureScrollView() expect(lineNumberHasClass(2, 'no-wrap')).toBe true expect(lineNumberHasClass(2, 'wrap-me')).toBe true @@ -582,7 +586,7 @@ describe "EditorComponent", -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 20 * lineHeightInPixels + 'px' - component.measureScrollView() + measureScrollView() cursorNodes = node.querySelectorAll('.cursor') expect(cursorNodes.length).toBe 1 @@ -781,7 +785,7 @@ describe "EditorComponent", -> inputNode = node.querySelector('.hidden-input') node.style.height = 5 * lineHeightInPixels + 'px' node.style.width = 10 * charWidth + 'px' - component.measureScrollView() + measureScrollView() expect(editor.getCursorScreenPosition()).toEqual [0, 0] editor.setScrollTop(3 * lineHeightInPixels) @@ -827,7 +831,7 @@ describe "EditorComponent", -> it "moves the cursor to the nearest screen position", -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 10 * charWidth + 'px' - component.measureScrollView() + measureScrollView() editor.setScrollTop(3.5 * lineHeightInPixels) editor.setScrollLeft(2 * charWidth) @@ -952,7 +956,7 @@ describe "EditorComponent", -> describe "scrolling", -> it "updates the vertical scrollbar when the scrollTop is changed in the model", -> node.style.height = 4.5 * lineHeightInPixels + 'px' - component.measureScrollView() + measureScrollView() expect(verticalScrollbarNode.scrollTop).toBe 0 @@ -961,7 +965,7 @@ describe "EditorComponent", -> it "updates the horizontal scrollbar and the x transform of the lines based on the scrollLeft of the model", -> node.style.width = 30 * charWidth + 'px' - component.measureScrollView() + measureScrollView() linesNode = node.querySelector('.lines') expect(linesNode.style['-webkit-transform']).toBe "translate3d(0px, 0px, 0px)" @@ -973,7 +977,7 @@ describe "EditorComponent", -> it "updates the scrollLeft of the model when the scrollLeft of the horizontal scrollbar changes", -> node.style.width = 30 * charWidth + 'px' - component.measureScrollView() + measureScrollView() expect(editor.getScrollLeft()).toBe 0 horizontalScrollbarNode.scrollLeft = 100 @@ -984,7 +988,7 @@ describe "EditorComponent", -> it "does not obscure the last line with the horizontal scrollbar", -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 10 * charWidth + 'px' - component.measureScrollView() + measureScrollView() editor.setScrollBottom(editor.getScrollHeight()) lastLineNode = component.lineNodeForScreenRow(editor.getLastScreenRow()) bottomOfLastLine = lastLineNode.getBoundingClientRect().bottom @@ -993,7 +997,7 @@ describe "EditorComponent", -> # Scroll so there's no space below the last line when the horizontal scrollbar disappears node.style.width = 100 * charWidth + 'px' - component.measureScrollView() + measureScrollView() bottomOfLastLine = lastLineNode.getBoundingClientRect().bottom bottomOfEditor = node.getBoundingClientRect().bottom expect(bottomOfLastLine).toBe bottomOfEditor @@ -1001,7 +1005,7 @@ describe "EditorComponent", -> it "does not obscure the last character of the longest line with the vertical scrollbar", -> node.style.height = 7 * lineHeightInPixels + 'px' node.style.width = 10 * charWidth + 'px' - component.measureScrollView() + measureScrollView() editor.setScrollLeft(Infinity) @@ -1015,19 +1019,19 @@ describe "EditorComponent", -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = '1000px' - component.measureScrollView() + measureScrollView() expect(verticalScrollbarNode.style.display).toBe '' expect(horizontalScrollbarNode.style.display).toBe 'none' node.style.width = 10 * charWidth + 'px' - component.measureScrollView() + measureScrollView() expect(verticalScrollbarNode.style.display).toBe '' expect(horizontalScrollbarNode.style.display).toBe '' node.style.height = 20 * lineHeightInPixels + 'px' - component.measureScrollView() + measureScrollView() expect(verticalScrollbarNode.style.display).toBe 'none' expect(horizontalScrollbarNode.style.display).toBe '' @@ -1035,7 +1039,7 @@ describe "EditorComponent", -> it "makes the dummy scrollbar divs only as tall/wide as the actual scrollbars", -> node.style.height = 4 * lineHeightInPixels + 'px' node.style.width = 10 * charWidth + 'px' - component.measureScrollView() + measureScrollView() atom.themes.applyStylesheet "test", """ ::-webkit-scrollbar { @@ -1058,19 +1062,19 @@ describe "EditorComponent", -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = '1000px' - component.measureScrollView() + measureScrollView() expect(verticalScrollbarNode.style.bottom).toBe '' expect(horizontalScrollbarNode.style.right).toBe verticalScrollbarNode.offsetWidth + 'px' expect(scrollbarCornerNode.style.display).toBe 'none' node.style.width = 10 * charWidth + 'px' - component.measureScrollView() + measureScrollView() expect(verticalScrollbarNode.style.bottom).toBe horizontalScrollbarNode.offsetHeight + 'px' expect(horizontalScrollbarNode.style.right).toBe verticalScrollbarNode.offsetWidth + 'px' expect(scrollbarCornerNode.style.display).toBe '' node.style.height = 20 * lineHeightInPixels + 'px' - component.measureScrollView() + measureScrollView() expect(verticalScrollbarNode.style.bottom).toBe horizontalScrollbarNode.offsetHeight + 'px' expect(horizontalScrollbarNode.style.right).toBe '' expect(scrollbarCornerNode.style.display).toBe 'none' @@ -1078,7 +1082,7 @@ describe "EditorComponent", -> it "accounts for the width of the gutter in the scrollWidth of the horizontal scrollbar", -> gutterNode = node.querySelector('.gutter') node.style.width = 10 * charWidth + 'px' - component.measureScrollView() + measureScrollView() expect(horizontalScrollbarNode.scrollWidth).toBe gutterNode.offsetWidth + editor.getScrollWidth() @@ -1090,7 +1094,7 @@ describe "EditorComponent", -> beforeEach -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 20 * charWidth + 'px' - component.measureScrollView() + measureScrollView() it "updates the scrollLeft or scrollTop on mousewheel events depending on which delta is greater (x or y)", -> expect(verticalScrollbarNode.scrollTop).toBe 0 @@ -1128,7 +1132,7 @@ describe "EditorComponent", -> it "keeps the line on the DOM if it is scrolled off-screen", -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 20 * charWidth + 'px' - component.measureScrollView() + measureScrollView() lineNode = node.querySelector('.line') wheelEvent = new WheelEvent('mousewheel', wheelDeltaX: 0, wheelDeltaY: -500) @@ -1140,7 +1144,7 @@ describe "EditorComponent", -> it "does not set the mouseWheelScreenRow if scrolling horizontally", -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 20 * charWidth + 'px' - component.measureScrollView() + measureScrollView() lineNode = node.querySelector('.line') wheelEvent = new WheelEvent('mousewheel', wheelDeltaX: 10, wheelDeltaY: 0) @@ -1184,7 +1188,7 @@ describe "EditorComponent", -> it "keeps the line number on the DOM if it is scrolled off-screen", -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 20 * charWidth + 'px' - component.measureScrollView() + measureScrollView() lineNumberNode = node.querySelectorAll('.line-number')[1] wheelEvent = new WheelEvent('mousewheel', wheelDeltaX: 0, wheelDeltaY: -500) diff --git a/src/editor-component.coffee b/src/editor-component.coffee index 7ac0e8c57..97bd1e30b 100644 --- a/src/editor-component.coffee +++ b/src/editor-component.coffee @@ -22,6 +22,7 @@ EditorComponent = React.createClass selectOnMouseMove: false batchingUpdates: false updateRequested: false + updatesSuppressed: false cursorsMoved: false selectionChanged: false selectionAdded: false @@ -180,10 +181,16 @@ EditorComponent = React.createClass @props.parentView.trigger 'editor:display-updated' requestUpdate: -> - if @batchingUpdates - @updateRequested = true - else - @forceUpdate() + unless @updatesSuppressed + if @batchingUpdates + @updateRequested = true + else + @forceUpdate() + + suppressUpdates: (fn) -> + @updatesSuppressed = true + fn() + @updatesSuppressed = false getRenderedRowRange: -> {editor, lineOverdrawMargin} = @props @@ -256,8 +263,6 @@ EditorComponent = React.createClass @subscribe editor, 'decoration-changed', @onDecorationChanged @subscribe editor.$scrollTop.changes, @onScrollTopChanged @subscribe editor.$scrollLeft.changes, @requestUpdate - @subscribe editor.$height.changes, @requestUpdate - @subscribe editor.$width.changes, @requestUpdate @subscribe editor.$defaultCharWidth.changes, @requestUpdate @subscribe editor.$lineHeightInPixels.changes, @requestUpdate @@ -596,13 +601,14 @@ EditorComponent = React.createClass {position} = getComputedStyle(editorNode) {width, height} = editorNode.style - if position is 'absolute' or height - clientHeight = scrollViewNode.clientHeight - editor.setHeight(clientHeight) if clientHeight > 0 + @suppressUpdates -> + if position is 'absolute' or height + clientHeight = scrollViewNode.clientHeight + editor.setHeight(clientHeight) if clientHeight > 0 - if position is 'absolute' or width - clientWidth = scrollViewNode.clientWidth - editor.setWidth(clientWidth) if clientWidth > 0 + if position is 'absolute' or width + clientWidth = scrollViewNode.clientWidth + editor.setWidth(clientWidth) if clientWidth > 0 measureLineHeightAndCharWidthsIfNeeded: (prevState) -> if not isEqualForProperties(prevState, @state, 'lineHeight', 'fontSize', 'fontFamily') From c06f5911c62a05d63aaffe9b2611892e44359a54 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 17 Jun 2014 13:26:56 -0700 Subject: [PATCH 05/11] Update editor height change spec --- spec/editor-component-spec.coffee | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/spec/editor-component-spec.coffee b/spec/editor-component-spec.coffee index 4d1b346da..f2354f9cd 100644 --- a/spec/editor-component-spec.coffee +++ b/spec/editor-component-spec.coffee @@ -1271,11 +1271,16 @@ describe "EditorComponent", -> describe "when the editor component is resized", -> it "updates the size in the editor model", -> + spyOn(editor, 'setHeight').andCallThrough() originalHeight = editor.getHeight() fourLinesHeight = 4 * editor.getLineHeightInPixels() node.style.height = "#{originalHeight - fourLinesHeight}px" - component.forceUpdate() - expect(editor.getHeight()).not.toBe originalHeight + + waitsFor -> + editor.setHeight.callCount > 0 + + runs -> + expect(editor.getHeight()).not.toBe originalHeight buildMouseEvent = (type, properties...) -> properties = extend({bubbles: true, cancelable: true}, properties...) From 0a671fc386aa08e7fc2bd7873c6f76b999755e8c Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 17 Jun 2014 13:28:05 -0700 Subject: [PATCH 06/11] Add a div that triggers overflowchanged events on resize --- src/editor-component.coffee | 33 +++++++++++++++++---------------- static/editor.less | 5 +++++ 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/editor-component.coffee b/src/editor-component.coffee index 97bd1e30b..d65697630 100644 --- a/src/editor-component.coffee +++ b/src/editor-component.coffee @@ -73,6 +73,9 @@ EditorComponent = React.createClass className += ' has-selection' if hasSelection div className: className, style: {fontSize, lineHeight, fontFamily}, tabIndex: -1, + div ref: 'overflowTester', className: 'overflow-tester', (div ref: 'overflowExpander', className: 'overflow-expander') + + GutterComponent { ref: 'gutter', editor, renderedRowRange, maxLineNumberDigits, scrollTop, scrollHeight, lineHeightInPixels, @pendingChanges, mouseWheelScreenRow, @@ -164,11 +167,9 @@ EditorComponent = React.createClass componentWillUnmount: -> @unsubscribe() - window.removeEventListener('resize', @onWindowResize) componentWillUpdate: -> if @props.editor.isAlive() - @requestScrollViewMeasurement() @props.parentView.trigger 'cursor:moved' if @cursorsMoved @props.parentView.trigger 'selection:changed' if @selectionChanged @@ -271,9 +272,11 @@ EditorComponent = React.createClass node.addEventListener 'mousewheel', @onMouseWheel node.addEventListener 'focus', @onFocus # For some reason, React's built in focus events seem to bubble + overflowTester = @refs.overflowTester.getDOMNode() + overflowTester.addEventListener 'overflowchanged', @requestScrollViewMeasurement + scrollViewNode = @refs.scrollView.getDOMNode() scrollViewNode.addEventListener 'scroll', @onScrollViewScroll - window.addEventListener('resize', @onWindowResize) listenForCommands: -> {parentView, editor, mini} = @props @@ -449,12 +452,6 @@ EditorComponent = React.createClass @pendingVerticalScrollDelta = 0 @pendingHorizontalScrollDelta = 0 - onScrollViewOverflowChanged: -> - @requestScrollViewMeasurement() - - onWindowResize: -> - @requestScrollViewMeasurement() - onScrollViewScroll: -> console.warn "EditorScrollView scroll position changed, and it shouldn't have. If you can reproduce this, please report it." scrollViewNode = @refs.scrollView.getDOMNode() @@ -597,18 +594,22 @@ EditorComponent = React.createClass {editor} = @props editorNode = @getDOMNode() + overflowExpander = @refs.overflowExpander.getDOMNode() scrollViewNode = @refs.scrollView.getDOMNode() {position} = getComputedStyle(editorNode) {width, height} = editorNode.style - @suppressUpdates -> - if position is 'absolute' or height - clientHeight = scrollViewNode.clientHeight - editor.setHeight(clientHeight) if clientHeight > 0 + if position is 'absolute' or height + clientHeight = scrollViewNode.clientHeight + if clientHeight > 0 + overflowExpander.style.height = clientHeight + "px" + editor.setHeight(clientHeight) - if position is 'absolute' or width - clientWidth = scrollViewNode.clientWidth - editor.setWidth(clientWidth) if clientWidth > 0 + if position is 'absolute' or width + clientWidth = scrollViewNode.clientWidth + if clientWidth > 0 + overflowExpander.style.height = clientHeight + "px" + editor.setWidth(clientWidth) measureLineHeightAndCharWidthsIfNeeded: (prevState) -> if not isEqualForProperties(prevState, @state, 'lineHeight', 'fontSize', 'fontFamily') diff --git a/static/editor.less b/static/editor.less index ea95b530d..cbdc4b970 100644 --- a/static/editor.less +++ b/static/editor.less @@ -3,6 +3,11 @@ @import "octicon-mixins"; .editor.react { + .overflow-tester { + height: 100%; + overflow: hidden; + } + .underlayer { position: absolute; top: 0; From 0255e44f00a4e5a77129af5afb5739b85acb386c Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 17 Jun 2014 13:38:08 -0700 Subject: [PATCH 07/11] Remove suppressUpdates --- src/editor-component.coffee | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/editor-component.coffee b/src/editor-component.coffee index d65697630..0bf755be9 100644 --- a/src/editor-component.coffee +++ b/src/editor-component.coffee @@ -22,7 +22,6 @@ EditorComponent = React.createClass selectOnMouseMove: false batchingUpdates: false updateRequested: false - updatesSuppressed: false cursorsMoved: false selectionChanged: false selectionAdded: false @@ -182,16 +181,10 @@ EditorComponent = React.createClass @props.parentView.trigger 'editor:display-updated' requestUpdate: -> - unless @updatesSuppressed - if @batchingUpdates - @updateRequested = true - else - @forceUpdate() - - suppressUpdates: (fn) -> - @updatesSuppressed = true - fn() - @updatesSuppressed = false + if @batchingUpdates + @updateRequested = true + else + @forceUpdate() getRenderedRowRange: -> {editor, lineOverdrawMargin} = @props From f3a4d32a325241f627b0d004e0964d7d6f549149 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 17 Jun 2014 14:07:26 -0700 Subject: [PATCH 08/11] Remove width setting of overflowExpander --- src/editor-component.coffee | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/editor-component.coffee b/src/editor-component.coffee index 0bf755be9..803d95c95 100644 --- a/src/editor-component.coffee +++ b/src/editor-component.coffee @@ -600,9 +600,7 @@ EditorComponent = React.createClass if position is 'absolute' or width clientWidth = scrollViewNode.clientWidth - if clientWidth > 0 - overflowExpander.style.height = clientHeight + "px" - editor.setWidth(clientWidth) + editor.setWidth(clientWidth) if clientWidth > 0 measureLineHeightAndCharWidthsIfNeeded: (prevState) -> if not isEqualForProperties(prevState, @state, 'lineHeight', 'fontSize', 'fontFamily') From 068c1e62496db208e864f824fa6ce5609dcf4eeb Mon Sep 17 00:00:00 2001 From: Corey Johnson & Nathan Sobo Date: Tue, 17 Jun 2014 16:07:07 -0700 Subject: [PATCH 09/11] Use polling to detect editor resize --- spec/editor-component-spec.coffee | 20 +++++++++++--------- src/editor-component.coffee | 29 ++++++++++++++--------------- static/editor.less | 5 ----- 3 files changed, 25 insertions(+), 29 deletions(-) diff --git a/spec/editor-component-spec.coffee b/spec/editor-component-spec.coffee index f2354f9cd..5ebdec586 100644 --- a/spec/editor-component-spec.coffee +++ b/spec/editor-component-spec.coffee @@ -1270,17 +1270,19 @@ describe "EditorComponent", -> expect(node.querySelector('.cursor').style['-webkit-transform']).toBe "translate3d(#{9 * charWidth}px, 0px, 0px)" describe "when the editor component is resized", -> - it "updates the size in the editor model", -> - spyOn(editor, 'setHeight').andCallThrough() - originalHeight = editor.getHeight() - fourLinesHeight = 4 * editor.getLineHeightInPixels() - node.style.height = "#{originalHeight - fourLinesHeight}px" + it "updates the component based on a new size", -> + editor.setSoftWrap(true) + newHeight = 4 * editor.getLineHeightInPixels() + "px" + expect(newHeight).toBeLessThan node.style.height + node.style.height = newHeight - waitsFor -> - editor.setHeight.callCount > 0 + advanceClock(component.scrollViewMeasurementInterval) + expect(node.querySelectorAll('.line')).toHaveLength(4 + lineOverdrawMargin + 1) - runs -> - expect(editor.getHeight()).not.toBe originalHeight + gutterWidth = node.querySelector('.gutter').offsetWidth + node.style.width = gutterWidth + 14 * charWidth + 'px' + advanceClock(component.scrollViewMeasurementInterval) + expect(node.querySelector('.line').textContent).toBe "var quicksort " buildMouseEvent = (type, properties...) -> properties = extend({bubbles: true, cancelable: true}, properties...) diff --git a/src/editor-component.coffee b/src/editor-component.coffee index 803d95c95..6f8c8de76 100644 --- a/src/editor-component.coffee +++ b/src/editor-component.coffee @@ -37,6 +37,7 @@ EditorComponent = React.createClass scrollViewMeasurementRequested: false measureLineHeightAndDefaultCharWidthWhenShown: false inputEnabled: true + scrollViewMeasurementInterval: 100 render: -> {focused, fontSize, lineHeight, fontFamily, showIndentGuide, showInvisibles, visible} = @state @@ -72,9 +73,6 @@ EditorComponent = React.createClass className += ' has-selection' if hasSelection div className: className, style: {fontSize, lineHeight, fontFamily}, tabIndex: -1, - div ref: 'overflowTester', className: 'overflow-tester', (div ref: 'overflowExpander', className: 'overflow-expander') - - GutterComponent { ref: 'gutter', editor, renderedRowRange, maxLineNumberDigits, scrollTop, scrollHeight, lineHeightInPixels, @pendingChanges, mouseWheelScreenRow, @@ -150,6 +148,8 @@ EditorComponent = React.createClass componentDidMount: -> {editor} = @props + @scrollViewMeasurementIntervalId = setInterval(@requestScrollViewMeasurement, @scrollViewMeasurementInterval) + @observeEditor() @listenForDOMEvents() @listenForCommands() @@ -166,6 +166,8 @@ EditorComponent = React.createClass componentWillUnmount: -> @unsubscribe() + clearInterval(@scrollViewMeasurementIntervalId) + @scrollViewMeasurementIntervalId = null componentWillUpdate: -> if @props.editor.isAlive() @@ -255,6 +257,8 @@ EditorComponent = React.createClass @subscribe editor, 'selection-removed selection-screen-range-changed', @onSelectionChanged @subscribe editor, 'selection-added', @onSelectionAdded @subscribe editor, 'decoration-changed', @onDecorationChanged + @subscribe editor.$height.changes, @requestUpdate + @subscribe editor.$width.changes, @requestUpdate @subscribe editor.$scrollTop.changes, @onScrollTopChanged @subscribe editor.$scrollLeft.changes, @requestUpdate @subscribe editor.$defaultCharWidth.changes, @requestUpdate @@ -265,9 +269,6 @@ EditorComponent = React.createClass node.addEventListener 'mousewheel', @onMouseWheel node.addEventListener 'focus', @onFocus # For some reason, React's built in focus events seem to bubble - overflowTester = @refs.overflowTester.getDOMNode() - overflowTester.addEventListener 'overflowchanged', @requestScrollViewMeasurement - scrollViewNode = @refs.scrollView.getDOMNode() scrollViewNode.addEventListener 'scroll', @onScrollViewScroll @@ -587,20 +588,18 @@ EditorComponent = React.createClass {editor} = @props editorNode = @getDOMNode() - overflowExpander = @refs.overflowExpander.getDOMNode() scrollViewNode = @refs.scrollView.getDOMNode() {position} = getComputedStyle(editorNode) {width, height} = editorNode.style - if position is 'absolute' or height - clientHeight = scrollViewNode.clientHeight - if clientHeight > 0 - overflowExpander.style.height = clientHeight + "px" - editor.setHeight(clientHeight) + editor.batchUpdates -> + if position is 'absolute' or height + clientHeight = scrollViewNode.clientHeight + editor.setHeight(clientHeight) if clientHeight > 0 - if position is 'absolute' or width - clientWidth = scrollViewNode.clientWidth - editor.setWidth(clientWidth) if clientWidth > 0 + if position is 'absolute' or width + clientWidth = scrollViewNode.clientWidth + editor.setWidth(clientWidth) if clientWidth > 0 measureLineHeightAndCharWidthsIfNeeded: (prevState) -> if not isEqualForProperties(prevState, @state, 'lineHeight', 'fontSize', 'fontFamily') diff --git a/static/editor.less b/static/editor.less index cbdc4b970..ea95b530d 100644 --- a/static/editor.less +++ b/static/editor.less @@ -3,11 +3,6 @@ @import "octicon-mixins"; .editor.react { - .overflow-tester { - height: 100%; - overflow: hidden; - } - .underlayer { position: absolute; top: 0; From 4564a39392857bbd30cdc8aad48d361d85c0b212 Mon Sep 17 00:00:00 2001 From: Corey Johnson & Nathan Sobo Date: Tue, 17 Jun 2014 16:09:23 -0700 Subject: [PATCH 10/11] Remove measureScrollView helper --- spec/editor-component-spec.coffee | 62 +++++++++++++++---------------- 1 file changed, 29 insertions(+), 33 deletions(-) diff --git a/spec/editor-component-spec.coffee b/spec/editor-component-spec.coffee index 5ebdec586..7a5d4ced0 100644 --- a/spec/editor-component-spec.coffee +++ b/spec/editor-component-spec.coffee @@ -47,19 +47,15 @@ describe "EditorComponent", -> node.style.height = editor.getLineCount() * lineHeightInPixels + 'px' node.style.width = '1000px' - measureScrollView() + component.measureScrollView() afterEach -> contentNode.style.width = '' - measureScrollView = -> - component.measureScrollView() - component.forceUpdate() - describe "line rendering", -> it "renders the currently-visible lines plus the overdraw margin", -> node.style.height = 4.5 * lineHeightInPixels + 'px' - measureScrollView() + component.measureScrollView() linesNode = node.querySelector('.lines') expect(linesNode.style['-webkit-transform']).toBe "translate3d(0px, 0px, 0px)" @@ -126,7 +122,7 @@ describe "EditorComponent", -> it "renders the .lines div at the full height of the editor if there aren't enough lines to scroll vertically", -> editor.setText('') node.style.height = '300px' - measureScrollView() + component.measureScrollView() linesNode = node.querySelector('.lines') expect(linesNode.offsetHeight).toBe 300 @@ -170,7 +166,7 @@ describe "EditorComponent", -> editor.setText "a line that wraps " editor.setSoftWrap(true) node.style.width = 16 * charWidth + 'px' - measureScrollView() + component.measureScrollView() it "doesn't show end of line invisibles at the end of wrapped lines", -> expect(component.lineNodeForScreenRow(0).textContent).toBe "a line that " @@ -246,7 +242,7 @@ describe "EditorComponent", -> it "renders the currently-visible line numbers", -> node.style.height = 4.5 * lineHeightInPixels + 'px' - measureScrollView() + component.measureScrollView() expect(node.querySelectorAll('.line-number').length).toBe 6 + 2 + 1 # line overdraw margin below + dummy line number expect(component.lineNumberNodeForScreenRow(0).textContent).toBe "#{nbsp}1" @@ -286,7 +282,7 @@ describe "EditorComponent", -> editor.setSoftWrap(true) node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 30 * charWidth + 'px' - measureScrollView() + component.measureScrollView() expect(node.querySelectorAll('.line-number').length).toBe 6 + lineOverdrawMargin + 1 # 1 dummy line node expect(component.lineNumberNodeForScreenRow(0).textContent).toBe "#{nbsp}1" @@ -432,7 +428,7 @@ describe "EditorComponent", -> describe "when decorations are applied to buffer rows", -> it "renders line number classes based on the decorations on their buffer row", -> node.style.height = 4.5 * lineHeightInPixels + 'px' - measureScrollView() + component.measureScrollView() expect(component.lineNumberNodeForScreenRow(9)).not.toBeDefined() @@ -469,7 +465,7 @@ describe "EditorComponent", -> editor.setSoftWrap(true) node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 30 * charWidth + 'px' - measureScrollView() + component.measureScrollView() expect(lineNumberHasClass(2, 'no-wrap')).toBe true expect(lineNumberHasClass(2, 'wrap-me')).toBe true @@ -586,7 +582,7 @@ describe "EditorComponent", -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 20 * lineHeightInPixels + 'px' - measureScrollView() + component.measureScrollView() cursorNodes = node.querySelectorAll('.cursor') expect(cursorNodes.length).toBe 1 @@ -785,7 +781,7 @@ describe "EditorComponent", -> inputNode = node.querySelector('.hidden-input') node.style.height = 5 * lineHeightInPixels + 'px' node.style.width = 10 * charWidth + 'px' - measureScrollView() + component.measureScrollView() expect(editor.getCursorScreenPosition()).toEqual [0, 0] editor.setScrollTop(3 * lineHeightInPixels) @@ -831,7 +827,7 @@ describe "EditorComponent", -> it "moves the cursor to the nearest screen position", -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 10 * charWidth + 'px' - measureScrollView() + component.measureScrollView() editor.setScrollTop(3.5 * lineHeightInPixels) editor.setScrollLeft(2 * charWidth) @@ -956,7 +952,7 @@ describe "EditorComponent", -> describe "scrolling", -> it "updates the vertical scrollbar when the scrollTop is changed in the model", -> node.style.height = 4.5 * lineHeightInPixels + 'px' - measureScrollView() + component.measureScrollView() expect(verticalScrollbarNode.scrollTop).toBe 0 @@ -965,7 +961,7 @@ describe "EditorComponent", -> it "updates the horizontal scrollbar and the x transform of the lines based on the scrollLeft of the model", -> node.style.width = 30 * charWidth + 'px' - measureScrollView() + component.measureScrollView() linesNode = node.querySelector('.lines') expect(linesNode.style['-webkit-transform']).toBe "translate3d(0px, 0px, 0px)" @@ -977,7 +973,7 @@ describe "EditorComponent", -> it "updates the scrollLeft of the model when the scrollLeft of the horizontal scrollbar changes", -> node.style.width = 30 * charWidth + 'px' - measureScrollView() + component.measureScrollView() expect(editor.getScrollLeft()).toBe 0 horizontalScrollbarNode.scrollLeft = 100 @@ -988,7 +984,7 @@ describe "EditorComponent", -> it "does not obscure the last line with the horizontal scrollbar", -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 10 * charWidth + 'px' - measureScrollView() + component.measureScrollView() editor.setScrollBottom(editor.getScrollHeight()) lastLineNode = component.lineNodeForScreenRow(editor.getLastScreenRow()) bottomOfLastLine = lastLineNode.getBoundingClientRect().bottom @@ -997,7 +993,7 @@ describe "EditorComponent", -> # Scroll so there's no space below the last line when the horizontal scrollbar disappears node.style.width = 100 * charWidth + 'px' - measureScrollView() + component.measureScrollView() bottomOfLastLine = lastLineNode.getBoundingClientRect().bottom bottomOfEditor = node.getBoundingClientRect().bottom expect(bottomOfLastLine).toBe bottomOfEditor @@ -1005,7 +1001,7 @@ describe "EditorComponent", -> it "does not obscure the last character of the longest line with the vertical scrollbar", -> node.style.height = 7 * lineHeightInPixels + 'px' node.style.width = 10 * charWidth + 'px' - measureScrollView() + component.measureScrollView() editor.setScrollLeft(Infinity) @@ -1019,19 +1015,19 @@ describe "EditorComponent", -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = '1000px' - measureScrollView() + component.measureScrollView() expect(verticalScrollbarNode.style.display).toBe '' expect(horizontalScrollbarNode.style.display).toBe 'none' node.style.width = 10 * charWidth + 'px' - measureScrollView() + component.measureScrollView() expect(verticalScrollbarNode.style.display).toBe '' expect(horizontalScrollbarNode.style.display).toBe '' node.style.height = 20 * lineHeightInPixels + 'px' - measureScrollView() + component.measureScrollView() expect(verticalScrollbarNode.style.display).toBe 'none' expect(horizontalScrollbarNode.style.display).toBe '' @@ -1039,7 +1035,7 @@ describe "EditorComponent", -> it "makes the dummy scrollbar divs only as tall/wide as the actual scrollbars", -> node.style.height = 4 * lineHeightInPixels + 'px' node.style.width = 10 * charWidth + 'px' - measureScrollView() + component.measureScrollView() atom.themes.applyStylesheet "test", """ ::-webkit-scrollbar { @@ -1062,19 +1058,19 @@ describe "EditorComponent", -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = '1000px' - measureScrollView() + component.measureScrollView() expect(verticalScrollbarNode.style.bottom).toBe '' expect(horizontalScrollbarNode.style.right).toBe verticalScrollbarNode.offsetWidth + 'px' expect(scrollbarCornerNode.style.display).toBe 'none' node.style.width = 10 * charWidth + 'px' - measureScrollView() + component.measureScrollView() expect(verticalScrollbarNode.style.bottom).toBe horizontalScrollbarNode.offsetHeight + 'px' expect(horizontalScrollbarNode.style.right).toBe verticalScrollbarNode.offsetWidth + 'px' expect(scrollbarCornerNode.style.display).toBe '' node.style.height = 20 * lineHeightInPixels + 'px' - measureScrollView() + component.measureScrollView() expect(verticalScrollbarNode.style.bottom).toBe horizontalScrollbarNode.offsetHeight + 'px' expect(horizontalScrollbarNode.style.right).toBe '' expect(scrollbarCornerNode.style.display).toBe 'none' @@ -1082,7 +1078,7 @@ describe "EditorComponent", -> it "accounts for the width of the gutter in the scrollWidth of the horizontal scrollbar", -> gutterNode = node.querySelector('.gutter') node.style.width = 10 * charWidth + 'px' - measureScrollView() + component.measureScrollView() expect(horizontalScrollbarNode.scrollWidth).toBe gutterNode.offsetWidth + editor.getScrollWidth() @@ -1094,7 +1090,7 @@ describe "EditorComponent", -> beforeEach -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 20 * charWidth + 'px' - measureScrollView() + component.measureScrollView() it "updates the scrollLeft or scrollTop on mousewheel events depending on which delta is greater (x or y)", -> expect(verticalScrollbarNode.scrollTop).toBe 0 @@ -1132,7 +1128,7 @@ describe "EditorComponent", -> it "keeps the line on the DOM if it is scrolled off-screen", -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 20 * charWidth + 'px' - measureScrollView() + component.measureScrollView() lineNode = node.querySelector('.line') wheelEvent = new WheelEvent('mousewheel', wheelDeltaX: 0, wheelDeltaY: -500) @@ -1144,7 +1140,7 @@ describe "EditorComponent", -> it "does not set the mouseWheelScreenRow if scrolling horizontally", -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 20 * charWidth + 'px' - measureScrollView() + component.measureScrollView() lineNode = node.querySelector('.line') wheelEvent = new WheelEvent('mousewheel', wheelDeltaX: 10, wheelDeltaY: 0) @@ -1188,7 +1184,7 @@ describe "EditorComponent", -> it "keeps the line number on the DOM if it is scrolled off-screen", -> node.style.height = 4.5 * lineHeightInPixels + 'px' node.style.width = 20 * charWidth + 'px' - measureScrollView() + component.measureScrollView() lineNumberNode = node.querySelectorAll('.line-number')[1] wheelEvent = new WheelEvent('mousewheel', wheelDeltaX: 0, wheelDeltaY: -500) From 9be1427891b84f89fd241c0b96e09b21ffc5f75b Mon Sep 17 00:00:00 2001 From: probablycorey Date: Tue, 17 Jun 2014 16:25:47 -0700 Subject: [PATCH 11/11] Request scrollView measurement on resize events --- src/editor-component.coffee | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/editor-component.coffee b/src/editor-component.coffee index 6f8c8de76..64b94aa82 100644 --- a/src/editor-component.coffee +++ b/src/editor-component.coffee @@ -257,10 +257,10 @@ EditorComponent = React.createClass @subscribe editor, 'selection-removed selection-screen-range-changed', @onSelectionChanged @subscribe editor, 'selection-added', @onSelectionAdded @subscribe editor, 'decoration-changed', @onDecorationChanged - @subscribe editor.$height.changes, @requestUpdate - @subscribe editor.$width.changes, @requestUpdate @subscribe editor.$scrollTop.changes, @onScrollTopChanged @subscribe editor.$scrollLeft.changes, @requestUpdate + @subscribe editor.$height.changes, @requestUpdate + @subscribe editor.$width.changes, @requestUpdate @subscribe editor.$defaultCharWidth.changes, @requestUpdate @subscribe editor.$lineHeightInPixels.changes, @requestUpdate @@ -271,6 +271,7 @@ EditorComponent = React.createClass scrollViewNode = @refs.scrollView.getDOMNode() scrollViewNode.addEventListener 'scroll', @onScrollViewScroll + window.addEventListener 'resize', @requestScrollViewMeasurement listenForCommands: -> {parentView, editor, mini} = @props