Commit 15ee1a41 authored by Kushal Pandya's avatar Kushal Pandya

Merge branch 'ph/removeUnifiedDiffComponentsFeatureFlag' into 'master'

Removes unified diff components feature flag [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!63744
parents e710f974 b964f4f5
<script>
import { GlLoadingIcon } from '@gitlab/ui';
import { mapActions, mapGetters, mapState } from 'vuex';
import { mapInline, mapParallel } from 'ee_else_ce/diffs/components/diff_row_utils';
import { mapParallel } from 'ee_else_ce/diffs/components/diff_row_utils';
import DiffFileDrafts from '~/batch_comments/components/diff_file_drafts.vue';
import draftCommentsMixin from '~/diffs/mixins/draft_comments';
import { diffViewerModes } from '~/ide/constants';
......@@ -9,7 +9,6 @@ import diffLineNoteFormMixin from '~/notes/mixins/diff_line_note_form';
import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue';
import NoPreviewViewer from '~/vue_shared/components/diff_viewer/viewers/no_preview.vue';
import NotDiffableViewer from '~/vue_shared/components/diff_viewer/viewers/not_diffable.vue';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import NoteForm from '../../notes/components/note_form.vue';
import eventHub from '../../notes/event_hub';
import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue';
......@@ -18,14 +17,10 @@ import { getDiffMode } from '../store/utils';
import DiffDiscussions from './diff_discussions.vue';
import DiffView from './diff_view.vue';
import ImageDiffOverlay from './image_diff_overlay.vue';
import InlineDiffView from './inline_diff_view.vue';
import ParallelDiffView from './parallel_diff_view.vue';
export default {
components: {
GlLoadingIcon,
InlineDiffView,
ParallelDiffView,
DiffView,
DiffViewer,
NoteForm,
......@@ -36,7 +31,7 @@ export default {
userAvatarLink,
DiffFileDrafts,
},
mixins: [diffLineNoteFormMixin, draftCommentsMixin, glFeatureFlagsMixin()],
mixins: [diffLineNoteFormMixin, draftCommentsMixin],
props: {
diffFile: {
type: Object,
......@@ -52,7 +47,6 @@ export default {
...mapState('diffs', ['projectPath']),
...mapGetters('diffs', [
'isInlineView',
'isParallelView',
'getCommentFormForDiffFile',
'diffLines',
'fileLineCodequality',
......@@ -86,15 +80,8 @@ export default {
return this.getUserData;
},
mappedLines() {
if (this.glFeatures.unifiedDiffComponents) {
return this.diffLines(this.diffFile, true).map(mapParallel(this)) || [];
}
// TODO: Everything below this line can be deleted when unifiedDiffComponents FF is removed
if (this.isInlineView) {
return this.diffFile.highlighted_diff_lines.map(mapInline(this));
}
return this.diffLines(this.diffFile).map(mapParallel(this));
// TODO: Do this data generation when we recieve a response to save a computed property being created
return this.diffLines(this.diffFile).map(mapParallel(this)) || [];
},
},
updated() {
......@@ -126,7 +113,7 @@ export default {
<template>
<div class="diff-content">
<div class="diff-viewer">
<template v-if="isTextFile && glFeatures.unifiedDiffComponents">
<template v-if="isTextFile">
<diff-view
:diff-file="diffFile"
:diff-lines="mappedLines"
......@@ -135,21 +122,6 @@ export default {
/>
<gl-loading-icon v-if="diffFile.renderingLines" size="md" class="mt-3" />
</template>
<template v-else-if="isTextFile">
<inline-diff-view
v-if="isInlineView"
:diff-file="diffFile"
:diff-lines="mappedLines"
:help-page-path="helpPagePath"
/>
<parallel-diff-view
v-else-if="isParallelView"
:diff-file="diffFile"
:diff-lines="mappedLines"
:help-page-path="helpPagePath"
/>
<gl-loading-icon v-if="diffFile.renderingLines" size="md" class="mt-3" />
</template>
<not-diffable-viewer v-else-if="notDiffable" />
<no-preview-viewer v-else-if="noPreview" />
<diff-viewer
......
......@@ -106,10 +106,7 @@ export default {
};
const getDiffLines = () => {
if (this.diffViewType === PARALLEL_DIFF_VIEW_TYPE) {
return this.diffLines(this.diffFile, this.glFeatures.unifiedDiffComponents).reduce(
combineSides,
[],
);
return this.diffLines(this.diffFile).reduce(combineSides, []);
}
return this.diffFile[INLINE_DIFF_LINES_KEY];
......
......@@ -210,6 +210,7 @@ export default {
<template>
<div :class="classNameMap" class="diff-grid-row diff-tr line_holder">
<div
:id="line.left && line.left.line_code"
data-testid="left-side"
class="diff-grid-left left-side"
v-bind="interopLeftAttributes"
......@@ -293,7 +294,6 @@ export default {
/>
</div>
<div
:id="line.left.line_code"
:key="line.left.line_code"
:class="[parallelViewLeftLineType, { parallel: !inline }]"
class="diff-td line_content with-coverage left-side"
......@@ -334,6 +334,7 @@ export default {
</div>
<div
v-if="!inline"
:id="line.right && line.right.line_code"
data-testid="right-side"
class="diff-grid-right right-side"
v-bind="interopRightAttributes"
......@@ -409,7 +410,6 @@ export default {
/>
</div>
<div
:id="line.right.line_code"
:key="line.right.rich_text"
:class="[
line.right.type,
......
......@@ -139,24 +139,3 @@ export const mapParallel = (content) => (line) => {
commentRowClasses: hasDiscussions(left) || hasDiscussions(right) ? '' : 'js-temp-notes-holder',
};
};
// TODO: Delete this function when unifiedDiffComponents FF is removed
export const mapInline = (content) => (line) => {
// Discussions/Comments
const renderCommentRow = line.hasForm || (line.discussions?.length && line.discussionsExpanded);
return {
...line,
renderDiscussion: Boolean(line.discussions?.length),
isMatchLine: isMatchLine(line.type),
commentRowClasses: line.discussions?.length ? '' : 'js-temp-notes-holder',
renderCommentRow,
hasDraft: content.shouldRenderDraftRow(content.diffFile.file_hash, line),
hasCommentForm: line.hasForm,
isMetaLine: isMetaLine(line.type),
isContextLine: isContextLine(line.type),
hasDiscussions: hasDiscussions(line),
lineHref: lineHref(line),
lineCode: lineCode(line),
};
};
<script>
import { GlTooltipDirective, GlIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui';
import { mapActions, mapGetters, mapState } from 'vuex';
import { CONTEXT_LINE_CLASS_NAME } from '../constants';
import { getInteropInlineAttributes } from '../utils/interoperability';
import DiffGutterAvatars from './diff_gutter_avatars.vue';
import {
isHighlighted,
shouldShowCommentButton,
shouldRenderCommentButton,
classNameMapCell,
addCommentTooltip,
} from './diff_row_utils';
export default {
components: {
DiffGutterAvatars,
GlIcon,
},
directives: {
GlTooltip: GlTooltipDirective,
SafeHtml,
},
props: {
fileHash: {
type: String,
required: true,
},
filePath: {
type: String,
required: true,
},
line: {
type: Object,
required: true,
},
isBottom: {
type: Boolean,
required: false,
default: false,
},
isCommented: {
type: Boolean,
required: false,
default: false,
},
},
data() {
return {
isHover: false,
};
},
computed: {
...mapGetters(['isLoggedIn']),
...mapGetters('diffs', ['fileLineCoverage']),
...mapState({
isHighlighted(state) {
return isHighlighted(state, this.line, this.isCommented);
},
}),
classNameMap() {
return [
this.line.type,
{
[CONTEXT_LINE_CLASS_NAME]: this.line.isContextLine,
},
];
},
inlineRowId() {
return this.line.line_code || `${this.fileHash}_${this.line.old_line}_${this.line.new_line}`;
},
coverageState() {
return this.fileLineCoverage(this.filePath, this.line.new_line);
},
classNameMapCell() {
return classNameMapCell({
line: this.line,
hll: this.isHighlighted,
isLoggedIn: this.isLoggedIn,
isHover: this.isHover,
});
},
addCommentTooltip() {
return addCommentTooltip(this.line);
},
shouldRenderCommentButton() {
return shouldRenderCommentButton(this.isLoggedIn, true);
},
shouldShowCommentButton() {
return shouldShowCommentButton(
this.isHover,
this.line.isContextLine,
this.line.isMetaLine,
this.line.hasDiscussions,
);
},
shouldShowAvatarsOnGutter() {
return this.line.hasDiscussions;
},
interopAttrs() {
return getInteropInlineAttributes(this.line);
},
},
mounted() {
this.scrollToLineIfNeededInline(this.line);
},
methods: {
...mapActions('diffs', [
'scrollToLineIfNeededInline',
'showCommentForm',
'setHighlightedRow',
'toggleLineDiscussions',
]),
handleMouseMove(e) {
// To show the comment icon on the gutter we need to know if we hover the line.
// Current table structure doesn't allow us to do this with CSS in both of the diff view types
this.isHover = e.type === 'mouseover';
},
handleCommentButton() {
this.showCommentForm({ lineCode: this.line.line_code, fileHash: this.fileHash });
},
},
};
</script>
<template>
<tr
:id="inlineRowId"
:class="classNameMap"
class="line_holder"
v-bind="interopAttrs"
@mouseover="handleMouseMove"
@mouseout="handleMouseMove"
>
<td ref="oldTd" class="diff-line-num old_line" :class="classNameMapCell">
<span
v-if="shouldRenderCommentButton"
ref="addNoteTooltip"
v-gl-tooltip
class="add-diff-note tooltip-wrapper"
:title="addCommentTooltip"
>
<button
v-show="shouldShowCommentButton"
ref="addDiffNoteButton"
type="button"
class="add-diff-note note-button js-add-diff-note-button"
:disabled="line.commentsDisabled"
:aria-label="addCommentTooltip"
@click="handleCommentButton"
>
<gl-icon :size="12" name="comment" />
</button>
</span>
<a
v-if="line.old_line"
ref="lineNumberRefOld"
:data-linenumber="line.old_line"
:href="line.lineHref"
@click="setHighlightedRow(line.lineCode)"
>
</a>
<diff-gutter-avatars
v-if="shouldShowAvatarsOnGutter"
:discussions="line.discussions"
:discussions-expanded="line.discussionsExpanded"
@toggleLineDiscussions="
toggleLineDiscussions({
lineCode: line.lineCode,
fileHash,
expanded: !line.discussionsExpanded,
})
"
/>
</td>
<td ref="newTd" class="diff-line-num new_line" :class="classNameMapCell">
<a
v-if="line.new_line"
ref="lineNumberRefNew"
:data-linenumber="line.new_line"
:href="line.lineHref"
@click="setHighlightedRow(line.lineCode)"
>
</a>
</td>
<td
v-gl-tooltip.hover
:title="coverageState.text"
:class="[line.type, coverageState.class, { hll: isHighlighted }]"
class="line-coverage"
></td>
<td
:key="line.line_code"
v-safe-html="line.rich_text"
:class="[
line.type,
{
hll: isHighlighted,
},
]"
class="line_content with-coverage"
></td>
</tr>
</template>
<script>
import { mapGetters, mapState } from 'vuex';
import DraftNote from '~/batch_comments/components/draft_note.vue';
import draftCommentsMixin from '~/diffs/mixins/draft_comments';
import { getCommentedLines } from '~/notes/components/multiline_comment_utils';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import DiffCommentCell from './diff_comment_cell.vue';
import DiffExpansionCell from './diff_expansion_cell.vue';
import inlineDiffTableRow from './inline_diff_table_row.vue';
export default {
components: {
DiffCommentCell,
inlineDiffTableRow,
DraftNote,
DiffExpansionCell,
},
mixins: [draftCommentsMixin, glFeatureFlagsMixin()],
props: {
diffFile: {
type: Object,
required: true,
},
diffLines: {
type: Array,
required: true,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
},
computed: {
...mapGetters('diffs', ['commitId']),
...mapState({
selectedCommentPosition: ({ notes }) => notes.selectedCommentPosition,
selectedCommentPositionHover: ({ notes }) => notes.selectedCommentPositionHover,
}),
diffLinesLength() {
return this.diffLines.length;
},
commentedLines() {
return getCommentedLines(
this.selectedCommentPosition || this.selectedCommentPositionHover,
this.diffLines,
);
},
},
userColorScheme: window.gon.user_color_scheme,
};
</script>
<template>
<table
:class="$options.userColorScheme"
:data-commit-id="commitId"
class="code diff-wrap-lines js-syntax-highlight text-file js-diff-inline-view"
>
<colgroup>
<col style="width: 50px" />
<col style="width: 50px" />
<col style="width: 8px" />
<col />
</colgroup>
<tbody>
<template v-for="(line, index) in diffLines">
<tr v-if="line.isMatchLine" :key="`expand-${index}`" class="line_expansion match">
<td colspan="4" class="text-center gl-font-regular">
<diff-expansion-cell
:file-hash="diffFile.file_hash"
:context-lines-path="diffFile.context_lines_path"
:line="line"
:is-top="index === 0"
:is-bottom="index + 1 === diffLinesLength"
/>
</td>
</tr>
<inline-diff-table-row
v-if="!line.isMatchLine"
:key="`${line.line_code || index}`"
:file-hash="diffFile.file_hash"
:file-path="diffFile.file_path"
:line="line"
:is-bottom="index + 1 === diffLinesLength"
:is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine"
/>
<tr
v-if="line.renderCommentRow"
:key="`icr-${line.line_code || index}`"
:class="line.commentRowClasses"
class="notes_holder"
>
<td class="notes-content" colspan="4">
<diff-comment-cell
:diff-file-hash="diffFile.file_hash"
:line="line"
:help-page-path="helpPagePath"
:has-draft="line.hasDraft"
/>
</td>
</tr>
<tr v-if="line.hasDraft" :key="`draft_${index}`" class="notes_holder js-temp-notes-holder">
<td class="notes-content" colspan="4">
<div class="content">
<draft-note
:draft="draftForLine(diffFile.file_hash, line)"
:diff-file="diffFile"
:line="line"
/>
</div>
</td>
</tr>
</template>
</tbody>
</table>
</template>
<script>
import { GlTooltipDirective, GlIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui';
import $ from 'jquery';
import { mapActions, mapGetters, mapState } from 'vuex';
import { CONTEXT_LINE_CLASS_NAME, PARALLEL_DIFF_VIEW_TYPE } from '../constants';
import {
getInteropOldSideAttributes,
getInteropNewSideAttributes,
} from '../utils/interoperability';
import DiffGutterAvatars from './diff_gutter_avatars.vue';
import * as utils from './diff_row_utils';
export default {
components: {
GlIcon,
DiffGutterAvatars,
},
directives: {
GlTooltip: GlTooltipDirective,
SafeHtml,
},
props: {
fileHash: {
type: String,
required: true,
},
filePath: {
type: String,
required: true,
},
line: {
type: Object,
required: true,
},
isBottom: {
type: Boolean,
required: false,
default: false,
},
isCommented: {
type: Boolean,
required: false,
default: false,
},
},
data() {
return {
isLeftHover: false,
isRightHover: false,
isCommentButtonRendered: false,
};
},
computed: {
...mapGetters('diffs', ['fileLineCoverage']),
...mapGetters(['isLoggedIn']),
...mapState({
isHighlighted(state) {
const line = this.line.left?.line_code ? this.line.left : this.line.right;
return utils.isHighlighted(state, line, this.isCommented);
},
}),
classNameMap() {
return {
[CONTEXT_LINE_CLASS_NAME]: this.line.isContextLineLeft,
[PARALLEL_DIFF_VIEW_TYPE]: true,
};
},
parallelViewLeftLineType() {
return utils.parallelViewLeftLineType(this.line, this.isHighlighted);
},
coverageState() {
return this.fileLineCoverage(this.filePath, this.line.right.new_line);
},
classNameMapCellLeft() {
return utils.classNameMapCell({
line: this.line.left,
hll: this.isHighlighted,
isLoggedIn: this.isLoggedIn,
isHover: this.isLeftHover,
});
},
classNameMapCellRight() {
return utils.classNameMapCell({
line: this.line.right,
hll: this.isHighlighted,
isLoggedIn: this.isLoggedIn,
isHover: this.isRightHover,
});
},
addCommentTooltipLeft() {
return utils.addCommentTooltip(this.line.left);
},
addCommentTooltipRight() {
return utils.addCommentTooltip(this.line.right);
},
shouldRenderCommentButton() {
return utils.shouldRenderCommentButton(this.isLoggedIn, this.isCommentButtonRendered);
},
shouldShowCommentButtonLeft() {
return utils.shouldShowCommentButton(
this.isLeftHover,
this.line.isContextLineLeft,
this.line.isMetaLineLeft,
this.line.hasDiscussionsLeft,
);
},
shouldShowCommentButtonRight() {
return utils.shouldShowCommentButton(
this.isRightHover,
this.line.isContextLineRight,
this.line.isMetaLineRight,
this.line.hasDiscussionsRight,
);
},
interopLeftAttributes() {
return getInteropOldSideAttributes(this.line.left);
},
interopRightAttributes() {
return getInteropNewSideAttributes(this.line.right);
},
},
mounted() {
this.scrollToLineIfNeededParallel(this.line);
this.unwatchShouldShowCommentButton = this.$watch(
(vm) => [vm.shouldShowCommentButtonLeft, vm.shouldShowCommentButtonRight].join(),
(newVal) => {
if (newVal) {
this.isCommentButtonRendered = true;
this.unwatchShouldShowCommentButton();
}
},
);
},
beforeDestroy() {
this.unwatchShouldShowCommentButton();
},
methods: {
...mapActions('diffs', [
'scrollToLineIfNeededParallel',
'showCommentForm',
'setHighlightedRow',
'toggleLineDiscussions',
]),
handleMouseMove(e) {
const isHover = e.type === 'mouseover';
const hoveringCell = e.target.closest('td');
const allCellsInHoveringRow = Array.from(e.currentTarget.children);
const hoverIndex = allCellsInHoveringRow.indexOf(hoveringCell);
if (hoverIndex >= 3) {
this.isRightHover = isHover;
} else {
this.isLeftHover = isHover;
}
},
// Prevent text selecting on both sides of parallel diff view
// Backport of the same code from legacy diff notes.
handleParallelLineMouseDown(e) {
const line = $(e.currentTarget);
const table = line.closest('table');
table.removeClass('left-side-selected right-side-selected');
const [lineClass] = ['left-side', 'right-side'].filter((name) => line.hasClass(name));
if (lineClass) {
table.addClass(`${lineClass}-selected`);
}
},
handleCommentButton(line) {
this.showCommentForm({ lineCode: line.line_code, fileHash: this.fileHash });
},
},
};
</script>
<template>
<tr
:class="classNameMap"
class="line_holder"
@mouseover="handleMouseMove"
@mouseout="handleMouseMove"
>
<template v-if="line.left && !line.isMatchLineLeft">
<td ref="oldTd" :class="classNameMapCellLeft" class="diff-line-num old_line">
<span
v-if="shouldRenderCommentButton"
ref="addNoteTooltipLeft"
v-gl-tooltip
class="add-diff-note tooltip-wrapper"
:title="addCommentTooltipLeft"
>
<button
v-show="shouldShowCommentButtonLeft"
ref="addDiffNoteButtonLeft"
type="button"
class="add-diff-note note-button js-add-diff-note-button"
:disabled="line.left.commentsDisabled"
:aria-label="addCommentTooltipLeft"
@click="handleCommentButton(line.left)"
>
<gl-icon :size="12" name="comment" />
</button>
</span>
<a
v-if="line.left.old_line"
ref="lineNumberRefOld"
:data-linenumber="line.left.old_line"
:href="line.lineHrefOld"
@click="setHighlightedRow(line.lineCode)"
>
</a>
<diff-gutter-avatars
v-if="line.hasDiscussionsLeft"
:discussions="line.left.discussions"
:discussions-expanded="line.left.discussionsExpanded"
@toggleLineDiscussions="
toggleLineDiscussions({
lineCode: line.left.line_code,
fileHash,
expanded: !line.left.discussionsExpanded,
})
"
/>
</td>
<td :class="parallelViewLeftLineType" class="line-coverage left-side"></td>
<td
:id="line.left.line_code"
:key="line.left.line_code"
v-safe-html="line.left.rich_text"
:class="parallelViewLeftLineType"
v-bind="interopLeftAttributes"
class="line_content with-coverage parallel left-side"
@mousedown="handleParallelLineMouseDown"
></td>
</template>
<template v-else>
<td class="diff-line-num old_line empty-cell"></td>
<td class="line-coverage left-side empty-cell"></td>
<td class="line_content with-coverage parallel left-side empty-cell"></td>
</template>
<template v-if="line.right && !line.isMatchLineRight">
<td ref="newTd" :class="classNameMapCellRight" class="diff-line-num new_line">
<span
v-if="shouldRenderCommentButton"
ref="addNoteTooltipRight"
v-gl-tooltip
class="add-diff-note tooltip-wrapper"
:title="addCommentTooltipRight"
>
<button
v-show="shouldShowCommentButtonRight"
ref="addDiffNoteButtonRight"
type="button"
class="add-diff-note note-button js-add-diff-note-button"
:disabled="line.right.commentsDisabled"
:aria-label="addCommentTooltipRight"
@click="handleCommentButton(line.right)"
>
<gl-icon :size="12" name="comment" />
</button>
</span>
<a
v-if="line.right.new_line"
ref="lineNumberRefNew"
:data-linenumber="line.right.new_line"
:href="line.lineHrefNew"
@click="setHighlightedRow(line.lineCode)"
>
</a>
<diff-gutter-avatars
v-if="line.hasDiscussionsRight"
:discussions="line.right.discussions"
:discussions-expanded="line.right.discussionsExpanded"
@toggleLineDiscussions="
toggleLineDiscussions({
lineCode: line.right.line_code,
fileHash,
expanded: !line.right.discussionsExpanded,
})
"
/>
</td>
<td
v-gl-tooltip.hover
:title="coverageState.text"
:class="[line.right.type, coverageState.class, { hll: isHighlighted }]"
class="line-coverage right-side"
></td>
<td
:id="line.right.line_code"
:key="line.right.rich_text"
v-safe-html="line.right.rich_text"
:class="[
line.right.type,
{
hll: isHighlighted,
},
]"
v-bind="interopRightAttributes"
class="line_content with-coverage parallel right-side"
@mousedown="handleParallelLineMouseDown"
></td>
</template>
<template v-else>
<td class="diff-line-num old_line empty-cell"></td>
<td class="line-coverage right-side empty-cell"></td>
<td class="line_content with-coverage parallel right-side empty-cell"></td>
</template>
</tr>
</template>
<script>
import { mapGetters, mapState } from 'vuex';
import DraftNote from '~/batch_comments/components/draft_note.vue';
import draftCommentsMixin from '~/diffs/mixins/draft_comments';
import { getCommentedLines } from '~/notes/components/multiline_comment_utils';
import DiffCommentCell from './diff_comment_cell.vue';
import DiffExpansionCell from './diff_expansion_cell.vue';
import parallelDiffTableRow from './parallel_diff_table_row.vue';
export default {
components: {
DiffExpansionCell,
parallelDiffTableRow,
DiffCommentCell,
DraftNote,
},
mixins: [draftCommentsMixin],
props: {
diffFile: {
type: Object,
required: true,
},
diffLines: {
type: Array,
required: true,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
},
computed: {
...mapGetters('diffs', ['commitId']),
...mapState({
selectedCommentPosition: ({ notes }) => notes.selectedCommentPosition,
selectedCommentPositionHover: ({ notes }) => notes.selectedCommentPositionHover,
}),
diffLinesLength() {
return this.diffLines.length;
},
commentedLines() {
return getCommentedLines(
this.selectedCommentPosition || this.selectedCommentPositionHover,
this.diffLines,
);
},
},
userColorScheme: window.gon.user_color_scheme,
};
</script>
<template>
<table
:class="$options.userColorScheme"
:data-commit-id="commitId"
class="code diff-wrap-lines js-syntax-highlight text-file"
>
<colgroup>
<col style="width: 50px" />
<col style="width: 8px" />
<col />
<col style="width: 50px" />
<col style="width: 8px" />
<col />
</colgroup>
<tbody>
<template v-for="(line, index) in diffLines">
<tr
v-if="line.isMatchLineLeft || line.isMatchLineRight"
:key="`expand-${index}`"
class="line_expansion match"
>
<td colspan="6" class="text-center gl-font-regular">
<diff-expansion-cell
:file-hash="diffFile.file_hash"
:context-lines-path="diffFile.context_lines_path"
:line="line.left"
:is-top="index === 0"
:is-bottom="index + 1 === diffLinesLength"
/>
</td>
</tr>
<parallel-diff-table-row
:key="line.line_code"
:file-hash="diffFile.file_hash"
:file-path="diffFile.file_path"
:line="line"
:is-bottom="index + 1 === diffLinesLength"
:is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine"
/>
<tr
v-if="line.renderCommentRow"
:key="`dcr-${line.line_code || index}`"
:class="line.commentRowClasses"
class="notes_holder"
>
<td class="notes-content parallel old" colspan="3">
<diff-comment-cell
v-if="line.left"
:line="line.left"
:diff-file-hash="diffFile.file_hash"
:help-page-path="helpPagePath"
:has-draft="line.left.hasDraft"
line-position="left"
/>
</td>
<td class="notes-content parallel new" colspan="3">
<diff-comment-cell
v-if="line.right"
:line="line.right"
:diff-file-hash="diffFile.file_hash"
:line-index="index"
:help-page-path="helpPagePath"
:has-draft="line.right.hasDraft"
line-position="right"
/>
</td>
</tr>
<tr
v-if="shouldRenderParallelDraftRow(diffFile.file_hash, line)"
:key="`drafts-${index}`"
:class="line.draftRowClasses"
class="notes_holder"
>
<td class="notes_line old"></td>
<td class="notes-content parallel old" colspan="2">
<div v-if="line.left && line.left.lineDraft.isDraft" class="content">
<draft-note :draft="line.left.lineDraft" :line="line.left" />
</div>
</td>
<td class="notes_line new"></td>
<td class="notes-content parallel new" colspan="2">
<div v-if="line.right && line.right.lineDraft.isDraft" class="content">
<draft-note :draft="line.right.lineDraft" :line="line.right" />
</div>
</td>
</tr>
</template>
</tbody>
</table>
</template>
......@@ -151,11 +151,7 @@ export const currentDiffIndex = (state) =>
state.diffFiles.findIndex((diff) => diff.file_hash === state.currentDiffFileId),
);
export const diffLines = (state) => (file, unifiedDiffComponents) => {
if (!unifiedDiffComponents && state.diffViewType === INLINE_DIFF_VIEW_TYPE) {
return null;
}
export const diffLines = (state) => (file) => {
return parallelizeDiffLines(
file.highlighted_diff_lines || [],
state.diffViewType === INLINE_DIFF_VIEW_TYPE,
......
......@@ -33,7 +33,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
push_frontend_feature_flag(:approvals_commented_by, @project, default_enabled: true)
push_frontend_feature_flag(:merge_request_widget_graphql, @project, default_enabled: :yaml)
push_frontend_feature_flag(:drag_comment_selection, @project, default_enabled: true)
push_frontend_feature_flag(:unified_diff_components, @project, default_enabled: true)
push_frontend_feature_flag(:default_merge_ref_for_diffs, @project, default_enabled: :yaml)
push_frontend_feature_flag(:core_security_mr_widget_counts, @project)
push_frontend_feature_flag(:diffs_gradual_load, @project, default_enabled: true)
......
---
name: unified_diff_components
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44974
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/268039
type: development
group: group::code review
default_enabled: true
milestone: '13.6'
import {
mapParallel as CEMapParallel,
mapInline as CEMapInline,
} from '~/diffs/components/diff_row_utils';
import { mapParallel as CEMapParallel } from '~/diffs/components/diff_row_utils';
export const mapParallel = (content) => (line) => {
let { left, right } = line;
......@@ -27,5 +24,3 @@ export const mapParallel = (content) => (line) => {
}),
};
};
export const mapInline = CEMapInline;
......@@ -259,8 +259,8 @@ RSpec.describe 'Merge request > Batch comments', :js do
end
def write_parallel_comment(line, **params)
find("td[id='#{line}']").hover
find(".is-over button").click
find("div[id='#{line}']").hover
find(".js-add-diff-note-button").click
write_comment(selector: "form[data-line-code='#{line}']", **params)
end
......
......@@ -132,7 +132,7 @@ RSpec.describe 'User comments on a diff', :js do
# In `files/ruby/popen.rb`
it 'allows comments for changes involving both sides' do
# click +15, select -13 add and verify comment
click_diff_line(find('div[data-path="files/ruby/popen.rb"] .new_line a[data-linenumber="15"]').find(:xpath, '../..'), 'right')
click_diff_line(find('div[data-path="files/ruby/popen.rb"] .right-side a[data-linenumber="15"]').find(:xpath, '../../..'), 'right')
add_comment('-13', '+15')
end
......@@ -141,7 +141,7 @@ RSpec.describe 'User comments on a diff', :js do
page.within('[data-path="files/ruby/popen.rb"]') do
all('.js-unfold-all')[0].click
end
click_diff_line(find('div[data-path="files/ruby/popen.rb"] .old_line a[data-linenumber="9"]').find(:xpath, '../..'), 'left')
click_diff_line(find('div[data-path="files/ruby/popen.rb"] .left-side a[data-linenumber="9"]').find(:xpath, '../..'), 'left')
add_comment('1', '-9')
end
......@@ -150,7 +150,7 @@ RSpec.describe 'User comments on a diff', :js do
page.within('[data-path="files/ruby/popen.rb"]') do
all('.js-unfold-all')[1].click
end
click_diff_line(find('div[data-path="files/ruby/popen.rb"] .old_line a[data-linenumber="21"]').find(:xpath, '../..'), 'left')
click_diff_line(find('div[data-path="files/ruby/popen.rb"] .left-side a[data-linenumber="21"]').find(:xpath, '../..'), 'left')
add_comment('18', '21')
end
......@@ -159,7 +159,7 @@ RSpec.describe 'User comments on a diff', :js do
page.within('[data-path="files/ruby/popen.rb"]') do
all('.js-unfold-down')[1].click
end
click_diff_line(find('div[data-path="files/ruby/popen.rb"] .old_line a[data-linenumber="30"]').find(:xpath, '../..'), 'left')
click_diff_line(find('div[data-path="files/ruby/popen.rb"] .left-side a[data-linenumber="30"]').find(:xpath, '../..'), 'left')
add_comment('+28', '37')
end
end
......
......@@ -16,14 +16,14 @@ RSpec.describe 'Batch diffs', :js do
wait_for_requests
# Add discussion to first line of first file
click_diff_line(find('.diff-file.file-holder:first-of-type tr.line_holder.new:first-of-type'))
click_diff_line(find('.diff-file.file-holder:first-of-type .line_holder .left-side:first-of-type'))
page.within('.js-discussion-note-form') do
fill_in('note_note', with: 'First Line Comment')
click_button('Add comment now')
end
# Add discussion to first line of last file
click_diff_line(find('.diff-file.file-holder:last-of-type tr.line_holder.new:first-of-type'))
click_diff_line(find('.diff-file.file-holder:last-of-type .line_holder .left-side:first-of-type'))
page.within('.js-discussion-note-form') do
fill_in('note_note', with: 'Last Line Comment')
click_button('Add comment now')
......
......@@ -10,7 +10,7 @@ RSpec.describe 'Merge request > User posts diff notes', :js do
let(:user) { project.creator }
let(:comment_button_class) { '.add-diff-note' }
let(:notes_holder_input_class) { 'js-temp-notes-holder' }
let(:notes_holder_input_xpath) { './following-sibling::*[contains(concat(" ", @class, " "), " notes_holder ")]' }
let(:notes_holder_input_xpath) { '..//following-sibling::*[contains(concat(" ", @class, " "), " notes_holder ")]' }
let(:test_note_comment) { 'this is a test note!' }
before do
......@@ -27,7 +27,7 @@ RSpec.describe 'Merge request > User posts diff notes', :js do
context 'with an old line on the left and no line on the right' do
it 'allows commenting on the left side' do
should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_23_22"]').find(:xpath, '..'), 'left')
should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_23_22"]'), 'left')
end
it 'does not allow commenting on the right side' do
......@@ -67,7 +67,7 @@ RSpec.describe 'Merge request > User posts diff notes', :js do
context 'with a match line' do
it 'does not allow commenting' do
line_holder = find('.match', match: :first).find(:xpath, '..')
line_holder = find('.match', match: :first)
match_should_not_allow_commenting(line_holder)
end
end
......@@ -81,17 +81,13 @@ RSpec.describe 'Merge request > User posts diff notes', :js do
wait_for_requests
end
# The first `.js-unfold` unfolds upwards, therefore the first
# `.line_holder` will be an unfolded line.
let(:line_holder) { first('#a5cc2925ca8258af241be7e5b0381edf30266302 .line_holder') }
it 'allows commenting on the left side' do
should_allow_commenting(line_holder, 'left')
should_allow_commenting(first('#a5cc2925ca8258af241be7e5b0381edf30266302 .line_holder [data-testid="left-side"]'))
end
it 'allows commenting on the right side' do
# Automatically shifts comment box to left side.
should_allow_commenting(line_holder, 'right')
should_allow_commenting(first('#a5cc2925ca8258af241be7e5b0381edf30266302 .line_holder [data-testid="right-side"]'))
end
end
end
......@@ -149,7 +145,7 @@ RSpec.describe 'Merge request > User posts diff notes', :js do
# The first `.js-unfold` unfolds upwards, therefore the first
# `.line_holder` will be an unfolded line.
let(:line_holder) { first('.line_holder[id="a5cc2925ca8258af241be7e5b0381edf30266302_1_1"]') }
let(:line_holder) { first('[id="a5cc2925ca8258af241be7e5b0381edf30266302_1_1"]') }
it 'allows commenting' do
should_allow_commenting line_holder
......
......@@ -30,8 +30,8 @@ RSpec.describe 'Merge request > User sees versions', :js do
line_code = "#{file_id}_#{line_code}"
page.within(diff_file_selector) do
find(".line_holder[id='#{line_code}'] td:nth-of-type(1)").hover
find(".line_holder[id='#{line_code}'] button").click
first("[id='#{line_code}']").hover
first("[id='#{line_code}'] [role='button']").click
page.within("form[data-line-code='#{line_code}']") do
fill_in "note[note]", with: comment
......
......@@ -24,7 +24,7 @@ RSpec.describe 'User views diffs', :js do
page.within('.file-holder[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd"]') do
expect(find('.text-file')).to have_content('fileutils')
expect(page).to have_selector('.new_line [data-linenumber="1"]', count: 1)
expect(page).to have_selector('[data-interop-type="new"] [data-linenumber="1"]')
end
end
......@@ -32,8 +32,8 @@ RSpec.describe 'User views diffs', :js do
page.within('.file-holder[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd"]') do
all('.js-unfold-all')[1].click
expect(page).to have_selector('.new_line [data-linenumber="24"]', count: 1)
expect(page).not_to have_selector('.new_line [data-linenumber="1"]')
expect(page).to have_selector('[data-interop-type="new"] [data-linenumber="24"]', count: 1)
expect(page).not_to have_selector('[data-interop-type="new"] [data-linenumber="1"]')
end
end
......
......@@ -4,8 +4,6 @@ import Vuex from 'vuex';
import DiffContentComponent from '~/diffs/components/diff_content.vue';
import DiffDiscussions from '~/diffs/components/diff_discussions.vue';
import DiffView from '~/diffs/components/diff_view.vue';
import InlineDiffView from '~/diffs/components/inline_diff_view.vue';
import ParallelDiffView from '~/diffs/components/parallel_diff_view.vue';
import { IMAGE_DIFF_POSITION_TYPE } from '~/diffs/constants';
import { diffViewerModes } from '~/ide/constants';
import NoteForm from '~/notes/components/note_form.vue';
......@@ -107,25 +105,10 @@ describe('DiffContent', () => {
});
const textDiffFile = { ...defaultProps.diffFile, viewer: { name: diffViewerModes.text } };
it('should render diff inline view if `isInlineView` is true', () => {
isInlineViewGetterMock.mockReturnValue(true);
createComponent({ props: { diffFile: textDiffFile } });
expect(wrapper.find(InlineDiffView).exists()).toBe(true);
});
it('should render parallel view if `isParallelView` getter is true', () => {
isParallelViewGetterMock.mockReturnValue(true);
createComponent({ props: { diffFile: textDiffFile } });
expect(wrapper.find(ParallelDiffView).exists()).toBe(true);
});
it('should render diff view if `unifiedDiffComponents` are true', () => {
isParallelViewGetterMock.mockReturnValue(true);
createComponent({
props: { diffFile: textDiffFile },
provide: { glFeatures: { unifiedDiffComponents: true } },
});
expect(wrapper.find(DiffView).exists()).toBe(true);
......
......@@ -258,30 +258,3 @@ describe('mapParallel', () => {
expect(mapped.right).toMatchObject(rightExpectation);
});
});
describe('mapInline', () => {
it('should assign computed properties to the line object', () => {
const content = {
diffFile: {},
shouldRenderDraftRow: () => false,
};
const line = {
discussions: [{}],
discussionsExpanded: true,
hasForm: true,
};
const expectation = {
commentRowClasses: '',
hasDiscussions: true,
isContextLine: false,
isMatchLine: false,
isMetaLine: false,
renderDiscussion: true,
hasDraft: false,
hasCommentForm: true,
};
const mapped = utils.mapInline(content)(line);
expect(mapped).toMatchObject(expectation);
});
});
import '~/behaviors/markdown/render_gfm';
import { getByText } from '@testing-library/dom';
import { mount } from '@vue/test-utils';
import { mapInline } from '~/diffs/components/diff_row_utils';
import InlineDiffView from '~/diffs/components/inline_diff_view.vue';
import { createStore } from '~/mr_notes/stores';
import discussionsMockData from '../mock_data/diff_discussions';
import diffFileMockData from '../mock_data/diff_file';
describe('InlineDiffView', () => {
let wrapper;
const getDiffFileMock = () => ({ ...diffFileMockData });
const getDiscussionsMockData = () => [{ ...discussionsMockData }];
const notesLength = getDiscussionsMockData()[0].notes.length;
const setup = (diffFile, diffLines) => {
const mockDiffContent = {
diffFile,
shouldRenderDraftRow: jest.fn(),
};
const store = createStore();
store.dispatch('diffs/setInlineDiffViewType');
wrapper = mount(InlineDiffView, {
store,
propsData: {
diffFile,
diffLines: diffLines.map(mapInline(mockDiffContent)),
},
});
};
describe('template', () => {
it('should have rendered diff lines', () => {
const diffFile = getDiffFileMock();
setup(diffFile, diffFile.highlighted_diff_lines);
expect(wrapper.findAll('tr.line_holder').length).toEqual(8);
expect(wrapper.findAll('tr.line_holder.new').length).toEqual(4);
expect(wrapper.findAll('tr.line_expansion.match').length).toEqual(1);
getByText(wrapper.element, /Bad dates/i);
});
it('should render discussions', () => {
const diffFile = getDiffFileMock();
diffFile.highlighted_diff_lines[1].discussions = getDiscussionsMockData();
diffFile.highlighted_diff_lines[1].discussionsExpanded = true;
setup(diffFile, diffFile.highlighted_diff_lines);
expect(wrapper.findAll('.notes_holder').length).toEqual(1);
expect(wrapper.findAll('.notes_holder .note').length).toEqual(notesLength + 1);
getByText(wrapper.element, 'comment 5');
wrapper.vm.$store.dispatch('setInitialNotes', []);
});
});
});
import { shallowMount, createLocalVue } from '@vue/test-utils';
import Vuex from 'vuex';
import parallelDiffTableRow from '~/diffs/components/parallel_diff_table_row.vue';
import ParallelDiffView from '~/diffs/components/parallel_diff_view.vue';
import { createStore } from '~/mr_notes/stores';
import diffFileMockData from '../mock_data/diff_file';
let wrapper;
const localVue = createLocalVue();
localVue.use(Vuex);
function factory() {
const diffFile = { ...diffFileMockData };
const store = createStore();
wrapper = shallowMount(ParallelDiffView, {
localVue,
store,
propsData: {
diffFile,
diffLines: diffFile.parallel_diff_lines,
},
});
}
describe('ParallelDiffView', () => {
afterEach(() => {
wrapper.destroy();
});
it('renders diff lines', () => {
factory();
expect(wrapper.findAll(parallelDiffTableRow).length).toBe(8);
});
});
......@@ -8,15 +8,6 @@ import {
getCodeElementFromLineNumber,
} from './diffs_interopability_api';
jest.mock('~/vue_shared/mixins/gl_feature_flags_mixin', () => () => ({
inject: {
glFeatures: {
from: 'window.gon.features',
default: () => global.window.gon?.features,
},
},
}));
const TEST_PROJECT_PATH = 'gitlab-org/gitlab-test';
const TEST_BASE_URL = `/${TEST_PROJECT_PATH}/-/merge_requests/1/`;
const TEST_DIFF_FILE = 'files/js/commit.coffee';
......@@ -114,48 +105,41 @@ describe('diffs third party interoperability', () => {
);
describe.each`
desc | unifiedDiffComponents | view | rowSelector | codeSelector | expectation
${'inline view'} | ${false} | ${'inline'} | ${'tr.line_holder'} | ${'td.line_content'} | ${EXPECT_INLINE}
${'parallel view left side'} | ${false} | ${'parallel'} | ${'tr.line_holder'} | ${'td.line_content.left-side'} | ${EXPECT_PARALLEL_LEFT_SIDE}
${'parallel view right side'} | ${false} | ${'parallel'} | ${'tr.line_holder'} | ${'td.line_content.right-side'} | ${EXPECT_PARALLEL_RIGHT_SIDE}
${'inline view'} | ${true} | ${'inline'} | ${'.diff-tr.line_holder'} | ${'.diff-td.line_content'} | ${EXPECT_INLINE}
${'parallel view left side'} | ${true} | ${'parallel'} | ${'.diff-tr.line_holder'} | ${'.diff-td.line_content.left-side'} | ${EXPECT_PARALLEL_LEFT_SIDE}
${'parallel view right side'} | ${true} | ${'parallel'} | ${'.diff-tr.line_holder'} | ${'.diff-td.line_content.right-side'} | ${EXPECT_PARALLEL_RIGHT_SIDE}
`(
'$desc (unifiedDiffComponents=$unifiedDiffComponents)',
({ unifiedDiffComponents, view, rowSelector, codeSelector, expectation }) => {
beforeEach(async () => {
global.jsdom.reconfigure({
url: `${TEST_HOST}/${TEST_BASE_URL}/diffs?view=${view}`,
});
window.gon.features = { unifiedDiffComponents };
vm = startDiffsApp();
await waitFor(() => expect(hasLines(rowSelector)).toBe(true));
desc | view | rowSelector | codeSelector | expectation
${'inline view'} | ${'inline'} | ${'.diff-tr.line_holder'} | ${'.diff-td.line_content'} | ${EXPECT_INLINE}
${'parallel view left side'} | ${'parallel'} | ${'.diff-tr.line_holder'} | ${'.diff-td.line_content.left-side'} | ${EXPECT_PARALLEL_LEFT_SIDE}
${'parallel view right side'} | ${'parallel'} | ${'.diff-tr.line_holder'} | ${'.diff-td.line_content.right-side'} | ${EXPECT_PARALLEL_RIGHT_SIDE}
`('$desc', ({ view, rowSelector, codeSelector, expectation }) => {
beforeEach(async () => {
global.jsdom.reconfigure({
url: `${TEST_HOST}/${TEST_BASE_URL}/diffs?view=${view}`,
});
it('should match diff model', () => {
const lines = findLineElements(rowSelector);
const codes = findCodeElements(lines, codeSelector);
vm = startDiffsApp();
expect(getCodeElementsInteropModel(codes)).toEqual(expectation);
});
await waitFor(() => expect(hasLines(rowSelector)).toBe(true));
});
it('should match diff model', () => {
const lines = findLineElements(rowSelector);
const codes = findCodeElements(lines, codeSelector);
expect(getCodeElementsInteropModel(codes)).toEqual(expectation);
});
it.each`
lineNumber | part | expectedText
${4} | ${'base'} | ${'new CommitFile(this)'}
${4} | ${'head'} | ${'new CommitFile(@)'}
${2} | ${'base'} | ${'constructor: ->'}
${2} | ${'head'} | ${'constructor: ->'}
`(
'should find code element lineNumber=$lineNumber part=$part',
({ lineNumber, part, expectedText }) => {
const codeElement = getCodeElementFromLineNumber(findDiffFile(), lineNumber, part);
expect(codeElement.textContent.trim()).toBe(expectedText);
},
);
},
);
it.each`
lineNumber | part | expectedText
${4} | ${'base'} | ${'new CommitFile(this)'}
${4} | ${'head'} | ${'new CommitFile(@)'}
${2} | ${'base'} | ${'constructor: ->'}
${2} | ${'head'} | ${'constructor: ->'}
`(
'should find code element lineNumber=$lineNumber part=$part',
({ lineNumber, part, expectedText }) => {
const codeElement = getCodeElementFromLineNumber(findDiffFile(), lineNumber, part);
expect(codeElement.textContent.trim()).toBe(expectedText);
},
);
});
});
......@@ -271,7 +271,6 @@ RSpec.configure do |config|
# See https://gitlab.com/gitlab-org/gitlab/-/issues/33867
stub_feature_flags(file_identifier_hash: false)
stub_feature_flags(unified_diff_components: false)
stub_feature_flags(diffs_virtual_scrolling: false)
# The following `vue_issues_list`/`vue_issuables_list` stubs can be removed
......
......@@ -3,8 +3,8 @@
module MergeRequestDiffHelpers
def click_diff_line(line_holder, diff_side = nil)
line = get_line_components(line_holder, diff_side)
line[:content].hover
line[:num].find('.js-add-diff-note-button', visible: false).send_keys(:return)
line_holder.hover
line[:num].find('.js-add-diff-note-button').click
end
def get_line_components(line_holder, diff_side = nil)
......
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