Commit fc0ff7cf authored by Samantha Ming's avatar Samantha Ming Committed by Mike Greiling

Replace ... with new expansion options

- expand upwards
- expand downwards
- expand all

in both inline and parallel views
parent cdac9ed8
<script>
import createFlash from '~/flash';
import { s__ } from '~/locale';
import { mapState, mapActions } from 'vuex';
import Icon from '~/vue_shared/components/icon.vue';
import { UNFOLD_COUNT } from '../constants';
import * as utils from '../store/utils';
import tooltip from '../../vue_shared/directives/tooltip';
const EXPAND_ALL = 0;
const EXPAND_UP = 1;
const EXPAND_DOWN = 2;
export default {
directives: {
tooltip,
},
components: {
Icon,
},
props: {
fileHash: {
type: String,
required: true,
},
contextLinesPath: {
type: String,
required: true,
},
line: {
type: Object,
required: true,
},
isTop: {
type: Boolean,
required: false,
default: false,
},
isBottom: {
type: Boolean,
required: false,
default: false,
},
colspan: {
type: Number,
required: false,
default: 3,
},
},
computed: {
...mapState({
diffViewType: state => state.diffs.diffViewType,
diffFiles: state => state.diffs.diffFiles,
}),
canExpandUp() {
return !this.isBottom;
},
canExpandDown() {
return this.isBottom || !this.isTop;
},
},
created() {
this.EXPAND_DOWN = EXPAND_DOWN;
this.EXPAND_UP = EXPAND_UP;
},
methods: {
...mapActions('diffs', ['loadMoreLines']),
getPrevLineNumber(oldLineNumber, newLineNumber) {
const diffFile = utils.findDiffFile(this.diffFiles, this.fileHash);
const indexForInline = utils.findIndexInInlineLines(diffFile.highlighted_diff_lines, {
oldLineNumber,
newLineNumber,
});
const prevLine = diffFile.highlighted_diff_lines[indexForInline - 2];
return (prevLine && prevLine.new_line) || 0;
},
callLoadMoreLines(
endpoint,
params,
lineNumbers,
fileHash,
isExpandDown = false,
nextLineNumbers = {},
) {
this.loadMoreLines({ endpoint, params, lineNumbers, fileHash, isExpandDown, nextLineNumbers })
.then(() => {
this.isRequesting = false;
})
.catch(() => {
createFlash(s__('Diffs|Something went wrong while fetching diff lines.'));
this.isRequesting = false;
});
},
handleExpandLines(type = EXPAND_ALL) {
if (this.isRequesting) {
return;
}
this.isRequesting = true;
const endpoint = this.contextLinesPath;
const { fileHash } = this;
const view = this.diffViewType;
const oldLineNumber = this.line.meta_data.old_pos || 0;
const newLineNumber = this.line.meta_data.new_pos || 0;
const offset = newLineNumber - oldLineNumber;
const expandOptions = { endpoint, fileHash, view, oldLineNumber, newLineNumber, offset };
if (type === EXPAND_UP) {
this.handleExpandUpLines(expandOptions);
} else if (type === EXPAND_DOWN) {
this.handleExpandDownLines(expandOptions);
} else {
this.handleExpandAllLines(expandOptions);
}
},
handleExpandUpLines(expandOptions = EXPAND_ALL) {
const { endpoint, fileHash, view, oldLineNumber, newLineNumber, offset } = expandOptions;
const bottom = this.isBottom;
const lineNumber = newLineNumber - 1;
const to = lineNumber;
let since = lineNumber - UNFOLD_COUNT;
let unfold = true;
const prevLineNumber = this.getPrevLineNumber(oldLineNumber, newLineNumber);
if (since <= prevLineNumber + 1) {
since = prevLineNumber + 1;
unfold = false;
}
const params = { since, to, bottom, offset, unfold, view };
const lineNumbers = { oldLineNumber, newLineNumber };
this.callLoadMoreLines(endpoint, params, lineNumbers, fileHash);
},
handleExpandDownLines(expandOptions) {
const {
endpoint,
fileHash,
view,
oldLineNumber: metaOldPos,
newLineNumber: metaNewPos,
offset,
} = expandOptions;
const bottom = true;
const nextLineNumbers = {
old_line: metaOldPos,
new_line: metaNewPos,
};
let unfold = true;
let isExpandDown = false;
let oldLineNumber = metaOldPos;
let newLineNumber = metaNewPos;
let lineNumber = metaNewPos + 1;
let since = lineNumber;
let to = lineNumber + UNFOLD_COUNT;
if (!this.isBottom) {
const prevLineNumber = this.getPrevLineNumber(oldLineNumber, newLineNumber);
isExpandDown = true;
oldLineNumber = prevLineNumber - offset;
newLineNumber = prevLineNumber;
lineNumber = prevLineNumber + 1;
since = lineNumber;
to = lineNumber + UNFOLD_COUNT;
if (to >= metaNewPos) {
to = metaNewPos - 1;
unfold = false;
}
}
const params = { since, to, bottom, offset, unfold, view };
const lineNumbers = { oldLineNumber, newLineNumber };
this.callLoadMoreLines(
endpoint,
params,
lineNumbers,
fileHash,
isExpandDown,
nextLineNumbers,
);
},
handleExpandAllLines(expandOptions) {
const { endpoint, fileHash, view, oldLineNumber, newLineNumber, offset } = expandOptions;
const bottom = this.isBottom;
const unfold = false;
let since;
let to;
if (this.isTop) {
since = 1;
to = newLineNumber - 1;
} else if (bottom) {
since = newLineNumber + 1;
to = -1;
} else {
const prevLineNumber = this.getPrevLineNumber(oldLineNumber, newLineNumber);
since = prevLineNumber + 1;
to = newLineNumber - 1;
}
const params = { since, to, bottom, offset, unfold, view };
const lineNumbers = { oldLineNumber, newLineNumber };
this.callLoadMoreLines(endpoint, params, lineNumbers, fileHash);
},
},
};
</script>
<template>
<td :colspan="colspan">
<div class="content">
<a
v-if="canExpandUp"
v-tooltip
class="cursor-pointer js-unfold unfold-icon"
data-placement="top"
data-container="body"
:title="__('Expand up')"
@click="handleExpandLines(EXPAND_UP)"
>
<!-- TODO: remove style & replace with correct icon, waiting for MR https://gitlab.com/gitlab-org/gitlab-design/issues/499 -->
<icon :size="12" name="expand-left" aria-hidden="true" style="transform: rotate(270deg);" />
</a>
<a class="mx-2 cursor-pointer js-unfold-all" @click="handleExpandLines()">
<span>{{ s__('Diffs|Show all lines') }}</span>
</a>
<a
v-if="canExpandDown"
v-tooltip
class="cursor-pointer js-unfold-down has-tooltip unfold-icon"
data-placement="top"
data-container="body"
:title="__('Expand down')"
@click="handleExpandLines(EXPAND_DOWN)"
>
<!-- TODO: remove style & replace with correct icon, waiting for MR https://gitlab.com/gitlab-org/gitlab-design/issues/499 -->
<icon :size="12" name="expand-left" aria-hidden="true" style="transform: rotate(90deg);" />
</a>
</div>
</td>
</template>
<script> <script>
import createFlash from '~/flash';
import { s__ } from '~/locale';
import { mapState, mapGetters, mapActions } from 'vuex'; import { mapState, mapGetters, mapActions } from 'vuex';
import Icon from '~/vue_shared/components/icon.vue'; import Icon from '~/vue_shared/components/icon.vue';
import DiffGutterAvatars from './diff_gutter_avatars.vue'; import DiffGutterAvatars from './diff_gutter_avatars.vue';
import { LINE_POSITION_RIGHT, UNFOLD_COUNT } from '../constants'; import { LINE_POSITION_RIGHT } from '../constants';
import * as utils from '../store/utils';
export default { export default {
components: { components: {
...@@ -115,89 +112,36 @@ export default { ...@@ -115,89 +112,36 @@ export default {
handleCommentButton() { handleCommentButton() {
this.showCommentForm({ lineCode: this.line.line_code, fileHash: this.fileHash }); this.showCommentForm({ lineCode: this.line.line_code, fileHash: this.fileHash });
}, },
handleLoadMoreLines() {
if (this.isRequesting) {
return;
}
this.isRequesting = true;
const endpoint = this.contextLinesPath;
const oldLineNumber = this.line.meta_data.old_pos || 0;
const newLineNumber = this.line.meta_data.new_pos || 0;
const offset = newLineNumber - oldLineNumber;
const bottom = this.isBottom;
const { fileHash } = this;
const view = this.diffViewType;
let unfold = true;
let lineNumber = newLineNumber - 1;
let since = lineNumber - UNFOLD_COUNT;
let to = lineNumber;
if (bottom) {
lineNumber = newLineNumber + 1;
since = lineNumber;
to = lineNumber + UNFOLD_COUNT;
} else {
const diffFile = utils.findDiffFile(this.diffFiles, this.fileHash);
const indexForInline = utils.findIndexInInlineLines(diffFile.highlighted_diff_lines, {
oldLineNumber,
newLineNumber,
});
const prevLine = diffFile.highlighted_diff_lines[indexForInline - 2];
const prevLineNumber = (prevLine && prevLine.new_line) || 0;
if (since <= prevLineNumber + 1) {
since = prevLineNumber + 1;
unfold = false;
}
}
const params = { since, to, bottom, offset, unfold, view };
const lineNumbers = { oldLineNumber, newLineNumber };
this.loadMoreLines({ endpoint, params, lineNumbers, fileHash })
.then(() => {
this.isRequesting = false;
})
.catch(() => {
createFlash(s__('Diffs|Something went wrong while fetching diff lines.'));
this.isRequesting = false;
});
},
}, },
}; };
</script> </script>
<template> <template>
<div> <div>
<span v-if="isMatchLine" class="context-cell" role="button" @click="handleLoadMoreLines" <button
>...</span v-if="shouldRenderCommentButton"
v-show="shouldShowCommentButton"
type="button"
class="add-diff-note js-add-diff-note-button qa-diff-comment"
title="Add a comment to this line"
@click="handleCommentButton"
>
<icon :size="12" name="comment" />
</button>
<a
v-if="lineNumber"
:data-linenumber="lineNumber"
:href="lineHref"
@click="setHighlightedRow(lineCode)"
> >
<template v-else> </a>
<button <diff-gutter-avatars
v-if="shouldRenderCommentButton" v-if="shouldShowAvatarsOnGutter"
v-show="shouldShowCommentButton" :discussions="line.discussions"
type="button" :discussions-expanded="line.discussionsExpanded"
class="add-diff-note js-add-diff-note-button qa-diff-comment" @toggleLineDiscussions="
title="Add a comment to this line" toggleLineDiscussions({ lineCode, fileHash, expanded: !line.discussionsExpanded })
@click="handleCommentButton" "
> />
<icon :size="12" name="comment" />
</button>
<a
v-if="lineNumber"
:data-linenumber="lineNumber"
:href="lineHref"
@click="setHighlightedRow(lineCode)"
>
</a>
<diff-gutter-avatars
v-if="shouldShowAvatarsOnGutter"
:discussions="line.discussions"
:discussions-expanded="line.discussionsExpanded"
@toggleLineDiscussions="
toggleLineDiscussions({ lineCode, fileHash, expanded: !line.discussionsExpanded })
"
/>
</template>
</div> </div>
</template> </template>
<script>
import Icon from '~/vue_shared/components/icon.vue';
import DiffExpansionCell from './diff_expansion_cell.vue';
import { MATCH_LINE_TYPE } from '../constants';
export default {
components: {
Icon,
DiffExpansionCell,
},
props: {
fileHash: {
type: String,
required: true,
},
contextLinesPath: {
type: String,
required: true,
},
line: {
type: Object,
required: true,
},
isTop: {
type: Boolean,
required: false,
default: false,
},
isBottom: {
type: Boolean,
required: false,
default: false,
},
},
computed: {
isMatchLine() {
return this.line.type === MATCH_LINE_TYPE;
},
},
};
</script>
<template>
<tr v-if="isMatchLine" class="line_expansion match">
<diff-expansion-cell
:file-hash="fileHash"
:context-lines-path="contextLinesPath"
:line="line"
:is-top="isTop"
:is-bottom="isBottom"
/>
</tr>
</template>
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
import { mapActions, mapState } from 'vuex'; import { mapActions, mapState } from 'vuex';
import DiffTableCell from './diff_table_cell.vue'; import DiffTableCell from './diff_table_cell.vue';
import { import {
MATCH_LINE_TYPE,
NEW_LINE_TYPE, NEW_LINE_TYPE,
OLD_LINE_TYPE, OLD_LINE_TYPE,
CONTEXT_LINE_TYPE, CONTEXT_LINE_TYPE,
...@@ -58,6 +59,9 @@ export default { ...@@ -58,6 +59,9 @@ export default {
inlineRowId() { inlineRowId() {
return this.line.line_code || `${this.fileHash}_${this.line.old_line}_${this.line.new_line}`; return this.line.line_code || `${this.fileHash}_${this.line.old_line}_${this.line.new_line}`;
}, },
isMatchLine() {
return this.line.type === MATCH_LINE_TYPE;
},
}, },
created() { created() {
this.newLineType = NEW_LINE_TYPE; this.newLineType = NEW_LINE_TYPE;
...@@ -81,6 +85,7 @@ export default { ...@@ -81,6 +85,7 @@ export default {
<template> <template>
<tr <tr
v-if="!isMatchLine"
:id="inlineRowId" :id="inlineRowId"
:class="classNameMap" :class="classNameMap"
class="line_holder" class="line_holder"
......
...@@ -3,6 +3,7 @@ import { mapGetters } from 'vuex'; ...@@ -3,6 +3,7 @@ import { mapGetters } from 'vuex';
import draftCommentsMixin from 'ee_else_ce/diffs/mixins/draft_comments'; import draftCommentsMixin from 'ee_else_ce/diffs/mixins/draft_comments';
import inlineDiffTableRow from './inline_diff_table_row.vue'; import inlineDiffTableRow from './inline_diff_table_row.vue';
import inlineDiffCommentRow from './inline_diff_comment_row.vue'; import inlineDiffCommentRow from './inline_diff_comment_row.vue';
import inlineDiffExpansionRow from './inline_diff_expansion_row.vue';
export default { export default {
components: { components: {
...@@ -10,6 +11,7 @@ export default { ...@@ -10,6 +11,7 @@ export default {
inlineDiffTableRow, inlineDiffTableRow,
InlineDraftCommentRow: () => InlineDraftCommentRow: () =>
import('ee_component/batch_comments/components/inline_draft_comment_row.vue'), import('ee_component/batch_comments/components/inline_draft_comment_row.vue'),
inlineDiffExpansionRow,
}, },
mixins: [draftCommentsMixin], mixins: [draftCommentsMixin],
props: { props: {
...@@ -43,10 +45,24 @@ export default { ...@@ -43,10 +45,24 @@ export default {
:data-commit-id="commitId" :data-commit-id="commitId"
class="code diff-wrap-lines js-syntax-highlight text-file js-diff-inline-view" class="code diff-wrap-lines js-syntax-highlight text-file js-diff-inline-view"
> >
<!-- Need to insert an empty row to solve "table-layout:fixed" equal width when expansion row is the first line -->
<tr>
<td style="width: 50px;"></td>
<td style="width: 50px;"></td>
<td></td>
</tr>
<tbody> <tbody>
<template v-for="(line, index) in diffLines"> <template v-for="(line, index) in diffLines">
<inline-diff-expansion-row
:key="`expand-${index}`"
:file-hash="diffFile.file_hash"
:context-lines-path="diffFile.context_lines_path"
:line="line"
:is-top="index === 0"
:is-bottom="index + 1 === diffLinesLength"
/>
<inline-diff-table-row <inline-diff-table-row
:key="line.line_code" :key="`${line.line_code || index}`"
:file-hash="diffFile.file_hash" :file-hash="diffFile.file_hash"
:context-lines-path="diffFile.context_lines_path" :context-lines-path="diffFile.context_lines_path"
:line="line" :line="line"
......
<script>
import { MATCH_LINE_TYPE } from '../constants';
import DiffExpansionCell from './diff_expansion_cell.vue';
export default {
components: {
DiffExpansionCell,
},
props: {
fileHash: {
type: String,
required: true,
},
contextLinesPath: {
type: String,
required: true,
},
line: {
type: Object,
required: true,
},
isTop: {
type: Boolean,
required: false,
default: false,
},
isBottom: {
type: Boolean,
required: false,
default: false,
},
},
computed: {
isMatchLineLeft() {
return this.line.left && this.line.left.type === MATCH_LINE_TYPE;
},
isMatchLineRight() {
return this.line.right && this.line.right.type === MATCH_LINE_TYPE;
},
},
};
</script>
<template>
<tr class="line_expansion match">
<template v-if="isMatchLineLeft || isMatchLineRight">
<diff-expansion-cell
:file-hash="fileHash"
:context-lines-path="contextLinesPath"
:line="line.left"
:is-top="isTop"
:is-bottom="isBottom"
:colspan="4"
/>
</template>
</tr>
</template>
...@@ -3,6 +3,7 @@ import { mapActions, mapState } from 'vuex'; ...@@ -3,6 +3,7 @@ import { mapActions, mapState } from 'vuex';
import $ from 'jquery'; import $ from 'jquery';
import DiffTableCell from './diff_table_cell.vue'; import DiffTableCell from './diff_table_cell.vue';
import { import {
MATCH_LINE_TYPE,
NEW_LINE_TYPE, NEW_LINE_TYPE,
OLD_LINE_TYPE, OLD_LINE_TYPE,
CONTEXT_LINE_TYPE, CONTEXT_LINE_TYPE,
...@@ -75,6 +76,12 @@ export default { ...@@ -75,6 +76,12 @@ export default {
}, },
]; ];
}, },
isMatchLineLeft() {
return this.line.left && this.line.left.type === MATCH_LINE_TYPE;
},
isMatchLineRight() {
return this.line.right && this.line.right.type === MATCH_LINE_TYPE;
},
}, },
created() { created() {
this.newLineType = NEW_LINE_TYPE; this.newLineType = NEW_LINE_TYPE;
...@@ -122,7 +129,7 @@ export default { ...@@ -122,7 +129,7 @@ export default {
@mouseover="handleMouseMove" @mouseover="handleMouseMove"
@mouseout="handleMouseMove" @mouseout="handleMouseMove"
> >
<template v-if="line.left"> <template v-if="line.left && !isMatchLineLeft">
<diff-table-cell <diff-table-cell
:file-hash="fileHash" :file-hash="fileHash"
:context-lines-path="contextLinesPath" :context-lines-path="contextLinesPath"
...@@ -148,7 +155,7 @@ export default { ...@@ -148,7 +155,7 @@ export default {
<td class="diff-line-num old_line empty-cell"></td> <td class="diff-line-num old_line empty-cell"></td>
<td class="line_content parallel left-side empty-cell"></td> <td class="line_content parallel left-side empty-cell"></td>
</template> </template>
<template v-if="line.right"> <template v-if="line.right && !isMatchLineRight">
<diff-table-cell <diff-table-cell
:file-hash="fileHash" :file-hash="fileHash"
:context-lines-path="contextLinesPath" :context-lines-path="contextLinesPath"
......
...@@ -3,9 +3,11 @@ import { mapGetters } from 'vuex'; ...@@ -3,9 +3,11 @@ import { mapGetters } from 'vuex';
import draftCommentsMixin from 'ee_else_ce/diffs/mixins/draft_comments'; import draftCommentsMixin from 'ee_else_ce/diffs/mixins/draft_comments';
import parallelDiffTableRow from './parallel_diff_table_row.vue'; import parallelDiffTableRow from './parallel_diff_table_row.vue';
import parallelDiffCommentRow from './parallel_diff_comment_row.vue'; import parallelDiffCommentRow from './parallel_diff_comment_row.vue';
import parallelDiffExpansionRow from './parallel_diff_expansion_row.vue';
export default { export default {
components: { components: {
parallelDiffExpansionRow,
parallelDiffTableRow, parallelDiffTableRow,
parallelDiffCommentRow, parallelDiffCommentRow,
ParallelDraftCommentRow: () => ParallelDraftCommentRow: () =>
...@@ -43,8 +45,23 @@ export default { ...@@ -43,8 +45,23 @@ export default {
:data-commit-id="commitId" :data-commit-id="commitId"
class="code diff-wrap-lines js-syntax-highlight text-file" class="code diff-wrap-lines js-syntax-highlight text-file"
> >
<!-- Need to insert an empty row to solve "table-layout:fixed" equal width when expansion row is the first line -->
<tr>
<td style="width: 50px;"></td>
<td></td>
<td style="width: 50px;"></td>
<td></td>
</tr>
<tbody> <tbody>
<template v-for="(line, index) in diffLines"> <template v-for="(line, index) in diffLines">
<parallel-diff-expansion-row
:key="`expand-${index}`"
:file-hash="diffFile.file_hash"
:context-lines-path="diffFile.context_lines_path"
:line="line"
:is-top="index === 0"
:is-bottom="index + 1 === diffLinesLength"
/>
<parallel-diff-table-row <parallel-diff-table-row
:key="line.line_code" :key="line.line_code"
:file-hash="diffFile.file_hash" :file-hash="diffFile.file_hash"
......
...@@ -183,7 +183,7 @@ export const cancelCommentForm = ({ commit }, { lineCode, fileHash }) => { ...@@ -183,7 +183,7 @@ export const cancelCommentForm = ({ commit }, { lineCode, fileHash }) => {
}; };
export const loadMoreLines = ({ commit }, options) => { export const loadMoreLines = ({ commit }, options) => {
const { endpoint, params, lineNumbers, fileHash } = options; const { endpoint, params, lineNumbers, fileHash, isExpandDown, nextLineNumbers } = options;
params.from_merge_request = true; params.from_merge_request = true;
...@@ -195,6 +195,8 @@ export const loadMoreLines = ({ commit }, options) => { ...@@ -195,6 +195,8 @@ export const loadMoreLines = ({ commit }, options) => {
contextLines, contextLines,
params, params,
fileHash, fileHash,
isExpandDown,
nextLineNumbers,
}); });
}); });
}; };
......
...@@ -71,18 +71,30 @@ export default { ...@@ -71,18 +71,30 @@ export default {
}, },
[types.ADD_CONTEXT_LINES](state, options) { [types.ADD_CONTEXT_LINES](state, options) {
const { lineNumbers, contextLines, fileHash } = options; const { lineNumbers, contextLines, fileHash, isExpandDown, nextLineNumbers } = options;
const { bottom } = options.params; const { bottom } = options.params;
const diffFile = findDiffFile(state.diffFiles, fileHash); const diffFile = findDiffFile(state.diffFiles, fileHash);
removeMatchLine(diffFile, lineNumbers, bottom); removeMatchLine(diffFile, lineNumbers, bottom);
const lines = addLineReferences(contextLines, lineNumbers, bottom).map(line => ({ const lines = addLineReferences(
...line, contextLines,
line_code: line.line_code || `${fileHash}_${line.old_line}_${line.new_line}`, lineNumbers,
discussions: line.discussions || [], bottom,
hasForm: false, isExpandDown,
})); nextLineNumbers,
).map(line => {
const lineCode =
line.type === 'match'
? `${fileHash}_${line.meta_data.old_pos}_${line.meta_data.new_pos}_match`
: line.line_code || `${fileHash}_${line.old_line}_${line.new_line}`;
return {
...line,
line_code: lineCode,
discussions: line.discussions || [],
hasForm: false,
};
});
addContextLines({ addContextLines({
inlineLines: diffFile.highlighted_diff_lines, inlineLines: diffFile.highlighted_diff_lines,
...@@ -90,6 +102,7 @@ export default { ...@@ -90,6 +102,7 @@ export default {
contextLines: lines, contextLines: lines,
bottom, bottom,
lineNumbers, lineNumbers,
isExpandDown,
}); });
}, },
......
...@@ -121,7 +121,7 @@ export function removeMatchLine(diffFile, lineNumbers, bottom) { ...@@ -121,7 +121,7 @@ export function removeMatchLine(diffFile, lineNumbers, bottom) {
diffFile.parallel_diff_lines.splice(indexForParallel + factor, 1); diffFile.parallel_diff_lines.splice(indexForParallel + factor, 1);
} }
export function addLineReferences(lines, lineNumbers, bottom) { export function addLineReferences(lines, lineNumbers, bottom, isExpandDown, nextLineNumbers) {
const { oldLineNumber, newLineNumber } = lineNumbers; const { oldLineNumber, newLineNumber } = lineNumbers;
const lineCount = lines.length; const lineCount = lines.length;
let matchLineIndex = -1; let matchLineIndex = -1;
...@@ -135,15 +135,20 @@ export function addLineReferences(lines, lineNumbers, bottom) { ...@@ -135,15 +135,20 @@ export function addLineReferences(lines, lineNumbers, bottom) {
new_line: bottom ? newLineNumber + index + 1 : newLineNumber + index - lineCount, new_line: bottom ? newLineNumber + index + 1 : newLineNumber + index - lineCount,
}); });
} }
return l; return l;
}); });
if (matchLineIndex > -1) { if (matchLineIndex > -1) {
const line = linesWithNumbers[matchLineIndex]; const line = linesWithNumbers[matchLineIndex];
const targetLine = bottom let targetLine;
? linesWithNumbers[matchLineIndex - 1]
: linesWithNumbers[matchLineIndex + 1]; if (isExpandDown) {
targetLine = nextLineNumbers;
} else if (bottom) {
targetLine = linesWithNumbers[matchLineIndex - 1];
} else {
targetLine = linesWithNumbers[matchLineIndex + 1];
}
Object.assign(line, { Object.assign(line, {
meta_data: { meta_data: {
...@@ -152,26 +157,27 @@ export function addLineReferences(lines, lineNumbers, bottom) { ...@@ -152,26 +157,27 @@ export function addLineReferences(lines, lineNumbers, bottom) {
}, },
}); });
} }
return linesWithNumbers; return linesWithNumbers;
} }
export function addContextLines(options) { export function addContextLines(options) {
const { inlineLines, parallelLines, contextLines, lineNumbers } = options; const { inlineLines, parallelLines, contextLines, lineNumbers, isExpandDown } = options;
const normalizedParallelLines = contextLines.map(line => ({ const normalizedParallelLines = contextLines.map(line => ({
left: line, left: line,
right: line, right: line,
line_code: line.line_code, line_code: line.line_code,
})); }));
const factor = isExpandDown ? 1 : 0;
if (options.bottom) { if (!isExpandDown && options.bottom) {
inlineLines.push(...contextLines); inlineLines.push(...contextLines);
parallelLines.push(...normalizedParallelLines); parallelLines.push(...normalizedParallelLines);
} else { } else {
const inlineIndex = findIndexInInlineLines(inlineLines, lineNumbers); const inlineIndex = findIndexInInlineLines(inlineLines, lineNumbers);
const parallelIndex = findIndexInParallelLines(parallelLines, lineNumbers); const parallelIndex = findIndexInParallelLines(parallelLines, lineNumbers);
inlineLines.splice(inlineIndex, 0, ...contextLines);
parallelLines.splice(parallelIndex, 0, ...normalizedParallelLines); inlineLines.splice(inlineIndex + factor, 0, ...contextLines);
parallelLines.splice(parallelIndex + factor, 0, ...normalizedParallelLines);
} }
} }
......
...@@ -101,6 +101,26 @@ pre.code, ...@@ -101,6 +101,26 @@ pre.code,
color: $white-code-color; color: $white-code-color;
} }
// Expansion line
.line_expansion {
background-color: $gray-light;
td {
border-top: 1px solid $border-color;
border-bottom: 1px solid $border-color;
text-align: center;
}
a {
color: $blue-600;
}
.unfold-icon {
display: inline-block;
padding: 8px 0;
}
}
// Diff line // Diff line
.line_holder { .line_holder {
&.match .line_content, &.match .line_content,
......
...@@ -408,6 +408,14 @@ table.code { ...@@ -408,6 +408,14 @@ table.code {
table-layout: fixed; table-layout: fixed;
border-radius: 0 0 $border-radius-default $border-radius-default; border-radius: 0 0 $border-radius-default $border-radius-default;
tr:first-of-type.line_expansion > td {
border-top: 0;
}
tr:nth-last-of-type(2).line_expansion > td {
border-bottom: 0;
}
tr.line_holder td { tr.line_holder td {
line-height: $code-line-height; line-height: $code-line-height;
font-size: $code-font-size; font-size: $code-font-size;
......
...@@ -9,7 +9,7 @@ module Blobs ...@@ -9,7 +9,7 @@ module Blobs
attribute :full, Boolean, default: false attribute :full, Boolean, default: false
attribute :since, GtOneCoercion attribute :since, GtOneCoercion
attribute :to, GtOneCoercion attribute :to, Integer
attribute :bottom, Boolean attribute :bottom, Boolean
attribute :unfold, Boolean, default: true attribute :unfold, Boolean, default: true
attribute :offset, Integer attribute :offset, Integer
...@@ -24,9 +24,7 @@ module Blobs ...@@ -24,9 +24,7 @@ module Blobs
@all_lines = blob.data.lines @all_lines = blob.data.lines
super(params) super(params)
if full? self.attributes = prepare_attributes
self.attributes = { since: 1, to: @all_lines.size, bottom: false, unfold: false, offset: 0, indent: 0 }
end
end end
# Returns an array of Gitlab::Diff::Line with match line added # Returns an array of Gitlab::Diff::Line with match line added
...@@ -43,7 +41,9 @@ module Blobs ...@@ -43,7 +41,9 @@ module Blobs
end end
def lines def lines
@lines ||= limit(highlight.lines).map(&:html_safe) strong_memoize(:lines) do
limit(highlight.lines).map(&:html_safe)
end
end end
def match_line_text def match_line_text
...@@ -56,10 +56,34 @@ module Blobs ...@@ -56,10 +56,34 @@ module Blobs
private private
def prepare_attributes
return attributes unless full? || to == -1
full_opts = {
since: 1,
to: all_lines_size,
bottom: false,
unfold: false,
offset: 0,
indent: 0
}
return full_opts if full?
full_opts.merge(attributes.slice(:since))
end
def all_lines_size
strong_memoize(:all_lines_size) do
@all_lines.size
end
end
def add_match_line(diff_lines) def add_match_line(diff_lines)
return unless unfold? return unless unfold?
return if bottom? && to >= all_lines_size
if bottom? && to < @all_lines.size if bottom? && to < all_lines_size
old_pos = to - offset old_pos = to - offset
new_pos = to new_pos = to
elsif since != 1 elsif since != 1
...@@ -75,7 +99,9 @@ module Blobs ...@@ -75,7 +99,9 @@ module Blobs
end end
def limited_blob_lines def limited_blob_lines
@limited_blob_lines ||= limit(@all_lines) strong_memoize(:limited_blob_lines) do
limit(@all_lines)
end
end end
def limit(lines) def limit(lines)
......
---
title: Add new expansion options for merge request diffs
merge_request: 30927
author:
type: added
...@@ -3899,6 +3899,9 @@ msgstr "" ...@@ -3899,6 +3899,9 @@ msgstr ""
msgid "Diffs|No file name available" msgid "Diffs|No file name available"
msgstr "" msgstr ""
msgid "Diffs|Show all lines"
msgstr ""
msgid "Diffs|Something went wrong while fetching diff lines." msgid "Diffs|Something went wrong while fetching diff lines."
msgstr "" msgstr ""
...@@ -4649,12 +4652,18 @@ msgstr "" ...@@ -4649,12 +4652,18 @@ msgstr ""
msgid "Expand all" msgid "Expand all"
msgstr "" msgstr ""
msgid "Expand down"
msgstr ""
msgid "Expand dropdown" msgid "Expand dropdown"
msgstr "" msgstr ""
msgid "Expand sidebar" msgid "Expand sidebar"
msgstr "" msgstr ""
msgid "Expand up"
msgstr ""
msgid "Expiration date" msgid "Expiration date"
msgstr "" msgstr ""
......
...@@ -68,12 +68,12 @@ describe 'Merge request > User posts diff notes', :js do ...@@ -68,12 +68,12 @@ describe 'Merge request > User posts diff notes', :js do
context 'with a match line' do context 'with a match line' do
it 'does not allow commenting on the left side' do it 'does not allow commenting on the left side' do
line_holder = find('.match', match: :first).find(:xpath, '..') line_holder = find('.match', match: :first).find(:xpath, '..')
should_not_allow_commenting(line_holder, 'left') match_should_not_allow_commenting(line_holder)
end end
it 'does not allow commenting on the right side' do it 'does not allow commenting on the right side' do
line_holder = find('.match', match: :first).find(:xpath, '..') line_holder = find('.match', match: :first).find(:xpath, '..')
should_not_allow_commenting(line_holder, 'right') match_should_not_allow_commenting(line_holder)
end end
end end
...@@ -136,7 +136,7 @@ describe 'Merge request > User posts diff notes', :js do ...@@ -136,7 +136,7 @@ describe 'Merge request > User posts diff notes', :js do
context 'with a match line' do context 'with a match line' do
it 'does not allow commenting' do it 'does not allow commenting' do
should_not_allow_commenting(find('.match', match: :first)) match_should_not_allow_commenting(find('.match', match: :first))
end end
end end
...@@ -222,7 +222,7 @@ describe 'Merge request > User posts diff notes', :js do ...@@ -222,7 +222,7 @@ describe 'Merge request > User posts diff notes', :js do
context 'with a match line' do context 'with a match line' do
it 'does not allow commenting' do it 'does not allow commenting' do
should_not_allow_commenting(find('.match', match: :first)) match_should_not_allow_commenting(find('.match', match: :first))
end end
end end
end end
...@@ -251,6 +251,10 @@ describe 'Merge request > User posts diff notes', :js do ...@@ -251,6 +251,10 @@ describe 'Merge request > User posts diff notes', :js do
expect(line[:num]).not_to have_css comment_button_class expect(line[:num]).not_to have_css comment_button_class
end end
def match_should_not_allow_commenting(line_holder)
expect(line_holder).not_to have_css comment_button_class
end
def write_comment_on_line(line_holder, diff_side) def write_comment_on_line(line_holder, diff_side)
click_diff_line(line_holder, diff_side) click_diff_line(line_holder, diff_side)
......
...@@ -17,11 +17,25 @@ describe 'User views diffs', :js do ...@@ -17,11 +17,25 @@ describe 'User views diffs', :js do
end end
shared_examples 'unfold diffs' do shared_examples 'unfold diffs' do
it 'unfolds diffs' do it 'unfolds diffs upwards' do
first('.js-unfold').click first('.js-unfold').click
expect(find('.file-holder[id="a5cc2925ca8258af241be7e5b0381edf30266302"] .text-file')).to have_content('.bundle') expect(find('.file-holder[id="a5cc2925ca8258af241be7e5b0381edf30266302"] .text-file')).to have_content('.bundle')
end end
it 'unfolds diffs to the start' do
first('.js-unfold-all').click
expect(find('.file-holder[id="a5cc2925ca8258af241be7e5b0381edf30266302"] .text-file')).to have_content('.rbc')
end
it 'unfolds diffs downwards' do
first('.js-unfold-down').click
expect(find('.file-holder[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd"] .text-file')).to have_content('.popen3')
end
it 'unfolds diffs to the end' do
page.all('.js-unfold-down').last
expect(find('.file-holder[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9"] .text-file')).to have_content('end')
end
end end
it 'shows diffs' do it 'shows diffs' do
......
import Vue from 'vue';
import store from '~/mr_notes/stores';
import DiffExpansionCell from '~/diffs/components/diff_expansion_cell.vue';
import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import diffFileMockData from '../mock_data/diff_file';
const EXPAND_UP_CLASS = '.js-unfold';
const EXPAND_DOWN_CLASS = '.js-unfold-down';
const EXPAND_ALL_CLASS = '.js-unfold-all';
describe('DiffExpansionCell', () => {
const matchLine = diffFileMockData.highlighted_diff_lines[5];
const createComponent = (options = {}) => {
const cmp = Vue.extend(DiffExpansionCell);
const defaults = {
fileHash: diffFileMockData.file_hash,
contextLinesPath: 'contextLinesPath',
line: matchLine,
isTop: false,
isBottom: false,
};
const props = Object.assign({}, defaults, options);
return createComponentWithStore(cmp, store, props).$mount();
};
describe('top row', () => {
it('should have "expand up" and "show all" option', () => {
const vm = createComponent({
isTop: true,
});
const el = vm.$el;
expect(el.querySelector(EXPAND_UP_CLASS)).not.toBe(null);
expect(el.querySelector(EXPAND_DOWN_CLASS)).toBe(null);
expect(el.querySelector(EXPAND_ALL_CLASS)).not.toBe(null);
});
});
describe('middle row', () => {
it('should have "expand down", "show all", "expand up" option', () => {
const vm = createComponent();
const el = vm.$el;
expect(el.querySelector(EXPAND_UP_CLASS)).not.toBe(null);
expect(el.querySelector(EXPAND_DOWN_CLASS)).not.toBe(null);
expect(el.querySelector(EXPAND_ALL_CLASS)).not.toBe(null);
});
});
describe('bottom row', () => {
it('should have "expand down" and "show all" option', () => {
const vm = createComponent({
isBottom: true,
});
const el = vm.$el;
expect(el.querySelector(EXPAND_UP_CLASS)).toBe(null);
expect(el.querySelector(EXPAND_DOWN_CLASS)).not.toBe(null);
expect(el.querySelector(EXPAND_ALL_CLASS)).not.toBe(null);
});
});
});
...@@ -70,15 +70,6 @@ describe('DiffLineGutterContent', () => { ...@@ -70,15 +70,6 @@ describe('DiffLineGutterContent', () => {
}); });
describe('template', () => { describe('template', () => {
it('should render three dots for context lines', () => {
const component = createComponent({
isMatchLine: true,
});
expect(component.$el.querySelector('span').classList.contains('context-cell')).toEqual(true);
expect(component.$el.innerText).toEqual('...');
});
it('should render comment button', () => { it('should render comment button', () => {
const component = createComponent({ const component = createComponent({
showCommentButton: true, showCommentButton: true,
......
import Vue from 'vue';
import store from '~/mr_notes/stores';
import InlineDiffExpansionRow from '~/diffs/components/inline_diff_expansion_row.vue';
import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import diffFileMockData from '../mock_data/diff_file';
describe('InlineDiffExpansionRow', () => {
const matchLine = diffFileMockData.highlighted_diff_lines[5];
const createComponent = (options = {}) => {
const cmp = Vue.extend(InlineDiffExpansionRow);
const defaults = {
fileHash: diffFileMockData.file_hash,
contextLinesPath: 'contextLinesPath',
line: matchLine,
isTop: false,
isBottom: false,
};
const props = Object.assign({}, defaults, options);
return createComponentWithStore(cmp, store, props).$mount();
};
describe('template', () => {
it('should render expansion row for match lines', () => {
const vm = createComponent();
expect(vm.$el.classList.contains('line_expansion')).toBe(true);
});
});
});
...@@ -28,9 +28,9 @@ describe('InlineDiffView', () => { ...@@ -28,9 +28,9 @@ describe('InlineDiffView', () => {
it('should have rendered diff lines', () => { it('should have rendered diff lines', () => {
const el = component.$el; const el = component.$el;
expect(el.querySelectorAll('tr.line_holder').length).toEqual(6); expect(el.querySelectorAll('tr.line_holder').length).toEqual(5);
expect(el.querySelectorAll('tr.line_holder.new').length).toEqual(2); expect(el.querySelectorAll('tr.line_holder.new').length).toEqual(2);
expect(el.querySelectorAll('tr.line_holder.match').length).toEqual(1); expect(el.querySelectorAll('tr.line_expansion.match').length).toEqual(1);
expect(el.textContent.indexOf('Bad dates')).toBeGreaterThan(-1); expect(el.textContent.indexOf('Bad dates')).toBeGreaterThan(-1);
}); });
......
import Vue from 'vue';
import store from '~/mr_notes/stores';
import ParallelDiffExpansionRow from '~/diffs/components/parallel_diff_expansion_row.vue';
import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import diffFileMockData from '../mock_data/diff_file';
describe('ParallelDiffExpansionRow', () => {
const matchLine = diffFileMockData.highlighted_diff_lines[5];
const createComponent = (options = {}) => {
const cmp = Vue.extend(ParallelDiffExpansionRow);
const defaults = {
fileHash: diffFileMockData.file_hash,
contextLinesPath: 'contextLinesPath',
line: matchLine,
isTop: false,
isBottom: false,
};
const props = Object.assign({}, defaults, options);
return createComponentWithStore(cmp, store, props).$mount();
};
describe('template', () => {
it('should render expansion row for match lines', () => {
const vm = createComponent();
expect(vm.$el.classList.contains('line_expansion')).toBe(true);
});
});
});
...@@ -380,7 +380,9 @@ describe('DiffsStoreActions', () => { ...@@ -380,7 +380,9 @@ describe('DiffsStoreActions', () => {
const params = { since: 6, to: 26 }; const params = { since: 6, to: 26 };
const lineNumbers = { oldLineNumber: 3, newLineNumber: 5 }; const lineNumbers = { oldLineNumber: 3, newLineNumber: 5 };
const fileHash = 'ff9200'; const fileHash = 'ff9200';
const options = { endpoint, params, lineNumbers, fileHash }; const isExpandDown = false;
const nextLineNumbers = {};
const options = { endpoint, params, lineNumbers, fileHash, isExpandDown, nextLineNumbers };
const mock = new MockAdapter(axios); const mock = new MockAdapter(axios);
const contextLines = { contextLines: [{ lineCode: 6 }] }; const contextLines = { contextLines: [{ lineCode: 6 }] };
mock.onGet(endpoint).reply(200, contextLines); mock.onGet(endpoint).reply(200, contextLines);
...@@ -392,7 +394,7 @@ describe('DiffsStoreActions', () => { ...@@ -392,7 +394,7 @@ describe('DiffsStoreActions', () => {
[ [
{ {
type: types.ADD_CONTEXT_LINES, type: types.ADD_CONTEXT_LINES,
payload: { lineNumbers, contextLines, params, fileHash }, payload: { lineNumbers, contextLines, params, fileHash, isExpandDown, nextLineNumbers },
}, },
], ],
[], [],
......
...@@ -81,6 +81,8 @@ describe('DiffsStoreMutations', () => { ...@@ -81,6 +81,8 @@ describe('DiffsStoreMutations', () => {
params: { params: {
bottom: true, bottom: true,
}, },
isExpandDown: false,
nextLineNumbers: {},
}; };
const diffFile = { const diffFile = {
file_hash: options.fileHash, file_hash: options.fileHash,
...@@ -108,6 +110,8 @@ describe('DiffsStoreMutations', () => { ...@@ -108,6 +110,8 @@ describe('DiffsStoreMutations', () => {
options.contextLines, options.contextLines,
options.lineNumbers, options.lineNumbers,
options.params.bottom, options.params.bottom,
options.isExpandDown,
options.nextLineNumbers,
); );
expect(addContextLinesSpy).toHaveBeenCalledWith({ expect(addContextLinesSpy).toHaveBeenCalledWith({
...@@ -116,6 +120,7 @@ describe('DiffsStoreMutations', () => { ...@@ -116,6 +120,7 @@ describe('DiffsStoreMutations', () => {
contextLines: options.contextLines, contextLines: options.contextLines,
bottom: options.params.bottom, bottom: options.params.bottom,
lineNumbers: options.lineNumbers, lineNumbers: options.lineNumbers,
isExpandDown: false,
}); });
}); });
}); });
......
...@@ -260,6 +260,17 @@ describe('DiffsStoreUtils', () => { ...@@ -260,6 +260,17 @@ describe('DiffsStoreUtils', () => {
expect(linesWithReferences[1].meta_data.old_pos).toEqual(2); expect(linesWithReferences[1].meta_data.old_pos).toEqual(2);
expect(linesWithReferences[1].meta_data.new_pos).toEqual(3); expect(linesWithReferences[1].meta_data.new_pos).toEqual(3);
}); });
it('should add correct line references when isExpandDown is true', () => {
const lines = [{ type: null }, { type: MATCH_LINE_TYPE }];
const linesWithReferences = utils.addLineReferences(lines, lineNumbers, false, true, {
old_line: 10,
new_line: 11,
});
expect(linesWithReferences[1].meta_data.old_pos).toEqual(10);
expect(linesWithReferences[1].meta_data.new_pos).toEqual(11);
});
}); });
describe('trimFirstCharOfLineContent', () => { describe('trimFirstCharOfLineContent', () => {
......
...@@ -39,6 +39,21 @@ describe Blobs::UnfoldPresenter do ...@@ -39,6 +39,21 @@ describe Blobs::UnfoldPresenter do
expect(result.indent).to eq(0) expect(result.indent).to eq(0)
end end
end end
context 'when to is -1' do
let(:params) { { full: false, since: 2, to: -1, bottom: true, offset: 1, indent: 1 } }
it 'sets other attributes' do
result = subject
expect(result.full?).to eq(false)
expect(result.since).to eq(2)
expect(result.to).to eq(blob.lines.size)
expect(result.bottom).to eq(false)
expect(result.offset).to eq(0)
expect(result.indent).to eq(0)
end
end
end end
describe '#diff_lines' do describe '#diff_lines' do
...@@ -83,8 +98,9 @@ describe Blobs::UnfoldPresenter do ...@@ -83,8 +98,9 @@ describe Blobs::UnfoldPresenter do
end end
end end
context 'when since is greater than 1' do context 'when "since" is greater than 1' do
let(:params) { { since: 5, to: 10, offset: 10 } } let(:default_params) { { since: 5, to: 10, offset: 10 } }
let(:params) { default_params }
it 'adds top match line' do it 'adds top match line' do
line = subject.diff_lines.first line = subject.diff_lines.first
...@@ -93,6 +109,38 @@ describe Blobs::UnfoldPresenter do ...@@ -93,6 +109,38 @@ describe Blobs::UnfoldPresenter do
expect(line.old_pos).to eq(5) expect(line.old_pos).to eq(5)
expect(line.new_pos).to eq(5) expect(line.new_pos).to eq(5)
end end
context '"to" is higher than blob size' do
let(:params) { default_params.merge(to: total_lines + 10, bottom: true) }
it 'does not add bottom match line' do
line = subject.diff_lines.last
expect(line.type).to be_nil
end
end
context '"to" is equal to blob size' do
let(:params) { default_params.merge(to: total_lines, bottom: true) }
it 'does not add bottom match line' do
line = subject.diff_lines.last
expect(line.type).to be_nil
end
end
context '"to" is less than blob size' do
let(:params) { default_params.merge(to: total_lines - 3, bottom: true) }
it 'adds bottom match line' do
line = subject.diff_lines.last
expect(line.type).to eq('match')
expect(line.old_pos).to eq(total_lines - 3 - params[:offset])
expect(line.new_pos).to eq(total_lines - 3)
end
end
end end
context 'when "to" is less than blob size' do context 'when "to" is less than blob size' do
...@@ -116,6 +164,22 @@ describe Blobs::UnfoldPresenter do ...@@ -116,6 +164,22 @@ describe Blobs::UnfoldPresenter do
expect(line.type).to be_nil expect(line.type).to be_nil
end end
end end
context 'when "to" is "-1"' do
let(:params) { { since: 10, to: -1, offset: 10, bottom: true } }
it 'does not add bottom match line' do
line = subject.diff_lines.last
expect(line.type).to be_nil
end
it 'last line is the latest blob line' do
line = subject.diff_lines.last
expect(line.text).to eq(total_lines.to_s)
end
end
end end
describe '#lines' do describe '#lines' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment