Merge pull request #431 from sharelatex/ja-scroll-review-panel-past-ace

Scroll the review panel past the limits of Ace

services/web/public/coffee/ide/editor/directives/aceEditor.coffee

@@ -184,6 +184,28 @@ define [
 							session = editor.getSession()
 							session.setScrollTop(session.getScrollTop() + newScreenPosition - previousScreenPosition)
 
+				scope.$on "#{scope.name}:set-scroll-size", (e, size) ->
+					# Make sure that the editor has enough scroll margin above and below
+					# to scroll the review panel with the given size
+					marginTop = size.overflowTop
+					maxHeight = editor.renderer.layerConfig.maxHeight
+					marginBottom = Math.max(size.height - maxHeight, 0)
+					setScrollMargins(marginTop, marginBottom)
+
+				setScrollMargins = (marginTop, marginBottom) ->
+					marginChanged = false
+					if editor.renderer.scrollMargin.top != marginTop
+						editor.renderer.scrollMargin.top = marginTop
+						marginChanged = true
+					if editor.renderer.scrollMargin.bottom != marginBottom
+						editor.renderer.scrollMargin.bottom = marginBottom
+						marginChanged = true
+					if marginChanged
+						editor.renderer.updateFull()
+
+				resetScrollMargins = () ->
+					setScrollMargins(0,0)
+
 				scope.$watch "theme", (value) ->
 					editor.setTheme("ace/theme/#{value}")
 
@@ -308,6 +330,8 @@ define [
 					sharejs_doc.attachToAce(editor)
 					editor.initing = false
 
+					resetScrollMargins()
+
 					# need to set annotations after attaching because attaching
 					# deletes and then inserts document content
 					session.setAnnotations scope.annotations

services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee

@@ -39,6 +39,9 @@ define [
 			$timeout () ->
 				$scope.$broadcast "review-panel:layout"
 
+		$scope.$on "review-panel:sizes", (e, sizes) ->
+			$scope.$broadcast "editor:set-scroll-size", sizes
+
 		$scope.$watch "ui.pdfLayout", (layout) ->
 			$scope.reviewPanel.layoutToLeft = (layout == "flat")
 		

services/web/public/coffee/ide/review-panel/directives/reviewPanelSorted.coffee

@@ -15,9 +15,11 @@ define [
 					if scope.ui.reviewPanelOpen
 						PADDING = 8
 						TOOLBAR_HEIGHT = 38
+						OVERVIEW_TOGGLE_HEIGHT = 57
 					else
 						PADDING = 4
 						TOOLBAR_HEIGHT = 4
+						OVERVIEW_TOGGLE_HEIGHT = 0
 					
 					entries = []
 					for el in element.find(".rp-entry-wrapper")
@@ -31,6 +33,7 @@ define [
 							entry.$layout_el = entry.$box_el
 						else
 							entry.$layout_el = entry.$indicator_el
+						entry.height = entry.$layout_el.height() # Do all of our DOM reads first for perfomance, see http://wilsonpage.co.uk/preventing-layout-thrashing/
 						entries.push entry
 					entries.sort (a,b) -> a.scope.entry.offset - b.scope.entry.offset
 					
@@ -50,19 +53,6 @@ define [
 					
 					sl_console.log "focused_entry_index", focused_entry_index
 
-					# As we go backwards, we run the risk of pushing things off the top of the editor.
-					# If we go through the entries before and assume they are as pushed together as they
-					# could be, we can work out the 'ceiling' that each one can't go through. I.e. the first
-					# on can't go beyond the toolbar height, the next one can't go beyond the bottom of the first
-					# one at this minimum height, etc.
-					heights = (entry.$layout_el.height() for entry in entries_before)
-					previousMinTop = TOOLBAR_HEIGHT
-					min_tops = []
-					for height in heights
-						min_tops.push previousMinTop
-						previousMinTop += PADDING + height
-					min_tops.reverse()
-
 					positionLayoutEl = ($callout_el, original_top, top) ->
 						if original_top <= top
 							$callout_el.removeClass("rp-entry-callout-inverted")
@@ -72,7 +62,7 @@ define [
 							$callout_el.css(top: top + line_height, height: original_top - top)
 
 					# Put the focused entry as close to where it wants to be as possible
-					focused_entry_top = Math.max(previousMinTop, focused_entry.scope.entry.screenPos.y)
+					focused_entry_top = Math.max(focused_entry.scope.entry.screenPos.y, TOOLBAR_HEIGHT)
 					focused_entry.$box_el.css(top: focused_entry_top)
 					focused_entry.$indicator_el.css(top: focused_entry_top)
 					positionLayoutEl(focused_entry.$callout_el, focused_entry.scope.entry.screenPos.y, focused_entry_top)
@@ -80,27 +70,39 @@ define [
 					previousBottom = focused_entry_top + focused_entry.$layout_el.height()
 					for entry in entries_after
 						original_top = entry.scope.entry.screenPos.y
-						height = entry.$layout_el.height()
+						height = entry.height
 						top = Math.max(original_top, previousBottom + PADDING)
 						previousBottom = top + height
 						entry.$box_el.css(top: top)
 						entry.$indicator_el.css(top: top)
 						positionLayoutEl(entry.$callout_el, original_top, top)
 						sl_console.log "ENTRY", {entry: entry.scope.entry, top}
+					
+					lastBottom = previousBottom
 
 					previousTop = focused_entry_top
 					entries_before.reverse() # Work through backwards, starting with the one just above
 					for entry, i in entries_before
 						original_top = entry.scope.entry.screenPos.y
-						height = entry.$layout_el.height()
+						height = entry.height
 						original_bottom = original_top + height
 						bottom = Math.min(original_bottom, previousTop - PADDING)
-						top = Math.max(bottom - height, min_tops[i])
+						top = bottom - height
 						previousTop = top
 						entry.$box_el.css(top: top)
 						entry.$indicator_el.css(top: top)
 						positionLayoutEl(entry.$callout_el, original_top, top)
 						sl_console.log "ENTRY", {entry: entry.scope.entry, top}
+
+					lastTop = top
+					if lastTop < TOOLBAR_HEIGHT
+						overflowTop = -lastTop + TOOLBAR_HEIGHT
+					else
+						overflowTop = 0
+					scope.$emit "review-panel:sizes", {
+						overflowTop: overflowTop,
+						height: previousBottom + OVERVIEW_TOGGLE_HEIGHT
+					}
 				
 				scope.$applyAsync () ->
 					layout()