Commit ca2de985 authored by Phil Hughes's avatar Phil Hughes

Load code navigation for merge request diffs

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/119036
parent a6373205
<script> <script>
import { mapActions, mapState } from 'vuex'; import { mapActions, mapState } from 'vuex';
import Popover from './popover.vue'; import Popover from './popover.vue';
import eventHub from '../../notes/event_hub';
export default { export default {
components: { components: {
...@@ -10,24 +11,27 @@ export default { ...@@ -10,24 +11,27 @@ export default {
...mapState(['currentDefinition', 'currentDefinitionPosition', 'definitionPathPrefix']), ...mapState(['currentDefinition', 'currentDefinitionPosition', 'definitionPathPrefix']),
}, },
mounted() { mounted() {
this.blobViewer = document.querySelector('.blob-viewer'); this.body = document.body;
eventHub.$on('showBlobInteractionZones', this.showBlobInteractionZones);
this.addGlobalEventListeners(); this.addGlobalEventListeners();
this.fetchData(); this.fetchData();
}, },
beforeDestroy() { beforeDestroy() {
eventHub.$off('showBlobInteractionZones', this.showBlobInteractionZones);
this.removeGlobalEventListeners(); this.removeGlobalEventListeners();
}, },
methods: { methods: {
...mapActions(['fetchData', 'showDefinition']), ...mapActions(['fetchData', 'showDefinition', 'showBlobInteractionZones']),
addGlobalEventListeners() { addGlobalEventListeners() {
if (this.blobViewer) { if (this.body) {
this.blobViewer.addEventListener('click', this.showDefinition); this.body.addEventListener('click', this.showDefinition);
} }
}, },
removeGlobalEventListeners() { removeGlobalEventListeners() {
if (this.blobViewer) { if (this.body) {
this.blobViewer.removeEventListener('click', this.showDefinition); this.body.removeEventListener('click', this.showDefinition);
} }
}, },
}, },
......
...@@ -5,10 +5,10 @@ import App from './components/app.vue'; ...@@ -5,10 +5,10 @@ import App from './components/app.vue';
Vue.use(Vuex); Vue.use(Vuex);
export default () => { export default initialData => {
const el = document.getElementById('js-code-navigation'); const el = document.getElementById('js-code-navigation');
store.dispatch('setInitialData', el.dataset); store.dispatch('setInitialData', initialData);
return new Vue({ return new Vue({
el, el,
......
...@@ -12,20 +12,25 @@ export default { ...@@ -12,20 +12,25 @@ export default {
fetchData({ commit, dispatch, state }) { fetchData({ commit, dispatch, state }) {
commit(types.REQUEST_DATA); commit(types.REQUEST_DATA);
axios state.blobs.forEach(({ path, codeNavigationPath }) => {
.get(state.codeNavUrl) axios
.then(({ data }) => { .get(codeNavigationPath)
const normalizedData = data.reduce((acc, d) => { .then(({ data }) => {
if (d.hover) { const normalizedData = data.reduce((acc, d) => {
acc[`${d.start_line}:${d.start_char}`] = d; if (d.hover) {
addInteractionClass(d); acc[`${d.start_line}:${d.start_char}`] = d;
} addInteractionClass(path, d);
return acc; }
}, {}); return acc;
}, {});
commit(types.REQUEST_DATA_SUCCESS, normalizedData);
}) commit(types.REQUEST_DATA_SUCCESS, { path, normalizedData });
.catch(() => dispatch('requestDataError')); })
.catch(() => dispatch('requestDataError'));
});
},
showBlobInteractionZones({ state }, path) {
Object.values(state.data[path]).forEach(d => addInteractionClass(path, d));
}, },
showDefinition({ commit, state }, { target: el }) { showDefinition({ commit, state }, { target: el }) {
let definition; let definition;
...@@ -39,15 +44,28 @@ export default { ...@@ -39,15 +44,28 @@ export default {
getCurrentHoverElement().classList.remove('hll'); getCurrentHoverElement().classList.remove('hll');
} }
if (el.classList.contains('js-code-navigation') && !isCurrentElementPopoverOpen) { const blobEl = el.closest('[data-path]');
if (!blobEl) {
commit(types.SET_CURRENT_DEFINITION, { definition, position });
return;
}
const data = state.data[blobEl.dataset.path];
if (!data) return;
if (el.closest('.js-code-navigation') && !isCurrentElementPopoverOpen) {
const { lineIndex, charIndex } = el.dataset; const { lineIndex, charIndex } = el.dataset;
const { x, y } = el.getBoundingClientRect();
position = { position = {
x: el.offsetLeft, x: x || 0,
y: el.offsetTop, y: y + window.scrollY || 0,
height: el.offsetHeight, height: el.offsetHeight,
}; };
definition = state.data[`${lineIndex}:${charIndex}`]; definition = data[`${lineIndex}:${charIndex}`];
el.classList.add('hll'); el.classList.add('hll');
......
import * as types from './mutation_types'; import * as types from './mutation_types';
export default { export default {
[types.SET_INITIAL_DATA](state, { codeNavUrl, definitionPathPrefix }) { [types.SET_INITIAL_DATA](state, { blobs, definitionPathPrefix }) {
state.codeNavUrl = codeNavUrl; state.blobs = blobs;
state.definitionPathPrefix = definitionPathPrefix; state.definitionPathPrefix = definitionPathPrefix;
}, },
[types.REQUEST_DATA](state) { [types.REQUEST_DATA](state) {
state.loading = true; state.loading = true;
}, },
[types.REQUEST_DATA_SUCCESS](state, data) { [types.REQUEST_DATA_SUCCESS](state, { path, normalizedData }) {
state.loading = false; state.loading = false;
state.data = data; state.data = { ...state.data, [path]: normalizedData };
}, },
[types.REQUEST_DATA_ERROR](state) { [types.REQUEST_DATA_ERROR](state) {
state.loading = false; state.loading = false;
......
export default () => ({ export default () => ({
projectPath: null, blobs: [],
commitId: null,
blobPath: null,
loading: false, loading: false,
data: null, data: null,
currentDefinition: null, currentDefinition: null,
......
...@@ -3,18 +3,25 @@ export const cachedData = new Map(); ...@@ -3,18 +3,25 @@ export const cachedData = new Map();
export const getCurrentHoverElement = () => cachedData.get('current'); export const getCurrentHoverElement = () => cachedData.get('current');
export const setCurrentHoverElement = el => cachedData.set('current', el); export const setCurrentHoverElement = el => cachedData.set('current', el);
export const addInteractionClass = d => { export const addInteractionClass = (path, d) => {
let charCount = 0; const lineNumber = d.start_line + 1;
const line = document.getElementById(`LC${d.start_line + 1}`); const lines = document
const el = [...line.childNodes].find(({ textContent }) => { .querySelector(`[data-path="${path}"]`)
if (charCount === d.start_char) return true; .querySelectorAll(`.blob-content #LC${lineNumber}, .line_content:not(.old) #LC${lineNumber}`);
charCount += textContent.length; if (!lines?.length) return;
return false;
}); lines.forEach(line => {
let charCount = 0;
const el = [...line.childNodes].find(({ textContent }) => {
if (charCount === d.start_char) return true;
charCount += textContent.length;
return false;
});
if (el) { if (el) {
el.setAttribute('data-char-index', d.start_char); el.setAttribute('data-char-index', d.start_char);
el.setAttribute('data-line-index', d.start_line); el.setAttribute('data-line-index', d.start_line);
el.classList.add('cursor-pointer', 'code-navigation', 'js-code-navigation'); el.classList.add('cursor-pointer', 'code-navigation', 'js-code-navigation');
} }
});
}; };
...@@ -12,6 +12,7 @@ import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_ ...@@ -12,6 +12,7 @@ import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_
import NoteForm from '../../notes/components/note_form.vue'; import NoteForm from '../../notes/components/note_form.vue';
import ImageDiffOverlay from './image_diff_overlay.vue'; import ImageDiffOverlay from './image_diff_overlay.vue';
import DiffDiscussions from './diff_discussions.vue'; import DiffDiscussions from './diff_discussions.vue';
import eventHub from '../../notes/event_hub';
import { IMAGE_DIFF_POSITION_TYPE } from '../constants'; import { IMAGE_DIFF_POSITION_TYPE } from '../constants';
import { getDiffMode } from '../store/utils'; import { getDiffMode } from '../store/utils';
import { diffViewerModes } from '~/ide/constants'; import { diffViewerModes } from '~/ide/constants';
...@@ -77,6 +78,13 @@ export default { ...@@ -77,6 +78,13 @@ export default {
return this.getUserData; return this.getUserData;
}, },
}, },
updated() {
if (window.gon?.features?.codeNavigation) {
this.$nextTick(() => {
eventHub.$emit('showBlobInteractionZones', this.diffFile.new_path);
});
}
},
methods: { methods: {
...mapActions('diffs', ['saveDiffDiscussion', 'closeDiffFileCommentForm']), ...mapActions('diffs', ['saveDiffDiscussion', 'closeDiffFileCommentForm']),
handleSaveNote(note) { handleSaveNote(note) {
......
...@@ -145,6 +145,7 @@ export default { ...@@ -145,6 +145,7 @@ export default {
:class="{ :class="{
'is-active': currentDiffFileId === file.file_hash, 'is-active': currentDiffFileId === file.file_hash,
}" }"
:data-path="file.new_path"
class="diff-file file-holder" class="diff-file file-holder"
> >
<diff-file-header <diff-file-header
......
...@@ -129,6 +129,18 @@ export const fetchDiffFilesBatch = ({ commit, state }) => { ...@@ -129,6 +129,18 @@ export const fetchDiffFilesBatch = ({ commit, state }) => {
if (!pagination.next_page) { if (!pagination.next_page) {
commit(types.SET_RETRIEVING_BATCHES, false); commit(types.SET_RETRIEVING_BATCHES, false);
if (gon.features?.codeNavigation) {
// eslint-disable-next-line promise/catch-or-return,promise/no-nesting
import('~/code_navigation').then(m =>
m.default({
blobs: state.diffFiles.map(f => ({
path: f.new_path,
codeNavigationPath: f.code_navigation_path,
})),
definitionPathPrefix: state.definitionPathPrefix,
}),
);
}
} }
return pagination.next_page; return pagination.next_page;
......
...@@ -33,8 +33,16 @@ document.addEventListener('DOMContentLoaded', () => { ...@@ -33,8 +33,16 @@ document.addEventListener('DOMContentLoaded', () => {
GpgBadges.fetch(); GpgBadges.fetch();
if (gon.features?.codeNavigation) { if (gon.features?.codeNavigation) {
const el = document.getElementById('js-code-navigation');
const { codeNavigationPath, blobPath, definitionPathPrefix } = el.dataset;
// eslint-disable-next-line promise/catch-or-return // eslint-disable-next-line promise/catch-or-return
import('~/code_navigation').then(m => m.default()); import('~/code_navigation').then(m =>
m.default({
blobs: [{ path: blobPath, codeNavigationPath }],
definitionPathPrefix,
}),
);
} }
if (gon.features?.suggestPipeline) { if (gon.features?.suggestPipeline) {
......
...@@ -23,6 +23,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -23,6 +23,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
push_frontend_feature_flag(:deploy_from_footer, @project, default_enabled: true) push_frontend_feature_flag(:deploy_from_footer, @project, default_enabled: true)
push_frontend_feature_flag(:single_mr_diff_view, @project, default_enabled: true) push_frontend_feature_flag(:single_mr_diff_view, @project, default_enabled: true)
push_frontend_feature_flag(:suggest_pipeline) if experiment_enabled?(:suggest_pipeline) push_frontend_feature_flag(:suggest_pipeline) if experiment_enabled?(:suggest_pipeline)
push_frontend_feature_flag(:code_navigation, @project)
end end
before_action do before_action do
......
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
#blob-content-holder.blob-content-holder #blob-content-holder.blob-content-holder
- if @code_navigation_path - if @code_navigation_path
#js-code-navigation{ data: { code_nav_url: @code_navigation_path, definition_path_prefix: project_blob_path(@project, @ref) } } #js-code-navigation{ data: { code_navigation_path: @code_navigation_path, blob_path: blob.path, definition_path_prefix: project_blob_path(@project, @ref) } }
%article.file-holder %article.file-holder
= render 'projects/blob/header', blob: blob = render 'projects/blob/header', blob: blob
= render 'projects/blob/content', blob: blob = render 'projects/blob/content', blob: blob
...@@ -51,6 +51,8 @@ ...@@ -51,6 +51,8 @@
.tab-content#diff-notes-app .tab-content#diff-notes-app
#js-diff-file-finder #js-diff-file-finder
- if native_code_navigation_enabled?(@project)
#js-code-navigation
= render "projects/merge_requests/tabs/pane", id: "notes", class: "notes voting_notes" do = render "projects/merge_requests/tabs/pane", id: "notes", class: "notes voting_notes" do
.row .row
%section.col-md-12 %section.col-md-12
......
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
%a.diff-line-num{ href: "#{link}#L#{i}", id: "L#{i}", 'data-line-number' => i } %a.diff-line-num{ href: "#{link}#L#{i}", id: "L#{i}", 'data-line-number' => i }
= link_icon = link_icon
= i = i
.blob-content{ data: { blob_id: blob.id } } .blob-content{ data: { blob_id: blob.id, path: blob.path } }
%pre.code.highlight %pre.code.highlight
%code %code
= blob.present.highlight = blob.present.highlight
...@@ -28,9 +28,9 @@ describe('Code navigation actions', () => { ...@@ -28,9 +28,9 @@ describe('Code navigation actions', () => {
describe('fetchData', () => { describe('fetchData', () => {
let mock; let mock;
const codeNavUrl = const codeNavigationPath =
'gitlab-org/gitlab-shell/-/jobs/1114/artifacts/raw/lsif/cmd/check/main.go.json'; 'gitlab-org/gitlab-shell/-/jobs/1114/artifacts/raw/lsif/cmd/check/main.go.json';
const state = { codeNavUrl }; const state = { blobs: [{ path: 'index.js', codeNavigationPath }] };
beforeEach(() => { beforeEach(() => {
window.gon = { api_version: '1' }; window.gon = { api_version: '1' };
...@@ -43,7 +43,7 @@ describe('Code navigation actions', () => { ...@@ -43,7 +43,7 @@ describe('Code navigation actions', () => {
describe('success', () => { describe('success', () => {
beforeEach(() => { beforeEach(() => {
mock.onGet(codeNavUrl).replyOnce(200, [ mock.onGet(codeNavigationPath).replyOnce(200, [
{ {
start_line: 0, start_line: 0,
start_char: 0, start_char: 0,
...@@ -66,7 +66,12 @@ describe('Code navigation actions', () => { ...@@ -66,7 +66,12 @@ describe('Code navigation actions', () => {
{ type: 'REQUEST_DATA' }, { type: 'REQUEST_DATA' },
{ {
type: 'REQUEST_DATA_SUCCESS', type: 'REQUEST_DATA_SUCCESS',
payload: { '0:0': { start_line: 0, start_char: 0, hover: { value: '123' } } }, payload: {
path: 'index.js',
normalizedData: {
'0:0': { start_line: 0, start_char: 0, hover: { value: '123' } },
},
},
}, },
], ],
[], [],
...@@ -83,13 +88,18 @@ describe('Code navigation actions', () => { ...@@ -83,13 +88,18 @@ describe('Code navigation actions', () => {
{ type: 'REQUEST_DATA' }, { type: 'REQUEST_DATA' },
{ {
type: 'REQUEST_DATA_SUCCESS', type: 'REQUEST_DATA_SUCCESS',
payload: { '0:0': { start_line: 0, start_char: 0, hover: { value: '123' } } }, payload: {
path: 'index.js',
normalizedData: {
'0:0': { start_line: 0, start_char: 0, hover: { value: '123' } },
},
},
}, },
], ],
[], [],
) )
.then(() => { .then(() => {
expect(addInteractionClass).toHaveBeenCalledWith({ expect(addInteractionClass).toHaveBeenCalledWith('index.js', {
start_line: 0, start_line: 0,
start_char: 0, start_char: 0,
hover: { value: '123' }, hover: { value: '123' },
...@@ -102,7 +112,7 @@ describe('Code navigation actions', () => { ...@@ -102,7 +112,7 @@ describe('Code navigation actions', () => {
describe('error', () => { describe('error', () => {
beforeEach(() => { beforeEach(() => {
mock.onGet(codeNavUrl).replyOnce(500); mock.onGet(codeNavigationPath).replyOnce(500);
}); });
it('dispatches requestDataError', done => { it('dispatches requestDataError', done => {
...@@ -118,11 +128,29 @@ describe('Code navigation actions', () => { ...@@ -118,11 +128,29 @@ describe('Code navigation actions', () => {
}); });
}); });
describe('showBlobInteractionZones', () => {
it('calls addInteractionClass with data for a path', () => {
const state = {
data: {
'index.js': { '0:0': 'test', '1:1': 'console.log' },
},
};
actions.showBlobInteractionZones({ state }, 'index.js');
expect(addInteractionClass).toHaveBeenCalled();
expect(addInteractionClass.mock.calls.length).toBe(2);
expect(addInteractionClass.mock.calls[0]).toEqual(['index.js', 'test']);
expect(addInteractionClass.mock.calls[1]).toEqual(['index.js', 'console.log']);
});
});
describe('showDefinition', () => { describe('showDefinition', () => {
let target; let target;
beforeEach(() => { beforeEach(() => {
target = document.createElement('div'); setFixtures('<div data-path="index.js"><div class="js-test"></div></div>');
target = document.querySelector('.js-test');
}); });
it('returns early when no data exists', done => { it('returns early when no data exists', done => {
...@@ -130,19 +158,7 @@ describe('Code navigation actions', () => { ...@@ -130,19 +158,7 @@ describe('Code navigation actions', () => {
}); });
it('commits SET_CURRENT_DEFINITION when target is not code navitation element', done => { it('commits SET_CURRENT_DEFINITION when target is not code navitation element', done => {
testAction( testAction(actions.showDefinition, { target }, { data: {} }, [], [], done);
actions.showDefinition,
{ target },
{ data: {} },
[
{
type: 'SET_CURRENT_DEFINITION',
payload: { definition: undefined, position: undefined },
},
],
[],
done,
);
}); });
it('commits SET_CURRENT_DEFINITION with LSIF data', done => { it('commits SET_CURRENT_DEFINITION with LSIF data', done => {
...@@ -153,7 +169,7 @@ describe('Code navigation actions', () => { ...@@ -153,7 +169,7 @@ describe('Code navigation actions', () => {
testAction( testAction(
actions.showDefinition, actions.showDefinition,
{ target }, { target },
{ data: { '0:0': { hover: 'test' } } }, { data: { 'index.js': { '0:0': { hover: 'test' } } } },
[ [
{ {
type: 'SET_CURRENT_DEFINITION', type: 'SET_CURRENT_DEFINITION',
...@@ -173,7 +189,7 @@ describe('Code navigation actions', () => { ...@@ -173,7 +189,7 @@ describe('Code navigation actions', () => {
return testAction( return testAction(
actions.showDefinition, actions.showDefinition,
{ target }, { target },
{ data: { '0:0': { hover: 'test' } } }, { data: { 'index.js': { '0:0': { hover: 'test' } } } },
[ [
{ {
type: 'SET_CURRENT_DEFINITION', type: 'SET_CURRENT_DEFINITION',
...@@ -194,7 +210,7 @@ describe('Code navigation actions', () => { ...@@ -194,7 +210,7 @@ describe('Code navigation actions', () => {
return testAction( return testAction(
actions.showDefinition, actions.showDefinition,
{ target }, { target },
{ data: { '0:0': { hover: 'test' } } }, { data: { 'index.js': { '0:0': { hover: 'test' } } } },
[ [
{ {
type: 'SET_CURRENT_DEFINITION', type: 'SET_CURRENT_DEFINITION',
......
...@@ -11,11 +11,11 @@ describe('Code navigation mutations', () => { ...@@ -11,11 +11,11 @@ describe('Code navigation mutations', () => {
describe('SET_INITIAL_DATA', () => { describe('SET_INITIAL_DATA', () => {
it('sets initial data', () => { it('sets initial data', () => {
mutations.SET_INITIAL_DATA(state, { mutations.SET_INITIAL_DATA(state, {
codeNavUrl: 'https://test.com/builds/1005', blobs: ['test'],
definitionPathPrefix: 'https://test.com/blob/master', definitionPathPrefix: 'https://test.com/blob/master',
}); });
expect(state.codeNavUrl).toBe('https://test.com/builds/1005'); expect(state.blobs).toEqual(['test']);
expect(state.definitionPathPrefix).toBe('https://test.com/blob/master'); expect(state.definitionPathPrefix).toBe('https://test.com/blob/master');
}); });
}); });
...@@ -36,9 +36,9 @@ describe('Code navigation mutations', () => { ...@@ -36,9 +36,9 @@ describe('Code navigation mutations', () => {
}); });
it('sets data', () => { it('sets data', () => {
mutations.REQUEST_DATA_SUCCESS(state, ['test']); mutations.REQUEST_DATA_SUCCESS(state, { path: 'index.js', normalizedData: ['test'] });
expect(state.data).toEqual(['test']); expect(state.data).toEqual({ 'index.js': ['test'] });
}); });
}); });
......
...@@ -36,7 +36,7 @@ describe('setCurrentHoverElement', () => { ...@@ -36,7 +36,7 @@ describe('setCurrentHoverElement', () => {
describe('addInteractionClass', () => { describe('addInteractionClass', () => {
beforeEach(() => { beforeEach(() => {
setFixtures( setFixtures(
'<div id="LC1"><span>console</span><span>.</span><span>log</span></div><div id="LC2"><span>function</span></div>', '<div data-path="index.js"><div class="blob-content"><div id="LC1"><span>console</span><span>.</span><span>log</span></div><div id="LC2"><span>function</span></div></div></div>',
); );
}); });
...@@ -48,7 +48,7 @@ describe('addInteractionClass', () => { ...@@ -48,7 +48,7 @@ describe('addInteractionClass', () => {
`( `(
'it sets code navigation attributes for line $line and character $char', 'it sets code navigation attributes for line $line and character $char',
({ line, char, index }) => { ({ line, char, index }) => {
addInteractionClass({ start_line: line, start_char: char }); addInteractionClass('index.js', { start_line: line, start_char: char });
expect(document.querySelectorAll(`#LC${line + 1} span`)[index].classList).toContain( expect(document.querySelectorAll(`#LC${line + 1} span`)[index].classList).toContain(
'js-code-navigation', 'js-code-navigation',
......
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