diff --git a/services/web/app/views/project/editor/review-panel.jade b/services/web/app/views/project/editor/review-panel.jade index 3e82e56daa..7c3779b079 100644 --- a/services/web/app/views/project/editor/review-panel.jade +++ b/services/web/app/views/project/editor/review-panel.jade @@ -23,8 +23,10 @@ comment-entry( entry="entry" users="users" - on-resolve="" - on-reply="submitReply(entry);" + on-resolve="resolveComment(entry, entry_id)" + on-unresolve="unresolveComment(entry_id)" + on-delete="deleteComment(entry_id)" + on-reply="submitReply(entry, entry_id);" on-indicator-click="toggleReviewPanel();" ) @@ -61,8 +63,10 @@ comment-entry( entry="entry" users="users" - on-resolve="" - on-reply="submitReply(entry);" + on-resolve="resolveComment(entry, entry_id)" + on-unresolve="unresolveComment(entry_id)" + on-delete="deleteComment(entry_id)" + on-reply="submitReply(entry, entry_id);" on-indicator-click="toggleReviewPanel();" ng-click="gotoEntry(doc_id, entry)" ) @@ -122,7 +126,7 @@ script(type='text/ng-template', id='changeEntryTemplate') script(type='text/ng-template', id='commentEntryTemplate') div - .rp-entry-callout.rp-entry-callout-comment + .rp-entry-callout.rp-entry-callout-comment(ng-if="!entry.resolved") .rp-entry-indicator( ng-class="{ 'rp-entry-indicator-focused': entry.focused }" ng-click="onIndicatorClick();" @@ -132,6 +136,7 @@ script(type='text/ng-template', id='commentEntryTemplate') ng-class="entry.focused ? 'rp-entry-focused' : '';" ) .rp-comment( + ng-if="!entry.resolved || entry.showWhenResolved" ng-repeat="comment in entry.thread" ng-class="users[comment.user_id].isSelf ? 'rp-comment-self' : '';" ) @@ -145,16 +150,24 @@ script(type='text/ng-template', id='commentEntryTemplate') | {{ comment.ts | date : 'MMM d, y h:mm a' }} |  •  span(style="color: hsl({{ users[comment.user_id].hue }}, 70%, 40%);") {{ users[comment.user_id].name }} - .rp-comment-reply + .rp-comment-reply(ng-if="!entry.resolved || entry.showWhenResolved") textarea.rp-comment-input( ng-model="entry.replyContent" ng-keypress="handleCommentReplyKeyPress($event);" - placeholder="Hit \"Enter\" to reply" + placeholder="{{ 'Hit \"Enter\" to reply' + (entry.resolved ? ' and re-open' : '') }}" ) + .rp-comment-resolved-description(ng-if="entry.resolved && !entry.showWhenResolved") + | Resolved Comment + a(href, ng-click="entry.showWhenResolved = true") Show .rp-entry-actions - a.rp-entry-button(href, ng-click="onResolve();") + a.rp-entry-button(href, ng-click="onResolve();", ng-if="!entry.resolved") i.fa.fa-check |  Mark as resolved + a.rp-entry-button(href, ng-click="onUnresolve();", ng-if="entry.resolved") + |  Re-open + a.rp-entry-button(href, ng-click="onDelete();", ng-if="entry.resolved") + |  Delete + script(type='text/ng-template', id='addCommentEntryTemplate') div 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 9c9a2bd7bc..95a10befcd 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 @@ -44,6 +44,15 @@ define [ @$scope.$on "change:reject", (e, change_id) => @rejectChangeId(change_id) + + @$scope.$on "comment:remove", (e, comment_id) => + @removeCommentId(comment_id) + + @$scope.$on "comment:resolve", (e, comment_id) => + @resolveCommentId(comment_id) + + @$scope.$on "comment:unresolve", (e, comment_id) => + @unresolveCommentId(comment_id) onChange = (e) => if !@editor.initing and @enabled @@ -92,7 +101,10 @@ define [ @changesTracker.off "delete:removed" @changesTracker.off "changes:moved" @changesTracker.off "comment:added" + @changesTracker.off "comment:moved" @changesTracker.off "comment:removed" + @changesTracker.off "comment:resolved" + @changesTracker.off "comment:unresolved" connectToChangesTracker: () -> @changesTracker.track_changes = @$scope.trackNewChanges @@ -119,6 +131,15 @@ define [ @changesTracker.on "comment:moved", (comment) => sl_console.log "[comment:moved]", comment @_onCommentMoved(comment) + @changesTracker.on "comment:removed", (comment) => + sl_console.log "[comment:removed]", comment + @_onCommentRemoved(comment) + @changesTracker.on "comment:resolved", (comment) => + sl_console.log "[comment:resolved]", comment + @_onCommentRemoved(comment) + @changesTracker.on "comment:unresolved", (comment) => + sl_console.log "[comment:unresolved]", comment + @_onCommentAdded(comment) redrawAnnotations: () -> for change in @changesTracker.changes @@ -179,6 +200,14 @@ define [ else throw new Error("unknown change: #{JSON.stringify(change)}") + removeCommentId: (comment_id) -> + @changesTracker.removeCommentId(comment_id) + + resolveCommentId: (comment_id) -> + @changesTracker.resolveCommentId(comment_id) + + unresolveCommentId: (comment_id) -> + @changesTracker.unresolveCommentId(comment_id) checkMapping: () -> session = @editor.getSession() @@ -190,22 +219,24 @@ define [ expected_markers = [] for change in @changesTracker.changes - op = change.op - {background_marker_id, callout_marker_id} = @changeIdToMarkerIdMap[change.id] - start = @_shareJsOffsetToAcePosition(op.p) - if op.i? - end = @_shareJsOffsetToAcePosition(op.p + op.i.length) - else if op.d? - end = start - expected_markers.push { marker_id: background_marker_id, start, end } - expected_markers.push { marker_id: callout_marker_id, start, end: start } + if @changeIdToMarkerIdMap[change.id]? + op = change.op + {background_marker_id, callout_marker_id} = @changeIdToMarkerIdMap[change.id] + start = @_shareJsOffsetToAcePosition(op.p) + if op.i? + end = @_shareJsOffsetToAcePosition(op.p + op.i.length) + else if op.d? + end = start + expected_markers.push { marker_id: background_marker_id, start, end } + expected_markers.push { marker_id: callout_marker_id, start, end: start } for comment in @changesTracker.comments - {background_marker_id, callout_marker_id} = @changeIdToMarkerIdMap[comment.id] - start = @_shareJsOffsetToAcePosition(comment.offset) - end = @_shareJsOffsetToAcePosition(comment.offset + comment.length) - expected_markers.push { marker_id: background_marker_id, start, end } - expected_markers.push { marker_id: callout_marker_id, start, end: start } + if @changeIdToMarkerIdMap[comment.id]? + {background_marker_id, callout_marker_id} = @changeIdToMarkerIdMap[comment.id] + start = @_shareJsOffsetToAcePosition(comment.offset) + end = @_shareJsOffsetToAcePosition(comment.offset + comment.length) + expected_markers.push { marker_id: background_marker_id, start, end } + expected_markers.push { marker_id: callout_marker_id, start, end: start } for {marker_id, start, end} in expected_markers marker = markers[marker_id] @@ -253,6 +284,7 @@ define [ new_entry = { type: "comment" thread: comment.metadata.thread + resolved: comment.metadata.resolved offset: comment.offset length: comment.length } @@ -281,7 +313,7 @@ define [ } for id, entry of entries - if entry.type == "comment" + if entry.type == "comment" and not entry.resolved entry.focused = (entry.offset <= cursor_offset <= entry.offset + entry.length) else if entry.type == "insert" entry.focused = (entry.offset <= cursor_offset <= entry.offset + entry.content.length) @@ -363,6 +395,7 @@ define [ _onInsertRemoved: (change) -> {background_marker_id, callout_marker_id} = @changeIdToMarkerIdMap[change.id] + delete @changeIdToMarkerIdMap[change.id] session = @editor.getSession() session.removeMarker background_marker_id session.removeMarker callout_marker_id @@ -370,20 +403,33 @@ define [ _onDeleteRemoved: (change) -> {background_marker_id, callout_marker_id} = @changeIdToMarkerIdMap[change.id] + delete @changeIdToMarkerIdMap[change.id] session = @editor.getSession() session.removeMarker background_marker_id session.removeMarker callout_marker_id @updateReviewEntriesScope() _onCommentAdded: (comment) -> - start = @_shareJsOffsetToAcePosition(comment.offset) - end = @_shareJsOffsetToAcePosition(comment.offset + comment.length) - session = @editor.getSession() - doc = session.getDocument() - background_range = new Range(start.row, start.column, end.row, end.column) - background_marker_id = session.addMarker background_range, "track-changes-marker track-changes-comment-marker", "text" - callout_marker_id = @_createCalloutMarker(start, "track-changes-comment-marker-callout") - @changeIdToMarkerIdMap[comment.id] = { background_marker_id, callout_marker_id } + if !@changeIdToMarkerIdMap[comment.id]? + # Only create new markers if they don't already exist + start = @_shareJsOffsetToAcePosition(comment.offset) + end = @_shareJsOffsetToAcePosition(comment.offset + comment.length) + session = @editor.getSession() + doc = session.getDocument() + background_range = new Range(start.row, start.column, end.row, end.column) + background_marker_id = session.addMarker background_range, "track-changes-marker track-changes-comment-marker", "text" + callout_marker_id = @_createCalloutMarker(start, "track-changes-comment-marker-callout") + @changeIdToMarkerIdMap[comment.id] = { background_marker_id, callout_marker_id } + @updateReviewEntriesScope() + + _onCommentRemoved: (comment) -> + if @changeIdToMarkerIdMap[comment.id]? + # Resolved comments may not have marker ids + {background_marker_id, callout_marker_id} = @changeIdToMarkerIdMap[comment.id] + delete @changeIdToMarkerIdMap[comment.id] + session = @editor.getSession() + session.removeMarker background_marker_id + session.removeMarker callout_marker_id @updateReviewEntriesScope() _aceRangeToShareJs: (range) -> @@ -436,13 +482,16 @@ define [ @updateReviewEntriesScope() _updateMarker: (change_id, start, end) -> + return if !@changeIdToMarkerIdMap[change_id]? session = @editor.getSession() markers = session.getMarkers() {background_marker_id, callout_marker_id} = @changeIdToMarkerIdMap[change_id] - background_marker = markers[background_marker_id] - background_marker.range.start = start - background_marker.range.end = end - callout_marker = markers[callout_marker_id] - callout_marker.range.start = start - callout_marker.range.end = start + if background_marker_id? + background_marker = markers[background_marker_id] + background_marker.range.start = start + background_marker.range.end = end + if callout_marker_id? + callout_marker = markers[callout_marker_id] + callout_marker.range.start = start + callout_marker.range.end = start diff --git a/services/web/public/coffee/ide/review-panel/ChangesTracker.coffee b/services/web/public/coffee/ide/review-panel/ChangesTracker.coffee index 250f1c6fe1..1ea9852ab1 100644 --- a/services/web/public/coffee/ide/review-panel/ChangesTracker.coffee +++ b/services/web/public/coffee/ide/review-panel/ChangesTracker.coffee @@ -61,6 +61,32 @@ define [ @emit "comment:added", comment return comment + getComment: (comment_id) -> + comment = null + for c in @comments + if c.id == comment_id + comment = c + break + return comment + + resolveCommentId: (comment_id) -> + comment = @getComment(comment_id) + return if !comment? + comment.metadata.resolved = true + @emit "comment:resolved", comment + + unresolveCommentId: (comment_id) -> + comment = @getComment(comment_id) + return if !comment? + comment.metadata.resolved = false + @emit "comment:unresolved", comment + + removeCommentId: (comment_id) -> + comment = @getComment(comment_id) + return if !comment? + @comments = @comments.filter (c) -> c.id != comment_id + @emit "comment:removed", comment + getChange: (change_id) -> change = null for c in @changes diff --git a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee index e126984cf4..d6027d745c 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -110,7 +110,8 @@ define [ # ev.target.blur() # $scope.submitReply(entry) - $scope.submitReply = (entry) -> + $scope.submitReply = (entry, entry_id) -> + $scope.unresolveComment(entry_id) entry.thread.push { content: entry.replyContent ts: new Date() @@ -141,7 +142,17 @@ define [ entry.replying = false entry.replyContent = "" $scope.$broadcast "review-panel:layout" + + $scope.resolveComment = (entry, entry_id) -> + entry.showWhenResolved = false + $scope.$broadcast "comment:resolve", entry_id + $scope.unresolveComment = (entry_id) -> + $scope.$broadcast "comment:unresolve", entry_id + + $scope.deleteComment = (entry_id) -> + $scope.$broadcast "comment:remove", entry_id + $scope.setSubView = (subView) -> $scope.reviewPanel.subView = subView diff --git a/services/web/public/coffee/ide/review-panel/directives/commentEntry.coffee b/services/web/public/coffee/ide/review-panel/directives/commentEntry.coffee index 3fe0bd93cf..1f15f688dd 100644 --- a/services/web/public/coffee/ide/review-panel/directives/commentEntry.coffee +++ b/services/web/public/coffee/ide/review-panel/directives/commentEntry.coffee @@ -10,6 +10,8 @@ define [ onResolve: "&" onReply: "&" onIndicatorClick: "&" + onDelete: "&" + onUnresolve: "&" link: (scope, element, attrs) -> scope.handleCommentReplyKeyPress = (ev) -> if ev.keyCode == 13 and !ev.shiftKey and !ev.ctrlKey and !ev.metaKey