Commit cfdf5b19 authored by Justin Boyson's avatar Justin Boyson Committed by Miguel Rincon

Fix saving state between hover and start change

Save two separate states for hover and start selection.
Make start selection be the default.
parent e118d7ba
......@@ -51,6 +51,7 @@ export default {
'scrollToDraft',
'toggleResolveDiscussion',
]),
...mapActions(['setSelectedCommentPositionHover']),
update(data) {
this.updateDraft(data);
},
......@@ -67,7 +68,11 @@ export default {
};
</script>
<template>
<article class="draft-note-component note-wrapper">
<article
class="draft-note-component note-wrapper"
@mouseenter="setSelectedCommentPositionHover(draft.position.line_range)"
@mouseleave="setSelectedCommentPositionHover()"
>
<ul class="notes draft-notes">
<noteable-note
:note="draft"
......
......@@ -148,10 +148,7 @@ export default {
<template>
<div class="content discussion-form discussion-form-container discussion-notes">
<div
v-if="glFeatures.multilineComments"
class="gl-mb-3 gl-text-gray-700 gl-border-gray-100 gl-border-b-solid gl-border-b-1 gl-pb-3"
>
<div v-if="glFeatures.multilineComments" class="gl-mb-3 gl-text-gray-700 gl-pb-3">
<multiline-comment-form
v-model="commentLineStart"
:line="line"
......
......@@ -37,6 +37,11 @@ export default {
required: false,
default: false,
},
isCommented: {
type: Boolean,
required: false,
default: false,
},
},
data() {
return {
......@@ -47,7 +52,10 @@ export default {
...mapGetters('diffs', ['fileLineCoverage']),
...mapState({
isHighlighted(state) {
return this.line.line_code !== null && this.line.line_code === state.diffs.highlightedRow;
if (this.isCommented) return true;
const lineCode = this.line.line_code;
return lineCode ? lineCode === state.diffs.highlightedRow : false;
},
}),
isContextLine() {
......
<script>
import { mapGetters } from 'vuex';
import { mapGetters, mapState } from 'vuex';
import draftCommentsMixin from '~/diffs/mixins/draft_comments';
import InlineDraftCommentRow from '~/batch_comments/components/inline_draft_comment_row.vue';
import inlineDiffTableRow from './inline_diff_table_row.vue';
import inlineDiffCommentRow from './inline_diff_comment_row.vue';
import inlineDiffExpansionRow from './inline_diff_expansion_row.vue';
import { getCommentedLines } from '~/notes/components/multiline_comment_utils';
export default {
components: {
......@@ -31,9 +32,19 @@ export 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,
};
......@@ -67,6 +78,7 @@ export default {
:file-path="diffFile.file_path"
:line="line"
:is-bottom="index + 1 === diffLinesLength"
:is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine"
/>
<inline-diff-comment-row
:key="`icr-${line.line_code || index}`"
......
......@@ -40,6 +40,11 @@ export default {
required: false,
default: false,
},
isCommented: {
type: Boolean,
required: false,
default: false,
},
},
data() {
return {
......@@ -51,6 +56,8 @@ export default {
...mapGetters('diffs', ['fileLineCoverage']),
...mapState({
isHighlighted(state) {
if (this.isCommented) return true;
const lineCode =
(this.line.left && this.line.left.line_code) ||
(this.line.right && this.line.right.line_code);
......
<script>
import { mapGetters } from 'vuex';
import { mapGetters, mapState } from 'vuex';
import draftCommentsMixin from '~/diffs/mixins/draft_comments';
import ParallelDraftCommentRow from '~/batch_comments/components/parallel_draft_comment_row.vue';
import parallelDiffTableRow from './parallel_diff_table_row.vue';
import parallelDiffCommentRow from './parallel_diff_comment_row.vue';
import parallelDiffExpansionRow from './parallel_diff_expansion_row.vue';
import { getCommentedLines } from '~/notes/components/multiline_comment_utils';
export default {
components: {
......@@ -31,9 +32,19 @@ export 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,
};
......@@ -69,6 +80,7 @@ export default {
:file-path="diffFile.file_path"
:line="line"
:is-bottom="index + 1 === diffLinesLength"
:is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine"
/>
<parallel-diff-comment-row
:key="`dcr-${line.line_code || index}`"
......
......@@ -74,7 +74,7 @@ export default {
},
},
methods: {
...mapActions(['toggleDiscussion']),
...mapActions(['toggleDiscussion', 'setSelectedCommentPositionHover']),
componentName(note) {
if (note.isPlaceholderNote) {
if (note.placeholderType === SYSTEM_NOTE) {
......@@ -99,7 +99,11 @@ export default {
<template>
<div class="discussion-notes">
<ul class="notes">
<ul
class="notes"
@mouseenter="setSelectedCommentPositionHover(discussion.position.line_range)"
@mouseleave="setSelectedCommentPositionHover()"
>
<template v-if="shouldGroupReplies">
<component
:is="componentName(firstNote)"
......
<script>
import { mapActions } from 'vuex';
import { GlFormSelect, GlSprintf } from '@gitlab/ui';
import { getSymbol, getLineClasses } from './multiline_comment_utils';
......@@ -39,8 +40,13 @@ export default {
old_line: line.old_line,
new_line: line.new_line,
};
this.highlightSelection();
},
destroyed() {
this.setSelectedCommentPosition();
},
methods: {
...mapActions(['setSelectedCommentPosition']),
getSymbol({ type }) {
return getSymbol(type);
},
......@@ -50,6 +56,16 @@ export default {
updateCommentLineStart(value) {
this.commentLineStart = value;
this.$emit('input', value);
this.highlightSelection();
},
highlightSelection() {
const { line_code, new_line, old_line, type } = this.line;
const updatedLineRange = {
start: { ...this.commentLineStart },
end: { line_code, new_line, old_line, type },
};
this.setSelectedCommentPosition(updatedLineRange);
},
},
};
......
......@@ -90,3 +90,22 @@ export function formatLineRange(start, end) {
end: extractProps(end),
};
}
export function getCommentedLines(selectedCommentPosition, diffLines) {
if (!selectedCommentPosition) {
// This structure simplifies the logic that consumes this result
// by keeping the returned shape the same and adjusting the bounds
// to something unreachable. This way our component logic stays:
// "if index between start and end"
return {
startLine: diffLines.length + 1,
endLine: diffLines.length + 1,
};
}
const { start, end } = selectedCommentPosition;
const startLine = diffLines.findIndex(l => l.line_code === start.line_code);
const endLine = diffLines.findIndex(l => l.line_code === end.line_code);
return { startLine, endLine };
}
......@@ -186,6 +186,7 @@ export default {
eventHub.$on('enterEditMode', ({ noteId }) => {
if (noteId === this.note.id) {
this.isEditing = true;
this.setSelectedCommentPositionHover();
this.scrollToNoteIfNeeded($(this.$el));
}
});
......@@ -205,9 +206,11 @@ export default {
'toggleResolveNote',
'scrollToNoteIfNeeded',
'updateAssignees',
'setSelectedCommentPositionHover',
]),
editHandler() {
this.isEditing = true;
this.setSelectedCommentPositionHover();
this.$emit('handleEdit');
},
deleteHandler() {
......@@ -284,6 +287,7 @@ export default {
} else {
this.isRequesting = false;
this.isEditing = true;
this.setSelectedCommentPositionHover();
this.$nextTick(() => {
const msg = __('Something went wrong while editing your comment. Please try again.');
Flash(msg, 'alert', this.$el);
......@@ -340,7 +344,7 @@ export default {
:line="line"
:comment-line-options="commentLineOptions"
:line-range="note.position.line_range"
class="gl-mb-3 gl-text-gray-700 gl-border-gray-100 gl-border-b-solid gl-border-b-1 gl-pb-3"
class="gl-mb-3 gl-text-gray-700 gl-pb-3"
/>
<div
v-else
......
......@@ -99,6 +99,14 @@ export const setDiscussionSortDirection = ({ commit }, direction) => {
commit(types.SET_DISCUSSIONS_SORT, direction);
};
export const setSelectedCommentPosition = ({ commit }, position) => {
commit(types.SET_SELECTED_COMMENT_POSITION, position);
};
export const setSelectedCommentPositionHover = ({ commit }, position) => {
commit(types.SET_SELECTED_COMMENT_POSITION_HOVER, position);
};
export const removeNote = ({ commit, dispatch, state }, note) => {
const discussion = state.discussions.find(({ id }) => id === note.discussion_id);
......
......@@ -12,6 +12,15 @@ export default () => ({
lastFetchedAt: null,
currentDiscussionId: null,
batchSuggestionsInfo: [],
/**
* selectedCommentPosition & selectedCommentPosition structures are the same as `position.line_range`:
* {
* start: { line_code: string, new_line: number, old_line:number, type: string },
* end: { line_code: string, new_line: number, old_line:number, type: string },
* }
*/
selectedCommentPosition: null,
selectedCommentPositionHover: null,
// View layer
isToggleStateButtonLoading: false,
......
......@@ -33,6 +33,8 @@ export const SET_EXPAND_DISCUSSIONS = 'SET_EXPAND_DISCUSSIONS';
export const UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS = 'UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS';
export const SET_CURRENT_DISCUSSION_ID = 'SET_CURRENT_DISCUSSION_ID';
export const SET_DISCUSSIONS_SORT = 'SET_DISCUSSIONS_SORT';
export const SET_SELECTED_COMMENT_POSITION = 'SET_SELECTED_COMMENT_POSITION';
export const SET_SELECTED_COMMENT_POSITION_HOVER = 'SET_SELECTED_COMMENT_POSITION_HOVER';
// Issue
export const CLOSE_ISSUE = 'CLOSE_ISSUE';
......
......@@ -308,6 +308,14 @@ export default {
state.discussionSortOrder = sort;
},
[types.SET_SELECTED_COMMENT_POSITION](state, position) {
state.selectedCommentPosition = position;
},
[types.SET_SELECTED_COMMENT_POSITION_HOVER](state, position) {
state.selectedCommentPositionHover = position;
},
[types.DISABLE_COMMENTS](state, value) {
state.commentsDisabled = value;
},
......
---
title: Highlight commented rows
merge_request: 34432
author:
type: added
import Vue from 'vue';
import { createComponentWithStore } from 'helpers/vue_mount_component_helper';
import { shallowMount } from '@vue/test-utils';
import { createStore } from '~/mr_notes/stores';
import InlineDiffTableRow from '~/diffs/components/inline_diff_table_row.vue';
import diffFileMockData from '../mock_data/diff_file';
describe('InlineDiffTableRow', () => {
let wrapper;
let vm;
const thisLine = diffFileMockData.highlighted_diff_lines[0];
beforeEach(() => {
vm = createComponentWithStore(Vue.extend(InlineDiffTableRow), createStore(), {
line: thisLine,
fileHash: diffFileMockData.file_hash,
filePath: diffFileMockData.file_path,
contextLinesPath: 'contextLinesPath',
isHighlighted: false,
}).$mount();
wrapper = shallowMount(InlineDiffTableRow, {
store: createStore(),
propsData: {
line: thisLine,
fileHash: diffFileMockData.file_hash,
filePath: diffFileMockData.file_path,
contextLinesPath: 'contextLinesPath',
isHighlighted: false,
},
});
vm = wrapper.vm;
});
it('does not add hll class to line content when line does not match highlighted row', done => {
vm.$nextTick()
.then(() => {
expect(vm.$el.querySelector('.line_content').classList).not.toContain('hll');
expect(wrapper.find('.line_content').classes('hll')).toBe(false);
})
.then(done)
.catch(done.fail);
......@@ -35,12 +39,19 @@ describe('InlineDiffTableRow', () => {
return vm.$nextTick();
})
.then(() => {
expect(vm.$el.querySelector('.line_content').classList).toContain('hll');
expect(wrapper.find('.line_content').classes('hll')).toBe(true);
})
.then(done)
.catch(done.fail);
});
it('adds hll class to lineContent when line is part of a multiline comment', () => {
wrapper.setProps({ isCommented: true });
return vm.$nextTick().then(() => {
expect(wrapper.find('.line_content').classes('hll')).toBe(true);
});
});
describe('sets coverage title and class', () => {
it('for lines with coverage', done => {
vm.$nextTick()
......@@ -53,10 +64,10 @@ describe('InlineDiffTableRow', () => {
return vm.$nextTick();
})
.then(() => {
const coverage = vm.$el.querySelector('.line-coverage');
const coverage = wrapper.find('.line-coverage');
expect(coverage.title).toContain('Test coverage: 5 hits');
expect(coverage.classList).toContain('coverage');
expect(coverage.attributes('title')).toContain('Test coverage: 5 hits');
expect(coverage.classes('coverage')).toBe(true);
})
.then(done)
.catch(done.fail);
......@@ -73,10 +84,10 @@ describe('InlineDiffTableRow', () => {
return vm.$nextTick();
})
.then(() => {
const coverage = vm.$el.querySelector('.line-coverage');
const coverage = wrapper.find('.line-coverage');
expect(coverage.title).toContain('No test coverage');
expect(coverage.classList).toContain('no-coverage');
expect(coverage.attributes('title')).toContain('No test coverage');
expect(coverage.classes('no-coverage')).toBe(true);
})
.then(done)
.catch(done.fail);
......@@ -90,11 +101,11 @@ describe('InlineDiffTableRow', () => {
return vm.$nextTick();
})
.then(() => {
const coverage = vm.$el.querySelector('.line-coverage');
const coverage = wrapper.find('.line-coverage');
expect(coverage.title).not.toContain('Coverage');
expect(coverage.classList).not.toContain('coverage');
expect(coverage.classList).not.toContain('no-coverage');
expect(coverage.attributes('title')).toBeUndefined();
expect(coverage.classes('coverage')).toBe(false);
expect(coverage.classes('no-coverage')).toBe(false);
})
.then(done)
.catch(done.fail);
......
import Vue from 'vue';
import { shallowMount } from '@vue/test-utils';
import { createComponentWithStore } from 'helpers/vue_mount_component_helper';
import { createStore } from '~/mr_notes/stores';
import ParallelDiffTableRow from '~/diffs/components/parallel_diff_table_row.vue';
......@@ -6,18 +7,24 @@ import diffFileMockData from '../mock_data/diff_file';
describe('ParallelDiffTableRow', () => {
describe('when one side is empty', () => {
let wrapper;
let vm;
const thisLine = diffFileMockData.parallel_diff_lines[0];
const rightLine = diffFileMockData.parallel_diff_lines[0].right;
beforeEach(() => {
vm = createComponentWithStore(Vue.extend(ParallelDiffTableRow), createStore(), {
line: thisLine,
fileHash: diffFileMockData.file_hash,
filePath: diffFileMockData.file_path,
contextLinesPath: 'contextLinesPath',
isHighlighted: false,
}).$mount();
wrapper = shallowMount(ParallelDiffTableRow, {
store: createStore(),
propsData: {
line: thisLine,
fileHash: diffFileMockData.file_hash,
filePath: diffFileMockData.file_path,
contextLinesPath: 'contextLinesPath',
isHighlighted: false,
},
});
vm = wrapper.vm;
});
it('does not highlight non empty line content when line does not match highlighted row', done => {
......@@ -42,6 +49,13 @@ describe('ParallelDiffTableRow', () => {
.then(done)
.catch(done.fail);
});
it('highlights nonempty line content when line is part of a multiline comment', () => {
wrapper.setProps({ isCommented: true });
return vm.$nextTick().then(() => {
expect(vm.$el.querySelector('.line_content.right-side').classList).toContain('hll');
});
});
});
describe('when both sides have content', () => {
......
......@@ -2,6 +2,7 @@ import {
getSymbol,
getStartLineNumber,
getEndLineNumber,
getCommentedLines,
} from '~/notes/components/multiline_comment_utils';
describe('Multiline comment utilities', () => {
......@@ -33,4 +34,30 @@ describe('Multiline comment utilities', () => {
expect(getSymbol(type)).toEqual(result);
});
});
describe('getCommentedLines', () => {
const diffLines = [{ line_code: '1' }, { line_code: '2' }, { line_code: '3' }];
it('returns a default object when `selectedCommentPosition` is not provided', () => {
expect(getCommentedLines(undefined, diffLines)).toEqual({ startLine: 4, endLine: 4 });
});
it('returns an object with startLine and endLine equal to 0', () => {
const selectedCommentPosition = {
start: { line_code: '1' },
end: { line_code: '1' },
};
expect(getCommentedLines(selectedCommentPosition, diffLines)).toEqual({
startLine: 0,
endLine: 0,
});
});
it('returns an object with startLine and endLine equal to 0 and 1', () => {
const selectedCommentPosition = {
start: { line_code: '1' },
end: { line_code: '2' },
};
expect(getCommentedLines(selectedCommentPosition, diffLines)).toEqual({
startLine: 0,
endLine: 1,
});
});
});
});
......@@ -175,6 +175,7 @@ describe('issue_note', () => {
store.hotUpdate({
actions: {
updateNote() {},
setSelectedCommentPositionHover() {},
},
});
const noteBodyComponent = wrapper.find(NoteBody);
......
......@@ -1127,6 +1127,19 @@ describe('Actions Notes Store', () => {
});
});
describe('setSelectedCommentPosition', () => {
it('calls the correct mutation with the correct args', done => {
testAction(
actions.setSelectedCommentPosition,
{},
{},
[{ type: mutationTypes.SET_SELECTED_COMMENT_POSITION, payload: {} }],
[],
done,
);
});
});
describe('softDeleteDescriptionVersion', () => {
const endpoint = '/path/to/diff/1';
const payload = {
......
......@@ -524,6 +524,26 @@ describe('Notes Store mutations', () => {
});
});
describe('SET_SELECTED_COMMENT_POSITION', () => {
it('should set comment position state', () => {
const state = {};
mutations.SET_SELECTED_COMMENT_POSITION(state, {});
expect(state.selectedCommentPosition).toEqual({});
});
});
describe('SET_SELECTED_COMMENT_POSITION_HOVER', () => {
it('should set comment hover position state', () => {
const state = {};
mutations.SET_SELECTED_COMMENT_POSITION_HOVER(state, {});
expect(state.selectedCommentPositionHover).toEqual({});
});
});
describe('DISABLE_COMMENTS', () => {
it('should set comments disabled state', () => {
const state = {};
......
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