diff --git a/services/web/app/views/project/editor/review-panel.pug b/services/web/app/views/project/editor/review-panel.pug index 3707af11eb..07f6bd081e 100644 --- a/services/web/app/views/project/editor/review-panel.pug +++ b/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 }" 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 77642c19e0..55590225d4 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 @@ -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) diff --git a/services/web/public/coffee/ide/review-panel/ReviewPanelManager.coffee b/services/web/public/coffee/ide/review-panel/ReviewPanelManager.coffee index 420c9c6139..85c0268609 100644 --- a/services/web/public/coffee/ide/review-panel/ReviewPanelManager.coffee +++ b/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" 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 fe4f8871f0..5a56baba2c 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/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 diff --git a/services/web/public/coffee/ide/review-panel/directives/aggregateChangeEntry.coffee b/services/web/public/coffee/ide/review-panel/directives/aggregateChangeEntry.coffee new file mode 100644 index 0000000000..46689193b2 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/services/web/public/coffee/ide/review-panel/directives/reviewPanelSorted.coffee b/services/web/public/coffee/ide/review-panel/directives/reviewPanelSorted.coffee index 236668c5ab..3445ac845a 100644 --- a/services/web/public/coffee/ide/review-panel/directives/reviewPanelSorted.coffee +++ b/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} diff --git a/services/web/public/stylesheets/app/editor/review-panel.less b/services/web/public/stylesheets/app/editor/review-panel.less index 861360b526..312b933b85 100644 --- a/services/web/public/stylesheets/app/editor/review-panel.less +++ b/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;