From b14de8e1f519b9b874033f783051814129af176c Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 28 Feb 2019 14:14:15 +0000 Subject: Add option to expand diff to full file The user can also toggle between the diff changes and the full file diff. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/19054 --- .../diffs/components/diff_file_header.vue | 36 ++++++++++++--- .../javascripts/diffs/components/edit_button.vue | 20 ++++++++- app/assets/javascripts/diffs/constants.js | 5 +++ app/assets/javascripts/diffs/store/actions.js | 35 +++++++++++++++ .../javascripts/diffs/store/mutation_types.js | 4 ++ app/assets/javascripts/diffs/store/mutations.js | 52 ++++++++++++++++++++++ app/assets/javascripts/diffs/store/utils.js | 40 ++++++++++++++++- app/assets/stylesheets/pages/diff.scss | 4 ++ 8 files changed, 187 insertions(+), 9 deletions(-) (limited to 'app/assets') diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index 4d33ad23f39..a5125c3d077 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -5,7 +5,7 @@ import { polyfillSticky } from '~/lib/utils/sticky'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import Icon from '~/vue_shared/components/icon.vue'; import FileIcon from '~/vue_shared/components/file_icon.vue'; -import { GlTooltipDirective } from '@gitlab/ui'; +import { GlButton, GlTooltipDirective, GlTooltip, GlLoadingIcon } from '@gitlab/ui'; import { truncateSha } from '~/lib/utils/text_utility'; import { __, s__, sprintf } from '~/locale'; import { diffViewerModes } from '~/ide/constants'; @@ -14,6 +14,9 @@ import DiffStats from './diff_stats.vue'; export default { components: { + GlTooltip, + GlLoadingIcon, + GlButton, ClipboardButton, EditButton, Icon, @@ -125,12 +128,15 @@ export default { isModeChanged() { return this.diffFile.viewer.name === diffViewerModes.mode_changed; }, + showExpandDiffToFullFileEnabled() { + return gon.features.expandDiffFullFile && !this.diffFile.is_fully_expanded; + }, }, mounted() { polyfillSticky(this.$refs.header); }, methods: { - ...mapActions('diffs', ['toggleFileDiscussions']), + ...mapActions('diffs', ['toggleFileDiscussions', 'toggleFullDiff']), handleToggleFile(e, checkTarget) { if ( !checkTarget || @@ -240,12 +246,30 @@ export default { v-html="viewReplacedFileButtonText" > - + + + - + + + + + + + +import { GlTooltipDirective, GlButton } from '@gitlab/ui'; +import Icon from '~/vue_shared/components/icon.vue'; + export default { + components: { + GlButton, + Icon, + }, + directives: { + GlTooltip: GlTooltipDirective, + }, props: { editPath: { type: String, @@ -27,5 +37,13 @@ export default { diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index 7002655ea49..6f380fe6ece 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -42,3 +42,8 @@ export const INITIAL_TREE_WIDTH = 320; export const MIN_TREE_WIDTH = 240; export const MAX_TREE_WIDTH = 400; export const TREE_HIDE_STATS_WIDTH = 260; + +export const OLD_LINE_KEY = 'old_line'; +export const NEW_LINE_KEY = 'new_line'; +export const TYPE_KEY = 'type'; +export const LEFT_LINE_KEY = 'left'; diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index c40775c3259..57ddc923a3e 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -309,5 +309,40 @@ export const cacheTreeListWidth = (_, size) => { localStorage.setItem(TREE_LIST_WIDTH_STORAGE_KEY, size); }; +export const requestFullDiff = ({ commit }, filePath) => commit(types.REQUEST_FULL_DIFF, filePath); +export const receiveFullDiffSucess = ({ commit }, { filePath, data }) => + commit(types.RECEIVE_FULL_DIFF_SUCCESS, { filePath, data }); +export const receiveFullDiffError = ({ commit }, filePath) => { + commit(types.RECEIVE_FULL_DIFF_ERROR, filePath); + createFlash(s__('MergeRequest|Error loading full diff. Please try again.')); +}; + +export const fetchFullDiff = ({ dispatch }, file) => + axios + .get(file.context_lines_path, { + params: { + full: true, + from_merge_request: true, + }, + }) + .then(({ data }) => dispatch('receiveFullDiffSucess', { filePath: file.file_path, data })) + .then(() => scrollToElement(`#${file.file_hash}`)) + .catch(() => dispatch('receiveFullDiffError', file.file_path)); + +export const toggleFullDiff = ({ dispatch, getters, state }, filePath) => { + const file = state.diffFiles.find(f => f.file_path === filePath); + + dispatch('requestFullDiff', filePath); + + if (file.isShowingFullFile) { + dispatch('loadCollapsedDiff', file) + .then(() => dispatch('assignDiscussionsToDiff', getters.getDiffFileDiscussions(file))) + .then(() => scrollToElement(`#${file.file_hash}`)) + .catch(() => dispatch('receiveFullDiffError', filePath)); + } else { + dispatch('fetchFullDiff', file); + } +}; + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index 71ad108ce88..b441b1de451 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -23,3 +23,7 @@ export const SET_TREE_DATA = 'SET_TREE_DATA'; export const SET_RENDER_TREE_LIST = 'SET_RENDER_TREE_LIST'; export const SET_SHOW_WHITESPACE = 'SET_SHOW_WHITESPACE'; export const TOGGLE_FILE_FINDER_VISIBLE = 'TOGGLE_FILE_FINDER_VISIBLE'; + +export const REQUEST_FULL_DIFF = 'REQUEST_FULL_DIFF'; +export const RECEIVE_FULL_DIFF_SUCCESS = 'RECEIVE_FULL_DIFF_SUCCESS'; +export const RECEIVE_FULL_DIFF_ERROR = 'RECEIVE_FULL_DIFF_ERROR'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 5a27388863c..45187d93fef 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -6,8 +6,10 @@ import { addContextLines, prepareDiffData, isDiscussionApplicableToLine, + convertExpandLines, } from './utils'; import * as types from './mutation_types'; +import { OLD_LINE_KEY, NEW_LINE_KEY, TYPE_KEY, LEFT_LINE_KEY } from '../constants'; export default { [types.SET_BASE_CONFIG](state, options) { @@ -248,4 +250,54 @@ export default { [types.TOGGLE_FILE_FINDER_VISIBLE](state, visible) { state.fileFinderVisible = visible; }, + [types.REQUEST_FULL_DIFF](state, filePath) { + const file = findDiffFile(state.diffFiles, filePath, 'file_path'); + + file.isLoadingFullFile = true; + }, + [types.RECEIVE_FULL_DIFF_ERROR](state, filePath) { + const file = findDiffFile(state.diffFiles, filePath, 'file_path'); + + file.isLoadingFullFile = false; + }, + [types.RECEIVE_FULL_DIFF_SUCCESS](state, { filePath, data }) { + const file = findDiffFile(state.diffFiles, filePath, 'file_path'); + + file.isShowingFullFile = true; + file.isLoadingFullFile = false; + + file.highlighted_diff_lines = convertExpandLines({ + diffLines: file.highlighted_diff_lines, + typeKey: [TYPE_KEY], + oldLineKey: [OLD_LINE_KEY], + newLineKey: [NEW_LINE_KEY], + data, + mapLine: ({ line, oldLine, newLine }) => ({ + ...line, + old_line: oldLine, + new_line: newLine, + line_code: `${file.file_hash}_${oldLine}_${newLine}`, + }), + }); + + file.parallel_diff_lines = convertExpandLines({ + diffLines: file.parallel_diff_lines, + typeKey: [LEFT_LINE_KEY, TYPE_KEY], + oldLineKey: [LEFT_LINE_KEY, OLD_LINE_KEY], + newLineKey: [LEFT_LINE_KEY, NEW_LINE_KEY], + data, + mapLine: ({ line, oldLine, newLine }) => ({ + left: { + ...line, + old_line: oldLine, + line_code: `${file.file_hash}_${oldLine}_${newLine}`, + }, + right: { + ...line, + new_line: newLine, + line_code: `${file.file_hash}_${newLine}_${oldLine}`, + }, + }), + }); + }, }; diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 247d1e65fea..27a79369a24 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -15,8 +15,8 @@ import { TREE_TYPE, } from '../constants'; -export function findDiffFile(files, hash) { - return files.filter(file => file.file_hash === hash)[0]; +export function findDiffFile(files, match, matchKey = 'file_hash') { + return files.find(file => file[matchKey] === match); } export const getReversePosition = linePosition => { @@ -250,6 +250,8 @@ export function prepareDiffData(diffData) { renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY, collapsed: file.viewer.name === diffViewerModes.text && showingLines > MAX_LINES_TO_BE_RENDERED, + isShowingFullFile: false, + isLoadingFullFile: false, discussions: [], }); } @@ -411,3 +413,37 @@ export const getDiffMode = diffFile => { diffModes.replaced ); }; + +export const convertExpandLines = ({ + diffLines, + data, + typeKey, + oldLineKey, + newLineKey, + mapLine, +}) => { + const dataLength = data.length; + + return diffLines.reduce((acc, line, i) => { + if (_.property(typeKey)(line) === 'match') { + const beforeLine = diffLines[i - 1]; + const afterLine = diffLines[i + 1]; + const beforeLineIndex = _.property(newLineKey)(beforeLine) || 0; + const afterLineIndex = _.property(newLineKey)(afterLine) - 1 || dataLength; + + acc.push( + ...data.slice(beforeLineIndex, afterLineIndex).map((l, index) => ({ + ...mapLine({ + line: { ...l, hasForm: false, discussions: [] }, + oldLine: (_.property(oldLineKey)(beforeLine) || 0) + index + 1, + newLine: (_.property(newLineKey)(beforeLine) || 0) + index + 1, + }), + })), + ); + } else { + acc.push(line); + } + + return acc; + }, []); +}; diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 0dbbe9e4c25..e50db5310a6 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -53,6 +53,10 @@ background-color: $gray-normal; } + a { + color: $gray-700; + } + svg { vertical-align: middle; top: -1px; -- cgit v1.2.3