Merge branch 'ph/213189/codeNavSameFile' into 'master'

Jump to definition in the same file

Closes #213189

See merge request gitlab-org/gitlab!30166

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

@@ -8,7 +8,12 @@ export default {
     Popover,
   },
   computed: {
-    ...mapState(['currentDefinition', 'currentDefinitionPosition', 'definitionPathPrefix']),
+    ...mapState([
+      'currentDefinition',
+      'currentDefinitionPosition',
+      'currentBlobPath',
+      'definitionPathPrefix',
+    ]),
   },
   mounted() {
     this.body = document.body;
@@ -44,5 +49,6 @@ export default {
     :position="currentDefinitionPosition"
     :data="currentDefinition"
     :definition-path-prefix="definitionPathPrefix"
+    :blob-path="currentBlobPath"
   />
 </template>

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

 <script>
-import { GlDeprecatedButton } from '@gitlab/ui';
+import { GlButton } from '@gitlab/ui';
 
 export default {
   components: {
-    GlDeprecatedButton,
+    GlButton,
   },
   props: {
     position: {
@@ -18,6 +18,10 @@ export default {
       type: String,
       required: true,
     },
+    blobPath: {
+      type: String,
+      required: true,
+    },
   },
   data() {
     return {
@@ -32,9 +36,18 @@ export default {
       };
     },
     definitionPath() {
-      return (
-        this.data.definition_path && `${this.definitionPathPrefix}/${this.data.definition_path}`
-      );
+      if (!this.data.definition_path) {
+        return null;
+      }
+
+      if (this.isDefinitionCurrentBlob) {
+        return `#${this.data.definition_path.split('#').pop()}`;
+      }
+
+      return `${this.definitionPathPrefix}/${this.data.definition_path}`;
+    },
+    isDefinitionCurrentBlob() {
+      return this.data.definition_path.indexOf(this.blobPath) === 0;
     },
   },
   watch: {
@@ -77,9 +90,15 @@ export default {
       </p>
     </div>
     <div v-if="definitionPath" class="popover-body">
-      <gl-deprecated-button :href="definitionPath" target="_blank" class="w-100" variant="default">
+      <gl-button
+        :href="definitionPath"
+        :target="isDefinitionCurrentBlob ? null : '_blank'"
+        class="w-100"
+        variant="default"
+        data-testid="go-to-definition-btn"
+      >
         {{ __('Go to definition') }}
-      </gl-deprecated-button>
+      </gl-button>
     </div>
   </div>
 </template>

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

@@ -52,7 +52,8 @@ export default {
       return;
     }
 
-    const data = state.data[blobEl.dataset.path];
+    const blobPath = blobEl.dataset.path;
+    const data = state.data[blobPath];
 
     if (!data) return;
 
@@ -72,6 +73,6 @@ export default {
       setCurrentHoverElement(el);
     }
 
-    commit(types.SET_CURRENT_DEFINITION, { definition, position });
+    commit(types.SET_CURRENT_DEFINITION, { definition, position, blobPath });
   },
 };

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

@@ -15,8 +15,9 @@ export default {
   [types.REQUEST_DATA_ERROR](state) {
     state.loading = false;
   },
-  [types.SET_CURRENT_DEFINITION](state, { definition, position }) {
+  [types.SET_CURRENT_DEFINITION](state, { definition, position, blobPath }) {
     state.currentDefinition = definition;
     state.currentDefinitionPosition = position;
+    state.currentBlobPath = blobPath;
   },
 };

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

@@ -4,4 +4,5 @@ export default () => ({
   data: null,
   currentDefinition: null,
   currentDefinitionPosition: null,
+  currentBlobPath: null,
 });

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

@@ -32,9 +32,10 @@ document.addEventListener('DOMContentLoaded', () => {
 
   GpgBadges.fetch();
 
-  if (gon.features?.codeNavigation) {
-    const el = document.getElementById('js-code-navigation');
-    const { codeNavigationPath, blobPath, definitionPathPrefix } = el.dataset;
+  const codeNavEl = document.getElementById('js-code-navigation');
+
+  if (gon.features?.codeNavigation && codeNavEl) {
+    const { codeNavigationPath, blobPath, definitionPathPrefix } = codeNavEl.dataset;
 
     // eslint-disable-next-line promise/catch-or-return
     import('~/code_navigation').then(m =>

changelogs/unreleased/ph-213189-codeNavSameFile.yml

+---
+title: Fix jump to definition linking to same file opening a new tab
+merge_request:
+author:
+type: changed

spec/frontend/code_navigation/components/__snapshots__/popover_spec.js.snap

@@ -23,17 +23,20 @@ exports[`Code navigation popover component renders popover 1`] = `
   <div
     class="popover-body"
   >
-    <gl-deprecated-button-stub
+    <gl-button-stub
+      category="tertiary"
       class="w-100"
-      href="http://test.com"
-      size="md"
+      data-testid="go-to-definition-btn"
+      href="http://gitlab.com/test.js#L20"
+      icon=""
+      size="medium"
       target="_blank"
       variant="default"
     >
       
       Go to definition
     
-    </gl-deprecated-button-stub>
+    </gl-button-stub>
   </div>
 </div>
 `;

spec/frontend/code_navigation/components/app_spec.js

@@ -48,6 +48,7 @@ describe('Code navigation app component', () => {
     factory({
       currentDefinition: { hover: 'console' },
       currentDefinitionPosition: { x: 0 },
+      currentBlobPath: 'index.js',
     });
 
     expect(wrapper.find(Popover).exists()).toBe(true);

spec/frontend/code_navigation/components/popover_spec.js

 import { shallowMount } from '@vue/test-utils';
 import Popover from '~/code_navigation/components/popover.vue';
 
-const DEFINITION_PATH_PREFIX = 'http:/';
+const DEFINITION_PATH_PREFIX = 'http://gitlab.com';
 
 const MOCK_CODE_DATA = Object.freeze({
   hover: [
@@ -10,7 +10,7 @@ const MOCK_CODE_DATA = Object.freeze({
       value: 'console.log',
     },
   ],
-  definition_path: 'test.com',
+  definition_path: 'test.js#L20',
 });
 
 const MOCK_DOCS_DATA = Object.freeze({
@@ -20,13 +20,15 @@ const MOCK_DOCS_DATA = Object.freeze({
       value: 'console.log',
     },
   ],
-  definition_path: 'test.com',
+  definition_path: 'test.js#L20',
 });
 
 let wrapper;
 
-function factory(position, data, definitionPathPrefix) {
-  wrapper = shallowMount(Popover, { propsData: { position, data, definitionPathPrefix } });
+function factory({ position, data, definitionPathPrefix, blobPath = 'index.js' }) {
+  wrapper = shallowMount(Popover, {
+    propsData: { position, data, definitionPathPrefix, blobPath },
+  });
 }
 
 describe('Code navigation popover component', () => {
@@ -35,14 +37,33 @@ describe('Code navigation popover component', () => {
   });
 
   it('renders popover', () => {
-    factory({ x: 0, y: 0, height: 0 }, MOCK_CODE_DATA, DEFINITION_PATH_PREFIX);
+    factory({
+      position: { x: 0, y: 0, height: 0 },
+      data: MOCK_CODE_DATA,
+      definitionPathPrefix: DEFINITION_PATH_PREFIX,
+    });
 
     expect(wrapper.element).toMatchSnapshot();
   });
 
+  it('renders link with hash to current file', () => {
+    factory({
+      position: { x: 0, y: 0, height: 0 },
+      data: MOCK_CODE_DATA,
+      definitionPathPrefix: DEFINITION_PATH_PREFIX,
+      blobPath: 'test.js',
+    });
+
+    expect(wrapper.find('[data-testid="go-to-definition-btn"]').attributes('href')).toBe('#L20');
+  });
+
   describe('code output', () => {
     it('renders code output', () => {
-      factory({ x: 0, y: 0, height: 0 }, MOCK_CODE_DATA, DEFINITION_PATH_PREFIX);
+      factory({
+        position: { x: 0, y: 0, height: 0 },
+        data: MOCK_CODE_DATA,
+        definitionPathPrefix: DEFINITION_PATH_PREFIX,
+      });
 
       expect(wrapper.find({ ref: 'code-output' }).exists()).toBe(true);
       expect(wrapper.find({ ref: 'doc-output' }).exists()).toBe(false);
@@ -51,7 +72,11 @@ describe('Code navigation popover component', () => {
 
   describe('documentation output', () => {
     it('renders code output', () => {
-      factory({ x: 0, y: 0, height: 0 }, MOCK_DOCS_DATA, DEFINITION_PATH_PREFIX);
+      factory({
+        position: { x: 0, y: 0, height: 0 },
+        data: MOCK_DOCS_DATA,
+        definitionPathPrefix: DEFINITION_PATH_PREFIX,
+      });
 
       expect(wrapper.find({ ref: 'code-output' }).exists()).toBe(false);
       expect(wrapper.find({ ref: 'doc-output' }).exists()).toBe(true);

spec/frontend/code_navigation/store/actions_spec.js

@@ -173,7 +173,11 @@ describe('Code navigation actions', () => {
         [
           {
             type: 'SET_CURRENT_DEFINITION',
-            payload: { definition: { hover: 'test' }, position: { height: 0, x: 0, y: 0 } },
+            payload: {
+              blobPath: 'index.js',
+              definition: { hover: 'test' },
+              position: { height: 0, x: 0, y: 0 },
+            },
           },
         ],
         [],
@@ -193,7 +197,11 @@ describe('Code navigation actions', () => {
         [
           {
             type: 'SET_CURRENT_DEFINITION',
-            payload: { definition: { hover: 'test' }, position: { height: 0, x: 0, y: 0 } },
+            payload: {
+              blobPath: 'index.js',
+              definition: { hover: 'test' },
+              position: { height: 0, x: 0, y: 0 },
+            },
           },
         ],
         [],
@@ -214,7 +222,11 @@ describe('Code navigation actions', () => {
         [
           {
             type: 'SET_CURRENT_DEFINITION',
-            payload: { definition: { hover: 'test' }, position: { height: 0, x: 0, y: 0 } },
+            payload: {
+              blobPath: 'index.js',
+              definition: { hover: 'test' },
+              position: { height: 0, x: 0, y: 0 },
+            },
           },
         ],
         [],