Commit 828bbfd3 authored by David O'Regan's avatar David O'Regan Committed by Natalia Tepluhina

Highlight currently focused/viewed file in file tree

parent 43e236b0
......@@ -133,6 +133,7 @@ export default {
'toggleFileDiscussions',
'toggleFileDiscussionWrappers',
'toggleFullDiff',
'toggleActiveFileByHash',
]),
handleToggleFile() {
this.$emit('toggleFile');
......@@ -149,6 +150,9 @@ export default {
const selector = this.diffContentIDSelector;
scrollToElement(document.querySelector(selector));
window.location.hash = selector;
if (!this.viewDiffsFileByFile) {
this.toggleActiveFileByHash(this.diffFile.file_hash);
}
}
},
},
......
......@@ -3,9 +3,10 @@
* This component is an iterative step towards refactoring and simplifying `vue_shared/components/file_row.vue`
* https://gitlab.com/gitlab-org/gitlab/-/merge_requests/23720
*/
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import FileRow from '~/vue_shared/components/file_row.vue';
import FileRowStats from './file_row_stats.vue';
import ChangedFileIcon from '~/vue_shared/components/changed_file_icon.vue';
import FileRowStats from './file_row_stats.vue';
export default {
name: 'DiffFileRow',
......@@ -14,6 +15,7 @@ export default {
FileRowStats,
ChangedFileIcon,
},
mixins: [glFeatureFlagsMixin()],
props: {
file: {
type: Object,
......@@ -28,11 +30,28 @@ export default {
required: false,
default: null,
},
viewedFiles: {
type: Object,
required: false,
default: () => ({}),
},
},
computed: {
showFileRowStats() {
return !this.hideFileStats && this.file.type === 'blob';
},
fileClasses() {
if (!this.glFeatures.highlightCurrentDiffRow) {
return '';
}
return this.file.type === 'blob' && !this.viewedFiles[this.file.fileHash]
? 'gl-font-weight-bold'
: '';
},
isActive() {
return this.currentDiffFileId === this.file.fileHash;
},
},
};
</script>
......@@ -41,8 +60,9 @@ export default {
<file-row
:file="file"
v-bind="$attrs"
:class="{ 'is-active': currentDiffFileId === file.fileHash }"
:class="{ 'is-active': isActive }"
class="diff-file-row"
:file-classes="fileClasses"
v-on="$listeners"
>
<file-row-stats v-if="showFileRowStats" :file="file" class="mr-1" />
......
......@@ -25,7 +25,7 @@ export default {
};
},
computed: {
...mapState('diffs', ['tree', 'renderTreeList', 'currentDiffFileId']),
...mapState('diffs', ['tree', 'renderTreeList', 'currentDiffFileId', 'viewedDiffFileIds']),
...mapGetters('diffs', ['allBlobs']),
filteredTreeList() {
const search = this.search.toLowerCase().trim();
......@@ -93,6 +93,7 @@ export default {
:key="file.key"
:file="file"
:level="0"
:viewed-files="viewedDiffFileIds"
:hide-file-stats="hideFileStats"
:file-row-component="$options.DiffFileRow"
:current-diff-file-id="currentDiffFileId"
......
......@@ -84,7 +84,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
commit(types.SET_BATCH_LOADING, false);
if (!isNoteLink && !state.currentDiffFileId) {
commit(types.UPDATE_CURRENT_DIFF_FILE_ID, diff_files[0].file_hash);
commit(types.VIEW_DIFF_FILE, diff_files[0].file_hash);
}
if (isNoteLink) {
......@@ -100,7 +100,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
!state.diffFiles.some(f => f.file_hash === state.currentDiffFileId) &&
!isNoteLink
) {
commit(types.UPDATE_CURRENT_DIFF_FILE_ID, state.diffFiles[0].file_hash);
commit(types.VIEW_DIFF_FILE, state.diffFiles[0].file_hash);
}
if (gon.features?.codeNavigation) {
......@@ -183,7 +183,7 @@ export const fetchCoverageFiles = ({ commit, state }) => {
export const setHighlightedRow = ({ commit }, lineCode) => {
const fileHash = lineCode.split('_')[0];
commit(types.SET_HIGHLIGHTED_ROW, lineCode);
commit(types.UPDATE_CURRENT_DIFF_FILE_ID, fileHash);
commit(types.VIEW_DIFF_FILE, fileHash);
};
// This is adding line discussions to the actual lines in the diff tree
......@@ -428,13 +428,17 @@ export const toggleTreeOpen = ({ commit }, path) => {
commit(types.TOGGLE_FOLDER_OPEN, path);
};
export const toggleActiveFileByHash = ({ commit }, hash) => {
commit(types.VIEW_DIFF_FILE, hash);
};
export const scrollToFile = ({ state, commit }, path) => {
if (!state.treeEntries[path]) return;
const { fileHash } = state.treeEntries[path];
document.location.hash = fileHash;
commit(types.UPDATE_CURRENT_DIFF_FILE_ID, fileHash);
commit(types.VIEW_DIFF_FILE, fileHash);
};
export const toggleShowTreeList = ({ commit, state }, saving = true) => {
......@@ -702,7 +706,7 @@ export const setCurrentDiffFileIdFromNote = ({ commit, state, rootGetters }, not
const fileHash = rootGetters.getDiscussion(note.discussion_id).diff_file?.file_hash;
if (fileHash && state.diffFiles.some(f => f.file_hash === fileHash)) {
commit(types.UPDATE_CURRENT_DIFF_FILE_ID, fileHash);
commit(types.VIEW_DIFF_FILE, fileHash);
}
};
......@@ -710,5 +714,5 @@ export const navigateToDiffFileIndex = ({ commit, state }, index) => {
const fileHash = state.diffFiles[index].file_hash;
document.location.hash = fileHash;
commit(types.UPDATE_CURRENT_DIFF_FILE_ID, fileHash);
commit(types.VIEW_DIFF_FILE, fileHash);
};
......@@ -34,6 +34,7 @@ export default () => ({
showTreeList: true,
currentDiffFileId: '',
projectPath: '',
viewedDiffFileIds: {},
commentForms: [],
highlightedRow: null,
renderTreeList: true,
......
......@@ -19,7 +19,7 @@ export const SET_LINE_DISCUSSIONS_FOR_FILE = 'SET_LINE_DISCUSSIONS_FOR_FILE';
export const REMOVE_LINE_DISCUSSIONS_FOR_FILE = 'REMOVE_LINE_DISCUSSIONS_FOR_FILE';
export const TOGGLE_FOLDER_OPEN = 'TOGGLE_FOLDER_OPEN';
export const TOGGLE_SHOW_TREE_LIST = 'TOGGLE_SHOW_TREE_LIST';
export const UPDATE_CURRENT_DIFF_FILE_ID = 'UPDATE_CURRENT_DIFF_FILE_ID';
export const VIEW_DIFF_FILE = 'VIEW_DIFF_FILE';
export const OPEN_DIFF_FILE_COMMENT_FORM = 'OPEN_DIFF_FILE_COMMENT_FORM';
export const UPDATE_DIFF_FILE_COMMENT_FORM = 'UPDATE_DIFF_FILE_COMMENT_FORM';
......
import Vue from 'vue';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import { PARALLEL_DIFF_VIEW_TYPE } from '../constants';
import {
......@@ -291,8 +292,9 @@ export default {
[types.TOGGLE_SHOW_TREE_LIST](state) {
state.showTreeList = !state.showTreeList;
},
[types.UPDATE_CURRENT_DIFF_FILE_ID](state, fileId) {
[types.VIEW_DIFF_FILE](state, fileId) {
state.currentDiffFileId = fileId;
Vue.set(state.viewedDiffFileIds, fileId, true);
},
[types.OPEN_DIFF_FILE_COMMENT_FORM](state, formData) {
state.commentForms.push({
......
......@@ -18,6 +18,11 @@ export default {
type: Number,
required: true,
},
fileClasses: {
type: String,
required: false,
default: '',
},
},
computed: {
isTree() {
......@@ -123,6 +128,7 @@ export default {
:style="levelIndentation"
class="file-row-name str-truncated"
data-qa-selector="file_name_content"
:class="fileClasses"
>
<file-icon
class="file-row-icon"
......
......@@ -39,6 +39,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
push_frontend_feature_flag(:hide_jump_to_next_unresolved_in_threads, default_enabled: true)
push_frontend_feature_flag(:merge_request_widget_graphql, @project)
push_frontend_feature_flag(:unified_diff_lines, @project)
push_frontend_feature_flag(:highlight_current_diff_row, @project)
end
before_action do
......
---
title: Highlight un-focused/un-viewed file's in file tree
merge_request: 27937
author:
type: changed
......@@ -44,6 +44,7 @@ describe('DiffFileHeader component', () => {
toggleFileDiscussions: jest.fn(),
toggleFileDiscussionWrappers: jest.fn(),
toggleFullDiff: jest.fn(),
toggleActiveFileByHash: jest.fn(),
},
},
},
......
......@@ -7,9 +7,12 @@ import ChangedFileIcon from '~/vue_shared/components/changed_file_icon.vue';
describe('Diff File Row component', () => {
let wrapper;
const createComponent = (props = {}) => {
const createComponent = (props = {}, highlightCurrentDiffRow = false) => {
wrapper = shallowMount(DiffFileRow, {
propsData: { ...props },
provide: {
glFeatures: { highlightCurrentDiffRow },
},
});
};
......@@ -56,6 +59,31 @@ describe('Diff File Row component', () => {
);
});
it.each`
features | fileType | isViewed | expected
${{ highlightCurrentDiffRow: true }} | ${'blob'} | ${false} | ${'gl-font-weight-bold'}
${{}} | ${'blob'} | ${true} | ${''}
${{}} | ${'tree'} | ${false} | ${''}
${{}} | ${'tree'} | ${true} | ${''}
`(
'with (features="$features", fileType="$fileType", isViewed=$isViewed), sets fileClasses="$expected"',
({ features, fileType, isViewed, expected }) => {
createComponent(
{
file: {
type: fileType,
fileHash: '#123456789',
},
level: 0,
hideFileStats: false,
viewedFiles: isViewed ? { '#123456789': true } : {},
},
features.highlightCurrentDiffRow,
);
expect(wrapper.find(FileRow).props('fileClasses')).toBe(expected);
},
);
describe('FileRowStats components', () => {
it.each`
type | hideFileStats | value | desc
......
import Vuex from 'vuex';
import { mount, createLocalVue } from '@vue/test-utils';
import { shallowMount, mount, createLocalVue } from '@vue/test-utils';
import TreeList from '~/diffs/components/tree_list.vue';
import createStore from '~/diffs/store/modules';
import FileTree from '~/vue_shared/components/file_tree.vue';
describe('Diffs tree list component', () => {
let wrapper;
let store;
const getFileRows = () => wrapper.findAll('.file-row');
const localVue = createLocalVue();
localVue.use(Vuex);
const createComponent = state => {
const store = new Vuex.Store({
const createComponent = (mountFn = mount) => {
wrapper = mountFn(TreeList, {
store,
localVue,
propsData: { hideFileStats: false },
});
};
beforeEach(() => {
store = new Vuex.Store({
modules: {
diffs: createStore(),
},
......@@ -23,61 +33,57 @@ describe('Diffs tree list component', () => {
addedLines: 10,
removedLines: 20,
...store.state.diffs,
...state,
};
});
wrapper = mount(TreeList, {
store,
localVue,
propsData: { hideFileStats: false },
const setupFilesInState = () => {
const treeEntries = {
'index.js': {
addedLines: 0,
changed: true,
deleted: false,
fileHash: 'test',
key: 'index.js',
name: 'index.js',
path: 'app/index.js',
removedLines: 0,
tempFile: true,
type: 'blob',
parentPath: 'app',
},
app: {
key: 'app',
path: 'app',
name: 'app',
type: 'tree',
tree: [],
},
};
Object.assign(store.state.diffs, {
treeEntries,
tree: [treeEntries['index.js'], treeEntries.app],
});
};
beforeEach(() => {
localStorage.removeItem('mr_diff_tree_list');
createComponent();
});
afterEach(() => {
wrapper.destroy();
});
it('renders empty text', () => {
expect(wrapper.text()).toContain('No files found');
describe('default', () => {
beforeEach(() => {
createComponent();
});
it('renders empty text', () => {
expect(wrapper.text()).toContain('No files found');
});
});
describe('with files', () => {
beforeEach(() => {
const treeEntries = {
'index.js': {
addedLines: 0,
changed: true,
deleted: false,
fileHash: 'test',
key: 'index.js',
name: 'index.js',
path: 'app/index.js',
removedLines: 0,
tempFile: true,
type: 'blob',
parentPath: 'app',
},
app: {
key: 'app',
path: 'app',
name: 'app',
type: 'tree',
tree: [],
},
};
createComponent({
treeEntries,
tree: [treeEntries['index.js'], treeEntries.app],
});
return wrapper.vm.$nextTick();
setupFilesInState();
createComponent();
});
it('renders tree', () => {
......@@ -136,4 +142,23 @@ describe('Diffs tree list component', () => {
});
});
});
describe('with viewedDiffFileIds', () => {
const viewedDiffFileIds = { fileId: '#12345' };
beforeEach(() => {
setupFilesInState();
store.state.diffs.viewedDiffFileIds = viewedDiffFileIds;
});
it('passes the viewedDiffFileIds to the FileTree', () => {
createComponent(shallowMount);
return wrapper.vm.$nextTick().then(() => {
// Have to use $attrs['viewed-files'] because we are passing down an object
// and attributes('') stringifies values (e.g. [object])...
expect(wrapper.find(FileTree).vm.$attrs['viewed-files']).toBe(viewedDiffFileIds);
});
});
});
});
......@@ -191,10 +191,10 @@ describe('DiffsStoreActions', () => {
{ type: types.SET_RETRIEVING_BATCHES, payload: true },
{ type: types.SET_DIFF_DATA_BATCH, payload: { diff_files: res1.diff_files } },
{ type: types.SET_BATCH_LOADING, payload: false },
{ type: types.UPDATE_CURRENT_DIFF_FILE_ID, payload: 'test' },
{ type: types.VIEW_DIFF_FILE, payload: 'test' },
{ type: types.SET_DIFF_DATA_BATCH, payload: { diff_files: res2.diff_files } },
{ type: types.SET_BATCH_LOADING, payload: false },
{ type: types.UPDATE_CURRENT_DIFF_FILE_ID, payload: 'test2' },
{ type: types.VIEW_DIFF_FILE, payload: 'test2' },
{ type: types.SET_RETRIEVING_BATCHES, payload: false },
],
[],
......@@ -300,7 +300,7 @@ describe('DiffsStoreActions', () => {
it('should mark currently selected diff and set lineHash and fileHash of highlightedRow', () => {
testAction(setHighlightedRow, 'ABC_123', {}, [
{ type: types.SET_HIGHLIGHTED_ROW, payload: 'ABC_123' },
{ type: types.UPDATE_CURRENT_DIFF_FILE_ID, payload: 'ABC' },
{ type: types.VIEW_DIFF_FILE, payload: 'ABC' },
]);
});
});
......@@ -904,7 +904,7 @@ describe('DiffsStoreActions', () => {
expect(document.location.hash).toBe('#test');
});
it('commits UPDATE_CURRENT_DIFF_FILE_ID', () => {
it('commits VIEW_DIFF_FILE', () => {
const state = {
treeEntries: {
path: {
......@@ -915,7 +915,7 @@ describe('DiffsStoreActions', () => {
scrollToFile({ state, commit }, 'path');
expect(commit).toHaveBeenCalledWith(types.UPDATE_CURRENT_DIFF_FILE_ID, 'test');
expect(commit).toHaveBeenCalledWith(types.VIEW_DIFF_FILE, 'test');
});
});
......@@ -1413,7 +1413,7 @@ describe('DiffsStoreActions', () => {
});
describe('setCurrentDiffFileIdFromNote', () => {
it('commits UPDATE_CURRENT_DIFF_FILE_ID', () => {
it('commits VIEW_DIFF_FILE', () => {
const commit = jest.fn();
const state = { diffFiles: [{ file_hash: '123' }] };
const rootGetters = {
......@@ -1423,10 +1423,10 @@ describe('DiffsStoreActions', () => {
setCurrentDiffFileIdFromNote({ commit, state, rootGetters }, '1');
expect(commit).toHaveBeenCalledWith(types.UPDATE_CURRENT_DIFF_FILE_ID, '123');
expect(commit).toHaveBeenCalledWith(types.VIEW_DIFF_FILE, '123');
});
it('does not commit UPDATE_CURRENT_DIFF_FILE_ID when discussion has no diff_file', () => {
it('does not commit VIEW_DIFF_FILE when discussion has no diff_file', () => {
const commit = jest.fn();
const state = { diffFiles: [{ file_hash: '123' }] };
const rootGetters = {
......@@ -1439,7 +1439,7 @@ describe('DiffsStoreActions', () => {
expect(commit).not.toHaveBeenCalled();
});
it('does not commit UPDATE_CURRENT_DIFF_FILE_ID when diff file does not exist', () => {
it('does not commit VIEW_DIFF_FILE when diff file does not exist', () => {
const commit = jest.fn();
const state = { diffFiles: [{ file_hash: '123' }] };
const rootGetters = {
......@@ -1454,12 +1454,12 @@ describe('DiffsStoreActions', () => {
});
describe('navigateToDiffFileIndex', () => {
it('commits UPDATE_CURRENT_DIFF_FILE_ID', done => {
it('commits VIEW_DIFF_FILE', done => {
testAction(
navigateToDiffFileIndex,
0,
{ diffFiles: [{ file_hash: '123' }] },
[{ type: types.UPDATE_CURRENT_DIFF_FILE_ID, payload: '123' }],
[{ type: types.VIEW_DIFF_FILE, payload: '123' }],
[],
done,
);
......
......@@ -737,11 +737,11 @@ describe('DiffsStoreMutations', () => {
});
});
describe('UPDATE_CURRENT_DIFF_FILE_ID', () => {
describe('VIEW_DIFF_FILE', () => {
it('updates currentDiffFileId', () => {
const state = createState();
mutations[types.UPDATE_CURRENT_DIFF_FILE_ID](state, 'somefileid');
mutations[types.VIEW_DIFF_FILE](state, 'somefileid');
expect(state.currentDiffFileId).toBe('somefileid');
});
......
......@@ -139,4 +139,16 @@ describe('File row component', () => {
expect(wrapper.vm.hasUrlAtCurrentRoute()).toBe(true);
});
it('render with the correct file classes prop', () => {
createComponent({
file: {
...file(),
},
level: 0,
fileClasses: 'font-weight-bold',
});
expect(wrapper.find('.file-row-name').classes()).toContain('font-weight-bold');
});
});
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