From abb4ef14d866bb091ba312579e11eb7841bd9a2a Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 26 Jan 2017 15:08:30 +0100 Subject: [PATCH 1/4] Streamline the update/rendering process to not do extra work --- .../views/project/editor/review-panel.jade | 1 - .../track-changes/TrackChangesManager.coffee | 55 ++++++++++++------- .../controllers/ReviewPanelController.coffee | 6 +- 3 files changed, 39 insertions(+), 23 deletions(-) diff --git a/services/web/app/views/project/editor/review-panel.jade b/services/web/app/views/project/editor/review-panel.jade index 9b31516b3f..590c1a6776 100644 --- a/services/web/app/views/project/editor/review-panel.jade +++ b/services/web/app/views/project/editor/review-panel.jade @@ -41,7 +41,6 @@ .rp-entry-list-inner .rp-entry-wrapper( ng-repeat="(entry_id, entry) in reviewPanel.entries[editor.open_doc_id]" - ng-if="!(entry.type === 'comment' && reviewPanel.commentThreads[entry.thread_id].resolved === true)" ) div(ng-if="entry.type === 'insert' || entry.type === 'delete'") change-entry( 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 2d57100cc5..ed15da2958 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 @@ -35,8 +35,8 @@ define [ @$scope.$on "comment:remove", (e, comment_id) => @removeCommentId(comment_id) - @$scope.$on "comment:resolve_thread", (e, thread_id) => - @resolveCommentByThreadId(thread_id) + @$scope.$on "comment:resolve_threads", (e, thread_ids) => + @resolveCommentByThreadIds(thread_ids) @$scope.$on "comment:unresolve_thread", (e, thread_id) => @unresolveCommentByThreadId(thread_id) @@ -105,29 +105,45 @@ define [ # ace has updated @rangesTracker.on "insert:added", (change) => sl_console.log "[insert:added]", change - setTimeout () => @_onInsertAdded(change) + setTimeout () => + @_onInsertAdded(change) + @broadcastChange() @rangesTracker.on "insert:removed", (change) => sl_console.log "[insert:removed]", change - setTimeout () => @_onInsertRemoved(change) + setTimeout () => + @_onInsertRemoved(change) + @broadcastChange() @rangesTracker.on "delete:added", (change) => sl_console.log "[delete:added]", change - setTimeout () => @_onDeleteAdded(change) + setTimeout () => + @_onDeleteAdded(change) + @broadcastChange() @rangesTracker.on "delete:removed", (change) => sl_console.log "[delete:removed]", change - setTimeout () => @_onDeleteRemoved(change) + setTimeout () => + @_onDeleteRemoved(change) + @broadcastChange() @rangesTracker.on "changes:moved", (changes) => sl_console.log "[changes:moved]", changes - setTimeout () => @_onChangesMoved(changes) + setTimeout () => + @_onChangesMoved(changes) + @broadcastChange() @rangesTracker.on "comment:added", (comment) => sl_console.log "[comment:added]", comment - setTimeout () => @_onCommentAdded(comment) + setTimeout () => + @_onCommentAdded(comment) + @broadcastChange() @rangesTracker.on "comment:moved", (comment) => sl_console.log "[comment:moved]", comment - setTimeout () => @_onCommentMoved(comment) + setTimeout () => + @_onCommentMoved(comment) + @broadcastChange() @rangesTracker.on "comment:removed", (comment) => sl_console.log "[comment:removed]", comment - setTimeout () => @_onCommentRemoved(comment) + setTimeout () => + @_onCommentRemoved(comment) + @broadcastChange() @rangesTracker.on "clear", () => @clearAnnotations() @@ -150,6 +166,8 @@ define [ for comment in @rangesTracker.comments @_onCommentAdded(comment) + + @broadcastChange() addComment: (offset, content, thread_id) -> op = { c: content, p: offset, t: thread_id } @@ -190,15 +208,20 @@ define [ removeCommentId: (comment_id) -> @rangesTracker.removeCommentId(comment_id) - resolveCommentByThreadId: (thread_id) -> + resolveCommentByThreadIds: (thread_ids) -> + resolve_ids = {} + for id in thread_ids + resolve_ids[id] = true for comment in @rangesTracker?.comments or [] - if comment.op.t == thread_id + if resolve_ids[comment.op.t] @_onCommentRemoved(comment) + @broadcastChange() unresolveCommentByThreadId: (thread_id) -> for comment in @rangesTracker?.comments or [] if comment.op.t == thread_id @_onCommentAdded(comment) + @broadcastChange() checkMapping: () -> # TODO: reintroduce this check @@ -303,7 +326,6 @@ define [ background_marker_id = session.addMarker background_range, "track-changes-marker track-changes-added-marker", "text" callout_marker_id = @_createCalloutMarker(start, "track-changes-added-marker-callout") @changeIdToMarkerIdMap[change.id] = { background_marker_id, callout_marker_id } - @broadcastChange() _onDeleteAdded: (change) -> position = @_shareJsOffsetToAcePosition(change.op.p) @@ -318,7 +340,6 @@ define [ callout_marker_id = @_createCalloutMarker(position, "track-changes-deleted-marker-callout") @changeIdToMarkerIdMap[change.id] = { background_marker_id, callout_marker_id } - @broadcastChange() _onInsertRemoved: (change) -> {background_marker_id, callout_marker_id} = @changeIdToMarkerIdMap[change.id] @@ -326,7 +347,6 @@ define [ session = @editor.getSession() session.removeMarker background_marker_id session.removeMarker callout_marker_id - @broadcastChange() _onDeleteRemoved: (change) -> {background_marker_id, callout_marker_id} = @changeIdToMarkerIdMap[change.id] @@ -334,7 +354,6 @@ define [ session = @editor.getSession() session.removeMarker background_marker_id session.removeMarker callout_marker_id - @broadcastChange() _onCommentAdded: (comment) -> if @rangesTracker.resolvedThreadIds[comment.op.t] @@ -350,7 +369,6 @@ define [ 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 } - @broadcastChange() _onCommentRemoved: (comment) -> if @changeIdToMarkerIdMap[comment.id]? @@ -360,7 +378,6 @@ define [ session = @editor.getSession() session.removeMarker background_marker_id session.removeMarker callout_marker_id - @broadcastChange() _aceRangeToShareJs: (range) -> lines = @editor.getSession().getDocument().getLines 0, range.row @@ -385,14 +402,12 @@ define [ end = start @_updateMarker(change.id, start, end) @editor.renderer.updateBackMarkers() - @broadcastChange() _onCommentMoved: (comment) -> start = @_shareJsOffsetToAcePosition(comment.op.p) end = @_shareJsOffsetToAcePosition(comment.op.p + comment.op.c.length) @_updateMarker(comment.id, start, end) @editor.renderer.updateBackMarkers() - @broadcastChange() _updateMarker: (change_id, start, end) -> return if !@changeIdToMarkerIdMap[change_id]? 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 5dc2cd3715..45b2e7294e 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -222,8 +222,10 @@ define [ delete delete_changes[comment.id] if $scope.reviewPanel.resolvedThreadIds[comment.op.t] new_comment = resolvedComments[comment.id] ?= {} + delete entries[comment.id] else new_comment = entries[comment.id] ?= {} + delete resolvedComments[comment.id] new_entry = { type: "comment" thread_id: comment.op.t @@ -356,7 +358,7 @@ define [ thread.resolved_by_user = formatUser(user) thread.resolved_at = new Date() $scope.reviewPanel.resolvedThreadIds[thread_id] = true - $scope.$broadcast "comment:resolve_thread", thread_id + $scope.$broadcast "comment:resolve_threads", [thread_id] _onCommentReopened = (thread_id) -> thread = getThread(thread_id) @@ -477,9 +479,9 @@ define [ for comment in thread.messages formatComment(comment) if thread.resolved_by_user? - $scope.$broadcast "comment:resolve_thread", thread_id thread.resolved_by_user = formatUser(thread.resolved_by_user) $scope.reviewPanel.resolvedThreadIds[thread_id] = true + $scope.$broadcast "comment:resolve_threads", [thread_id] $scope.reviewPanel.commentThreads = threads $timeout () -> $scope.$broadcast "review-panel:layout" From 76328ff93f2292eb1e59ea7ebe9dc70197cbcfd1 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 26 Jan 2017 15:12:59 +0100 Subject: [PATCH 2/4] Tell other clients when threads are deleted --- .../review-panel/controllers/ReviewPanelController.coffee | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 45b2e7294e..ac687386f8 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -66,6 +66,10 @@ define [ ide.socket.on "reopen-thread", (thread_id) -> _onCommentReopened(thread_id) + ide.socket.on "delete-thread", (thread_id) -> + _onThreadDeleted(thread_id) + $scope.$apply () -> + ide.socket.on "edit-message", (thread_id, message_id, content) -> _onCommentEdited(thread_id, message_id, content) $scope.$apply () -> @@ -372,6 +376,7 @@ define [ _onThreadDeleted = (thread_id) -> delete $scope.reviewPanel.resolvedThreadIds[thread_id] delete $scope.reviewPanel.commentThreads[thread_id] + $scope.$broadcast "comment:remove", thread_id _onCommentEdited = (thread_id, comment_id, content) -> thread = getThread(thread_id) @@ -396,7 +401,6 @@ define [ 'X-CSRF-Token': window.csrfToken } }) - $scope.$broadcast "comment:remove", entry_id event_tracking.sendMB "rp-comment-delete" $scope.saveEdit = (thread_id, comment) -> From d9b774bb9b02e8c3cd2f08bac8fea6d9c9ca53fe Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 27 Jan 2017 17:12:17 +0100 Subject: [PATCH 3/4] Sycn track changes based on doc state, not editor state --- services/web/public/coffee/ide/editor/Document.coffee | 3 +++ services/web/public/coffee/ide/editor/EditorManager.coffee | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/services/web/public/coffee/ide/editor/Document.coffee b/services/web/public/coffee/ide/editor/Document.coffee index 9d7eca813a..1de01b5467 100644 --- a/services/web/public/coffee/ide/editor/Document.coffee +++ b/services/web/public/coffee/ide/editor/Document.coffee @@ -84,6 +84,9 @@ define [ setTrackingChanges: (track_changes) -> @doc.track_changes = track_changes + getTrackingChanges: () -> + !!@doc.track_changes + setTrackChangesIdSeeds: (id_seeds) -> @doc.track_changes_id_seeds = id_seeds diff --git a/services/web/public/coffee/ide/editor/EditorManager.coffee b/services/web/public/coffee/ide/editor/EditorManager.coffee index 22fcef1a69..9b658ac87b 100644 --- a/services/web/public/coffee/ide/editor/EditorManager.coffee +++ b/services/web/public/coffee/ide/editor/EditorManager.coffee @@ -162,7 +162,7 @@ define [ @_syncTimeout = null want = @$scope.editor.wantTrackChanges - have = @$scope.editor.trackChanges + have = doc.getTrackingChanges() if want == have return From 30618d33db61295a9c0c9f2467a80a25fe74abb7 Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 27 Jan 2017 17:13:11 +0100 Subject: [PATCH 4/4] Add in extra check to set ui state --- services/web/public/coffee/ide/editor/EditorManager.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/public/coffee/ide/editor/EditorManager.coffee b/services/web/public/coffee/ide/editor/EditorManager.coffee index 9b658ac87b..3b6672fae8 100644 --- a/services/web/public/coffee/ide/editor/EditorManager.coffee +++ b/services/web/public/coffee/ide/editor/EditorManager.coffee @@ -164,6 +164,7 @@ define [ want = @$scope.editor.wantTrackChanges have = doc.getTrackingChanges() if want == have + @$scope.editor.trackChanges = want return do tryToggle = () =>