diff options
11 files changed, 121 insertions, 10 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 7cc4e6a2c3a..b5b05df4d34 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -114,11 +114,15 @@ export default { this.adjustView(); }, methods: { - ...mapActions('diffs', ['setBaseConfig', 'fetchDiffFiles']), + ...mapActions('diffs', ['setBaseConfig', 'fetchDiffFiles', 'startRenderDiffsQueue']), fetchData() { - this.fetchDiffFiles().catch(() => { - createFlash(__('Something went wrong on our end. Please try again!')); - }); + this.fetchDiffFiles() + .then(() => { + requestIdleCallback(this.startRenderDiffsQueue, { timeout: 1000 }); + }) + .catch(() => { + createFlash(__('Something went wrong on our end. Please try again!')); + }); if (!this.isNotesFetched) { eventHub.$emit('fetchNotesData'); diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index 7e7058d8d08..59e9ba08b8b 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -46,16 +46,25 @@ export default { showExpandMessage() { return this.isCollapsed && !this.isLoadingCollapsedDiff && !this.file.tooLarge; }, + showLoadingIcon() { + return this.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed); + }, }, methods: { ...mapActions('diffs', ['loadCollapsedDiff']), handleToggle() { const { collapsed, highlightedDiffLines, parallelDiffLines } = this.file; - if (collapsed && !highlightedDiffLines && !parallelDiffLines.length) { + if ( + collapsed && + !highlightedDiffLines && + parallelDiffLines !== undefined && + !parallelDiffLines.length + ) { this.handleLoadCollapsedDiff(); } else { this.file.collapsed = !this.file.collapsed; + this.file.renderIt = true; } }, handleLoadCollapsedDiff() { @@ -65,6 +74,7 @@ export default { .then(() => { this.isLoadingCollapsedDiff = false; this.file.collapsed = false; + this.file.renderIt = true; }) .catch(() => { this.isLoadingCollapsedDiff = false; @@ -121,12 +131,12 @@ export default { </div> <diff-content - v-if="!isCollapsed" + v-if="!isCollapsed && file.renderIt" :class="{ hidden: isCollapsed || file.tooLarge }" :diff-file="file" /> <loading-icon - v-if="isLoadingCollapsedDiff" + v-else-if="showLoadingIcon" class="diff-content loading" /> <div diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index 2fa8367f528..f68afa44837 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -25,3 +25,6 @@ export const CONTEXT_LINE_CLASS_NAME = 'diff-expanded'; export const UNFOLD_COUNT = 20; export const COUNT_OF_AVATARS_IN_GUTTER = 3; export const LENGTH_OF_AVATAR_TOOLTIP = 17; + +export const LINES_TO_BE_RENDERED_DIRECTLY = 100; +export const MAX_LINES_TO_BE_RENDERED = 2000; diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 27001142257..4ab6ceb249a 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -29,6 +29,27 @@ export const fetchDiffFiles = ({ state, commit }) => { .then(handleLocationHash); }; +export const startRenderDiffsQueue = ({ state, commit }) => { + const checkItem = () => { + const nextFile = state.diffFiles.find( + file => !file.renderIt && (!file.collapsed || !file.text), + ); + if (nextFile) { + requestAnimationFrame(() => { + commit(types.RENDER_FILE, nextFile); + }); + requestIdleCallback( + () => { + checkItem(); + }, + { timeout: 1000 }, + ); + } + }; + + checkItem(); +}; + export const setInlineDiffViewType = ({ commit }) => { commit(types.SET_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE); diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index 2c8e1a1466f..c999d637d50 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -8,3 +8,4 @@ export const REMOVE_COMMENT_FORM_LINE = 'REMOVE_COMMENT_FORM_LINE'; export const ADD_CONTEXT_LINES = 'ADD_CONTEXT_LINES'; export const ADD_COLLAPSED_DIFFS = 'ADD_COLLAPSED_DIFFS'; export const EXPAND_ALL_FILES = 'EXPAND_ALL_FILES'; +export const RENDER_FILE = 'RENDER_FILE'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index a98b2be89a3..0522e32c410 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -2,6 +2,7 @@ import Vue from 'vue'; import _ from 'underscore'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { findDiffFile, addLineReferences, removeMatchLine, addContextLines } from './utils'; +import { LINES_TO_BE_RENDERED_DIRECTLY, MAX_LINES_TO_BE_RENDERED } from '../constants'; import * as types from './mutation_types'; export default { @@ -15,8 +16,48 @@ export default { }, [types.SET_DIFF_DATA](state, data) { + const diffData = convertObjectPropsToCamelCase(data, { deep: true }); + let showingLines = 0; + const filesLength = diffData.diffFiles.length; + let i; + for (i = 0; i < filesLength; i += 1) { + const file = diffData.diffFiles[i]; + if (file.parallelDiffLines) { + const linesLength = file.parallelDiffLines.length; + let u = 0; + for (u = 0; u < linesLength; u += 1) { + const line = file.parallelDiffLines[u]; + if (line.left) delete line.left.text; + if (line.right) delete line.right.text; + } + } + + if (file.highlightedDiffLines) { + const linesLength = file.highlightedDiffLines.length; + let u; + for (u = 0; u < linesLength; u += 1) { + const line = file.highlightedDiffLines[u]; + delete line.text; + } + } + + if (file.highlightedDiffLines) { + showingLines += file.parallelDiffLines.length; + } + Object.assign(file, { + renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY, + collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED, + }); + } + Object.assign(state, { - ...convertObjectPropsToCamelCase(data, { deep: true }), + ...diffData, + }); + }, + + [types.RENDER_FILE](state, file) { + Object.assign(file, { + renderIt: true, }); }, diff --git a/changelogs/unreleased/tz-mr-incremental-rendering.yml b/changelogs/unreleased/tz-mr-incremental-rendering.yml new file mode 100644 index 00000000000..a35fa200363 --- /dev/null +++ b/changelogs/unreleased/tz-mr-incremental-rendering.yml @@ -0,0 +1,4 @@ +title: Incremental rendering with Vue on merge request page +merge_request: 21063 +author: +type: performance diff --git a/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb b/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb index c1608be402a..fd4175d5227 100644 --- a/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb +++ b/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb @@ -28,7 +28,7 @@ describe 'Merge request > User sees MR with deleted source branch', :js do click_on 'Changes' wait_for_requests - expect(page).to have_selector('.diffs.tab-pane .nothing-here-block') + expect(page).to have_selector('.diffs.tab-pane .file-holder') expect(page).to have_content('Source branch does not exist.') end end diff --git a/spec/javascripts/diffs/components/diff_file_spec.js b/spec/javascripts/diffs/components/diff_file_spec.js index 7a4616ec8eb..44a38f7ca82 100644 --- a/spec/javascripts/diffs/components/diff_file_spec.js +++ b/spec/javascripts/diffs/components/diff_file_spec.js @@ -22,11 +22,18 @@ describe('DiffFile', () => { expect(el.id).toEqual(fileHash); expect(el.classList.contains('diff-file')).toEqual(true); + expect(el.querySelectorAll('.diff-content.hidden').length).toEqual(0); expect(el.querySelector('.js-file-title')).toBeDefined(); expect(el.querySelector('.file-title-name').innerText.indexOf(filePath) > -1).toEqual(true); expect(el.querySelector('.js-syntax-highlight')).toBeDefined(); - expect(el.querySelectorAll('.line_content').length > 5).toEqual(true); + + expect(vm.file.renderIt).toEqual(false); + vm.file.renderIt = true; + + vm.$nextTick(() => { + expect(el.querySelectorAll('.line_content').length > 5).toEqual(true); + }); }); describe('collapsed', () => { @@ -34,6 +41,7 @@ describe('DiffFile', () => { expect(vm.$el.querySelectorAll('.diff-content').length).toEqual(1); expect(vm.file.collapsed).toEqual(false); vm.file.collapsed = true; + vm.file.renderIt = true; vm.$nextTick(() => { expect(vm.$el.querySelectorAll('.diff-content').length).toEqual(0); diff --git a/spec/javascripts/diffs/mock_data/diff_file.js b/spec/javascripts/diffs/mock_data/diff_file.js index d3bf9525924..cce36ecc91f 100644 --- a/spec/javascripts/diffs/mock_data/diff_file.js +++ b/spec/javascripts/diffs/mock_data/diff_file.js @@ -39,6 +39,7 @@ export default { viewPath: '/gitlab-org/gitlab-test/blob/spooky-stuff/CHANGELOG', replacedViewPath: null, collapsed: false, + renderIt: false, tooLarge: false, contextLinesPath: '/gitlab-org/gitlab-test/blob/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG/diff', diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index 1af49f4985c..8f89984c6e5 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -1,6 +1,7 @@ import mutations from '~/diffs/store/mutations'; import * as types from '~/diffs/store/mutation_types'; import { INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants'; +import diffFileMockData from '../mock_data/diff_file'; describe('DiffsStoreMutations', () => { describe('SET_BASE_CONFIG', () => { @@ -24,6 +25,23 @@ describe('DiffsStoreMutations', () => { }); }); + describe('SET_DIFF_DATA', () => { + it('should set diff data type properly', () => { + const state = {}; + const diffMock = { + diff_files: [diffFileMockData], + }; + + mutations[types.SET_DIFF_DATA](state, diffMock); + + const firstLine = state.diffFiles[0].parallelDiffLines[0]; + + expect(firstLine.right.text).toBeUndefined(); + expect(state.diffFiles[0].renderIt).toEqual(true); + expect(state.diffFiles[0].collapsed).toEqual(false); + }); + }); + describe('SET_DIFF_VIEW_TYPE', () => { it('should set diff view type properly', () => { const state = {}; |