From 016e750308b6e927b53d8c050b3f3bb60e9ca4aa Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 20 Oct 2017 18:56:43 +0100 Subject: Fix performance of sticky.js Closes #39332 --- app/assets/javascripts/init_changes_dropdown.js | 2 +- app/assets/javascripts/lib/utils/sticky.js | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/init_changes_dropdown.js b/app/assets/javascripts/init_changes_dropdown.js index f785ed29e6c..e41f374b2e5 100644 --- a/app/assets/javascripts/init_changes_dropdown.js +++ b/app/assets/javascripts/init_changes_dropdown.js @@ -1,7 +1,7 @@ import stickyMonitor from './lib/utils/sticky'; export default () => { - stickyMonitor(document.querySelector('.js-diff-files-changed')); + stickyMonitor(document.querySelector('.js-diff-files-changed'), 76); $('.js-diff-stats-dropdown').glDropdown({ filterable: true, diff --git a/app/assets/javascripts/lib/utils/sticky.js b/app/assets/javascripts/lib/utils/sticky.js index 64db42701ce..098afcfa1b4 100644 --- a/app/assets/javascripts/lib/utils/sticky.js +++ b/app/assets/javascripts/lib/utils/sticky.js @@ -28,14 +28,10 @@ export const isSticky = (el, scrollY, stickyTop, insertPlaceholder) => { } }; -export default (el, insertPlaceholder = true) => { +export default (el, stickyTop, insertPlaceholder = true) => { if (!el) return; - const computedStyle = window.getComputedStyle(el); - - if (!/sticky/.test(computedStyle.position)) return; - - const stickyTop = parseInt(computedStyle.top, 10); + if (typeof CSS === 'undefined' || !(CSS.supports('(position: -webkit-sticky) or (position: sticky)'))) return; document.addEventListener('scroll', () => isSticky(el, window.scrollY, stickyTop, insertPlaceholder), { passive: true, -- cgit v1.2.1 From 35daaa36e0ee74e28422a3b088fd5b01249ea9b7 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 23 Oct 2017 10:21:38 +0100 Subject: calculate the stickyTop instead of hard coding a variable --- app/assets/javascripts/dispatcher.js | 3 ++- app/assets/javascripts/init_changes_dropdown.js | 4 ++-- app/assets/javascripts/merge_request_tabs.js | 10 +++++++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 5ca1708f1b3..44d6bd42f7f 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -239,7 +239,8 @@ import Diff from './diff'; break; case 'projects:compare:show': new Diff(); - initChangesDropdown(); + const paddingTop = 16; + initChangesDropdown(document.querySelector('.navbar-gitlab').offsetHeight - paddingTop); break; case 'projects:branches:new': case 'projects:branches:create': diff --git a/app/assets/javascripts/init_changes_dropdown.js b/app/assets/javascripts/init_changes_dropdown.js index e41f374b2e5..1bab7965c19 100644 --- a/app/assets/javascripts/init_changes_dropdown.js +++ b/app/assets/javascripts/init_changes_dropdown.js @@ -1,7 +1,7 @@ import stickyMonitor from './lib/utils/sticky'; -export default () => { - stickyMonitor(document.querySelector('.js-diff-files-changed'), 76); +export default (stickyTop) => { + stickyMonitor(document.querySelector('.js-diff-files-changed'), stickyTop); $('.js-diff-stats-dropdown').glDropdown({ filterable: true, diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 789ccf48190..e64eb60231a 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -67,6 +67,9 @@ import Diff from './diff'; class MergeRequestTabs { constructor({ action, setUrl, stubLocation } = {}) { + const mergeRequestTabs = document.querySelector('.js-tabs-affix'); + const paddingTop = 16; + this.diffsLoaded = false; this.pipelinesLoaded = false; this.commitsLoaded = false; @@ -76,6 +79,11 @@ import Diff from './diff'; this.setCurrentAction = this.setCurrentAction.bind(this); this.tabShown = this.tabShown.bind(this); this.showTab = this.showTab.bind(this); + this.stickyTop = document.querySelector('.navbar-gitlab').offsetHeight - paddingTop; + + if (mergeRequestTabs) { + this.stickyTop += mergeRequestTabs.offsetHeight; + } if (stubLocation) { location = stubLocation; @@ -278,7 +286,7 @@ import Diff from './diff'; const $container = $('#diffs'); $container.html(data.html); - initChangesDropdown(); + initChangesDropdown(this.stickyTop); if (typeof gl.diffNotesCompileComponents !== 'undefined') { gl.diffNotesCompileComponents(); -- cgit v1.2.1 From f03d9a9b42102a3eb88ba8a31059aef29d3675f2 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 23 Oct 2017 14:24:51 +0100 Subject: fixed karma tests --- app/assets/javascripts/merge_request_tabs.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index e64eb60231a..54c1b7a268e 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -68,6 +68,7 @@ import Diff from './diff'; constructor({ action, setUrl, stubLocation } = {}) { const mergeRequestTabs = document.querySelector('.js-tabs-affix'); + const navbar = document.querySelector('.navbar-gitlab'); const paddingTop = 16; this.diffsLoaded = false; @@ -79,7 +80,7 @@ import Diff from './diff'; this.setCurrentAction = this.setCurrentAction.bind(this); this.tabShown = this.tabShown.bind(this); this.showTab = this.showTab.bind(this); - this.stickyTop = document.querySelector('.navbar-gitlab').offsetHeight - paddingTop; + this.stickyTop = navbar ? navbar.offsetHeight - paddingTop : 0; if (mergeRequestTabs) { this.stickyTop += mergeRequestTabs.offsetHeight; -- cgit v1.2.1