From a6108480f52d399f1d94c2a2d10c9f9630b00af7 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Wed, 31 May 2017 14:47:25 +0100 Subject: [PATCH 01/20] Add directive to show change entries. --- .../review-panel/ReviewPanelManager.coffee | 1 + .../directives/aggregateChangeEntry.coffee | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 services/web/public/coffee/ide/review-panel/directives/aggregateChangeEntry.coffee 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/directives/aggregateChangeEntry.coffee b/services/web/public/coffee/ide/review-panel/directives/aggregateChangeEntry.coffee new file mode 100644 index 0000000000..dc3b79b840 --- /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 = 40 + 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", (contentLength) -> + # scope.needsCollapsing = contentLength > scope.contentLimit \ No newline at end of file From b6cef2e3d7604d4a53e897add1f6441a8ee295ef Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Wed, 31 May 2017 15:53:14 +0100 Subject: [PATCH 02/20] Include directive; add some styling. --- .../app/views/project/editor/review-panel.pug | 60 +++++++++++++++++++ .../stylesheets/app/editor/review-panel.less | 6 +- 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/services/web/app/views/project/editor/review-panel.pug b/services/web/app/views/project/editor/review-panel.pug index 3707af11eb..2a12f567b4 100644 --- a/services/web/app/views/project/editor/review-panel.pug +++ b/services/web/app/views/project/editor/review-panel.pug @@ -82,6 +82,17 @@ permissions="permissions" ) + div(ng-if="entry.type === 'agg-change'") + aggregate-change-entry( + entry="entry" + user="users[entry.metadata.user_id]" + on-reject="rejectChange(entry_id);" + on-accept="acceptChange(entry_id);" + on-indicator-click="toggleReviewPanel();" + on-body-click="gotoEntry(editor.open_doc_id, entry)" + permissions="permissions" + ) + div(ng-if="entry.type === 'comment'") comment-entry( entry="entry" @@ -147,6 +158,17 @@ permissions="permissions" ) + div(ng-if="entry.type === 'agg-change'") + aggregate-change-entry( + entry="entry" + user="users[entry.metadata.user_id]" + on-reject="rejectChange(entry_id);" + on-accept="acceptChange(entry_id);" + on-indicator-click="toggleReviewPanel();" + on-body-click="gotoEntry(editor.open_doc_id, entry)" + permissions="permissions" + ) + div(ng-if="entry.type === 'comment'") comment-entry( entry="entry" @@ -222,6 +244,44 @@ 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 + | Changed + del.rp-content-highlight {{ entry.content }} + | for + ins.rp-content-highlight {{ entry.metadata.agg_op.content }} + //- span(ng-switch-when="insert") #{translate("tracked_change_added")}  + //- ins.rp-content-highlight {{ entry.content | limitTo:(isCollapsed ? contentLimit : entry.content.length) }} + //- 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/stylesheets/app/editor/review-panel.less b/services/web/public/stylesheets/app/editor/review-panel.less index 861360b526..cb60afd62f 100644 --- a/services/web/public/stylesheets/app/editor/review-panel.less +++ b/services/web/public/stylesheets/app/editor/review-panel.less @@ -299,7 +299,8 @@ transition: none; } - &-insert { + &-insert, + &-aggregate { border-color: @rp-green; } @@ -389,7 +390,7 @@ font-weight: @rp-semibold-weight; text-decoration: none; - .rp-entry-delete & { + del& { text-decoration: line-through; } } @@ -780,6 +781,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; From 3cf8f26583d5f4afd82c35b044f8fe7545b4085c Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Wed, 31 May 2017 16:23:24 +0100 Subject: [PATCH 03/20] Add collapse and expand behaviour to aggregate change entries. --- .../controllers/ReviewPanelController.coffee | 26 ++++++++++++++++--- .../directives/aggregateChangeEntry.coffee | 6 ++--- 2 files changed, 25 insertions(+), 7 deletions(-) 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..a08e8a93cb 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -225,6 +225,7 @@ define [ changed = false + # 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 @@ -233,23 +234,40 @@ define [ for change_id, change of resolvedComments delete_changes[change_id] = true + potential_aggregate = false + prev_insertion = null + for change in rangesTracker.changes changed = true + aggregate_entry = false 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 + + if potential_aggregate and change.op.d and change.op.p == prev_insertion.op.p + prev_insertion.op.i.length + aggregate_entry = true + 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 } + + if aggregate_entry + new_entry.type = "agg-change" + new_entry.metadata.agg_op = entries[prev_insertion.id] + delete entries[prev_insertion.id] + 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) diff --git a/services/web/public/coffee/ide/review-panel/directives/aggregateChangeEntry.coffee b/services/web/public/coffee/ide/review-panel/directives/aggregateChangeEntry.coffee index dc3b79b840..46689193b2 100644 --- a/services/web/public/coffee/ide/review-panel/directives/aggregateChangeEntry.coffee +++ b/services/web/public/coffee/ide/review-panel/directives/aggregateChangeEntry.coffee @@ -13,7 +13,7 @@ define [ onIndicatorClick: "&" onBodyClick: "&" link: (scope, element, attrs) -> - scope.contentLimit = 40 + scope.contentLimit = 35 scope.isCollapsed = true scope.needsCollapsing = false @@ -26,5 +26,5 @@ define [ $timeout () -> scope.$emit "review-panel:layout" - # scope.$watch "entry.content.length", (contentLength) -> - # scope.needsCollapsing = contentLength > scope.contentLimit \ No newline at end of file + scope.$watch "entry.content.length + entry.metadata.agg_op.content.length", (contentLength) -> + scope.needsCollapsing = contentLength > scope.contentLimit \ No newline at end of file From 11c0644490bfd124c45e00cf20c94948db355b9b Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Thu, 1 Jun 2017 11:47:55 +0100 Subject: [PATCH 04/20] Aggregate changes; make accept and reject work. --- .../app/views/project/editor/review-panel.pug | 16 ++++++-------- .../controllers/ReviewPanelController.coffee | 22 +++++++++++++++---- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/services/web/app/views/project/editor/review-panel.pug b/services/web/app/views/project/editor/review-panel.pug index 2a12f567b4..2366cedc06 100644 --- a/services/web/app/views/project/editor/review-panel.pug +++ b/services/web/app/views/project/editor/review-panel.pug @@ -86,8 +86,8 @@ aggregate-change-entry( entry="entry" user="users[entry.metadata.user_id]" - on-reject="rejectChange(entry_id);" - on-accept="acceptChange(entry_id);" + on-reject="rejectAggChange(entry_id, entry.metadata.agg_op_id);" + on-accept="acceptAggChange(entry_id, entry.metadata.agg_op_id);" on-indicator-click="toggleReviewPanel();" on-body-click="gotoEntry(editor.open_doc_id, entry)" permissions="permissions" @@ -264,13 +264,11 @@ script(type='text/ng-template', id='aggregateChangeEntryTemplate') del.rp-content-highlight {{ entry.content }} | for ins.rp-content-highlight {{ entry.metadata.agg_op.content }} - //- span(ng-switch-when="insert") #{translate("tracked_change_added")}  - //- ins.rp-content-highlight {{ entry.content | limitTo:(isCollapsed ? contentLimit : entry.content.length) }} - //- a.rp-collapse-toggle( - //- href - //- ng-if="needsCollapsing" - //- ng-click="toggleCollapse();" - //- ) {{ isCollapsed ? '... (#{translate("show_all")})' : ' (#{translate("show_less")})' }} + 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 }} 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 a08e8a93cb..c296bc9c72 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -256,6 +256,7 @@ define [ if aggregate_entry new_entry.type = "agg-change" new_entry.metadata.agg_op = entries[prev_insertion.id] + new_entry.metadata.agg_op_id = prev_insertion.id delete entries[prev_insertion.id] for key, value of new_entry @@ -356,10 +357,23 @@ define [ $scope.$broadcast "change:reject", entry_id event_tracking.sendMB "rp-change-rejected", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini' } + $scope.acceptAggChange = (entry_id1, entry_id2) -> + _doAcceptMultipleChanges [ entry_id1, entry_id2 ] + event_tracking.sendMB "rp-agg-change-accepted", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini' } + + $scope.rejectAggChange = (entry_id1, entry_id2) -> + _doRejectMultipleChanges [ entry_id1, entry_id2 ] + event_tracking.sendMB "rp-agg-change-rejected", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini' } + + _doAcceptMultipleChanges = (change_ids) -> + $http.post "/project/#{$scope.project_id}/doc/#{$scope.editor.open_doc_id}/changes/accept", { change_ids, _csrf: window.csrfToken} + $scope.$broadcast "change:bulk-accept", change_ids + + _doRejectMultipleChanges = (change_ids) -> + $scope.$broadcast "change:bulk-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 + _doAcceptMultipleChanges $scope.reviewPanel.selectedEntryIds.slice() $scope.reviewPanel.selectedEntryIds = [] event_tracking.sendMB "rp-bulk-accept", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini', @@ -367,7 +381,7 @@ define [ } bulkReject = () -> - $scope.$broadcast "change:bulk-reject", $scope.reviewPanel.selectedEntryIds.slice() + _doRejectMultipleChanges $scope.reviewPanel.selectedEntryIds.slice() $scope.reviewPanel.selectedEntryIds = [] event_tracking.sendMB "rp-bulk-reject", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini', From 1490ea7b048790cd3215f4633c5cfedef002a44e Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Thu, 1 Jun 2017 15:03:37 +0100 Subject: [PATCH 05/20] Change aggregated change model: the main change is now the insertion, deletion is stored in metadata. --- .../app/views/project/editor/review-panel.pug | 4 +-- .../controllers/ReviewPanelController.coffee | 25 ++++++++++++------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/services/web/app/views/project/editor/review-panel.pug b/services/web/app/views/project/editor/review-panel.pug index 2366cedc06..f7fe16bdcd 100644 --- a/services/web/app/views/project/editor/review-panel.pug +++ b/services/web/app/views/project/editor/review-panel.pug @@ -261,9 +261,9 @@ script(type='text/ng-template', id='aggregateChangeEntryTemplate') .rp-entry-details .rp-entry-description | Changed - del.rp-content-highlight {{ entry.content }} + del.rp-content-highlight {{ entry.metadata.agg_op.content }} | for - ins.rp-content-highlight {{ entry.metadata.agg_op.content }} + ins.rp-content-highlight {{ entry.content }} a.rp-collapse-toggle( href ng-if="needsCollapsing" 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 c296bc9c72..956fdc9215 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 = {} @@ -254,13 +259,12 @@ define [ } if aggregate_entry - new_entry.type = "agg-change" - new_entry.metadata.agg_op = entries[prev_insertion.id] - new_entry.metadata.agg_op_id = prev_insertion.id - delete entries[prev_insertion.id] - - for key, value of new_entry - entries[change.id][key] = value + entries[prev_insertion.id].type = "agg-change" + entries[prev_insertion.id].metadata.agg_op = new_entry + entries[prev_insertion.id].metadata.agg_op_id = change.id + else + for key, value of new_entry + entries[change.id][key] = value if change.op.i potential_aggregate = true @@ -314,8 +318,11 @@ 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.nVisibleSelectedChanges = 0 + console.log selection_offset_start, selection_offset_end delete entries["add-comment"] delete entries["bulk-actions"] From 2a0454f2f1992b5e27bdd2baa5ed1ffe7f58f604 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Thu, 1 Jun 2017 15:18:43 +0100 Subject: [PATCH 06/20] Integrate aggregate changes with bulk actions. --- .../app/views/project/editor/review-panel.pug | 10 ++++----- .../controllers/ReviewPanelController.coffee | 21 +++++++++++++------ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/services/web/app/views/project/editor/review-panel.pug b/services/web/app/views/project/editor/review-panel.pug index f7fe16bdcd..a33fa28e7d 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" @@ -117,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( 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 956fdc9215..e9aed7600a 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -230,7 +230,6 @@ define [ changed = false - # 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 @@ -321,8 +320,7 @@ define [ # 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.nVisibleSelectedChanges = 0 - console.log selection_offset_start, selection_offset_end + $scope.reviewPanel.nVisibleSelectedChanges = 0 delete entries["add-comment"] delete entries["bulk-actions"] @@ -344,11 +342,22 @@ define [ else if entry.type == "insert" isEntryWithinSelection = 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 + if isEntryWithinSelection + $scope.reviewPanel.selectedEntryIds.push id + $scope.reviewPanel.nVisibleSelectedChanges++ else if entry.type == "delete" isEntryWithinSelection = selection_offset_start <= entry.offset <= selection_offset_end entry.focused = (entry.offset == selection_offset_start) - $scope.reviewPanel.selectedEntryIds.push id if isEntryWithinSelection + if isEntryWithinSelection + $scope.reviewPanel.selectedEntryIds.push id + $scope.reviewPanel.nVisibleSelectedChanges++ + else if entry.type == "agg-change" + isEntryWithinSelection = 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) + if isEntryWithinSelection + $scope.reviewPanel.selectedEntryIds.push id, entry.metadata.agg_op_id + $scope.reviewPanel.nVisibleSelectedChanges++ + else if entry.type in [ "add-comment", "bulk-actions" ] and selection entry.focused = true @@ -406,7 +415,7 @@ define [ controller: "BulkActionsModalController" resolve: isAccept: () -> isAccept - nChanges: () -> $scope.reviewPanel.selectedEntryIds.length + nChanges: () -> $scope.reviewPanel.nVisibleSelectedChanges scope: $scope.$new() }).result.then (isAccept) -> if isAccept From b2a7686204a7506d96fcb3cb37caaa6b2604e06f Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Thu, 1 Jun 2017 15:28:37 +0100 Subject: [PATCH 07/20] DRYness. --- .../controllers/ReviewPanelController.coffee | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) 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 e9aed7600a..76ad8c84cb 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -337,29 +337,25 @@ 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) - if isEntryWithinSelection - $scope.reviewPanel.selectedEntryIds.push id - $scope.reviewPanel.nVisibleSelectedChanges++ 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) - if isEntryWithinSelection - $scope.reviewPanel.selectedEntryIds.push id - $scope.reviewPanel.nVisibleSelectedChanges++ else if entry.type == "agg-change" - 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) - if isEntryWithinSelection - $scope.reviewPanel.selectedEntryIds.push id, entry.metadata.agg_op_id - $scope.reviewPanel.nVisibleSelectedChanges++ - else if entry.type in [ "add-comment", "bulk-actions" ] and selection entry.focused = true + + if isChangeEntryAndWithinSelection + $scope.reviewPanel.selectedEntryIds.push id + $scope.reviewPanel.selectedEntryIds.push entry.metadata.agg_op_id if entry.type == "agg-change" + $scope.reviewPanel.nVisibleSelectedChanges++ $scope.$broadcast "review-panel:recalculate-screen-positions" $scope.$broadcast "review-panel:layout" @@ -391,17 +387,19 @@ define [ bulkAccept = () -> _doAcceptMultipleChanges $scope.reviewPanel.selectedEntryIds.slice() $scope.reviewPanel.selectedEntryIds = [] + $scope.reviewPanel.nVisibleSelectedChanges = 0 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 = () -> _doRejectMultipleChanges $scope.reviewPanel.selectedEntryIds.slice() $scope.reviewPanel.selectedEntryIds = [] + $scope.reviewPanel.nVisibleSelectedChanges = 0 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 = () -> From 26260fc11f8a961cbdab84ba6ce68cc71f2bca64 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Thu, 1 Jun 2017 16:33:50 +0100 Subject: [PATCH 08/20] Prevent aggregation of changes from different users. --- .../review-panel/controllers/ReviewPanelController.coffee | 7 ++++++- 1 file changed, 6 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 76ad8c84cb..1fbb1bd22b 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -247,7 +247,12 @@ define [ delete delete_changes[change.id] entries[change.id] ?= {} - if potential_aggregate and change.op.d and change.op.p == prev_insertion.op.p + prev_insertion.op.i.length + 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 + ) aggregate_entry = true new_entry = { From e2edf4184fc8dc2bbba115c26a41daf58f77aa3b Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Fri, 2 Jun 2017 11:43:43 +0100 Subject: [PATCH 09/20] Explicit naming. --- services/web/app/views/project/editor/review-panel.pug | 4 ++-- .../review-panel/controllers/ReviewPanelController.coffee | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/services/web/app/views/project/editor/review-panel.pug b/services/web/app/views/project/editor/review-panel.pug index a33fa28e7d..9e2a26ac6d 100644 --- a/services/web/app/views/project/editor/review-panel.pug +++ b/services/web/app/views/project/editor/review-panel.pug @@ -82,7 +82,7 @@ permissions="permissions" ) - div(ng-if="entry.type === 'agg-change'") + div(ng-if="entry.type === 'aggregate-change'") aggregate-change-entry( entry="entry" user="users[entry.metadata.user_id]" @@ -158,7 +158,7 @@ permissions="permissions" ) - div(ng-if="entry.type === 'agg-change'") + div(ng-if="entry.type === 'aggregate-change'") aggregate-change-entry( entry="entry" user="users[entry.metadata.user_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 1fbb1bd22b..6aebe0fbb3 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -263,7 +263,7 @@ define [ } if aggregate_entry - entries[prev_insertion.id].type = "agg-change" + entries[prev_insertion.id].type = "aggregate-change" entries[prev_insertion.id].metadata.agg_op = new_entry entries[prev_insertion.id].metadata.agg_op_id = change.id else @@ -351,7 +351,7 @@ define [ else if entry.type == "delete" isChangeEntryAndWithinSelection = selection_offset_start <= entry.offset <= selection_offset_end entry.focused = (entry.offset == selection_offset_start) - else if entry.type == "agg-change" + 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 @@ -359,7 +359,7 @@ define [ if isChangeEntryAndWithinSelection $scope.reviewPanel.selectedEntryIds.push id - $scope.reviewPanel.selectedEntryIds.push entry.metadata.agg_op_id if entry.type == "agg-change" + $scope.reviewPanel.selectedEntryIds.push entry.metadata.agg_op_id if entry.type == "aggregate-change" $scope.reviewPanel.nVisibleSelectedChanges++ $scope.$broadcast "review-panel:recalculate-screen-positions" From 8d8bcab1e85a94ee4288983516b2166cf1672e89 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Mon, 5 Jun 2017 10:41:59 +0100 Subject: [PATCH 10/20] Consolidate change accept and reject code. --- .../track-changes/TrackChangesManager.coffee | 10 ++---- .../controllers/ReviewPanelController.coffee | 32 +++++++------------ 2 files changed, 14 insertions(+), 28 deletions(-) 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 e759b9c236..e3f30b447d 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) => 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 6aebe0fbb3..57a0bba30d 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -79,7 +79,7 @@ define [ if doc_id != $scope.editor.open_doc_id getChangeTracker(doc_id).removeChangeId(change_id) else - $scope.$broadcast "change:accept", change_id + $scope.$broadcast "changes:accept", [ change_id ] updateEntries(doc_id) $scope.$apply () -> @@ -87,7 +87,7 @@ define [ 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 () -> @@ -365,29 +365,20 @@ define [ $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.acceptAggChange = (entry_id1, entry_id2) -> - _doAcceptMultipleChanges [ entry_id1, entry_id2 ] - event_tracking.sendMB "rp-agg-change-accepted", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini' } + $scope.acceptChanges = (change_ids) -> + _doAcceptMultipleChanges change_ids + event_tracking.sendMB "rp-changes-accepted", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini' } - $scope.rejectAggChange = (entry_id1, entry_id2) -> - _doRejectMultipleChanges [ entry_id1, entry_id2 ] - event_tracking.sendMB "rp-agg-change-rejected", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini' } + $scope.rejectChanges = (change_ids) -> + _doRejectMultipleChanges change_ids + event_tracking.sendMB "rp-changes-rejected", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini' } _doAcceptMultipleChanges = (change_ids) -> $http.post "/project/#{$scope.project_id}/doc/#{$scope.editor.open_doc_id}/changes/accept", { change_ids, _csrf: window.csrfToken} - $scope.$broadcast "change:bulk-accept", change_ids + $scope.$broadcast "changes:accept", change_ids _doRejectMultipleChanges = (change_ids) -> - $scope.$broadcast "change:bulk-reject", change_ids + $scope.$broadcast "changes:reject", change_ids bulkAccept = () -> _doAcceptMultipleChanges $scope.reviewPanel.selectedEntryIds.slice() @@ -410,7 +401,8 @@ define [ $scope.showBulkAcceptDialog = () -> showBulkActionsDialog true - $scope.showBulkRejectDialog = () -> showBulkActionsDialog false + $scope.showBulkRejectDialog = () -> + showBulkActionsDialog false showBulkActionsDialog = (isAccept) -> $modal.open({ From 6c3cdbcc3aa5bbd2f3a656cabd2fd6ebf216aaec Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Mon, 5 Jun 2017 10:43:03 +0100 Subject: [PATCH 11/20] Remove dead code, backend does not send single change events anymore. --- .../review-panel/controllers/ReviewPanelController.coffee | 8 -------- 1 file changed, 8 deletions(-) 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 57a0bba30d..1c0929084e 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -74,14 +74,6 @@ 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 "changes: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 From d4ac91f75e6ca8d16dfc672a98fb473764da7fbf Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Mon, 5 Jun 2017 10:52:11 +0100 Subject: [PATCH 12/20] Use correct handlers in directives. --- .../web/app/views/project/editor/review-panel.pug | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/services/web/app/views/project/editor/review-panel.pug b/services/web/app/views/project/editor/review-panel.pug index 9e2a26ac6d..d3f548d771 100644 --- a/services/web/app/views/project/editor/review-panel.pug +++ b/services/web/app/views/project/editor/review-panel.pug @@ -75,8 +75,8 @@ change-entry( entry="entry" user="users[entry.metadata.user_id]" - on-reject="rejectChange(entry_id);" - on-accept="acceptChange(entry_id);" + on-reject="rejectChanges([ entry_id ]);" + on-accept="acceptChanges([ entry_id ]);" on-indicator-click="toggleReviewPanel();" on-body-click="gotoEntry(editor.open_doc_id, entry)" permissions="permissions" @@ -86,8 +86,8 @@ aggregate-change-entry( entry="entry" user="users[entry.metadata.user_id]" - on-reject="rejectAggChange(entry_id, entry.metadata.agg_op_id);" - on-accept="acceptAggChange(entry_id, entry.metadata.agg_op_id);" + on-reject="rejectChanges([ entry_id, entry.metadata.agg_op_id ]);" + on-accept="acceptChanges([ entry_id, entry.metadata.agg_op_id ]);" on-indicator-click="toggleReviewPanel();" on-body-click="gotoEntry(editor.open_doc_id, entry)" permissions="permissions" @@ -153,7 +153,6 @@ change-entry( entry="entry" user="users[entry.metadata.user_id]" - on-indicator-click="toggleReviewPanel();" ng-click="gotoEntry(doc.doc.id, entry)" permissions="permissions" ) @@ -162,10 +161,7 @@ aggregate-change-entry( entry="entry" user="users[entry.metadata.user_id]" - on-reject="rejectChange(entry_id);" - on-accept="acceptChange(entry_id);" - on-indicator-click="toggleReviewPanel();" - on-body-click="gotoEntry(editor.open_doc_id, entry)" + ng-click="gotoEntry(editor.open_doc_id, entry)" permissions="permissions" ) @@ -176,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" ) From e616a62d689f5dc64a2fd7ae9f65cf29ffba5fc8 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Mon, 5 Jun 2017 16:29:55 +0100 Subject: [PATCH 13/20] Save entry ids inside entry object, not only as the key. --- .../app/views/project/editor/review-panel.pug | 10 ++-- .../controllers/ReviewPanelController.coffee | 56 +++++++++---------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/services/web/app/views/project/editor/review-panel.pug b/services/web/app/views/project/editor/review-panel.pug index d3f548d771..65a431f6fd 100644 --- a/services/web/app/views/project/editor/review-panel.pug +++ b/services/web/app/views/project/editor/review-panel.pug @@ -75,8 +75,8 @@ change-entry( entry="entry" user="users[entry.metadata.user_id]" - on-reject="rejectChanges([ entry_id ]);" - on-accept="acceptChanges([ 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" @@ -86,8 +86,8 @@ aggregate-change-entry( entry="entry" user="users[entry.metadata.user_id]" - on-reject="rejectChanges([ entry_id, entry.metadata.agg_op_id ]);" - on-accept="acceptChanges([ entry_id, entry.metadata.agg_op_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" @@ -256,7 +256,7 @@ script(type='text/ng-template', id='aggregateChangeEntryTemplate') .rp-entry-details .rp-entry-description | Changed - del.rp-content-highlight {{ entry.metadata.agg_op.content }} + del.rp-content-highlight {{ entry.metadata.replaced_content }} | for ins.rp-content-highlight {{ entry.content }} a.rp-collapse-toggle( 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 1c0929084e..9352d8f4c8 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -224,20 +224,19 @@ 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 - aggregate_entry = false - delete delete_changes[change.id] - entries[change.id] ?= {} if ( potential_aggregate and @@ -245,20 +244,20 @@ define [ change.op.p == prev_insertion.op.p + prev_insertion.op.i.length and change.metadata.user_id == prev_insertion.metadata.user_id ) - aggregate_entry = true - - 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 - } - - if aggregate_entry + # An actual aggregate op. entries[prev_insertion.id].type = "aggregate-change" - entries[prev_insertion.id].metadata.agg_op = new_entry - entries[prev_insertion.id].metadata.agg_op_id = change.id + 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 @@ -287,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 } @@ -350,30 +350,30 @@ define [ entry.focused = true if isChangeEntryAndWithinSelection - $scope.reviewPanel.selectedEntryIds.push id - $scope.reviewPanel.selectedEntryIds.push entry.metadata.agg_op_id if entry.type == "aggregate-change" + 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.acceptChanges = (change_ids) -> - _doAcceptMultipleChanges 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) -> - _doRejectMultipleChanges change_ids + _doRejectChanges change_ids event_tracking.sendMB "rp-changes-rejected", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini' } - _doAcceptMultipleChanges = (change_ids) -> + _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 - _doRejectMultipleChanges = (change_ids) -> + _doRejectChanges = (change_ids) -> $scope.$broadcast "changes:reject", change_ids bulkAccept = () -> - _doAcceptMultipleChanges $scope.reviewPanel.selectedEntryIds.slice() + _doAcceptChanges $scope.reviewPanel.selectedEntryIds.slice() $scope.reviewPanel.selectedEntryIds = [] $scope.reviewPanel.nVisibleSelectedChanges = 0 event_tracking.sendMB "rp-bulk-accept", { @@ -382,7 +382,7 @@ define [ } bulkReject = () -> - _doRejectMultipleChanges $scope.reviewPanel.selectedEntryIds.slice() + _doRejectChanges $scope.reviewPanel.selectedEntryIds.slice() $scope.reviewPanel.selectedEntryIds = [] $scope.reviewPanel.nVisibleSelectedChanges = 0 event_tracking.sendMB "rp-bulk-reject", { From 5a75663a0ca5bbb52a9289b5cf404ccadc966620 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Mon, 5 Jun 2017 17:17:47 +0100 Subject: [PATCH 14/20] Do bulk rejects in reverse order. --- .../aceEditor/track-changes/TrackChangesManager.coffee | 2 ++ 1 file changed, 2 insertions(+) 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 e3f30b447d..842d27ac39 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 @@ -214,6 +214,8 @@ define [ rejectChangeIds: (change_ids) -> changes = @rangesTracker.getChanges(change_ids) return if changes.length == 0 + changes.sort((a, b) -> b.op.p - a.op.p) + session = @editor.getSession() for change in changes if change.op.d? From af93fc186972908fb92fc6952c6fd6533af27df2 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Tue, 6 Jun 2017 10:00:50 +0100 Subject: [PATCH 15/20] Make aggregate entries work when the review panel is minimised. --- services/web/public/stylesheets/app/editor/review-panel.less | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/public/stylesheets/app/editor/review-panel.less b/services/web/public/stylesheets/app/editor/review-panel.less index cb60afd62f..4a5ca516ea 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; } From 81a5a5311daa7ff5256fa5245a8919f07a262d66 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Tue, 6 Jun 2017 10:03:33 +0100 Subject: [PATCH 16/20] Clear selection after bulk actions. --- .../controllers/ReviewPanelController.coffee | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) 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 9352d8f4c8..2cb12eb983 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -374,8 +374,9 @@ define [ bulkAccept = () -> _doAcceptChanges $scope.reviewPanel.selectedEntryIds.slice() - $scope.reviewPanel.selectedEntryIds = [] - $scope.reviewPanel.nVisibleSelectedChanges = 0 + $timeout () -> + $scope.reviewPanel.selectedEntryIds = [] + $scope.reviewPanel.nVisibleSelectedChanges = 0 event_tracking.sendMB "rp-bulk-accept", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini', nEntries: $scope.reviewPanel.nVisibleSelectedChanges @@ -383,8 +384,9 @@ define [ bulkReject = () -> _doRejectChanges $scope.reviewPanel.selectedEntryIds.slice() - $scope.reviewPanel.selectedEntryIds = [] - $scope.reviewPanel.nVisibleSelectedChanges = 0 + $timeout () -> + $scope.reviewPanel.selectedEntryIds = [] + $scope.reviewPanel.nVisibleSelectedChanges = 0 event_tracking.sendMB "rp-bulk-reject", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini', nEntries: $scope.reviewPanel.nVisibleSelectedChanges From afa011c813ffde77681a1e2b466f951f90d92abe Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Tue, 6 Jun 2017 16:11:00 +0100 Subject: [PATCH 17/20] Avoid flickering when elements enter the review panel. --- .../directives/reviewPanelSorted.coffee | 15 ++++++++++++--- .../stylesheets/app/editor/review-panel.less | 1 + 2 files changed, 13 insertions(+), 3 deletions(-) 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..5178178387 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,10 @@ 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 + 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 +76,10 @@ 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 + 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 +95,10 @@ 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 + 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 4a5ca516ea..312b933b85 100644 --- a/services/web/public/stylesheets/app/editor/review-panel.less +++ b/services/web/public/stylesheets/app/editor/review-panel.less @@ -261,6 +261,7 @@ } } .rp-state-current-file-expanded & { + visibility: hidden; left: 5px; right: 5px; width: auto; From 34796b18c5ee622dfcc4ddfb83d402ccc6344d3f Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Tue, 6 Jun 2017 16:46:36 +0100 Subject: [PATCH 18/20] Add translation keys. --- services/web/app/views/project/editor/review-panel.pug | 4 ++-- .../ide/review-panel/directives/reviewPanelSorted.coffee | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/services/web/app/views/project/editor/review-panel.pug b/services/web/app/views/project/editor/review-panel.pug index 65a431f6fd..07f6bd081e 100644 --- a/services/web/app/views/project/editor/review-panel.pug +++ b/services/web/app/views/project/editor/review-panel.pug @@ -255,9 +255,9 @@ script(type='text/ng-template', id='aggregateChangeEntryTemplate') i.fa.fa-pencil .rp-entry-details .rp-entry-description - | Changed + | #{translate("aggregate_changed")} del.rp-content-highlight {{ entry.metadata.replaced_content }} - | for + | #{translate("aggregate_to")} ins.rp-content-highlight {{ entry.content }} a.rp-collapse-toggle( href 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 5178178387..3445ac845a 100644 --- a/services/web/public/coffee/ide/review-panel/directives/reviewPanelSorted.coffee +++ b/services/web/public/coffee/ide/review-panel/directives/reviewPanelSorted.coffee @@ -65,6 +65,8 @@ define [ focused_entry_top = Math.max(focused_entry.scope.entry.screenPos.y, TOOLBAR_HEIGHT) 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) @@ -78,6 +80,8 @@ define [ previousBottom = top + height 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) @@ -97,6 +101,8 @@ define [ previousTop = 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) From 53b18e041ff0494377be8718b8a4e36749dd7722 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Wed, 7 Jun 2017 14:03:58 +0100 Subject: [PATCH 19/20] Trigger selection-related logic after accepting or rejecting changes. --- .../aceEditor/track-changes/TrackChangesManager.coffee | 4 +++- .../review-panel/controllers/ReviewPanelController.coffee | 6 ------ 2 files changed, 3 insertions(+), 7 deletions(-) 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 12417d2e09..59c119d929 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 @@ -210,6 +210,7 @@ define [ acceptChangeIds: (change_ids) -> @rangesTracker.removeChangeIds(change_ids) @updateAnnotations() + @updateFocus() rejectChangeIds: (change_ids) -> changes = @rangesTracker.getChanges(change_ids) @@ -288,7 +289,8 @@ define [ session.$fromReject = false else throw new Error("unknown change: #{JSON.stringify(change)}") - + @updateFocus() + removeCommentId: (comment_id) -> @rangesTracker.removeCommentId(comment_id) @updateAnnotations() 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 2cb12eb983..5a56baba2c 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -374,9 +374,6 @@ define [ bulkAccept = () -> _doAcceptChanges $scope.reviewPanel.selectedEntryIds.slice() - $timeout () -> - $scope.reviewPanel.selectedEntryIds = [] - $scope.reviewPanel.nVisibleSelectedChanges = 0 event_tracking.sendMB "rp-bulk-accept", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini', nEntries: $scope.reviewPanel.nVisibleSelectedChanges @@ -384,9 +381,6 @@ define [ bulkReject = () -> _doRejectChanges $scope.reviewPanel.selectedEntryIds.slice() - $timeout () -> - $scope.reviewPanel.selectedEntryIds = [] - $scope.reviewPanel.nVisibleSelectedChanges = 0 event_tracking.sendMB "rp-bulk-reject", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini', nEntries: $scope.reviewPanel.nVisibleSelectedChanges From 240d6d6f6b7330d8c712954a0dcbdd2308373d72 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Wed, 7 Jun 2017 14:21:05 +0100 Subject: [PATCH 20/20] Resolve timing issue when triggering selection-related logic. --- .../aceEditor/track-changes/TrackChangesManager.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 59c119d929..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 @@ -289,8 +289,8 @@ define [ session.$fromReject = false else throw new Error("unknown change: #{JSON.stringify(change)}") - @updateFocus() - + setTimeout () => @updateFocus() + removeCommentId: (comment_id) -> @rangesTracker.removeCommentId(comment_id) @updateAnnotations()