From 8c65cb58358133374d5cccbd4082844080ef88c0 Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 8 Nov 2016 15:12:25 +0000 Subject: [PATCH] Track and highlight comment ranges --- .../track-changes/ChangesTracker.coffee | 51 +++++++++++-- .../track-changes/TrackChangesManager.coffee | 76 +++++++++++++++++-- .../web/public/stylesheets/app/editor.less | 6 ++ 3 files changed, 121 insertions(+), 12 deletions(-) diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/ChangesTracker.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/ChangesTracker.coffee index 4c87a2ddd3..fed7605ba4 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/ChangesTracker.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/ChangesTracker.coffee @@ -49,16 +49,57 @@ define [ # Ids are used to uniquely identify a change, e.g. for updating it in the database, or keeping in # sync with Ace ranges. @changes = [] + @comments = [] @id = 0 + addComment: (offset, length, metadata) -> + # TODO: Don't allow overlapping comments? + @comments.push comment = { + id: @_newId() + offset, length, metadata + } + @emit "comment:added", comment + applyOp: (op, metadata) -> # Apply an op that has been applied to the document to our changes to keep them up to date if op.i? - @applyInsert(op, metadata) + @applyInsertToChanges(op, metadata) + @applyInsertToComments(op) else if op.d? - @applyDelete(op, metadata) - - applyInsert: (op, metadata) -> + @applyDeleteToChanges(op, metadata) + @applyDeleteToComments(op) + + applyInsertToComments: (op) -> + for comment in @comments + if op.p <= comment.offset + comment.offset += op.i.length + @emit "comment:moved", comment + else if op.p < comment.offset + comment.length + comment.length += op.i.length + @emit "comment:moved", comment + + applyDeleteToComments: (op) -> + op_start = op.p + op_length = op.d.length + op_end = op.p + op_length + for comment in @comments + comment_end = comment.offset + comment.length + if op_end <= comment.offset + # delete is fully before comment + comment.offset -= op_length + @emit "comment:moved", comment + else if op_start >= comment_end + # delete is fully after comment, nothing to do + else + # delete and comment overlap + delete_length_before = Math.max(0, comment.offset - op_start) + delete_length_after = Math.max(0, op_end - comment_end) + delete_length_overlapping = op_length - delete_length_before - delete_length_after + comment.offset = Math.min(comment.offset, op_start) + comment.length -= delete_length_overlapping + @emit "comment:moved", comment + + applyInsertToChanges: (op, metadata) -> op_start = op.p op_length = op.i.length op_end = op.p + op_length @@ -148,7 +189,7 @@ define [ if moved_changes.length > 0 @emit "changes:moved", moved_changes - applyDelete: (op, metadata) -> + applyDeleteToChanges: (op, metadata) -> op_start = op.p op_length = op.d.length op_end = op.p + op_length diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/TrackChangesManager.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/TrackChangesManager.coffee index c5e2cb5669..4ab381dbf4 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/TrackChangesManager.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/TrackChangesManager.coffee @@ -12,7 +12,7 @@ define [ @changesTracker.track_changes = true @changeIdToMarkerIdMap = {} @enabled = false - window.changesTracker ?= @changesTracker + window.trackChangesManager ?= @ @changesTracker.on "insert:added", (change) => sl_console.log "[insert:added]", change @@ -30,6 +30,13 @@ define [ sl_console.log "[changes:moved]", changes @_onChangesMoved(changes) + @changesTracker.on "comment:added", (comment) => + sl_console.log "[comment:added]", comment + @_onCommentAdded(comment) + @changesTracker.on "comment:moved", (comment) => + sl_console.log "[comment:moved]", comment + @_onCommentMoved(comment) + onChange = (e) => if !@editor.initing and @enabled # This change is trigger by a sharejs 'change' event, which is before the @@ -62,6 +69,19 @@ define [ @editor.renderer.on "resize", () => @recalculateReviewEntriesScreenPositions() + addComment: (offset, length, comment) -> + @changesTracker.addComment offset, length, { + comment: comment + user_id: window.user_id + } + + addCommentToSelection: (comment) -> + range = @editor.getSelectionRange() + offset = @_aceRangeToShareJs(range.start) + end = @_aceRangeToShareJs(range.end) + length = end - offset + @addComment(offset, length, comment) + checkMapping: () -> session = @editor.getSession() @@ -104,6 +124,12 @@ define [ content: change.op.i or change.op.d offset: change.op.p } + for comment in @changesTracker.comments + @$scope.reviewPanel.entries[comment.id] = { + content: comment.metadata.comment + offset: comment.offset + } + @recalculateReviewEntriesScreenPositions() recalculateReviewEntriesScreenPositions: () -> @@ -178,16 +204,52 @@ define [ session.removeMarker marker_id @updateReviewEntriesScope() - _aceChangeToShareJs: (delta) -> - start = delta.start - lines = @editor.getSession().getDocument().getLines 0, start.row + _onCommentAdded: (comment) -> + start = @_shareJsOffsetToAcePosition(comment.offset) + end = @_shareJsOffsetToAcePosition(comment.offset + comment.length) + session = @editor.getSession() + doc = session.getDocument() + ace_range = new Range(start.row, start.column, end.row, end.column) + + hue = ColorManager.getHueForUserId(comment.metadata.user_id) + colorScheme = ColorManager.getColorScheme(hue, @element) + markerLayer = @editor.renderer.$markerBack + klass = "track-changes-comment-marker" + style = "border-color: #{colorScheme.cursor}" + marker_id = session.addMarker ace_range, klass, (html, range, left, top, config) -> + if range.isMultiLine() + markerLayer.drawTextMarker(html, range, klass, config, style) + else + markerLayer.drawSingleLineMarker(html, range, "#{klass} ace_start", config, 0, style) + + @changeIdToMarkerIdMap[comment.id] = marker_id + @updateReviewEntriesScope() + + _onCommentMoved: (comment) -> + start = @_shareJsOffsetToAcePosition(comment.offset) + end = @_shareJsOffsetToAcePosition(comment.offset + comment.length) + session = @editor.getSession() + ace_range = new Range(start.row, start.column, end.row, end.column) + marker_id = @changeIdToMarkerIdMap[comment.id] + markers = session.getMarkers() + marker = markers[marker_id] + marker.range.start = start + marker.range.end = end + @editor.renderer.updateBackMarkers() + @updateReviewEntriesScope() + + _aceRangeToShareJs: (range) -> + lines = @editor.getSession().getDocument().getLines 0, range.row offset = 0 for line, i in lines - offset += if i < start.row + offset += if i < range.row line.length else - start.column - offset += start.row # Include newlines + range.column + offset += range.row # Include newlines + + _aceChangeToShareJs: (delta) -> + offset = @_aceRangeToShareJs(delta.start) text = delta.lines.join('\n') switch delta.action diff --git a/services/web/public/stylesheets/app/editor.less b/services/web/public/stylesheets/app/editor.less index c42d210ae0..702ee8338b 100644 --- a/services/web/public/stylesheets/app/editor.less +++ b/services/web/public/stylesheets/app/editor.less @@ -147,6 +147,12 @@ border-bottom: 1px dashed green; background-color: hsl(100, 70%, 85%); } + .track-changes-comment-marker { + border-radius: 0; + position: absolute; + border-bottom: 1px dashed orange; + background-color: #fde5b7; + } .track-changes-deleted-marker { border-radius: 0; position: absolute;