Change review panel to feel like part of ace

services/web/app/views/project/editor/editor.jade

@@ -8,46 +8,44 @@ div.full-size(
 	initial-size-east="'50%'"
 	minimum-restore-size-east="300"
 )
-	.ui-layout-center
-		#review-panel-wrapper.full-size(
-			layout="review"
-			spacing-open="12"
-			spacing-closed="0"
-			init-closed-east="true"
-			open-east="ui.reviewPanelOpen"
-		)
-			.ui-layout-center
-				.loading-panel(ng-show="!editor.sharejs_doc || editor.opening")
-					span(ng-show="editor.open_doc_id")
-						i.fa.fa-spin.fa-refresh
-						|   #{translate("loading")}...
-					span(ng-show="!editor.open_doc_id")
-						i.fa.fa-arrow-left
-						|   #{translate("open_a_file_on_the_left")}
+	.ui-layout-center(ng-controller="ReviewPanelController")
+		.loading-panel(ng-show="!editor.sharejs_doc || editor.opening")
+			span(ng-show="editor.open_doc_id")
+				i.fa.fa-spin.fa-refresh
+				|   #{translate("loading")}...
+			span(ng-show="!editor.open_doc_id")
+				i.fa.fa-arrow-left
+				|   #{translate("open_a_file_on_the_left")}
 
-				#editor(
-					ace-editor="editor",
-					ng-show="!!editor.sharejs_doc && !editor.opening"
-					theme="settings.theme",
-					keybindings="settings.mode",
-					font-size="settings.fontSize",
-					auto-complete="settings.autoComplete",
-					spell-check="true",
-					spell-check-language="project.spellCheckLanguage",
-					highlights="onlineUserCursorHighlights[editor.open_doc_id]"
-					show-print-margin="false",
-					sharejs-doc="editor.sharejs_doc",
-					last-updated="editor.last_updated",
-					cursor-position="editor.cursorPosition",
-					goto-line="editor.gotoLine",
-					resize-on="layout:main:resize,layout:pdf:resize,layout:review:resize",
-					annotations="pdf.logEntryAnnotations[editor.open_doc_id]",
-					read-only="!permissions.write",
-					on-ctrl-enter="recompileViaKey"
-					syntax-validation="settings.syntaxValidation"
-				)
-			.ui-layout-east
-				strong Hello world
+		#editor.has-review-panel(
+			ace-editor="editor",
+			ng-show="!!editor.sharejs_doc && !editor.opening"
+			theme="settings.theme",
+			keybindings="settings.mode",
+			font-size="settings.fontSize",
+			auto-complete="settings.autoComplete",
+			spell-check="true",
+			spell-check-language="project.spellCheckLanguage",
+			highlights="onlineUserCursorHighlights[editor.open_doc_id]"
+			show-print-margin="false",
+			sharejs-doc="editor.sharejs_doc",
+			last-updated="editor.last_updated",
+			cursor-position="editor.cursorPosition",
+			goto-line="editor.gotoLine",
+			resize-on="layout:main:resize,layout:pdf:resize,layout:review:resize",
+			annotations="pdf.logEntryAnnotations[editor.open_doc_id]",
+			read-only="!permissions.write",
+			on-ctrl-enter="recompileViaKey",
+			syntax-validation="settings.syntaxValidation",
+			review-panel="reviewPanel",
+			on-scroll="onScroll",
+			scroll-events="scrollEvents"
+		)
+		#review-panel
+			.review-panel-scroller
+				.review-entry-list
+					.review-entry(ng-repeat="(entry_id, entry) in reviewPanel.entries", ng-style="{'top': entry.screenPos.y}")
+						{{ entry.content }}
 
 	.ui-layout-east
 		div(ng-if="ui.pdfLayout == 'sideBySide'")

services/web/public/coffee/ide.coffee

