Commit 44a0121a authored by Sam Bigelow's avatar Sam Bigelow Committed by Phil Hughes

Resolve "Merge request refactor does not highlight selected line"

parent 0d5cbd16
......@@ -102,6 +102,12 @@ export default {
if (this.shouldShow) {
this.fetchData();
}
const id = window && window.location && window.location.hash;
if (id) {
this.setHighlightedRow(id.slice(1));
}
},
created() {
this.adjustView();
......@@ -114,6 +120,7 @@ export default {
'fetchDiffFiles',
'startRenderDiffsQueue',
'assignDiscussionsToDiff',
'setHighlightedRow',
]),
fetchData() {
this.fetchDiffFiles()
......
......@@ -72,6 +72,13 @@ export default {
diffFiles: state => state.diffs.diffFiles,
}),
...mapGetters(['isLoggedIn']),
lineCode() {
return (
this.line.line_code ||
(this.line.left && this.line.line.left.line_code) ||
(this.line.right && this.line.right.line_code)
);
},
lineHref() {
return `#${this.line.line_code || ''}`;
},
......@@ -97,7 +104,7 @@ export default {
},
},
methods: {
...mapActions('diffs', ['loadMoreLines', 'showCommentForm']),
...mapActions('diffs', ['loadMoreLines', 'showCommentForm', 'setHighlightedRow']),
handleCommentButton() {
this.showCommentForm({ lineCode: this.line.line_code, fileHash: this.fileHash });
},
......@@ -168,7 +175,13 @@ export default {
>
<icon :size="12" name="comment" />
</button>
<a v-if="lineNumber" :data-linenumber="lineNumber" :href="lineHref"> </a>
<a
v-if="lineNumber"
:data-linenumber="lineNumber"
:href="lineHref"
@click="setHighlightedRow(lineCode);"
>
</a>
<diff-gutter-avatars v-if="shouldShowAvatarsOnGutter" :discussions="line.discussions" />
</template>
</div>
......
<script>
import { mapGetters } from 'vuex';
import { mapGetters, mapActions } from 'vuex';
import DiffLineGutterContent from './diff_line_gutter_content.vue';
import {
MATCH_LINE_TYPE,
......@@ -30,6 +30,11 @@ export default {
type: String,
required: true,
},
isHighlighted: {
type: Boolean,
required: true,
default: false,
},
diffViewType: {
type: String,
required: false,
......@@ -85,6 +90,7 @@ export default {
const { type } = this.line;
return {
hll: this.isHighlighted,
[type]: type,
[LINE_UNFOLD_CLASS_NAME]: this.isMatchLine,
[LINE_HOVER_CLASS_NAME]:
......@@ -99,6 +105,7 @@ export default {
return this.lineType === OLD_LINE_TYPE ? this.line.old_line : this.line.new_line;
},
},
methods: mapActions('diffs', ['setHighlightedRow']),
};
</script>
......
<script>
import { mapGetters, mapActions } from 'vuex';
import { mapGetters, mapActions, mapState } from 'vuex';
import DiffTableCell from './diff_table_cell.vue';
import {
NEW_LINE_TYPE,
......@@ -40,6 +40,11 @@ export default {
};
},
computed: {
...mapState({
isHighlighted(state) {
return this.line.line_code !== null && this.line.line_code === state.diffs.highlightedRow;
},
}),
...mapGetters('diffs', ['isInlineView']),
isContextLine() {
return this.line.type === CONTEXT_LINE_TYPE;
......@@ -91,6 +96,7 @@ export default {
:is-bottom="isBottom"
:is-hover="isHover"
:show-comment-button="true"
:is-highlighted="isHighlighted"
class="diff-line-num old_line"
/>
<diff-table-cell
......@@ -100,8 +106,18 @@ export default {
:line-type="newLineType"
:is-bottom="isBottom"
:is-hover="isHover"
:is-highlighted="isHighlighted"
class="diff-line-num new_line qa-new-diff-line"
/>
<td :class="line.type" class="line_content" v-html="line.rich_text"></td>
<td
:class="[
line.type,
{
hll: isHighlighted,
},
]"
class="line_content"
v-html="line.rich_text"
></td>
</tr>
</template>
<script>
import { mapActions } from 'vuex';
import { mapActions, mapState } from 'vuex';
import $ from 'jquery';
import DiffTableCell from './diff_table_cell.vue';
import {
......@@ -43,6 +43,15 @@ export default {
};
},
computed: {
...mapState({
isHighlighted(state) {
const lineCode =
(this.line.left && this.line.left.line_code) ||
(this.line.right && this.line.right.line_code);
return lineCode ? lineCode === state.diffs.highlightedRow : false;
},
}),
isContextLine() {
return this.line.left && this.line.left.type === CONTEXT_LINE_TYPE;
},
......@@ -57,7 +66,14 @@ export default {
return OLD_NO_NEW_LINE_TYPE;
}
return this.line.left ? this.line.left.type : EMPTY_CELL_TYPE;
const lineTypeClass = this.line.left ? this.line.left.type : EMPTY_CELL_TYPE;
return [
lineTypeClass,
{
hll: this.isHighlighted,
},
];
},
},
created() {
......@@ -114,6 +130,7 @@ export default {
:line-type="oldLineType"
:is-bottom="isBottom"
:is-hover="isLeftHover"
:is-highlighted="isHighlighted"
:show-comment-button="true"
:diff-view-type="parallelDiffViewType"
line-position="left"
......@@ -139,6 +156,7 @@ export default {
:line-type="newLineType"
:is-bottom="isBottom"
:is-hover="isRightHover"
:is-highlighted="isHighlighted"
:show-comment-button="true"
:diff-view-type="parallelDiffViewType"
line-position="right"
......@@ -146,7 +164,12 @@ export default {
/>
<td
:id="line.right.line_code"
:class="line.right.type"
:class="[
line.right.type,
{
hll: isHighlighted,
},
]"
class="line_content parallel right-side"
@mousedown.native="handleParallelLineMouseDown"
v-html="line.right.rich_text"
......
......@@ -33,6 +33,10 @@ export const fetchDiffFiles = ({ state, commit }) => {
.then(handleLocationHash);
};
export const setHighlightedRow = ({ commit }, lineCode) => {
commit(types.SET_HIGHLIGHTED_ROW, lineCode);
};
// This is adding line discussions to the actual lines in the diff tree
// once for parallel and once for inline mode
export const assignDiscussionsToDiff = (
......@@ -127,7 +131,7 @@ export const loadMoreLines = ({ commit }, options) => {
export const scrollToLineIfNeededInline = (_, line) => {
const hash = getLocationHash();
if (hash && line.lineCode === hash) {
if (hash && line.line_code === hash) {
handleLocationHash();
}
};
......@@ -137,7 +141,7 @@ export const scrollToLineIfNeededParallel = (_, line) => {
if (
hash &&
((line.left && line.left.lineCode === hash) || (line.right && line.right.lineCode === hash))
((line.left && line.left.line_code === hash) || (line.right && line.right.line_code === hash))
) {
handleLocationHash();
}
......
......@@ -26,4 +26,5 @@ export default () => ({
currentDiffFileId: '',
projectPath: '',
commentForms: [],
highlightedRow: null,
});
......@@ -17,3 +17,4 @@ export const UPDATE_CURRENT_DIFF_FILE_ID = 'UPDATE_CURRENT_DIFF_FILE_ID';
export const OPEN_DIFF_FILE_COMMENT_FORM = 'OPEN_DIFF_FILE_COMMENT_FORM';
export const UPDATE_DIFF_FILE_COMMENT_FORM = 'UPDATE_DIFF_FILE_COMMENT_FORM';
export const CLOSE_DIFF_FILE_COMMENT_FORM = 'CLOSE_DIFF_FILE_COMMENT_FORM';
export const SET_HIGHLIGHTED_ROW = 'SET_HIGHLIGHTED_ROW';
......@@ -241,4 +241,7 @@ export default {
[types.CLOSE_DIFF_FILE_COMMENT_FORM](state, fileHash) {
state.commentForms = state.commentForms.filter(form => form.fileHash !== fileHash);
},
[types.SET_HIGHLIGHTED_ROW](state, lineCode) {
state.highlightedRow = lineCode;
},
};
---
title: When user clicks linenumber in MR changes, highlight that line
merge_request:
author:
type: fixed
......@@ -13,6 +13,8 @@ describe('diffs/components/app', () => {
beforeEach(() => {
// setup globals (needed for component to mount :/)
window.mrTabs = jasmine.createSpyObj('mrTabs', ['resetViewContainer']);
window.mrTabs.expandViewContainer = jasmine.createSpy();
window.location.hash = 'ABC_123';
// setup component
const store = createDiffsStore();
......@@ -39,4 +41,15 @@ describe('diffs/components/app', () => {
it('does not show commit info', () => {
expect(vm.$el).not.toContainElement('.blob-commit-info');
});
it('sets highlighted row if hash exists in location object', done => {
vm.$props.shouldShow = true;
vm.$nextTick()
.then(() => {
expect(vm.$store.state.diffs.highlightedRow).toBe('ABC_123');
})
.then(done)
.catch(done.fail);
});
});
import Vue from 'vue';
import store from '~/mr_notes/stores';
import DiffTableCell from '~/diffs/components/diff_table_cell.vue';
import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import diffFileMockData from '../mock_data/diff_file';
describe('DiffTableCell', () => {
const createComponent = options =>
createComponentWithStore(Vue.extend(DiffTableCell), store, {
line: diffFileMockData.highlighted_diff_lines[0],
fileHash: diffFileMockData.file_hash,
contextLinesPath: 'contextLinesPath',
...options,
}).$mount();
it('does not highlight row when isHighlighted prop is false', done => {
const vm = createComponent({ isHighlighted: false });
vm.$nextTick()
.then(() => {
expect(vm.$el.classList).not.toContain('hll');
})
.then(done)
.catch(done.fail);
});
it('highlights row when isHighlighted prop is true', done => {
const vm = createComponent({ isHighlighted: true });
vm.$nextTick()
.then(() => {
expect(vm.$el.classList).toContain('hll');
})
.then(done)
.catch(done.fail);
});
});
import Vue from 'vue';
import store from '~/mr_notes/stores';
import InlineDiffTableRow from '~/diffs/components/inline_diff_table_row.vue';
import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import diffFileMockData from '../mock_data/diff_file';
describe('InlineDiffTableRow', () => {
let vm;
const thisLine = diffFileMockData.highlighted_diff_lines[0];
beforeEach(() => {
vm = createComponentWithStore(Vue.extend(InlineDiffTableRow), store, {
line: thisLine,
fileHash: diffFileMockData.file_hash,
contextLinesPath: 'contextLinesPath',
isHighlighted: false,
}).$mount();
});
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');
})
.then(done)
.catch(done.fail);
});
it('adds hll class to lineContent when line is the highlighted row', done => {
vm.$nextTick()
.then(() => {
vm.$store.state.diffs.highlightedRow = thisLine.line_code;
return vm.$nextTick();
})
.then(() => {
expect(vm.$el.querySelector('.line_content').classList).toContain('hll');
})
.then(done)
.catch(done.fail);
});
});
import Vue from 'vue';
import { createStore } from '~/mr_notes/stores';
import ParallelDiffTableRow from '~/diffs/components/parallel_diff_table_row.vue';
import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import diffFileMockData from '../mock_data/diff_file';
describe('ParallelDiffTableRow', () => {
describe('when one side is empty', () => {
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,
contextLinesPath: 'contextLinesPath',
isHighlighted: false,
}).$mount();
});
it('does not highlight non empty line content when line does not match highlighted row', done => {
vm.$nextTick()
.then(() => {
expect(vm.$el.querySelector('.line_content.right-side').classList).not.toContain('hll');
})
.then(done)
.catch(done.fail);
});
it('highlights nonempty line content when line is the highlighted row', done => {
vm.$nextTick()
.then(() => {
vm.$store.state.diffs.highlightedRow = rightLine.line_code;
return vm.$nextTick();
})
.then(() => {
expect(vm.$el.querySelector('.line_content.right-side').classList).toContain('hll');
})
.then(done)
.catch(done.fail);
});
});
describe('when both sides have content', () => {
let vm;
const thisLine = diffFileMockData.parallel_diff_lines[2];
const rightLine = diffFileMockData.parallel_diff_lines[2].right;
beforeEach(() => {
vm = createComponentWithStore(Vue.extend(ParallelDiffTableRow), createStore(), {
line: thisLine,
fileHash: diffFileMockData.file_hash,
contextLinesPath: 'contextLinesPath',
isHighlighted: false,
}).$mount();
});
it('does not highlight either line when line does not match highlighted row', done => {
vm.$nextTick()
.then(() => {
expect(vm.$el.querySelector('.line_content.right-side').classList).not.toContain('hll');
expect(vm.$el.querySelector('.line_content.left-side').classList).not.toContain('hll');
})
.then(done)
.catch(done.fail);
});
it('adds hll class to lineContent when line is the highlighted row', done => {
vm.$nextTick()
.then(() => {
vm.$store.state.diffs.highlightedRow = rightLine.line_code;
return vm.$nextTick();
})
.then(() => {
expect(vm.$el.querySelector('.line_content.right-side').classList).toContain('hll');
expect(vm.$el.querySelector('.line_content.left-side').classList).toContain('hll');
})
.then(done)
.catch(done.fail);
});
});
});
......@@ -22,6 +22,7 @@ import actions, {
expandAllFiles,
toggleFileDiscussions,
saveDiffDiscussion,
setHighlightedRow,
toggleTreeOpen,
scrollToFile,
toggleShowTreeList,
......@@ -92,6 +93,14 @@ describe('DiffsStoreActions', () => {
});
});
describe('setHighlightedRow', () => {
it('should set lineHash and fileHash of highlightedRow', () => {
testAction(setHighlightedRow, 'ABC_123', {}, [
{ type: types.SET_HIGHLIGHTED_ROW, payload: 'ABC_123' },
]);
});
});
describe('assignDiscussionsToDiff', () => {
it('should merge discussions into diffs', done => {
const state = {
......@@ -469,7 +478,7 @@ describe('DiffsStoreActions', () => {
describe('scrollToLineIfNeededInline', () => {
const lineMock = {
lineCode: 'ABC_123',
line_code: 'ABC_123',
};
it('should not call handleLocationHash when there is not hash', () => {
......@@ -520,7 +529,7 @@ describe('DiffsStoreActions', () => {
const lineMock = {
left: null,
right: {
lineCode: 'ABC_123',
line_code: 'ABC_123',
},
};
......
......@@ -360,6 +360,16 @@ describe('DiffsStoreMutations', () => {
});
});
describe('Set highlighted row', () => {
it('sets highlighted row', () => {
const state = createState();
mutations[types.SET_HIGHLIGHTED_ROW](state, 'ABC_123');
expect(state.highlightedRow).toBe('ABC_123');
});
});
describe('TOGGLE_LINE_HAS_FORM', () => {
it('sets hasForm on lines', () => {
const file = {
......
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