Various UX improvements to the code navigation popover

Only show the code navigation areas when hovering over a line
Increase width of the popover
Allow the popover to be scrolled veritcally
Show 'this is the definition' text instead of a button
for areas which are the current definition

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/213190, https://gitlab.com/gitlab-org/gitlab/-/issues/218586, https://gitlab.com/gitlab-org/gitlab/-/issues/213188

app/assets/javascripts/blob/viewer/index.js

@@ -3,6 +3,7 @@ import '~/behaviors/markdown/render_gfm';
 import Flash from '../../flash';
 import { handleLocationHash } from '../../lib/utils/common_utils';
 import axios from '../../lib/utils/axios_utils';
+import eventHub from '../../notes/event_hub';
 import { __ } from '~/locale';
 
 const loadRichBlobViewer = type => {
@@ -178,6 +179,10 @@ export default class BlobViewer {
       viewer.innerHTML = data.html;
       viewer.setAttribute('data-loaded', 'true');
 
+      if (window.gon?.features?.codeNavigation) {
+        eventHub.$emit('showBlobInteractionZones', viewer.dataset.path);
+      }
+
       return viewer;
     });
   }

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

@@ -31,6 +31,9 @@ export default {
     };
   },
   computed: {
+    isCurrentDefinition() {
+      return this.data.definitionLineNumber - 1 === this.position.lineIndex;
+    },
     positionStyles() {
       return {
         left: `${this.position.x - this.offsetLeft}px`,
@@ -43,7 +46,7 @@ export default {
       }
 
       if (this.isDefinitionCurrentBlob) {
-        return `#${this.data.definition_path.split('#').pop()}`;
+        return `#L${this.data.definitionLineNumber}`;
       }
 
       return `${this.definitionPathPrefix}/${this.data.definition_path}`;
@@ -79,19 +82,29 @@ export default {
     class="popover code-navigation-popover popover-font-size-normal gl-popover bs-popover-bottom show"
   >
     <div :style="{ left: `${offsetLeft}px` }" class="arrow"></div>
-    <div v-for="(hover, index) in data.hover" :key="index" class="border-bottom">
+    <div class="overflow-auto code-navigation-popover-container">
+      <div
+        v-for="(hover, index) in data.hover"
+        :key="index"
+        :class="{ 'border-bottom': index !== data.hover.length - 1 }"
+      >
         <pre
           v-if="hover.language"
           ref="code-output"
           :class="$options.colorScheme"
-        class="border-0 bg-transparent m-0 code highlight"
+          class="border-0 bg-transparent m-0 code highlight text-wrap"
         ><doc-line v-for="(tokens, tokenIndex) in hover.tokens" :key="tokenIndex" :language="hover.language" :tokens="tokens"/></pre>
-      <p v-else ref="doc-output" class="p-3 m-0">
+        <p v-else ref="doc-output" class="p-3 m-0 gl-font-base">
           {{ hover.value }}
         </p>
       </div>
-    <div v-if="definitionPath" class="popover-body">
+    </div>
+    <div v-if="definitionPath || isCurrentDefinition" class="popover-body border-top">
+      <span v-if="isCurrentDefinition" class="gl-font-weight-bold gl-font-base">
+        {{ s__('CodeIntelligence|This is the definition') }}
+      </span>
       <gl-button
+        v-else
         :href="definitionPath"
         :target="isDefinitionCurrentBlob ? null : '_blank'"
         class="w-100"

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

@@ -18,7 +18,10 @@ export default {
         .then(({ data }) => {
           const normalizedData = data.reduce((acc, d) => {
             if (d.hover) {
-              acc[`${d.start_line}:${d.start_char}`] = d;
+              acc[`${d.start_line}:${d.start_char}`] = {
+                ...d,
+                definitionLineNumber: parseInt(d.definition_path?.split('#L').pop() || 0, 10),
+              };
               addInteractionClass(path, d);
             }
             return acc;
@@ -67,6 +70,7 @@ export default {
         x: x || 0,
         y: y + window.scrollY || 0,
         height: el.offsetHeight,
+        lineIndex: parseInt(lineIndex, 10),
       };
       definition = data[`${lineIndex}:${charIndex}`];
 

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

@@ -22,6 +22,7 @@ export const addInteractionClass = (path, d) => {
       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');
+      el.closest('.line').classList.add('code-navigation-line');
     }
   });
 };

app/assets/stylesheets/framework/files.scss

@@ -500,16 +500,27 @@ span.idiff {
   border: transparent;
 }
 
-.code-navigation {
+.code-navigation-line:hover {
+  .code-navigation {
     border-bottom: 1px $gray-darkest dashed;
 
     &:hover {
       border-bottom-color: $almost-black;
     }
+  }
+}
+
+.code-navigation-popover.popover {
+  max-width: calc(min(#{px-to-rem(560px)}, calc(100vw - #{$gl-padding-32})));
+}
+
+.code-navigation-popover-container {
+  max-height: px-to-rem(320px);
 }
 
-.code-navigation-popover {
-  max-width: 450px;
+.code-navigation-popover .code {
+  padding-left: $grid-size * 3;
+  text-indent: -$grid-size * 2;
 }
 
 .tree-item-link {

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

@@ -5,7 +5,7 @@
 - external_embed = local_assigns.fetch(:external_embed, false)
 
 - viewer_url = local_assigns.fetch(:viewer_url) { url_for(safe_params.merge(viewer: viewer.type, format: :json)) } if load_async
-.blob-viewer{ data: { type: viewer.type, rich_type: rich_type, url: viewer_url }, class: ('hidden' if hidden) }
+.blob-viewer{ data: { type: viewer.type, rich_type: rich_type, url: viewer_url, path: viewer.blob.path }, class: ('hidden' if hidden) }
   - if render_error
     = render 'projects/blob/render_error', viewer: viewer
   - elsif load_async

changelogs/unreleased/ph-codeNavigationUXImprovements.yml

+---
+title: Improved UX of the code navigation popover
+merge_request:
+author:
+type: changed

locale/gitlab.pot

@@ -5805,6 +5805,9 @@ msgstr ""
 msgid "Code owners"
 msgstr ""
 
+msgid "CodeIntelligence|This is the definition"
+msgstr ""
+
 msgid "CodeOwner|Pattern"
 msgstr ""
 

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

@@ -11,10 +11,13 @@ exports[`Code navigation popover component renders popover 1`] = `
   />
    
   <div
-    class="border-bottom"
+    class="overflow-auto code-navigation-popover-container"
+  >
+    <div
+      class=""
     >
       <pre
-      class="border-0 bg-transparent m-0 code highlight"
+        class="border-0 bg-transparent m-0 code highlight text-wrap"
       >
         <span
           class="line"
@@ -39,15 +42,16 @@ exports[`Code navigation popover component renders popover 1`] = `
         </span>
       </pre>
     </div>
+  </div>
    
   <div
-    class="popover-body"
+    class="popover-body border-top"
   >
     <gl-button-stub
       category="tertiary"
       class="w-100"
       data-testid="go-to-definition-btn"
-      href="http://gitlab.com/test.js#L20"
+      href="http://gitlab.com/test.js"
       icon=""
       size="medium"
       target="_blank"

spec/frontend/code_navigation/components/popover_spec.js

@@ -26,7 +26,8 @@ const MOCK_CODE_DATA = Object.freeze({
       ],
     },
   ],
-  definition_path: 'test.js#L20',
+  definition_path: 'test.js',
+  definitionLineNumber: 20,
 });
 
 const MOCK_DOCS_DATA = Object.freeze({

spec/frontend/code_navigation/store/actions_spec.js

@@ -69,7 +69,12 @@ describe('Code navigation actions', () => {
               payload: {
                 path: 'index.js',
                 normalizedData: {
-                  '0:0': { start_line: 0, start_char: 0, hover: { value: '123' } },
+                  '0:0': {
+                    definitionLineNumber: 0,
+                    start_line: 0,
+                    start_char: 0,
+                    hover: { value: '123' },
+                  },
                 },
               },
             },
@@ -91,7 +96,12 @@ describe('Code navigation actions', () => {
               payload: {
                 path: 'index.js',
                 normalizedData: {
-                  '0:0': { start_line: 0, start_char: 0, hover: { value: '123' } },
+                  '0:0': {
+                    definitionLineNumber: 0,
+                    start_line: 0,
+                    start_char: 0,
+                    hover: { value: '123' },
+                  },
                 },
               },
             },
@@ -159,7 +169,9 @@ describe('Code navigation actions', () => {
     let target;
 
     beforeEach(() => {
-      setFixtures('<div data-path="index.js"><div class="js-test"></div></div>');
+      setFixtures(
+        '<div data-path="index.js"><div class="line"><div class="js-test"></div></div></div>',
+      );
       target = document.querySelector('.js-test');
     });
 
@@ -186,7 +198,7 @@ describe('Code navigation actions', () => {
             payload: {
               blobPath: 'index.js',
               definition: { hover: 'test' },
-              position: { height: 0, x: 0, y: 0 },
+              position: { height: 0, x: 0, y: 0, lineIndex: 0 },
             },
           },
         ],
@@ -210,7 +222,7 @@ describe('Code navigation actions', () => {
             payload: {
               blobPath: 'index.js',
               definition: { hover: 'test' },
-              position: { height: 0, x: 0, y: 0 },
+              position: { height: 0, x: 0, y: 0, lineIndex: 0 },
             },
           },
         ],
@@ -235,7 +247,7 @@ describe('Code navigation actions', () => {
             payload: {
               blobPath: 'index.js',
               definition: { hover: 'test' },
-              position: { height: 0, x: 0, y: 0 },
+              position: { height: 0, x: 0, y: 0, lineIndex: 0 },
             },
           },
         ],

spec/frontend/code_navigation/utils/index_spec.js

@@ -36,7 +36,7 @@ describe('setCurrentHoverElement', () => {
 describe('addInteractionClass', () => {
   beforeEach(() => {
     setFixtures(
-      '<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>',
+      '<div data-path="index.js"><div class="blob-content"><div id="LC1" class="line"><span>console</span><span>.</span><span>log</span></div><div id="LC2" class="line"><span>function</span></div></div></div>',
     );
   });