@@ -9,6 +9,7 @@ define [
 	"ide/pdf/PdfManager"
 	"ide/binary-files/BinaryFilesManager"
 	"ide/references/ReferencesManager"
+	"ide/review-panel/ReviewPanelManager"
 	"ide/SafariScrollPatcher"
 	"ide/settings/index"
 	"ide/share/index"
@@ -41,6 +42,7 @@ define [
 	PdfManager
 	BinaryFilesManager
 	ReferencesManager
+	ReviewPanelManager
 	SafariScrollPatcher
 ) ->
 

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

@@ -42,9 +42,12 @@ define [
 				text: "="
 				readOnly: "="
 				annotations: "="
-				navigateHighlights: "=",
+				navigateHighlights: "="
 				onCtrlEnter: "="
 				syntaxValidation: "="
+				reviewPanel: "="
+				onScroll: "="
+				scrollEvents: "="
 			}
 			link: (scope, element, attrs) ->
 				# Don't freak out if we're already in an apply callback
@@ -210,6 +213,15 @@ define [
 					if updateCount == 100
 						event_tracking.send 'editor-interaction', 'multi-doc-update'
 					scope.$emit "#{scope.name}:change"
+				
+				onScroll = (scrollTop) ->
+					return if !scope.onScroll?
+					height = editor.renderer.layerConfig.maxHeight
+					scope.onScroll(scrollTop, height)
+					
+				if scope.scrollEvents?
+					scope.scrollEvents.on "scroll", (position) ->
+						editor.getSession().setScrollTop(position)
 
 				attachToAce = (sharejs_doc) ->
 					lines = sharejs_doc.getSnapshot().split("\n")
@@ -222,6 +234,8 @@ define [
 
 					doc = session.getDocument()
 					doc.on "change", onChange
+					
+					session.on "changeScrollTop", onScroll
 
 					sharejs_doc.on "remoteop.recordForUndo", () =>
 						undoManager.nextUpdateIsRemote = true
@@ -240,6 +254,8 @@ define [
 					sharejs_doc.off "remoteop.recordForUndo"
 
 					session = editor.getSession()
+					session.off "changeScrollTop"
+					
 					doc = session.getDocument()
 					doc.off "change", onChange
 

services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/TrackChangesManager.coffee

@@ -9,6 +9,7 @@ define [
 			@changesTracker = new ChangesTracker()
 			@changeIdToMarkerIdMap = {}
 			@enabled = false
+			console.log "Track Changes", @$scope.reviewPanel
 
 			@changesTracker.on "insert:added", (change) =>
 				@_onInsertAdded(change)
@@ -27,12 +28,18 @@ define [
 					setTimeout () =>
 						@checkMapping()
 					, 100
+			
+			# onScroll = () =>
+			# 	@recalculateReviewEntriesScreenPositions()
 
 			@editor.on "changeSession", (e) =>
 				e.oldSession?.getDocument().off "change", onChange
 				e.session.getDocument().on "change", onChange
+				# e.oldSession?.off "changeScrollTop", onScroll
+				# e.session.on "changeScrollTop", onScroll
 			@editor.getSession().getDocument().on "change", onChange
-		
+			# @editor.getSession().on "changeScrollTop", onScroll
+
 		checkMapping: () ->
 			session = @editor.getSession()
 
@@ -65,9 +72,28 @@ define [
 		
 		applyChange: (delta) ->
 			op = @_aceChangeToShareJs(delta)
-			console.log "Applying change", delta, op
 			@changesTracker.applyOp(op)
 		
+		updateReviewEntriesScope: () ->
+			# TODO: Update in place so Angular doesn't have to redo EVERYTHING
+			@$scope.reviewPanel.entries = {}
+			for change in @changesTracker.changes
+				@$scope.reviewPanel.entries[change.id] = {
+					content: change.op.i or change.op.d
+					offset: change.op.p
+				}
+			@recalculateReviewEntriesScreenPositions()
+		
+		recalculateReviewEntriesScreenPositions: () ->
+			session = @editor.getSession()
+			renderer = @editor.renderer
+			for entry_id, entry of @$scope.reviewPanel.entries
+				doc_position = @_shareJsOffsetToAcePosition(entry.offset)
+				screen_position = session.documentToScreenPosition(doc_position.row, doc_position.column)
+				y = screen_position.row * renderer.lineHeight
+				entry.screenPos = { y }
+			@$scope.$apply()
+				
 		_onInsertAdded: (change) ->
 			start = @_shareJsOffsetToAcePosition(change.op.p)
 			end = @_shareJsOffsetToAcePosition(change.op.p + change.op.i.length)
@@ -76,6 +102,7 @@ define [
 			ace_range = new Range(start.row, start.column, end.row, end.column)
 			marker_id = session.addMarker(ace_range, "track-changes-added-marker", "text")
 			@changeIdToMarkerIdMap[change.id] = marker_id
+			@updateReviewEntriesScope()
 		
 		_onDeleteAdded: (change) ->
 			position = @_shareJsOffsetToAcePosition(change.op.p)
@@ -97,16 +124,19 @@ define [
 
 			marker_id = session.addMarker(ace_range, "track-changes-deleted-marker", "text")
 			@changeIdToMarkerIdMap[change.id] = marker_id
+			@updateReviewEntriesScope()
 		
 		_onInsertRemoved: (change) ->
 			marker_id = @changeIdToMarkerIdMap[change.id]
 			session = @editor.getSession()
 			session.removeMarker marker_id
+			@updateReviewEntriesScope()
 		
 		_onDeleteRemoved: (change) ->
 			marker_id = @changeIdToMarkerIdMap[change.id]
 			session = @editor.getSession()
 			session.removeMarker marker_id
+			@updateReviewEntriesScope()
 		
 		_aceChangeToShareJs: (delta) ->
 			start = delta.start
@@ -138,6 +168,8 @@ define [
 		_onChangesMoved: (changes) ->
 			session = @editor.getSession()
 			markers = session.getMarkers()
+			# TODO: PERFORMANCE: Only run through the Ace lines once, and calculate all
+			# change positions as we go.
 			for change in changes
 				start = @_shareJsOffsetToAcePosition(change.op.p)
 				if change.op.i?
@@ -146,9 +178,9 @@ define [
 					end = start
 				marker_id = @changeIdToMarkerIdMap[change.id]
 				marker = markers[marker_id]
-				console.log "moving marker", {marker, start, end, change}
 				marker.range.start = start
 				marker.range.end = end
+			@updateReviewEntriesScope()
 	
 	class ChangesTracker extends EventEmitter
 		# The purpose of this class is to track a set of inserts and deletes to a document, like

services/web/public/coffee/ide/review-panel/ReviewPanelManager.coffee

@@ -0,0 +1,3 @@
+define [
+	"ide/review-panel/controllers/ReviewPanelController"
+], () ->

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

@@ -0,0 +1,43 @@
+define [
+	"base",
+	"utils/EventEmitter"
+], (App, EventEmitter) ->
+	App.controller "ReviewPanelController", ($scope, $element) ->
+		$scope.reviewPanel =
+			entries: {}
+			
+		scroller = $element.find(".review-panel-scroller")
+		list = $element.find(".review-entry-list")
+
+		ignoreNextPanelEvent = false
+		ignoreNextAceEvent = false
+
+		$scope.onScroll = (scrollTop, height) ->
+			if ignoreNextAceEvent
+				# console.log "Ignoring ace event"
+				ignoreNextAceEvent = false
+			else
+				ignoreNextPanelEvent = true
+				list.height(height)
+				scroller.scrollTop(scrollTop)
+		
+		$scope.scrollEvents = new EventEmitter()
+	
+		scrollAce = (e) ->
+			now = new Date()
+			if ignoreNextPanelEvent
+				# console.log "Ignoring review panel event"
+				ignoreNextPanelEvent = false
+			else
+				# console.log "review panel scrolled", e
+				ignoreNextAceEvent = true
+				$scope.scrollEvents.emit "scroll", e.target.scrollTop
+				lastScroll = now
+
+		previousScroll = new Date()
+		scroller.on "scroll", scrollAce
+		ace.require("ace/lib/event").addMouseWheelListener scroller[0], (e) ->
+			deltaY = e.wheelY
+			# console.log "mousewheel", deltaY
+			scroller.scrollTop(scroller.scrollTop() + deltaY * 4)
+			e.preventDefault()

services/web/public/stylesheets/app/editor/review-panel.less

@@ -1,5 +1,46 @@
-#review-panel-wrapper {
-	> .ui-layout-resizer > .ui-layout-toggler {
-		display: none !important;
+#review-panel {
+	position: absolute;
+	width: 160px;
+	top: 0px;
+	bottom: 0px;
+	right: 0px;
+	background-color: #eee;
+	overflow: hidden;
+}
+
+.review-panel-scroller {
+	position: absolute;
+	top: 0;
+	bottom: 0;
+	left: 0;
+	right: -30px; // Hide scroll bar
+	overflow-y: scroll;
+}
+
+.review-entry-list {
+	position: relative;
+	width: 160px;
+}
+
+.review-entry {
+	position: absolute;
+	font-size: 12px;
+	padding: 2px 6px;
+	border: 1px solid #999;
+	margin: 0 6px;
+	background-color: white;
+	max-width: 148px;
+	word-wrap: break-word;
+}
+
+#editor.has-review-panel {
+	right: 160px;
+	left: 0px;
+	width: auto;
+	.ace-editor-body {
+		overflow: visible;
+		.ace_scrollbar-v {
+			right: -160px;
+		}
 	}
 }