Merge branch 'master' into pr-collapse-overview-entries

services/web/app/views/project/editor/pdf.pug

@@ -302,7 +302,7 @@ div.full-size.pdf(ng-controller="PdfController")
 			.alert.alert-danger(ng-show="pdf.validation.conflictedPaths")
 				div
 					strong #{translate("conflicting_paths_found")}
-				div #{translate("following_paths_conflict")}
+				div !{translate("following_paths_conflict")}
 				div
 					li(ng-repeat="entry in pdf.validation.conflictedPaths") {{ '/'+entry['path'] }}
 

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

@@ -47,6 +47,7 @@
 		.rp-entry-list-inner
 			.rp-entry-wrapper(
 				ng-repeat="(entry_id, entry) in reviewPanel.entries[editor.open_doc_id]"
+				ng-if="entry.visible"
 			)
 				div(ng-if="entry.type === 'insert' || entry.type === 'delete'")
 					change-entry(
@@ -79,6 +80,7 @@
 						on-submit="submitNewComment(content);"
 						on-cancel="cancelNewComment();"
 						on-indicator-click="toggleReviewPanel();"
+						layout-to-left="reviewPanel.layoutToLeft"
 					)
 
 	.rp-entry-list(
@@ -322,7 +324,7 @@ script(type='text/ng-template', id='addCommentEntryTemplate')
 			ng-if="!commentState.adding"
 			ng-click="startNewComment(); onIndicatorClick();"
 			tooltip="Add a comment"
-			tooltip-placement="right"
+			tooltip-placement="{{ layoutToLeft ? 'left' : 'right' }}"
 			tooltip-append-to-body="true"
 		)
 			i.fa.fa-commenting

services/web/app/views/subscriptions/group/successful_join.pug

@@ -20,6 +20,6 @@ block content
 								.col-md-12  
 							.row
 								.span-md-12
-									a.btn.btn-success(href="/project") #{translate("Done")} 
+									a.btn.btn-success(href="/project") #{translate("done")} 
 
 

services/web/app/views/user/sessions.pug

@@ -17,7 +17,7 @@ block content
 
 						div
 							p.small
-								| #{translate("clear_sessions_description")}
+								| !{translate("clear_sessions_description")}
 
 						div
 							div(ng-if="state.otherSessions.length == 0")

services/web/app/views/user/settings.pug

@@ -107,7 +107,7 @@ block content
 													span.small.text-primary(ng-show="changePasswordForm.newPassword2.$error.areEqual && changePasswordForm.newPassword2.$dirty")
 														| #{translate("doesnt_match")} 
 													span.small.text-primary(ng-show="!changePasswordForm.newPassword2.$error.areEqual && changePasswordForm.newPassword2.$invalid && changePasswordForm.newPassword2.$dirty")
