diff --git a/services/web/app/views/project/editor/pdf.pug b/services/web/app/views/project/editor/pdf.pug index ba03f068a6..e75a2919b7 100644 --- a/services/web/app/views/project/editor/pdf.pug +++ b/services/web/app/views/project/editor/pdf.pug @@ -302,7 +302,7 @@ div.full-size.pdf(ng-controller="PdfController") .alert.alert-danger(ng-show="pdf.validation.conflictedPaths") div strong #{translate("conflicting_paths_found")} - div #{translate("following_paths_conflict")} + div !{translate("following_paths_conflict")} div li(ng-repeat="entry in pdf.validation.conflictedPaths") {{ '/'+entry['path'] }} diff --git a/services/web/app/views/project/editor/review-panel.pug b/services/web/app/views/project/editor/review-panel.pug index afa5b4244e..59ac7e5baf 100644 --- a/services/web/app/views/project/editor/review-panel.pug +++ b/services/web/app/views/project/editor/review-panel.pug @@ -47,6 +47,7 @@ .rp-entry-list-inner .rp-entry-wrapper( ng-repeat="(entry_id, entry) in reviewPanel.entries[editor.open_doc_id]" + ng-if="entry.visible" ) div(ng-if="entry.type === 'insert' || entry.type === 'delete'") change-entry( @@ -79,6 +80,7 @@ on-submit="submitNewComment(content);" on-cancel="cancelNewComment();" on-indicator-click="toggleReviewPanel();" + layout-to-left="reviewPanel.layoutToLeft" ) .rp-entry-list( @@ -322,7 +324,7 @@ script(type='text/ng-template', id='addCommentEntryTemplate') ng-if="!commentState.adding" ng-click="startNewComment(); onIndicatorClick();" tooltip="Add a comment" - tooltip-placement="right" + tooltip-placement="{{ layoutToLeft ? 'left' : 'right' }}" tooltip-append-to-body="true" ) i.fa.fa-commenting diff --git a/services/web/app/views/subscriptions/group/successful_join.pug b/services/web/app/views/subscriptions/group/successful_join.pug index 6de282aa36..9e748f4c38 100644 --- a/services/web/app/views/subscriptions/group/successful_join.pug +++ b/services/web/app/views/subscriptions/group/successful_join.pug @@ -20,6 +20,6 @@ block content .col-md-12   .row .span-md-12 - a.btn.btn-success(href="/project") #{translate("Done")} + a.btn.btn-success(href="/project") #{translate("done")} diff --git a/services/web/app/views/user/sessions.pug b/services/web/app/views/user/sessions.pug index 948ed2d94f..c952daea03 100644 --- a/services/web/app/views/user/sessions.pug +++ b/services/web/app/views/user/sessions.pug @@ -17,7 +17,7 @@ block content div p.small - | #{translate("clear_sessions_description")} + | !{translate("clear_sessions_description")} div div(ng-if="state.otherSessions.length == 0") diff --git a/services/web/app/views/user/settings.pug b/services/web/app/views/user/settings.pug index a4217d8ca4..c358909110 100644 --- a/services/web/app/views/user/settings.pug +++ b/services/web/app/views/user/settings.pug @@ -107,7 +107,7 @@ block content span.small.text-primary(ng-show="changePasswordForm.newPassword2.$error.areEqual && changePasswordForm.newPassword2.$dirty") | #{translate("doesnt_match")} span.small.text-primary(ng-show="!changePasswordForm.newPassword2.$error.areEqual && changePasswordForm.newPassword2.$invalid && changePasswordForm.newPassword2.$dirty") - | #{translate("Invalid Password")} + | #{translate("invalid_password")} .actions button.btn.btn-primary( type='submit', diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor.coffee index 9205cb7b87..faa1159e00 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor.coffee @@ -184,6 +184,28 @@ define [ session = editor.getSession() session.setScrollTop(session.getScrollTop() + newScreenPosition - previousScreenPosition) + scope.$on "#{scope.name}:set-scroll-size", (e, size) -> + # Make sure that the editor has enough scroll margin above and below + # to scroll the review panel with the given size + marginTop = size.overflowTop + maxHeight = editor.renderer.layerConfig.maxHeight + marginBottom = Math.max(size.height - maxHeight, 0) + setScrollMargins(marginTop, marginBottom) + + setScrollMargins = (marginTop, marginBottom) -> + marginChanged = false + if editor.renderer.scrollMargin.top != marginTop + editor.renderer.scrollMargin.top = marginTop + marginChanged = true + if editor.renderer.scrollMargin.bottom != marginBottom + editor.renderer.scrollMargin.bottom = marginBottom + marginChanged = true + if marginChanged + editor.renderer.updateFull() + + resetScrollMargins = () -> + setScrollMargins(0,0) + scope.$watch "theme", (value) -> editor.setTheme("ace/theme/#{value}") @@ -308,6 +330,8 @@ define [ sharejs_doc.attachToAce(editor) editor.initing = false + resetScrollMargins() + # need to set annotations after attaching because attaching # deletes and then inserts document content session.setAnnotations scope.annotations 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 b75cb07dba..38b3e5678c 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 @@ -60,6 +60,18 @@ define [ onChangeSession = (e) => @clearAnnotations() @redrawAnnotations() + @editor.session.on "changeScrollTop", onChangeScroll + + _scrollTimeout = null + onChangeScroll = () => + if _scrollTimeout? + return + else + _scrollTimeout = setTimeout () => + @recalculateVisibleEntries() + @$scope.$apply() + _scrollTimeout = null + , 200 bindToAce = () => @editor.on "changeSelection", onChangeSelection @@ -282,6 +294,7 @@ define [ recalculateReviewEntriesScreenPositions: () -> session = @editor.getSession() renderer = @editor.renderer + {firstRow, lastRow} = renderer.layerConfig entries = @_getCurrentDocEntries() for entry_id, entry of entries or {} doc_position = @_shareJsOffsetToAcePosition(entry.offset) @@ -290,9 +303,24 @@ define [ entry.screenPos ?= {} entry.screenPos.y = y entry.docPos = doc_position - + @recalculateVisibleEntries() @$scope.$apply() - + + recalculateVisibleEntries: () -> + OFFSCREEN_ROWS = 20 + CULL_AFTER = 100 # With less than this number of entries, don't bother culling to avoid little UI jumps when scrolling. + {firstRow, lastRow} = @editor.renderer.layerConfig + entries = @_getCurrentDocEntries() or {} + entriesLength = Object.keys(entries).length + changed = false + for entry_id, entry of entries + old = entry.visible + entry.visible = (entriesLength < CULL_AFTER) or (firstRow - OFFSCREEN_ROWS <= entry.docPos.row <= lastRow + OFFSCREEN_ROWS) + if (entry.visible != old) + changed = true + if changed + @$scope.$emit "editor:track-changes:visibility_changed" + _getCurrentDocEntries: () -> doc_id = @$scope.docId entries = @$scope.reviewPanel.entries[doc_id] ?= {} diff --git a/services/web/public/coffee/ide/review-panel/RangesTracker.coffee b/services/web/public/coffee/ide/review-panel/RangesTracker.coffee index e31b84f051..865ecf4ef6 100644 --- a/services/web/public/coffee/ide/review-panel/RangesTracker.coffee +++ b/services/web/public/coffee/ide/review-panel/RangesTracker.coffee @@ -105,6 +105,7 @@ load = (EventEmitter) -> throw new Error("unknown op type") addComment: (op, metadata) -> + # TODO: Don't allow overlapping comments? @comments.push comment = { id: op.t or @newId() op: # Copy because we'll modify in place @@ -167,12 +168,14 @@ load = (EventEmitter) -> op_length = op.i.length op_end = op.p + op_length + already_merged = false previous_change = null moved_changes = [] remove_changes = [] new_changes = [] - for change in @changes + + for change, i in @changes change_start = change.op.p if change.op.d? @@ -200,6 +203,16 @@ load = (EventEmitter) -> # Only merge inserts if they are from the same user is_same_user = metadata.user_id == change.metadata.user_id + # If this is an insert op at the end of an existing insert with a delete following, and it cancels out the following + # delete then we shouldn't append it to this insert, but instead only cancel the following delete. + # E.g. + # foo|<--- about to insert 'b' here + # inserted 'foo' --^ ^-- deleted 'bar' + # should become just 'foo' not 'foob' (with the delete marker becoming just 'ar'), . + next_change = @changes[i+1] + is_op_adjacent_to_next_delete = next_change? and next_change.op.d? and op.p == change_end and next_change.op.p == op.p + will_op_cancel_next_delete = is_op_adjacent_to_next_delete and next_change.op.d.slice(0, op.i.length) == op.i + # If there is a delete at the start of the insert, and we're inserting # at the start, we SHOULDN'T merge since the delete acts as a partition. # The previous op will be the delete, but it's already been shifted by this insert @@ -222,7 +235,8 @@ load = (EventEmitter) -> if @track_changes and is_change_overlapping and !is_insert_blocked_by_delete and - !already_merged and + !already_merged and + !will_op_cancel_next_delete and is_same_user offset = op_start - change_start change.op.i = change.op.i.slice(0, offset) + op.i + change.op.i.slice(offset) @@ -398,8 +412,8 @@ load = (EventEmitter) -> _addOp: (op, metadata) -> change = { id: @newId() - op: op - metadata: metadata + op: @_clone(op) # Don't take a reference to the existing op since we'll modify this in place with future changes + metadata: @_clone(metadata) } @changes.push change @@ -471,6 +485,11 @@ load = (EventEmitter) -> else # Only update to the current change if we haven't removed it. previous_change = change return { moved_changes, remove_changes } + + _clone: (object) -> + clone = {} + (clone[k] = v for k,v of object) + return clone if define? define ["utils/EventEmitter"], load 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 07f429036f..2f66cc1530 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -40,6 +40,9 @@ define [ $timeout () -> $scope.$broadcast "review-panel:layout" + $scope.$on "review-panel:sizes", (e, sizes) -> + $scope.$broadcast "editor:set-scroll-size", sizes + $scope.$watch "ui.pdfLayout", (layout) -> $scope.reviewPanel.layoutToLeft = (layout == "flat") @@ -266,7 +269,11 @@ define [ updateEntries(doc_id) $scope.$broadcast "review-panel:recalculate-screen-positions" $scope.$broadcast "review-panel:layout" - + + $scope.$on "editor:track-changes:visibility_changed", () -> + $timeout () -> + $scope.$broadcast "review-panel:layout", false + $scope.$on "editor:focus:changed", (e, selection_offset_start, selection_offset_end, selection) -> doc_id = $scope.editor.open_doc_id entries = getDocEntries(doc_id) diff --git a/services/web/public/coffee/ide/review-panel/directives/addCommentEntry.coffee b/services/web/public/coffee/ide/review-panel/directives/addCommentEntry.coffee index c23c37b674..c30eaadd0f 100644 --- a/services/web/public/coffee/ide/review-panel/directives/addCommentEntry.coffee +++ b/services/web/public/coffee/ide/review-panel/directives/addCommentEntry.coffee @@ -9,6 +9,7 @@ define [ onSubmit: "&" onCancel: "&" onIndicatorClick: "&" + layoutToLeft: "=" link: (scope, element, attrs) -> scope.state = isAdding: false 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 4028406713..b76f891e7f 100644 --- a/services/web/public/coffee/ide/review-panel/directives/reviewPanelSorted.coffee +++ b/services/web/public/coffee/ide/review-panel/directives/reviewPanelSorted.coffee @@ -15,9 +15,11 @@ define [ if scope.ui.reviewPanelOpen PADDING = 8 TOOLBAR_HEIGHT = 38 + OVERVIEW_TOGGLE_HEIGHT = 57 else PADDING = 4 TOOLBAR_HEIGHT = 4 + OVERVIEW_TOGGLE_HEIGHT = 0 entries = [] for el in element.find(".rp-entry-wrapper") @@ -31,6 +33,7 @@ define [ entry.$layout_el = entry.$box_el else entry.$layout_el = entry.$indicator_el + entry.height = entry.$layout_el.height() # Do all of our DOM reads first for perfomance, see http://wilsonpage.co.uk/preventing-layout-thrashing/ entries.push entry entries.sort (a,b) -> a.scope.entry.offset - b.scope.entry.offset @@ -50,19 +53,6 @@ define [ sl_console.log "focused_entry_index", focused_entry_index - # As we go backwards, we run the risk of pushing things off the top of the editor. - # If we go through the entries before and assume they are as pushed together as they - # could be, we can work out the 'ceiling' that each one can't go through. I.e. the first - # on can't go beyond the toolbar height, the next one can't go beyond the bottom of the first - # one at this minimum height, etc. - heights = (entry.$layout_el.height() for entry in entries_before) - previousMinTop = TOOLBAR_HEIGHT - min_tops = [] - for height in heights - min_tops.push previousMinTop - previousMinTop += PADDING + height - min_tops.reverse() - positionLayoutEl = ($callout_el, original_top, top) -> if original_top <= top $callout_el.removeClass("rp-entry-callout-inverted") @@ -72,7 +62,7 @@ define [ $callout_el.css(top: top + line_height, height: original_top - top) # Put the focused entry as close to where it wants to be as possible - focused_entry_top = Math.max(previousMinTop, focused_entry.scope.entry.screenPos.y) + focused_entry_top = Math.max(focused_entry.scope.entry.screenPos.y, TOOLBAR_HEIGHT) focused_entry.$box_el.css(top: focused_entry_top) focused_entry.$indicator_el.css(top: focused_entry_top) positionLayoutEl(focused_entry.$callout_el, focused_entry.scope.entry.screenPos.y, focused_entry_top) @@ -80,27 +70,39 @@ define [ previousBottom = focused_entry_top + focused_entry.$layout_el.height() for entry in entries_after original_top = entry.scope.entry.screenPos.y - height = entry.$layout_el.height() + height = entry.height top = Math.max(original_top, previousBottom + PADDING) previousBottom = top + height entry.$box_el.css(top: top) entry.$indicator_el.css(top: top) positionLayoutEl(entry.$callout_el, original_top, top) sl_console.log "ENTRY", {entry: entry.scope.entry, top} + + lastBottom = previousBottom previousTop = focused_entry_top entries_before.reverse() # Work through backwards, starting with the one just above for entry, i in entries_before original_top = entry.scope.entry.screenPos.y - height = entry.$layout_el.height() + height = entry.height original_bottom = original_top + height bottom = Math.min(original_bottom, previousTop - PADDING) - top = Math.max(bottom - height, min_tops[i]) + top = bottom - height previousTop = top entry.$box_el.css(top: top) entry.$indicator_el.css(top: top) positionLayoutEl(entry.$callout_el, original_top, top) sl_console.log "ENTRY", {entry: entry.scope.entry, top} + + lastTop = top + if lastTop < TOOLBAR_HEIGHT + overflowTop = -lastTop + TOOLBAR_HEIGHT + else + overflowTop = 0 + scope.$emit "review-panel:sizes", { + overflowTop: overflowTop, + height: previousBottom + OVERVIEW_TOGGLE_HEIGHT + } scope.$applyAsync () -> layout() diff --git a/services/web/public/img/review-icon-sprite@2x.png b/services/web/public/img/review-icon-sprite@2x.png new file mode 100644 index 0000000000..7a76326d63 Binary files /dev/null and b/services/web/public/img/review-icon-sprite@2x.png differ diff --git a/services/web/public/stylesheets/app/editor.less b/services/web/public/stylesheets/app/editor.less index 250269a7a2..fdf18d7b38 100644 --- a/services/web/public/stylesheets/app/editor.less +++ b/services/web/public/stylesheets/app/editor.less @@ -181,6 +181,19 @@ } } +// Hack to solve an issue where scrollbars aren't visible in Safari. +// Safari seems to clip part of the scrollbar element. By giving the +// element a background, we're telling Safari that it *really* needs to +// paint the whole area. See https://github.com/ajaxorg/ace/issues/2872 +.ace_scrollbar-inner { + background-color: #FFF; + opacity: 0.01; + + .ace_dark & { + background-color: #000; + } +} + .ui-layout-resizer { width: 6px; background-color: #f4f4f4; diff --git a/services/web/public/stylesheets/app/editor/review-panel.less b/services/web/public/stylesheets/app/editor/review-panel.less index 5b422cbd0a..4a22ac3634 100644 --- a/services/web/public/stylesheets/app/editor/review-panel.less +++ b/services/web/public/stylesheets/app/editor/review-panel.less @@ -49,11 +49,11 @@ } &[disabled] { - opacity: 0.5; + background-color: tint(@rp-highlight-blue, 50%); &:hover, &:focus { - background-color: @rp-highlight-blue; + background-color: tint(@rp-highlight-blue, 50%); } } } @@ -463,6 +463,7 @@ margin-top: 3px; overflow-x: hidden; min-height: 3em; + max-height: 400px; } .rp-icon-delete { @@ -784,6 +785,10 @@ .toolbar .btn-full-height:active & { background-position-y: -60px; } + + @media (min-device-pixel-ratio: 1.5), (min-resolution: 144dpi) { + background-image: url('/img/review-icon-sprite@2x.png'); + } } .resolved-comments-toggle {