diff options
author | Phil Hughes <me@iamphill.com> | 2018-07-12 12:42:56 +0000 |
---|---|---|
committer | Phil Hughes <me@iamphill.com> | 2018-07-12 12:42:56 +0000 |
commit | 78df5fe231cb7b3f2c772749bfee1e1aa10aceba (patch) | |
tree | fe2c74e6ac8615811c4ee0669b758adb93fb3b94 | |
parent | eddf34f4af8a2cfdcea2586a91b0fc38a7ba0277 (diff) | |
parent | 289530a084d2bfec6de2d89bda3e8ddff46c4312 (diff) | |
download | gitlab-ce-78df5fe231cb7b3f2c772749bfee1e1aa10aceba.tar.gz |
Merge branch '48789-remove-event-listeners-scroll' into 'master'
Resolve "Improve performance of MR Changes tab: reduce event listeners on scroll event"
Closes #48639 and #48789
See merge request gitlab-org/gitlab-ce!20558
-rw-r--r-- | app/assets/javascripts/diffs/components/app.vue | 16 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/components/changed_files.vue | 41 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/components/changed_files_dropdown.vue | 2 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/components/diff_file.vue | 37 | ||||
-rw-r--r-- | app/assets/stylesheets/pages/diff.scss | 6 | ||||
-rw-r--r-- | changelogs/unreleased/48789-remove-event-listeners-scroll.yml | 6 | ||||
-rw-r--r-- | locale/gitlab.pot | 3 |
7 files changed, 27 insertions, 84 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 0327fceb38d..1d1415fe6ca 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -41,11 +41,6 @@ export default { required: true, }, }, - data() { - return { - activeFile: '', - }; - }, computed: { ...mapState({ isLoading: state => state.diffs.isLoading, @@ -126,14 +121,6 @@ export default { eventHub.$emit('fetchNotesData'); } }, - setActive(filePath) { - this.activeFile = filePath; - }, - unsetActive(filePath) { - if (this.activeFile === filePath) { - this.activeFile = ''; - } - }, adjustView() { if (this.shouldShow && this.isParallelView) { window.mrTabs.expandViewContainer(); @@ -195,7 +182,6 @@ export default { <changed-files :diff-files="diffFiles" - :active-file="activeFile" /> <div @@ -207,8 +193,6 @@ export default { :key="file.newPath" :file="file" :current-user="currentUser" - @setActive="setActive(file.filePath)" - @unsetActive="unsetActive(file.filePath)" /> </div> <no-changes v-else /> diff --git a/app/assets/javascripts/diffs/components/changed_files.vue b/app/assets/javascripts/diffs/components/changed_files.vue index 9d29357d800..97751db1254 100644 --- a/app/assets/javascripts/diffs/components/changed_files.vue +++ b/app/assets/javascripts/diffs/components/changed_files.vue @@ -16,13 +16,6 @@ export default { ClipboardButton, }, mixins: [changedFilesMixin], - props: { - activeFile: { - type: String, - required: false, - default: '', - }, - }, data() { return { isStuck: false, @@ -70,7 +63,7 @@ export default { pluralize, handleScroll() { if (!this.updating) { - requestAnimationFrame(this.updateIsStuck); + this.$nextTick(this.updateIsStuck); this.updating = true; } }, @@ -148,25 +141,8 @@ export default { /> <span - v-show="activeFile" - class="prepend-left-5" - > - <strong class="prepend-right-5"> - {{ truncatedDiffPath(activeFile) }} - </strong> - <clipboard-button - :text="activeFile" - :title="s__('Copy file name to clipboard')" - tooltip-placement="bottom" - tooltip-container="body" - class="btn btn-default btn-transparent btn-clipboard" - /> - </span> - - <span - v-show="!isStuck" - id="diff-stats" - class="diff-stats-additions-deletions-expanded" + class="js-diff-stats-additions-deletions-expanded + diff-stats-additions-deletions-expanded" > with <strong class="cgreen"> @@ -177,6 +153,17 @@ export default { {{ pluralize(`${sumRemovedLines} deletion`, sumRemovedLines) }} </strong> </span> + <div + class="js-diff-stats-additions-deletions-collapsed + diff-stats-additions-deletions-collapsed float-right d-sm-none" + > + <strong class="cgreen"> + +{{ sumAddedLines }} + </strong> + <strong class="cred"> + -{{ sumRemovedLines }} + </strong> + </div> </div> </div> </div> diff --git a/app/assets/javascripts/diffs/components/changed_files_dropdown.vue b/app/assets/javascripts/diffs/components/changed_files_dropdown.vue index b38d217fbe3..045688a32bf 100644 --- a/app/assets/javascripts/diffs/components/changed_files_dropdown.vue +++ b/app/assets/javascripts/diffs/components/changed_files_dropdown.vue @@ -40,7 +40,7 @@ export default { {{ n__('%d changed file', '%d changed files', diffFiles.length) }} </span> <icon - :size="8" + class="caret-icon" name="chevron-down" /> </button> diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index a61e368249a..944084f05c9 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -25,7 +25,6 @@ export default { }, data() { return { - isActive: false, isLoadingCollapsedDiff: false, forkMessageVisible: false, }; @@ -48,12 +47,6 @@ export default { return this.isCollapsed && !this.isLoadingCollapsedDiff && !this.file.tooLarge; }, }, - mounted() { - document.addEventListener('scroll', this.handleScroll); - }, - beforeDestroy() { - document.removeEventListener('scroll', this.handleScroll); - }, methods: { ...mapActions('diffs', ['loadCollapsedDiff']), handleToggle() { @@ -65,36 +58,6 @@ export default { this.file.collapsed = !this.file.collapsed; } }, - handleScroll() { - if (!this.updating) { - requestAnimationFrame(this.scrollUpdate.bind(this)); - this.updating = true; - } - }, - scrollUpdate() { - const header = document.querySelector('.js-diff-files-changed'); - if (!header) { - this.updating = false; - return; - } - - const { top, bottom } = this.$el.getBoundingClientRect(); - const { top: topOfFixedHeader, bottom: bottomOfFixedHeader } = header.getBoundingClientRect(); - - const headerOverlapsContent = top < topOfFixedHeader && bottom > bottomOfFixedHeader; - const fullyAboveHeader = bottom < bottomOfFixedHeader; - const fullyBelowHeader = top > topOfFixedHeader; - - if (headerOverlapsContent && !this.isActive) { - this.$emit('setActive'); - this.isActive = true; - } else if (this.isActive && (fullyAboveHeader || fullyBelowHeader)) { - this.$emit('unsetActive'); - this.isActive = false; - } - - this.updating = false; - }, handleLoadCollapsedDiff() { this.isLoadingCollapsedDiff = true; diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 7e89f8998fb..5e39bbb9890 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -518,6 +518,12 @@ outline: none; color: $gl-link-hover-color; } + + .caret-icon { + position: relative; + top: 2px; + left: -1px; + } } // Mobile diff --git a/changelogs/unreleased/48789-remove-event-listeners-scroll.yml b/changelogs/unreleased/48789-remove-event-listeners-scroll.yml new file mode 100644 index 00000000000..9cc3f7adc36 --- /dev/null +++ b/changelogs/unreleased/48789-remove-event-listeners-scroll.yml @@ -0,0 +1,6 @@ +--- +title: Improves performance on Merge Request diff tab by removing the scroll event + listeners being added to every file +merge_request: +author: +type: performance diff --git a/locale/gitlab.pot b/locale/gitlab.pot index eb3433b3ba2..c669e2e1622 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1666,9 +1666,6 @@ msgstr "" msgid "Copy commit SHA to clipboard" msgstr "" -msgid "Copy file name to clipboard" -msgstr "" - msgid "Copy file path to clipboard" msgstr "" |