-														| #{translate("Invalid Password")} 
+														| #{translate("invalid_password")} 
 												.actions
 													button.btn.btn-primary(
 														type='submit',

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/editor/directives/aceEditor/track-changes/TrackChangesManager.coffee

@@ -60,6 +60,18 @@ define [
 			onChangeSession = (e) =>
 				@clearAnnotations()
 				@redrawAnnotations()
+				@editor.session.on "changeScrollTop", onChangeScroll
+
+			_scrollTimeout = null
+			onChangeScroll = () =>
+				if _scrollTimeout?
+					return
+				else
+					_scrollTimeout = setTimeout () =>
+						@recalculateVisibleEntries()
+						@$scope.$apply()
+						_scrollTimeout = null
+					, 200
 
 			bindToAce = () =>
 				@editor.on "changeSelection", onChangeSelection
@@ -282,6 +294,7 @@ define [
 		recalculateReviewEntriesScreenPositions: () ->
 			session = @editor.getSession()
 			renderer = @editor.renderer
+			{firstRow, lastRow} = renderer.layerConfig
 			entries = @_getCurrentDocEntries()
 			for entry_id, entry of entries or {}
 				doc_position = @_shareJsOffsetToAcePosition(entry.offset)
@@ -290,9 +303,24 @@ define [
 				entry.screenPos ?= {}
 				entry.screenPos.y = y
 				entry.docPos = doc_position
-
+			@recalculateVisibleEntries()
 			@$scope.$apply()
 
+		recalculateVisibleEntries: () ->
+			OFFSCREEN_ROWS = 20
+			CULL_AFTER = 100 # With less than this number of entries, don't bother culling to avoid little UI jumps when scrolling.
+			{firstRow, lastRow} = @editor.renderer.layerConfig
+			entries = @_getCurrentDocEntries() or {}
+			entriesLength = Object.keys(entries).length
+			changed = false
+			for entry_id, entry of entries
+				old = entry.visible
+				entry.visible = (entriesLength < CULL_AFTER) or (firstRow - OFFSCREEN_ROWS <= entry.docPos.row <= lastRow + OFFSCREEN_ROWS)
+				if (entry.visible != old)
+					changed = true
+			if changed
+				@$scope.$emit "editor:track-changes:visibility_changed"
+
 		_getCurrentDocEntries: () ->
 			doc_id = @$scope.docId
 			entries = @$scope.reviewPanel.entries[doc_id] ?= {}

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

@@ -105,6 +105,7 @@ load = (EventEmitter) ->
 				throw new Error("unknown op type")
 			
 		addComment: (op, metadata) ->
+			# TODO: Don't allow overlapping comments?
 			@comments.push comment = {
 				id: op.t or @newId()
 				op: # Copy because we'll modify in place
@@ -167,12 +168,14 @@ load = (EventEmitter) ->
 			op_length = op.i.length
 			op_end = op.p + op_length
 
+
 			already_merged = false
 			previous_change = null
 			moved_changes = []
 			remove_changes = []
 			new_changes = []
-			for change in @changes
+
+			for change, i in @changes
 				change_start = change.op.p
 				
 				if change.op.d?
@@ -200,6 +203,16 @@ load = (EventEmitter) ->
 					# Only merge inserts if they are from the same user
 					is_same_user = metadata.user_id == change.metadata.user_id
 					
+					# If this is an insert op at the end of an existing insert with a delete following, and it cancels out the following
+					# delete then we shouldn't append it to this insert, but instead only cancel the following delete.
+					# E.g.
+					#                   foo|<--- about to insert 'b' here
+					#  inserted 'foo'  --^ ^-- deleted 'bar'
+					# should become just 'foo' not 'foob' (with the delete marker becoming just 'ar'), .
+					next_change = @changes[i+1]
+					is_op_adjacent_to_next_delete = next_change? and next_change.op.d? and op.p == change_end and next_change.op.p == op.p
+					will_op_cancel_next_delete = is_op_adjacent_to_next_delete and next_change.op.d.slice(0, op.i.length) == op.i
+					
 					# If there is a delete at the start of the insert, and we're inserting
 					# at the start, we SHOULDN'T merge since the delete acts as a partition.
 					# The previous op will be the delete, but it's already been shifted by this insert
@@ -223,6 +236,7 @@ load = (EventEmitter) ->
 							is_change_overlapping and
 							!is_insert_blocked_by_delete and
 							!already_merged and
+							!will_op_cancel_next_delete and
 							is_same_user
 						offset = op_start - change_start
 						change.op.i = change.op.i.slice(0, offset) + op.i + change.op.i.slice(offset)
@@ -398,8 +412,8 @@ load = (EventEmitter) ->
 		_addOp: (op, metadata) ->
 			change = {
 				id: @newId()
-				op: op
-				metadata: metadata
+				op: @_clone(op) # Don't take a reference to the existing op since we'll modify this in place with future changes
+				metadata: @_clone(metadata)
 			}
 			@changes.push change
 
@@ -472,6 +486,11 @@ load = (EventEmitter) ->
 					previous_change = change
 			return { moved_changes, remove_changes }
 		
+		_clone: (object) ->
+			clone = {}
+			(clone[k] = v for k,v of object)
+			return clone
+
 if define?
 	define ["utils/EventEmitter"], load
 else

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

@@ -40,6 +40,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")
 		
@@ -267,6 +270,10 @@ define [
 			$scope.$broadcast "review-panel:recalculate-screen-positions"
 			$scope.$broadcast "review-panel:layout"
 
+		$scope.$on "editor:track-changes:visibility_changed", () ->
+			$timeout () ->
+				$scope.$broadcast "review-panel:layout", false
+
 		$scope.$on "editor:focus:changed", (e, selection_offset_start, selection_offset_end, selection) ->
 			doc_id = $scope.editor.open_doc_id
 			entries = getDocEntries(doc_id)

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

@@ -9,6 +9,7 @@ define [
 			onSubmit: "&"
 			onCancel: "&"
 			onIndicatorClick: "&"
+			layoutToLeft: "="
 		link: (scope, element, attrs) ->
 			scope.state =
 				isAdding: false

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,7 +70,7 @@ 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)
@@ -88,20 +78,32 @@ define [
 						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()
 				

services/web/public/stylesheets/app/editor.less

@@ -181,6 +181,19 @@
 	}
 }
 
+// Hack to solve an issue where scrollbars aren't visible in Safari.
+// Safari seems to clip part of the scrollbar element. By giving the 
+// element a background, we're telling Safari that it *really* needs to 
+// paint the whole area. See https://github.com/ajaxorg/ace/issues/2872
+.ace_scrollbar-inner {
+	background-color: #FFF;
+	opacity: 0.01;
+
+	.ace_dark & {
+		background-color: #000;
+	}
+}
+
 .ui-layout-resizer {
 	width: 6px;
 	background-color: #f4f4f4;

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

@@ -49,11 +49,11 @@
 	}
 
 	&[disabled] {
-		opacity: 0.5;
+		background-color: tint(@rp-highlight-blue, 50%);
 
 		&:hover,
 		&:focus {
-			background-color: @rp-highlight-blue;
+			background-color: tint(@rp-highlight-blue, 50%);
 		}
 	}
 }
@@ -463,6 +463,7 @@
 		margin-top: 3px;
 		overflow-x: hidden;
 		min-height: 3em;
+		max-height: 400px;
 	}
 
 .rp-icon-delete {
@@ -784,6 +785,10 @@
 	.toolbar .btn-full-height:active & {
 		background-position-y: -60px;
 	}
+	
+	@media (min-device-pixel-ratio: 1.5), (min-resolution: 144dpi) {
+		background-image: url('/img/review-icon-sprite@2x.png');
+	}
 }
 
 .resolved-comments-toggle {