Merge pull request #518 from sharelatex/pr-aggregate-insert-delete

Aggregate adjacent inserts and deletes

services/web/app/views/project/editor/review-panel.pug

@@ -8,20 +8,20 @@
 		) !{translate("track_changes_is_on")}
 		a.rp-bulk-actions-btn(
 			href
-			ng-if="reviewPanel.selectedEntryIds.length > 1"
+			ng-if="reviewPanel.nVisibleSelectedChanges > 1"
 			ng-click="showBulkAcceptDialog();"
 		) 
 			i.fa.fa-check
 			|  #{translate("accept_all")}
-			| ({{ reviewPanel.selectedEntryIds.length }})
+			| ({{ reviewPanel.nVisibleSelectedChanges }})
 		a.rp-bulk-actions-btn(
 			href
-			ng-if="reviewPanel.selectedEntryIds.length > 1"
+			ng-if="reviewPanel.nVisibleSelectedChanges > 1"
 			ng-click="showBulkRejectDialog();"
 		) 
 			i.fa.fa-times
 			|  #{translate("reject_all")}
-			| ({{ reviewPanel.selectedEntryIds.length }})
+			| ({{ reviewPanel.nVisibleSelectedChanges }})
 		a.rp-add-comment-btn(
 			href
 			ng-if="reviewPanel.entries[editor.open_doc_id]['add-comment'] != null"
@@ -75,8 +75,19 @@
 					change-entry(
 						entry="entry"
 						user="users[entry.metadata.user_id]"
-						on-reject="rejectChange(entry_id);"
-						on-accept="acceptChange(entry_id);"
+						on-reject="rejectChanges(entry.entry_ids);"
+						on-accept="acceptChanges(entry.entry_ids);"
+						on-indicator-click="toggleReviewPanel();"
+						on-body-click="gotoEntry(editor.open_doc_id, entry)"
+						permissions="permissions"
+					)
+
+				div(ng-if="entry.type === 'aggregate-change'")
+					aggregate-change-entry(
+						entry="entry"
+						user="users[entry.metadata.user_id]"
+						on-reject="rejectChanges(entry.entry_ids);"
+						on-accept="acceptChanges(entry.entry_ids);"
 						on-indicator-click="toggleReviewPanel();"
 						on-body-click="gotoEntry(editor.open_doc_id, entry)"
 						permissions="permissions"
@@ -106,7 +117,7 @@
 					bulk-actions-entry(
 						on-bulk-accept="showBulkAcceptDialog();"
 						on-bulk-reject="showBulkRejectDialog();"
-						n-entries="reviewPanel.selectedEntryIds.length"
+						n-entries="reviewPanel.nVisibleSelectedChanges"
 					)
 
 	.rp-entry-list(
@@ -142,11 +153,18 @@
 						change-entry(
 							entry="entry"
 							user="users[entry.metadata.user_id]"
-							on-indicator-click="toggleReviewPanel();"
 							ng-click="gotoEntry(doc.doc.id, entry)"
 							permissions="permissions"
 						)
 
+					div(ng-if="entry.type === 'aggregate-change'")
+						aggregate-change-entry(
+							entry="entry"
+							user="users[entry.metadata.user_id]"
+							ng-click="gotoEntry(editor.open_doc_id, entry)"
+							permissions="permissions"
+						)
+
 					div(ng-if="entry.type === 'comment'")
 						comment-entry(
 							entry="entry"
@@ -154,7 +172,6 @@
 							on-reply="submitReply(entry, entry_id);"
 							on-save-edit="saveEdit(entry.thread_id, comment)"
 							on-delete="deleteComment(entry.thread_id, comment)"
-							on-indicator-click="toggleReviewPanel();"
 							ng-click="gotoEntry(doc.doc.id, entry)"
 							permissions="permissions"
 						)
@@ -222,6 +239,42 @@ script(type='text/ng-template', id='changeEntryTemplate')
 					i.fa.fa-check
 					|  #{translate("accept")}
 
+script(type='text/ng-template', id='aggregateChangeEntryTemplate')
+	div
+		.rp-entry-callout.rp-entry-callout-aggregate
+		.rp-entry-indicator(
+			ng-class="{ 'rp-entry-indicator-focused': entry.focused }"
+			ng-click="onIndicatorClick();"
+		)
+			i.fa.fa-pencil
+		.rp-entry.rp-entry-aggregate(
+			ng-class="{ 'rp-entry-focused': entry.focused }"
+		)
+			.rp-entry-body
+				.rp-entry-action-icon
+					i.fa.fa-pencil
+				.rp-entry-details
+					.rp-entry-description
+						| #{translate("aggregate_changed")} 
+						del.rp-content-highlight {{ entry.metadata.replaced_content }}
+						|  #{translate("aggregate_to")} 
+						ins.rp-content-highlight {{ entry.content }}
+						a.rp-collapse-toggle(
+							href
+							ng-if="needsCollapsing"
+							ng-click="toggleCollapse();"
+						) {{ isCollapsed ? '... (#{translate("show_all")})' : ' (#{translate("show_less")})' }}
+					.rp-entry-metadata
+						| {{ entry.metadata.ts | date : 'MMM d, y h:mm a' }} • 
+						span.rp-entry-user(style="color: hsl({{ user.hue }}, 70%, 40%);") {{ user.name }} 
+			.rp-entry-actions(ng-if="permissions.write")
+				a.rp-entry-button(href, ng-click="onReject();") 
+					i.fa.fa-times
+					|  #{translate("reject")}
+				a.rp-entry-button(href, ng-click="onAccept();") 
+					i.fa.fa-check
+					|  #{translate("accept")}
+
 script(type='text/ng-template', id='commentEntryTemplate')
 	.rp-comment-wrapper(
 		ng-class="{ 'rp-comment-wrapper-resolving': state.animating }"

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

@@ -26,16 +26,10 @@ define [
 			@$scope.$on "comment:select_line", (e) =>
 				@selectLineIfNoSelection()
 			
-			@$scope.$on "change:accept", (e, change_id) =>
-				@acceptChangeIds([ change_id ])
-			
-			@$scope.$on "change:reject", (e, change_id) =>
-				@rejectChangeIds([ change_id ])
-
-			@$scope.$on "change:bulk-accept", (e, change_ids) =>
+			@$scope.$on "changes:accept", (e, change_ids) =>
 				@acceptChangeIds(change_ids)
 
-			@$scope.$on "change:bulk-reject", (e, change_ids) =>
+			@$scope.$on "changes:reject", (e, change_ids) =>
 				@rejectChangeIds(change_ids)
 			
 			@$scope.$on "comment:remove", (e, comment_id) =>
@@ -216,6 +210,7 @@ define [
 		acceptChangeIds: (change_ids) ->
 			@rangesTracker.removeChangeIds(change_ids)
 			@updateAnnotations()
+			@updateFocus()
 
 		rejectChangeIds: (change_ids) ->
 			changes = @rangesTracker.getChanges(change_ids)
@@ -294,6 +289,7 @@ define [
 					session.$fromReject = false
 				else
 					throw new Error("unknown change: #{JSON.stringify(change)}")
+			setTimeout () => @updateFocus()
 
 		removeCommentId: (comment_id) ->
 			@rangesTracker.removeCommentId(comment_id)

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

@@ -5,6 +5,7 @@ define [
 	"ide/review-panel/directives/reviewPanelSorted"
 	"ide/review-panel/directives/reviewPanelToggle"
 	"ide/review-panel/directives/changeEntry"
+	"ide/review-panel/directives/aggregateChangeEntry"
 	"ide/review-panel/directives/commentEntry"
 	"ide/review-panel/directives/addCommentEntry"
 	"ide/review-panel/directives/bulkActionsEntry"

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

@@ -26,7 +26,12 @@ define [
 			resolvedThreadIds: {}
 			rendererData: {}
 			loadingThreads: false
-			selectedEntryIds: []
+			# All selected changes. If a aggregated change (insertion + deletion) is selection, the two ids
+			# will be present. The length of this array will differ from the count below (see explanation).
+			selectedEntryIds: [] 
+			# A count of user-facing selected changes. An aggregated change (insertion + deletion) will count
+			# as only one.
+			nVisibleSelectedChanges: 0
 
 		window.addEventListener "beforeunload", () ->
 			collapsedStates = {}
@@ -69,20 +74,12 @@ define [
 			$scope.$apply()
 			$timeout () ->
 				$scope.$broadcast "review-panel:layout"
-		
-		ide.socket.on "accept-change", (doc_id, change_id) ->
-			if doc_id != $scope.editor.open_doc_id
-				getChangeTracker(doc_id).removeChangeId(change_id)
-			else
-				$scope.$broadcast "change:accept", change_id
-			updateEntries(doc_id)
-			$scope.$apply () ->
 
 		ide.socket.on "accept-changes", (doc_id, change_ids) ->
 			if doc_id != $scope.editor.open_doc_id
 				getChangeTracker(doc_id).removeChangeIds(change_ids)
 			else
-				$scope.$broadcast "change:bulk-accept", change_ids
+				$scope.$broadcast "changes:accept", change_ids
 			updateEntries(doc_id)
 			$scope.$apply () ->
 		
@@ -227,28 +224,49 @@ define [
 
 			# Assume we'll delete everything until we see it, then we'll remove it from this object
 			delete_changes = {}
-			for change_id, change of entries
-				if change_id != "add-comment"
-					delete_changes[change_id] = true 
-			for change_id, change of resolvedComments
-				delete_changes[change_id] = true 
+			for id, change of entries
+				if id not in [ "add-comment", "bulk-actions" ]
+					for entry_id in change.entry_ids
+						delete_changes[entry_id] = true 
+			for id, change of resolvedComments
+				for entry_id in change.entry_ids
+					delete_changes[entry_id] = true 
+
+			potential_aggregate = false
+			prev_insertion = null
 
 			for change in rangesTracker.changes
 				changed = true
-				delete delete_changes[change.id]
-				entries[change.id] ?= {}
-					
-				# Update in place to avoid a full DOM redraw via angular
-				metadata = {}
-				metadata[key] = value for key, value of change.metadata
-				new_entry = {
-					type: if change.op.i then "insert" else "delete"
-					content: change.op.i or change.op.d
-					offset: change.op.p
-					metadata: change.metadata
-				}
-				for key, value of new_entry
-					entries[change.id][key] = value
+				
+				if (
+					potential_aggregate and 
+					change.op.d and 
+					change.op.p == prev_insertion.op.p + prev_insertion.op.i.length and
+					change.metadata.user_id == prev_insertion.metadata.user_id
+				)
+					# An actual aggregate op.
+					entries[prev_insertion.id].type = "aggregate-change"
+					entries[prev_insertion.id].metadata.replaced_content = change.op.d
+					entries[prev_insertion.id].entry_ids.push change.id
+				else
+					entries[change.id] ?= {}
+					delete delete_changes[change.id]
+					new_entry = {
+						type: if change.op.i then "insert" else "delete"
+						entry_ids: [ change.id ]
+						content: change.op.i or change.op.d
+						offset: change.op.p
+						metadata: change.metadata
+					}
+					for key, value of new_entry
+						entries[change.id][key] = value
+
+				if change.op.i
+					potential_aggregate = true
+					prev_insertion = change
+				else
+					potential_aggregate = false
+					prev_insertion = null
 
 				if !$scope.users[change.metadata.user_id]?
 					refreshChangeUsers(change.metadata.user_id)
@@ -268,6 +286,7 @@ define [
 				new_entry = {
 					type: "comment"
 					thread_id: comment.op.t
+					entry_ids: [ comment.id ]
 					content: comment.op.c
 					offset: comment.op.p
 				}
@@ -295,8 +314,10 @@ define [
 		$scope.$on "editor:focus:changed", (e, selection_offset_start, selection_offset_end, selection) ->
 			doc_id = $scope.editor.open_doc_id
 			entries = getDocEntries(doc_id)
+			# All selected changes will be added to this array.
 			$scope.reviewPanel.selectedEntryIds = []
-
+			# Count of user-visible changes, i.e. an aggregated change will count as one.
+			$scope.reviewPanel.nVisibleSelectedChanges = 0
 			delete entries["add-comment"]
 			delete entries["bulk-actions"]
 
@@ -313,53 +334,63 @@ define [
 				}
 			
 			for id, entry of entries
+				isChangeEntryAndWithinSelection = false
 				if entry.type == "comment" and not $scope.reviewPanel.resolvedThreadIds[entry.thread_id]
 					entry.focused = (entry.offset <= selection_offset_start <= entry.offset + entry.content.length)
 				else if entry.type == "insert"
-					isEntryWithinSelection = entry.offset >= selection_offset_start and entry.offset + entry.content.length <= selection_offset_end
+					isChangeEntryAndWithinSelection = entry.offset >= selection_offset_start and entry.offset + entry.content.length <= selection_offset_end
 					entry.focused = (entry.offset <= selection_offset_start <= entry.offset + entry.content.length)
-					$scope.reviewPanel.selectedEntryIds.push id if isEntryWithinSelection
 				else if entry.type == "delete"
-					isEntryWithinSelection = selection_offset_start <= entry.offset <= selection_offset_end
+					isChangeEntryAndWithinSelection = selection_offset_start <= entry.offset <= selection_offset_end
 					entry.focused = (entry.offset == selection_offset_start)
-					$scope.reviewPanel.selectedEntryIds.push id if isEntryWithinSelection
+				else if entry.type == "aggregate-change"
+					isChangeEntryAndWithinSelection = entry.offset >= selection_offset_start and entry.offset + entry.content.length <= selection_offset_end
+					entry.focused = (entry.offset <= selection_offset_start <= entry.offset + entry.content.length)
 				else if entry.type in [ "add-comment", "bulk-actions" ] and selection
 					entry.focused = true
+
+				if isChangeEntryAndWithinSelection
+					for entry_id in entry.entry_ids
+						$scope.reviewPanel.selectedEntryIds.push entry_id
+					$scope.reviewPanel.nVisibleSelectedChanges++
 			
 			$scope.$broadcast "review-panel:recalculate-screen-positions"
 			$scope.$broadcast "review-panel:layout"
 
-		$scope.acceptChange = (entry_id) ->
-			$http.post "/project/#{$scope.project_id}/doc/#{$scope.editor.open_doc_id}/changes/#{entry_id}/accept", {_csrf: window.csrfToken}
-			$scope.$broadcast "change:accept", entry_id
-			event_tracking.sendMB "rp-change-accepted", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini' }
-		
-		$scope.rejectChange = (entry_id) ->
-			$scope.$broadcast "change:reject", entry_id
-			event_tracking.sendMB "rp-change-rejected", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini' }
-		
+		$scope.acceptChanges = (change_ids) ->
+			_doAcceptChanges change_ids
+			event_tracking.sendMB "rp-changes-accepted", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini' }
+
+		$scope.rejectChanges = (change_ids) ->
+			_doRejectChanges change_ids
+			event_tracking.sendMB "rp-changes-rejected", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini' }
+
+		_doAcceptChanges = (change_ids) ->
+			$http.post "/project/#{$scope.project_id}/doc/#{$scope.editor.open_doc_id}/changes/accept", { change_ids, _csrf: window.csrfToken}
+			$scope.$broadcast "changes:accept", change_ids
+
+		_doRejectChanges = (change_ids) ->
+			$scope.$broadcast "changes:reject", change_ids
+
 		bulkAccept = () ->
-			entry_ids = $scope.reviewPanel.selectedEntryIds.slice()
-			$http.post "/project/#{$scope.project_id}/doc/#{$scope.editor.open_doc_id}/changes/accept", { change_ids: entry_ids, _csrf: window.csrfToken}
-			$scope.$broadcast "change:bulk-accept", entry_ids
-			$scope.reviewPanel.selectedEntryIds = []
+			_doAcceptChanges $scope.reviewPanel.selectedEntryIds.slice()
 			event_tracking.sendMB "rp-bulk-accept", { 
 				view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini',  
-				nEntries: $scope.reviewPanel.selectedEntryIds.length
+				nEntries: $scope.reviewPanel.nVisibleSelectedChanges
 			}
 
 		bulkReject = () ->
-			$scope.$broadcast "change:bulk-reject", $scope.reviewPanel.selectedEntryIds.slice()
-			$scope.reviewPanel.selectedEntryIds = []
+			_doRejectChanges $scope.reviewPanel.selectedEntryIds.slice()
 			event_tracking.sendMB "rp-bulk-reject", { 
 				view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini',  
-				nEntries: $scope.reviewPanel.selectedEntryIds.length
+				nEntries: $scope.reviewPanel.nVisibleSelectedChanges
 			}
 
 		$scope.showBulkAcceptDialog = () ->
 			showBulkActionsDialog true
 
-		$scope.showBulkRejectDialog = () -> showBulkActionsDialog false
+		$scope.showBulkRejectDialog = () -> 
+			showBulkActionsDialog false
 
 		showBulkActionsDialog = (isAccept) ->
 			$modal.open({
@@ -367,7 +398,7 @@ define [
 				controller: "BulkActionsModalController"
 				resolve:
 					isAccept: () -> isAccept
-					nChanges: () -> $scope.reviewPanel.selectedEntryIds.length
+					nChanges: () -> $scope.reviewPanel.nVisibleSelectedChanges
 				scope: $scope.$new()
 			}).result.then (isAccept) ->
 				if isAccept

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

@@ -0,0 +1,30 @@
+define [
+	"base"
+], (App) ->
+	App.directive "aggregateChangeEntry", ($timeout) ->
+		restrict: "E"
+		templateUrl: "aggregateChangeEntryTemplate"
+		scope: 
+			entry: "="
+			user: "="
+			permissions: "="
+			onAccept: "&"
+			onReject: "&"
+			onIndicatorClick: "&"
+			onBodyClick: "&"
+		link: (scope, element, attrs) ->
+			scope.contentLimit = 35
+			scope.isCollapsed = true
+			scope.needsCollapsing = false
+
+			element.on "click", (e) ->
+				if $(e.target).is('.rp-entry, .rp-entry-description, .rp-entry-body, .rp-entry-action-icon i')
+					scope.onBodyClick()
+
+			scope.toggleCollapse = () ->
+				scope.isCollapsed = !scope.isCollapsed
+				$timeout () ->
+					scope.$emit "review-panel:layout"
+
+			scope.$watch "entry.content.length + entry.metadata.agg_op.content.length", (contentLength) ->
+					scope.needsCollapsing = contentLength > scope.contentLimit

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

@@ -63,7 +63,12 @@ define [
 
 					# Put the focused entry as close to where it wants to be as possible
 					focused_entry_top = Math.max(focused_entry.scope.entry.screenPos.y, TOOLBAR_HEIGHT)
-					focused_entry.$box_el.css(top: focused_entry_top)
+					focused_entry.$box_el.css(
+						top: focused_entry_top
+						# The entry element is invisible by default, to avoid flickering when positioning for 
+						# the first time. Here we make sure it becomes visible after having a "top" value.
+						visibility: "visible"
+					)
 					focused_entry.$indicator_el.css(top: focused_entry_top)
 					positionLayoutEl(focused_entry.$callout_el, focused_entry.scope.entry.screenPos.y, focused_entry_top)
 
@@ -73,7 +78,12 @@ define [
 						height = entry.height
 						top = Math.max(original_top, previousBottom + PADDING)
 						previousBottom = top + height
-						entry.$box_el.css(top: top)
+						entry.$box_el.css(
+							top: top
+							# The entry element is invisible by default, to avoid flickering when positioning for 
+							# the first time. Here we make sure it becomes visible after having a "top" value.
+							visibility: "visible"
+						)
 						entry.$indicator_el.css(top: top)
 						positionLayoutEl(entry.$callout_el, original_top, top)
 						sl_console.log "ENTRY", {entry: entry.scope.entry, top}
@@ -89,7 +99,12 @@ define [
 						bottom = Math.min(original_bottom, previousTop - PADDING)
 						top = bottom - height
 						previousTop = top
-						entry.$box_el.css(top: top)
+						entry.$box_el.css(
+							top: top
+							# The entry element is invisible by default, to avoid flickering when positioning for 
+							# the first time. Here we make sure it becomes visible after having a "top" value.
+							visibility: "visible"
+						)
 						entry.$indicator_el.css(top: top)
 						positionLayoutEl(entry.$callout_el, original_top, top)
 						sl_console.log "ENTRY", {entry: entry.scope.entry, top}

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

@@ -210,6 +210,7 @@
 .rp-entry-wrapper {
 	&:hover .rp-entry-insert,
 	&:hover .rp-entry-delete,
+	&:hover .rp-entry-aggregate,
 	&:hover .rp-entry-comment {
 		display: block;
 	}
@@ -260,6 +261,7 @@
 		}
 	}
 	.rp-state-current-file-expanded & {
+		visibility: hidden;
 		left: 5px;
 		right: 5px;
 		width: auto;
@@ -299,7 +301,8 @@
 		transition: none;
 	}
 
-	&-insert {
+	&-insert,
+	&-aggregate {
 		border-color: @rp-green;
 	}
 
@@ -389,7 +392,7 @@
 				font-weight: @rp-semibold-weight;
 				text-decoration: none;
 
-				.rp-entry-delete & {
+				del& {
 					text-decoration: line-through;
 				}
 			}
@@ -780,6 +783,7 @@
 	}
 	.track-changes-added-marker-callout {
 		border-bottom: 1px dashed @rp-green;
+		z-index: 1;
 	}
 	.track-changes-comment-marker-callout {
 		border-bottom: 1px dashed @rp-yellow;