From 0c89db37a5d514c32c592c4b7212c625c5fe29c3 Mon Sep 17 00:00:00 2001
From: Nathan Sobo <nathan@github.com>
Date: Fri, 11 May 2012 17:51:47 -0600
Subject: [PATCH] Editor scroll position is correctly restored on refresh.

When we attach the editor we need to scroll it to the correct position before rendering lines. But to scroll it to the right position, the lines container and the scrollbar need to be assigned a height that allows them to scroll. So now we call `prepareForVerticalScrolling` on attach, then set the scroll position based on the edit session and render the visible lines.
---
 spec/app/editor-spec.coffee | 20 ++++++++++++++++++--
 src/app/editor.coffee       | 31 +++++++++++++++++++++++++------
 2 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/spec/app/editor-spec.coffee b/spec/app/editor-spec.coffee
index f94881b4d..2cba35f58 100644
--- a/spec/app/editor-spec.coffee
+++ b/spec/app/editor-spec.coffee
@@ -45,7 +45,14 @@ describe "Editor", ->
 
   describe ".copy()", ->
     it "builds a new editor with the same edit sessions, cursor position, and scroll position as the receiver", ->
-      editor.setCursorScreenPosition([1, 1])
+      rootView.attachToDom()
+      rootView.height(8 * editor.lineHeight)
+      rootView.width(50 * editor.charWidth)
+
+      editor.setCursorScreenPosition([5, 20])
+      advanceClock()
+      editor.verticalScrollbar.scrollTop(1.5 * editor.lineHeight)
+      editor.scrollView.scrollLeft(44)
 
       # prove this test covers serialization and deserialization
       spyOn(editor, 'serialize').andCallThrough()
@@ -54,12 +61,21 @@ describe "Editor", ->
       newEditor = editor.copy()
       expect(editor.serialize).toHaveBeenCalled()
       expect(Editor.deserialize).toHaveBeenCalled()
+
       expect(newEditor.buffer).toBe editor.buffer
       expect(newEditor.getCursorScreenPosition()).toEqual editor.getCursorScreenPosition()
-
       expect(newEditor.editSessions[0]).toEqual(editor.editSessions[0])
       expect(newEditor.editSessions[0]).not.toBe(editor.editSessions[0])
 
+      newEditor.height(editor.height())
+      newEditor.width(editor.width())
+      rootView.remove()
+      newEditor.attachToDom()
+      advanceClock() # ensure any deferred scrollTo code completes (this was causing a regression)
+      expect(newEditor.verticalScrollbar.scrollTop()).toBe 1.5 * editor.lineHeight
+      expect(newEditor.lines.css('padding-top')).toBe "#{editor.lineHeight}px"
+      expect(newEditor.scrollView.scrollLeft()).toBe 44
+
   describe ".setBuffer(buffer)", ->
     it "sets the cursor to the beginning of the file", ->
       expect(editor.getCursorScreenPosition()).toEqual(row: 0, column: 0)
diff --git a/src/app/editor.coffee b/src/app/editor.coffee
index 289235690..ffdba9736 100644
--- a/src/app/editor.coffee
+++ b/src/app/editor.coffee
@@ -232,9 +232,15 @@ class Editor extends View
     @attached = true
     @subscribeToFontSize()
     @calculateDimensions()
-    @renderLines()
-    @hiddenInput.width(@charWidth)
     @setMaxLineLength() if @softWrap
+    @prepareForVerticalScrolling()
+    @setScrollPositionFromActiveEditSession()
+    @renderLines()
+    # TODO: The redundant assignment of scrollLeft below is needed because the lines weren't render
+    # rendered when we called setScrollPositionFromActiveEditSession above. Remove this when we fix
+    # that problem by setting the width of the lines container based on the max line width
+    @scrollView.scrollLeft(@getActiveEditSession().scrollLeft ? 0)
+    @hiddenInput.width(@charWidth)
     @focus() if @isFocused
     @trigger 'editor-open', [this]
 
@@ -250,18 +256,22 @@ class Editor extends View
       @compositeSelection.mergeIntersectingSelections({reverse})
       @syncCursorAnimations()
 
+  prepareForVerticalScrolling: ->
+    linesHeight = @lineHeight * @screenLineCount()
+    @verticalScrollbarContent.height(linesHeight)
+    @lines.css('padding-bottom', linesHeight)
+
   renderLines: ->
     @lineCache = []
     @lines.find('.line').remove()
 
-    @firstRenderedScreenRow = 0
+    @firstRenderedScreenRow = @getFirstVisibleScreenRow()
     @lastRenderedScreenRow = @getLastVisibleScreenRow()
 
     @insertLineElements(0, @buildLineElements(@firstRenderedScreenRow, @lastRenderedScreenRow))
+    @lines.css('padding-top', @firstRenderedScreenRow * @lineHeight)
     @lines.css('padding-bottom', (@getLastScreenRow() - @lastRenderedScreenRow) * @lineHeight)
 
-    @verticalScrollbarContent.height(@lineHeight * @screenLineCount())
-
   updateLines: ->
     firstVisibleScreenRow = @getFirstVisibleScreenRow()
     lastVisibleScreenRow = @getLastVisibleScreenRow()
@@ -355,10 +365,18 @@ class Editor extends View
     editSession = @editSessions[index]
     throw new Error("Edit session not found") unless editSession
     @setBuffer(editSession.buffer) unless @buffer == editSession.buffer
+    @activeEditSessionIndex = index
+    @setScrollPositionFromActiveEditSession() if @attached
     @setCursorScreenPosition(editSession.cursorScreenPosition ? [0, 0])
+
+  getActiveEditSession: ->
+    @editSessions[@activeEditSessionIndex]
+
+  setScrollPositionFromActiveEditSession: ->
+    editSession = @getActiveEditSession()
     @verticalScrollbar.scrollTop(editSession.scrollTop ? 0)
     @scrollView.scrollLeft(editSession.scrollLeft ? 0)
-    @activeEditSessionIndex = index
+    @verticalScrollbar.trigger 'scroll'
 
   saveCurrentEditSession: ->
     @editSessions[@activeEditSessionIndex] =
@@ -657,6 +675,7 @@ class Editor extends View
     @buffer.getMode()
 
   scrollTo: (pixelPosition) ->
+    return unless @attached
     _.defer => # Optimization
       @scrollVertically(pixelPosition)
       @scrollHorizontally(pixelPosition)