From 379afe4aa5a7388c512d6e4638d7055917f8b22f Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 16 Nov 2016 15:23:29 +0000 Subject: [PATCH] Refactor scroll handling into directive --- .../web/app/views/project/editor/editor.jade | 4 +- .../views/project/editor/review-panel.jade | 10 ++-- .../controllers/ReviewPanelController.coffee | 55 ++----------------- .../directives/reviewPanelSorted.coffee | 47 ++++++++++++++++ .../stylesheets/app/editor/review-panel.less | 19 +++---- 5 files changed, 67 insertions(+), 68 deletions(-) diff --git a/services/web/app/views/project/editor/editor.jade b/services/web/app/views/project/editor/editor.jade index 816b4026b0..acf5b547f5 100644 --- a/services/web/app/views/project/editor/editor.jade +++ b/services/web/app/views/project/editor/editor.jade @@ -40,8 +40,8 @@ div.full-size( on-ctrl-enter="recompileViaKey", syntax-validation="settings.syntaxValidation", review-panel="reviewPanel", - on-scroll="onScroll", - scroll-events="scrollEvents", + on-scroll="scrollBindings.onAceScroll", + scroll-events="scrollBindings.reviewPanelEvents", track-changes-enabled="trackChangesFeatureFlag", track-new-changes= "reviewPanel.trackNewChanges", changes-tracker="reviewPanel.changesTracker", diff --git a/services/web/app/views/project/editor/review-panel.jade b/services/web/app/views/project/editor/review-panel.jade index b09e1314fe..f5470b45b3 100644 --- a/services/web/app/views/project/editor/review-panel.jade +++ b/services/web/app/views/project/editor/review-panel.jade @@ -2,11 +2,11 @@ .review-panel-toolbar | Track Changes input(type="checkbox", ng-model="reviewPanel.trackNewChanges") - .review-panel-scroller - .rp-entry-list( - review-panel-sorted - ng-if="reviewPanel.subView === SubViews.CUR_FILE" - ) + .rp-entry-list( + review-panel-sorted + ng-if="reviewPanel.subView === SubViews.CUR_FILE" + ) + .rp-entry-list-inner .rp-entry-wrapper( ng-repeat="(entry_id, entry) in reviewPanel.entries[editor.open_doc_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 4b08118879..a926bd94ed 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -19,6 +19,10 @@ define [ adding: false content: "" + # Used to communicate between Ace and reviewPanelSorted directive + $scope.scrollBindings = + reviewPanelEvents: new EventEmitter() + changesTrackers = {} $scope.$watch "editor.open_doc_id", (open_doc_id) -> @@ -29,58 +33,9 @@ define [ # TODO Just for prototyping purposes; remove afterwards. mockedUserId = '12345abc' - scroller = $element.find(".review-panel-scroller") - list = $element.find(".rp-entry-list") - - # Use these to avoid unnecessary updates. Scrolling one - # panel causes us to scroll the other panel, but there's no - # need to trigger the event back to the original panel. - ignoreNextPanelEvent = false - ignoreNextAceEvent = false - - $scope.scrollEvents = new EventEmitter() - - scrollPanel = (scrollTop, height) -> - if ignoreNextAceEvent - ignoreNextAceEvent = false - else - ignoreNextPanelEvent = true - list.height(height) - scroller.scrollTop(scrollTop) - - scrollAce = (e) -> - if ignoreNextPanelEvent - ignoreNextPanelEvent = false - else - ignoreNextAceEvent = true - $scope.scrollEvents.emit "scroll", e.target.scrollTop - - $scope.$watch "ui.reviewPanelOpen", (reviewPanelOpen) -> - return if !reviewPanelOpen? - if reviewPanelOpen - scroller.on "scroll", scrollAce - $scope.onScroll = scrollPanel # Passed into the editor directive for it to call - else - scroller.off "scroll" - $scope.onScroll = null - $timeout () -> - $scope.$broadcast "review-panel:toggle" - $scope.$broadcast "review-panel:layout" - + #TODO: Doesn't work anymore now entries is first indexed by doc_id $scope.$watch (() -> Object.keys($scope.reviewPanel.entries).length), (nEntries) -> $scope.reviewPanel.hasEntries = nEntries > 0 - - # If we listen for scroll events in the review panel natively, then with a Mac trackpad - # the scroll is very smooth (natively done I'd guess), but we don't get polled regularly - # enough to keep Ace in step, and it noticeably lags. If instead, we borrow the manual - # mousewheel/trackpad scrolling behaviour from Ace, and turn mousewheel events into - # scroll events ourselves, then it makes the review panel slightly less smooth (barely) - # noticeable, but keeps it perfectly in step with Ace. - ace.require("ace/lib/event").addMouseWheelListener scroller[0], (e) -> - deltaY = e.wheelY - # console.log "mousewheel", deltaY - scroller.scrollTop(scroller.scrollTop() + deltaY * 4) - e.preventDefault() $scope.acceptChange = (entry_id) -> $scope.$broadcast "change:accept", entry_id 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 23bb8754b4..b62d8e59e4 100644 --- a/services/web/public/coffee/ide/review-panel/directives/reviewPanelSorted.coffee +++ b/services/web/public/coffee/ide/review-panel/directives/reviewPanelSorted.coffee @@ -88,4 +88,51 @@ define [ scope.$on "review-panel:layout", () -> scope.$evalAsync () -> layout() + + ## Scroll lock with Ace + scroller = element + list = element.find(".rp-entry-list-inner") + + # If we listen for scroll events in the review panel natively, then with a Mac trackpad + # the scroll is very smooth (natively done I'd guess), but we don't get polled regularly + # enough to keep Ace in step, and it noticeably lags. If instead, we borrow the manual + # mousewheel/trackpad scrolling behaviour from Ace, and turn mousewheel events into + # scroll events ourselves, then it makes the review panel slightly less smooth (barely) + # noticeable, but keeps it perfectly in step with Ace. + ace.require("ace/lib/event").addMouseWheelListener scroller[0], (e) -> + deltaY = e.wheelY + # console.log "mousewheel", deltaY + scroller.scrollTop(scroller.scrollTop() + deltaY * 4) + e.preventDefault() + + # Use these to avoid unnecessary updates. Scrolling one + # panel causes us to scroll the other panel, but there's no + # need to trigger the event back to the original panel. + ignoreNextPanelEvent = false + ignoreNextAceEvent = false + + scrollPanel = (scrollTop, height) -> + console.log "Scrolling panel", scrollTop, height, list, scroller + if ignoreNextAceEvent + ignoreNextAceEvent = false + else + ignoreNextPanelEvent = true + list.height(height) + scroller.scrollTop(scrollTop) + + scrollAce = (e) -> + if ignoreNextPanelEvent + ignoreNextPanelEvent = false + else + ignoreNextAceEvent = true + scope.scrollBindings.reviewPanelEvents.emit "scroll", e.target.scrollTop + + scroller.on "scroll", scrollAce + scope.scrollBindings.onAceScroll = scrollPanel + + scope.$watch "ui.reviewPanelOpen", (reviewPanelOpen) -> + return if !reviewPanelOpen? + $timeout () -> + scope.$broadcast "review-panel:toggle" + scope.$broadcast "review-panel:layout" } \ No newline at end of file diff --git a/services/web/public/stylesheets/app/editor/review-panel.less b/services/web/public/stylesheets/app/editor/review-panel.less index 8270b0f782..c6008a5b87 100644 --- a/services/web/public/stylesheets/app/editor/review-panel.less +++ b/services/web/public/stylesheets/app/editor/review-panel.less @@ -70,6 +70,7 @@ border-left: solid 1px @rp-border-grey; font-size: @rp-base-font-size; color: @rp-type-blue; + overflow: hidden; // Stop inner hidden scroll bar showing } .review-panel-toolbar { @@ -86,18 +87,18 @@ z-index: 2; } -.review-panel-scroller { +.rp-entry-list { + // TODO: Use a more cross-browser method of hiding the scroll bar + width: @review-off-width + @rp-scroller-offset; + padding-right: @rp-scroller-offset; + overflow-y: scroll; position: absolute; top: 0; bottom: 0; - left: 0; - // TODO: Use a more cross-browser method of hiding the scroll bar - right: -@rp-scroller-offset; // Hide scroll bar } -.rp-entry-list { +.rp-entry-list-inner { position: relative; - width: @review-off-width; } .rp-entry-indicator { @@ -438,10 +439,6 @@ } } - .review-panel-scroller { - overflow-y: scroll; - } - #review-panel { display: block; width: @review-panel-width; @@ -453,7 +450,7 @@ } .rp-entry-list { - width: @review-panel-width; + width: @review-panel-width + @rp-scroller-offset; } .rp-entry {