diff options
author | Tim Zallmann <tzallmann@gitlab.com> | 2018-09-05 10:28:49 +0200 |
---|---|---|
committer | Tim Zallmann <tzallmann@gitlab.com> | 2018-09-07 12:25:50 +0200 |
commit | c9bacfd6823de77de6b60db39190190b502681e0 (patch) | |
tree | d75660a1878a08d681d0aec594e5e8e813caa285 /app | |
parent | cee271ee3476fa357cbc8ba4414cad6118b04d7e (diff) | |
download | gitlab-ce-c9bacfd6823de77de6b60db39190190b502681e0.tar.gz |
Fixed Resolving, Loading more and Line Bugs
Diffstat (limited to 'app')
11 files changed, 119 insertions, 95 deletions
diff --git a/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js b/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js index 5ed13488788..ae31005a2d2 100644 --- a/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js +++ b/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js @@ -25,44 +25,44 @@ const ResolveDiscussionBtn = Vue.extend({ }; }, computed: { - showButton: function () { + showButton: function() { if (this.discussion) { return this.discussion.isResolvable(); } else { return false; } }, - isDiscussionResolved: function () { + isDiscussionResolved: function() { if (this.discussion) { return this.discussion.isResolved(); } else { return false; } }, - buttonText: function () { + buttonText: function() { if (this.isDiscussionResolved) { - return "Unresolve discussion"; + return 'Unresolve discussion'; } else { - return "Resolve discussion"; + return 'Resolve discussion'; } }, - loading: function () { + loading: function() { if (this.discussion) { return this.discussion.loading; } else { return false; } - } + }, }, - created: function () { + created: function() { CommentsStore.createDiscussion(this.discussionId, this.canResolve); this.discussion = CommentsStore.state[this.discussionId]; }, methods: { - resolve: function () { + resolve: function() { ResolveService.toggleResolveForDiscussion(this.mergeRequestId, this.discussionId); - } + }, }, }); diff --git a/app/assets/javascripts/diff_notes/services/resolve.js b/app/assets/javascripts/diff_notes/services/resolve.js index 0b3568e432d..e69eaad4423 100644 --- a/app/assets/javascripts/diff_notes/services/resolve.js +++ b/app/assets/javascripts/diff_notes/services/resolve.js @@ -8,9 +8,7 @@ window.gl = window.gl || {}; class ResolveServiceClass { constructor(root) { - this.noteResource = Vue.resource( - `${root}/notes{/noteId}/resolve?html=true`, - ); + this.noteResource = Vue.resource(`${root}/notes{/noteId}/resolve?html=true`); this.discussionResource = Vue.resource( `${root}/merge_requests{/mergeRequestId}/discussions{/discussionId}/resolve?html=true`, ); @@ -51,10 +49,7 @@ class ResolveServiceClass { discussion.updateHeadline(data); }) .catch( - () => - new Flash( - 'An error occurred when trying to resolve a discussion. Please try again.', - ), + () => new Flash('An error occurred when trying to resolve a discussion. Please try again.'), ); } diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 7ec7ee893af..13ff79029d0 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -112,6 +112,7 @@ export default { }, created() { this.adjustView(); + eventHub.$once('fetchedNotesData', this.setDiscussions); }, methods: { ...mapActions('diffs', [ @@ -128,12 +129,7 @@ export default { () => { this.startRenderDiffsQueue() .then(() => { - requestIdleCallback( - () => { - this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode); - }, - { timeout: 1000 }, - ); + this.setDiscussions(); }) .catch(() => { createFlash(__('Something went wrong on our end. Please try again!')); @@ -150,6 +146,16 @@ export default { eventHub.$emit('fetchNotesData'); } }, + setDiscussions() { + if (this.isNotesFetched) { + requestIdleCallback( + () => { + this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode); + }, + { timeout: 1000 }, + ); + } + }, adjustView() { if (this.shouldShow && this.isParallelView) { window.mrTabs.expandViewContainer(); diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index 59e9ba08b8b..a0197e95456 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -1,5 +1,5 @@ <script> -import { mapActions } from 'vuex'; +import { mapActions, mapGetters } from 'vuex'; import _ from 'underscore'; import { __, sprintf } from '~/locale'; import createFlash from '~/flash'; @@ -30,6 +30,7 @@ export default { }; }, computed: { + ...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']), isCollapsed() { return this.file.collapsed || false; }, @@ -44,23 +45,22 @@ export default { ); }, showExpandMessage() { - return this.isCollapsed && !this.isLoadingCollapsedDiff && !this.file.tooLarge; + return ( + !this.isCollapsed && + !this.file.highlightedDiffLines && + !this.isLoadingCollapsedDiff && + !this.file.tooLarge + ); }, showLoadingIcon() { return this.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed); }, }, methods: { - ...mapActions('diffs', ['loadCollapsedDiff']), + ...mapActions('diffs', ['loadCollapsedDiff', 'assignDiscussionsToDiff']), handleToggle() { - const { collapsed, highlightedDiffLines, parallelDiffLines } = this.file; - - if ( - collapsed && - !highlightedDiffLines && - parallelDiffLines !== undefined && - !parallelDiffLines.length - ) { + const { highlightedDiffLines, parallelDiffLines } = this.file; + if (!highlightedDiffLines && parallelDiffLines !== undefined && !parallelDiffLines.length) { this.handleLoadCollapsedDiff(); } else { this.file.collapsed = !this.file.collapsed; @@ -76,6 +76,15 @@ export default { this.file.collapsed = false; this.file.renderIt = true; }) + .then(() => this.$nextTick()) + .then(() => { + requestIdleCallback( + () => { + this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode); + }, + { timeout: 1000 }, + ); + }) .catch(() => { this.isLoadingCollapsedDiff = false; createFlash(__('Something went wrong on our end. Please try again!')); @@ -136,7 +145,7 @@ export default { :diff-file="file" /> <loading-icon - v-else-if="showLoadingIcon" + v-if="showLoadingIcon" class="diff-content loading" /> <div diff --git a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue index f07a048d6a3..88bf026c0dd 100644 --- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue +++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue @@ -73,7 +73,7 @@ export default { }), ...mapGetters(['isLoggedIn']), lineHref() { - return this.line && this.line.code ? `#${this.line.code}` : '#'; + return this.line && this.line.lineCode ? `#${this.line.lineCode}` : '#'; }, shouldShowCommentButton() { return ( @@ -93,7 +93,6 @@ export default { if (this.line && !this.line.type && this.linePosition === LINE_POSITION_RIGHT) { return false; } - return this.showCommentButton && this.hasDiscussions; }, }, diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 1838394422a..d04fa5d2cae 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -29,7 +29,7 @@ export const fetchDiffFiles = ({ state, commit }) => { .then(handleLocationHash); }; -export const assignDiscussionsToDiff = ({ state }, allLineDiscussions) => { +export const assignDiscussionsToDiff = ({ state, commit }, allLineDiscussions) => { Object.values(allLineDiscussions).forEach(discussions => { if (discussions.length > 0) { const { fileHash } = discussions[0]; @@ -40,12 +40,11 @@ export const assignDiscussionsToDiff = ({ state }, allLineDiscussions) => { (line.left && line.left.lineCode === discussions[0].line_code) || (line.right && line.right.lineCode === discussions[0].line_code), ); - if (targetLine) { if (targetLine.left && targetLine.left.lineCode === discussions[0].line_code) { - Object.assign(targetLine.left, { discussions }); + commit(types.SET_LINE_DISCUSSIONS, { line: targetLine.left, discussions }); } else { - Object.assign(targetLine.right, { discussions }); + commit(types.SET_LINE_DISCUSSIONS, { line: targetLine.right, discussions }); } } @@ -55,7 +54,7 @@ export const assignDiscussionsToDiff = ({ state }, allLineDiscussions) => { ); if (targetInlineLine) { - Object.assign(targetInlineLine, { discussions }); + commit(types.SET_LINE_DISCUSSIONS, { line: targetInlineLine, discussions }); } } } @@ -63,7 +62,7 @@ export const assignDiscussionsToDiff = ({ state }, allLineDiscussions) => { }); }; -export const removeDiscussionsFromDiff = ({ state }, removeDiscussion) => { +export const removeDiscussionsFromDiff = ({ state, commit }, removeDiscussion) => { const { fileHash } = removeDiscussion; const selectedFile = state.diffFiles.find(file => file.fileHash === fileHash); if (selectedFile) { @@ -74,7 +73,13 @@ export const removeDiscussionsFromDiff = ({ state }, removeDiscussion) => { ); if (targetLine) { - Object.assign(targetLine.right, { discussions: [] }); + if (targetLine) { + if (targetLine.left && targetLine.left.lineCode === removeDiscussion.line_code) { + commit(types.REMOVE_LINE_DISCUSSIONS, targetLine.left); + } else { + commit(types.REMOVE_LINE_DISCUSSIONS, targetLine.right); + } + } } const targetInlineLine = selectedFile.highlightedDiffLines.find( @@ -82,7 +87,7 @@ export const removeDiscussionsFromDiff = ({ state }, removeDiscussion) => { ); if (targetInlineLine) { - Object.assign(targetInlineLine, { discussions: [] }); + commit(types.REMOVE_LINE_DISCUSSIONS, targetInlineLine); } } }; diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index c999d637d50..80f2807682a 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -9,3 +9,5 @@ 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'; +export const SET_LINE_DISCUSSIONS = 'SET_LINE_DISCUSSIONS'; +export const REMOVE_LINE_DISCUSSIONS = 'REMOVE_LINE_DISCUSSIONS'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index af40a657211..f7f6a187e0d 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -6,9 +6,8 @@ import { addLineReferences, removeMatchLine, addContextLines, - trimFirstCharOfLineContent, + prepareDiffData, } from './utils'; -import { LINES_TO_BE_RENDERED_DIRECTLY, MAX_LINES_TO_BE_RENDERED } from '../constants'; import * as types from './mutation_types'; export default { @@ -23,40 +22,7 @@ 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) { - line.left = trimFirstCharOfLineContent(line.left); - } - if (line.right) { - line.right = trimFirstCharOfLineContent(line.right); - } - } - } - - if (file.highlightedDiffLines) { - const linesLength = file.highlightedDiffLines.length; - let u; - for (u = 0; u < linesLength; u += 1) { - trimFirstCharOfLineContent(file.highlightedDiffLines[u]); - } - showingLines += file.parallelDiffLines.length; - } - - Object.assign(file, { - renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY, - collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED, - }); - } + prepareDiffData(diffData); Object.assign(state, { ...diffData, @@ -106,12 +72,9 @@ export default { [types.ADD_COLLAPSED_DIFFS](state, { file, data }) { const normalizedData = convertObjectPropsToCamelCase(data, { deep: true }); + prepareDiffData(normalizedData); const [newFileData] = normalizedData.diffFiles.filter(f => f.fileHash === file.fileHash); - - if (newFileData) { - const index = _.findIndex(state.diffFiles, f => f.fileHash === file.fileHash); - state.diffFiles.splice(index, 1, newFileData); - } + Object.assign(file, { ...newFileData }); }, [types.EXPAND_ALL_FILES](state) { @@ -120,4 +83,16 @@ export default { collapsed: false, })); }, + + [types.SET_LINE_DISCUSSIONS](state, { line, discussions }) { + Object.assign(line, { + discussions, + }); + }, + + [types.REMOVE_LINE_DISCUSSIONS](state, line) { + Object.assign(line, { + discussions: [], + }); + }, }; diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 5a29d3e1ac7..eb9bab09d38 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -8,6 +8,8 @@ import { NEW_LINE_TYPE, OLD_LINE_TYPE, MATCH_LINE_TYPE, + LINES_TO_BE_RENDERED_DIRECTLY, + MAX_LINES_TO_BE_RENDERED, } from '../constants'; export function findDiffFile(files, hash) { @@ -179,6 +181,43 @@ export function trimFirstCharOfLineContent(line = {}) { return parsedLine; } +export function prepareDiffData(diffData) { + 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) { + line.left = trimFirstCharOfLineContent(line.left); + } + if (line.right) { + line.right = trimFirstCharOfLineContent(line.right); + } + } + } + + if (file.highlightedDiffLines) { + const linesLength = file.highlightedDiffLines.length; + let u; + for (u = 0; u < linesLength; u += 1) { + trimFirstCharOfLineContent(file.highlightedDiffLines[u]); + } + showingLines += file.parallelDiffLines.length; + } + + Object.assign(file, { + renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY, + collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED, + }); + } +} + export function getDiffRefsByLineCode(diffFiles) { return diffFiles.reduce((acc, diffFile) => { const { baseSha, headSha, startSha } = diffFile.diffRefs; diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue index 9b8713b40fb..7f9d23b211b 100644 --- a/app/assets/javascripts/notes/components/notes_app.vue +++ b/app/assets/javascripts/notes/components/notes_app.vue @@ -138,6 +138,7 @@ export default { .then(() => { this.isLoading = false; this.setNotesFetchedState(true); + eventHub.$emit('fetchedNotesData'); }) .then(() => this.$nextTick()) .then(() => this.checkLocationHash()) diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js index ab6a95e2601..b6fd19dedf9 100644 --- a/app/assets/javascripts/notes/stores/mutations.js +++ b/app/assets/javascripts/notes/stores/mutations.js @@ -185,16 +185,9 @@ export default { [types.UPDATE_DISCUSSION](state, noteData) { const note = noteData; - let index = 0; - - state.discussions.forEach((n, i) => { - if (n.id === note.id) { - index = i; - } - }); - + const selectedDiscussion = state.discussions.find(disc => disc.id === note.id); note.expanded = true; // override expand flag to prevent collapse - state.discussions.splice(index, 1, note); + Object.assign(selectedDiscussion, { ...note }); }, [types.CLOSE_ISSUE](state) { |