Merge branch 'ph/119036/codeNavDiffs' into 'master'

Load code navigation for merge request diffs

Closes #119036

See merge request gitlab-org/gitlab!28083

app/assets/javascripts/code_navigation/components/app.vue

 <script>
 import { mapActions, mapState } from 'vuex';
 import Popover from './popover.vue';
+import eventHub from '../../notes/event_hub';
 
 export default {
   components: {
@@ -10,24 +11,27 @@ export default {
     ...mapState(['currentDefinition', 'currentDefinitionPosition', 'definitionPathPrefix']),
   },
   mounted() {
-    this.blobViewer = document.querySelector('.blob-viewer');
+    this.body = document.body;
+
+    eventHub.$on('showBlobInteractionZones', this.showBlobInteractionZones);
 
     this.addGlobalEventListeners();
     this.fetchData();
   },
   beforeDestroy() {
+    eventHub.$off('showBlobInteractionZones', this.showBlobInteractionZones);
     this.removeGlobalEventListeners();
   },
   methods: {
-    ...mapActions(['fetchData', 'showDefinition']),
+    ...mapActions(['fetchData', 'showDefinition', 'showBlobInteractionZones']),
     addGlobalEventListeners() {
-      if (this.blobViewer) {
-        this.blobViewer.addEventListener('click', this.showDefinition);
+      if (this.body) {
+        this.body.addEventListener('click', this.showDefinition);
       }
     },
     removeGlobalEventListeners() {
-      if (this.blobViewer) {
-        this.blobViewer.removeEventListener('click', this.showDefinition);
+      if (this.body) {
+        this.body.removeEventListener('click', this.showDefinition);
       }
     },
   },

app/assets/javascripts/code_navigation/index.js

@@ -5,10 +5,10 @@ import App from './components/app.vue';
 
 Vue.use(Vuex);
 
-export default () => {
+export default initialData => {
   const el = document.getElementById('js-code-navigation');
 
-  store.dispatch('setInitialData', el.dataset);
+  store.dispatch('setInitialData', initialData);
 
   return new Vue({
     el,

app/assets/javascripts/code_navigation/store/actions.js

@@ -12,20 +12,25 @@ export default {
   fetchData({ commit, dispatch, state }) {
     commit(types.REQUEST_DATA);
 
-    axios
-      .get(state.codeNavUrl)
-      .then(({ data }) => {
-        const normalizedData = data.reduce((acc, d) => {
-          if (d.hover) {
-            acc[`${d.start_line}:${d.start_char}`] = d;
-            addInteractionClass(d);
-          }
-          return acc;
-        }, {});
-
-        commit(types.REQUEST_DATA_SUCCESS, normalizedData);
-      })
-      .catch(() => dispatch('requestDataError'));
+    state.blobs.forEach(({ path, codeNavigationPath }) => {
+      axios
+        .get(codeNavigationPath)
+        .then(({ data }) => {
+          const normalizedData = data.reduce((acc, d) => {
+            if (d.hover) {
+              acc[`${d.start_line}:${d.start_char}`] = d;
+              addInteractionClass(path, d);
+            }
+            return acc;
+          }, {});
+
+          commit(types.REQUEST_DATA_SUCCESS, { path, normalizedData });
+        })
+        .catch(() => dispatch('requestDataError'));
+    });
+  },
+  showBlobInteractionZones({ state }, path) {
+    Object.values(state.data[path]).forEach(d => addInteractionClass(path, d));
   },
   showDefinition({ commit, state }, { target: el }) {
     let definition;
@@ -39,15 +44,28 @@ export default {
       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 { x, y } = el.getBoundingClientRect();
 
       position = {
-        x: el.offsetLeft,
-        y: el.offsetTop,
+        x: x || 0,
+        y: y + window.scrollY || 0,
         height: el.offsetHeight,
       };
-      definition = state.data[`${lineIndex}:${charIndex}`];
+      definition = data[`${lineIndex}:${charIndex}`];
 
       el.classList.add('hll');
 

app/assets/javascripts/code_navigation/store/mutations.js

 import * as types from './mutation_types';
 
 export default {
-  [types.SET_INITIAL_DATA](state, { codeNavUrl, definitionPathPrefix }) {
-    state.codeNavUrl = codeNavUrl;
+  [types.SET_INITIAL_DATA](state, { blobs, definitionPathPrefix }) {
+    state.blobs = blobs;
     state.definitionPathPrefix = definitionPathPrefix;
   },
   [types.REQUEST_DATA](state) {
     state.loading = true;
   },
-  [types.REQUEST_DATA_SUCCESS](state, data) {
+  [types.REQUEST_DATA_SUCCESS](state, { path, normalizedData }) {
     state.loading = false;
-    state.data = data;
+    state.data = { ...state.data, [path]: normalizedData };
   },
   [types.REQUEST_DATA_ERROR](state) {
     state.loading = false;

app/assets/javascripts/code_navigation/store/state.js

 export default () => ({
-  projectPath: null,
-  commitId: null,
-  blobPath: null,
+  blobs: [],
   loading: false,
   data: null,
   currentDefinition: null,

app/assets/javascripts/code_navigation/utils/index.js

@@ -3,18 +3,25 @@ export const cachedData = new Map();
 export const getCurrentHoverElement = () => cachedData.get('current');
 export const setCurrentHoverElement = el => cachedData.set('current', el);
 
-export const addInteractionClass = d => {
-  let charCount = 0;
-  const line = document.getElementById(`LC${d.start_line + 1}`);
-  const el = [...line.childNodes].find(({ textContent }) => {
-    if (charCount === d.start_char) return true;
-    charCount += textContent.length;
-    return false;
-  });
+export const addInteractionClass = (path, d) => {
+  const lineNumber = d.start_line + 1;
+  const lines = document
+    .querySelector(`[data-path="${path}"]`)
+    .querySelectorAll(`.blob-content #LC${lineNumber}, .line_content:not(.old) #LC${lineNumber}`);
+  if (!lines?.length) return;
+
+  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) {
-    el.setAttribute('data-char-index', d.start_char);
-    el.setAttribute('data-line-index', d.start_line);
-    el.classList.add('cursor-pointer', 'code-navigation', 'js-code-navigation');
-  }
+    if (el) {
+      el.setAttribute('data-char-index', d.start_char);
+      el.setAttribute('data-line-index', d.start_line);
+      el.classList.add('cursor-pointer', 'code-navigation', 'js-code-navigation');
+    }
+  });
 };

app/assets/javascripts/diffs/components/diff_content.vue

@@ -12,6 +12,7 @@ import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_
 import NoteForm from '../../notes/components/note_form.vue';
 import ImageDiffOverlay from './image_diff_overlay.vue';
 import DiffDiscussions from './diff_discussions.vue';
+import eventHub from '../../notes/event_hub';
 import { IMAGE_DIFF_POSITION_TYPE } from '../constants';
 import { getDiffMode } from '../store/utils';
 import { diffViewerModes } from '~/ide/constants';
@@ -77,6 +78,13 @@ export default {
       return this.getUserData;
     },
   },
+  updated() {
+    if (window.gon?.features?.codeNavigation) {
+      this.$nextTick(() => {
+        eventHub.$emit('showBlobInteractionZones', this.diffFile.new_path);
+      });
+    }
+  },
   methods: {
     ...mapActions('diffs', ['saveDiffDiscussion', 'closeDiffFileCommentForm']),
     handleSaveNote(note) {

app/assets/javascripts/diffs/components/diff_file.vue

@@ -145,6 +145,7 @@ export default {
     :class="{
       'is-active': currentDiffFileId === file.file_hash,
     }"
+    :data-path="file.new_path"
     class="diff-file file-holder"
   >
     <diff-file-header

app/assets/javascripts/diffs/store/actions.js

@@ -129,6 +129,18 @@ export const fetchDiffFilesBatch = ({ commit, state }) => {
 
         if (!pagination.next_page) {
           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;

app/assets/javascripts/pages/projects/blob/show/index.js

@@ -33,8 +33,16 @@ document.addEventListener('DOMContentLoaded', () => {
   GpgBadges.fetch();
 
   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
-    import('~/code_navigation').then(m => m.default());
+    import('~/code_navigation').then(m =>
+      m.default({
+        blobs: [{ path: blobPath, codeNavigationPath }],
+        definitionPathPrefix,
+      }),
+    );
   }
 
   if (gon.features?.suggestPipeline) {

app/controllers/projects/merge_requests_controller.rb

@@ -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(:single_mr_diff_view, @project, default_enabled: true)
     push_frontend_feature_flag(:suggest_pipeline) if experiment_enabled?(:suggest_pipeline)
+    push_frontend_feature_flag(:code_navigation, @project)
   end
 
   before_action do

app/views/projects/blob/_blob.html.haml

@@ -10,7 +10,7 @@
 
 #blob-content-holder.blob-content-holder
   - 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
     = render 'projects/blob/header', blob: blob
     = render 'projects/blob/content', blob: blob

app/views/projects/merge_requests/show.html.haml

@@ -51,6 +51,8 @@
 
     .tab-content#diff-notes-app
       #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
         .row
           %section.col-md-12

app/views/shared/_file_highlight.html.haml

@@ -10,7 +10,7 @@
         %a.diff-line-num{ href: "#{link}#L#{i}", id: "L#{i}", 'data-line-number' => i }
           = link_icon
           = i
-  .blob-content{ data: { blob_id: blob.id } }
+  .blob-content{ data: { blob_id: blob.id, path: blob.path } }
     %pre.code.highlight
       %code
         = blob.present.highlight

spec/frontend/code_navigation/store/actions_spec.js

@@ -28,9 +28,9 @@ describe('Code navigation actions', () => {
   describe('fetchData', () => {
     let mock;
 
-    const codeNavUrl =
+    const codeNavigationPath =
       '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(() => {
       window.gon = { api_version: '1' };
@@ -43,7 +43,7 @@ describe('Code navigation actions', () => {
 
     describe('success', () => {
       beforeEach(() => {
-        mock.onGet(codeNavUrl).replyOnce(200, [
+        mock.onGet(codeNavigationPath).replyOnce(200, [
           {
             start_line: 0,
             start_char: 0,
@@ -66,7 +66,12 @@ describe('Code navigation actions', () => {
             { type: 'REQUEST_DATA' },
             {
               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', () => {
             { type: 'REQUEST_DATA' },
             {
               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(() => {
-            expect(addInteractionClass).toHaveBeenCalledWith({
+            expect(addInteractionClass).toHaveBeenCalledWith('index.js', {
               start_line: 0,
               start_char: 0,
               hover: { value: '123' },
@@ -102,7 +112,7 @@ describe('Code navigation actions', () => {
 
     describe('error', () => {
       beforeEach(() => {
-        mock.onGet(codeNavUrl).replyOnce(500);
+        mock.onGet(codeNavigationPath).replyOnce(500);
       });
 
       it('dispatches requestDataError', done => {
@@ -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', () => {
     let target;
 
     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 => {
@@ -130,19 +158,7 @@ describe('Code navigation actions', () => {
     });
 
     it('commits SET_CURRENT_DEFINITION when target is not code navitation element', done => {
-      testAction(
-        actions.showDefinition,
-        { target },
-        { data: {} },
-        [
-          {
-            type: 'SET_CURRENT_DEFINITION',
-            payload: { definition: undefined, position: undefined },
-          },
-        ],
-        [],
-        done,
-      );
+      testAction(actions.showDefinition, { target }, { data: {} }, [], [], done);
     });
 
     it('commits SET_CURRENT_DEFINITION with LSIF data', done => {
@@ -153,7 +169,7 @@ describe('Code navigation actions', () => {
       testAction(
         actions.showDefinition,
         { target },
-        { data: { '0:0': { hover: 'test' } } },
+        { data: { 'index.js': { '0:0': { hover: 'test' } } } },
         [
           {
             type: 'SET_CURRENT_DEFINITION',
@@ -173,7 +189,7 @@ describe('Code navigation actions', () => {
       return testAction(
         actions.showDefinition,
         { target },
-        { data: { '0:0': { hover: 'test' } } },
+        { data: { 'index.js': { '0:0': { hover: 'test' } } } },
         [
           {
             type: 'SET_CURRENT_DEFINITION',
@@ -194,7 +210,7 @@ describe('Code navigation actions', () => {
       return testAction(
         actions.showDefinition,
         { target },
-        { data: { '0:0': { hover: 'test' } } },
+        { data: { 'index.js': { '0:0': { hover: 'test' } } } },
         [
           {
             type: 'SET_CURRENT_DEFINITION',

spec/frontend/code_navigation/store/mutations_spec.js

@@ -11,11 +11,11 @@ describe('Code navigation mutations', () => {
   describe('SET_INITIAL_DATA', () => {
     it('sets initial data', () => {
       mutations.SET_INITIAL_DATA(state, {
-        codeNavUrl: 'https://test.com/builds/1005',
+        blobs: ['test'],
         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');
     });
   });
@@ -36,9 +36,9 @@ describe('Code navigation mutations', () => {
     });
 
     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'] });
     });
   });
 

spec/frontend/code_navigation/utils/index_spec.js

@@ -36,7 +36,7 @@ describe('setCurrentHoverElement', () => {
 describe('addInteractionClass', () => {
   beforeEach(() => {
     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', () => {
   `(
     'it sets code navigation attributes for line $line and character $char',
     ({ 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(
         'js-code-navigation',