Merge branch 'ph/35172/improveFileListLinks' into 'master'

Improve usability of the links in the file listing

Closes #35172

See merge request gitlab-org/gitlab!27571

app/assets/javascripts/repository/components/table/row.vue

 <script>
 import { escapeRegExp } from 'lodash';
 import { GlBadge, GlLink, GlSkeletonLoading, GlTooltipDirective, GlLoadingIcon } from '@gitlab/ui';
-import { visitUrl, escapeFileUrl } from '~/lib/utils/url_utility';
+import { escapeFileUrl } from '~/lib/utils/url_utility';
 import TimeagoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
 import Icon from '~/vue_shared/components/icon.vue';
 import { getIconName } from '../../utils/icon';
@@ -117,39 +117,37 @@ export default {
       return this.commit && this.commit.lockLabel;
     },
   },
-  methods: {
-    openRow(e) {
-      if (e.target.tagName === 'A') return;
-
-      if (this.isFolder && !e.metaKey) {
-        this.$router.push(this.routerLinkTo);
-      } else {
-        visitUrl(this.url, e.metaKey);
-      }
-    },
-  },
 };
 </script>
 
 <template>
-  <tr :class="`file_${id}`" class="tree-item" @click="openRow">
-    <td class="tree-item-file-name">
+  <tr class="tree-item">
+    <td class="tree-item-file-name cursor-default position-relative">
       <gl-loading-icon
         v-if="path === loadingPath"
         size="sm"
         inline
         class="d-inline-block align-text-bottom fa-fw"
       />
-      <i v-else :aria-label="type" role="img" :class="iconName" class="fa fa-fw"></i>
       <component
         :is="linkComponent"
         ref="link"
         :to="routerLinkTo"
         :href="url"
-        class="str-truncated"
+        :class="{
+          'is-submodule': isSubmodule,
+        }"
+        class="tree-item-link str-truncated"
         data-qa-selector="file_name_link"
       >
-        {{ fullPath }}
+        <i
+          v-if="!loadingPath"
+          :aria-label="type"
+          role="img"
+          :class="iconName"
+          class="fa fa-fw mr-1"
+        ></i
+        ><span class="position-relative">{{ fullPath }}</span>
       </component>
       <!-- eslint-disable-next-line @gitlab/vue-require-i18n-strings -->
       <gl-badge v-if="lfsOid" variant="default" class="label-lfs ml-1">LFS</gl-badge>
@@ -165,7 +163,7 @@ export default {
         class="ml-2 vertical-align-middle"
       />
     </td>
-    <td class="d-none d-sm-table-cell tree-commit">
+    <td class="d-none d-sm-table-cell tree-commit cursor-default">
       <gl-link
         v-if="commit"
         :href="commit.commitPath"
@@ -176,7 +174,7 @@ export default {
       </gl-link>
       <gl-skeleton-loading v-else :lines="1" class="h-auto" />
     </td>
-    <td class="tree-time-ago text-right">
+    <td class="tree-time-ago text-right cursor-default">
       <timeago-tooltip v-if="commit" :time="commit.committedDate" />
       <gl-skeleton-loading v-else :lines="1" class="ml-auto h-auto w-50" />
     </td>

app/assets/stylesheets/framework/files.scss

@@ -511,3 +511,21 @@ span.idiff {
 .code-navigation-popover {
   max-width: 450px;
 }
+
+.tree-item-link {
+  &:not(.is-submodule) {
+    span {
+      z-index: 2;
+    }
+
+    &::before {
+      content: '';
+      position: absolute;
+      left: 0;
+      top: 0;
+      width: 100%;
+      height: 100%;
+      z-index: 1;
+    }
+  }
+}

spec/frontend/repository/components/table/__snapshots__/row_spec.js.snap

@@ -2,25 +2,28 @@
 
 exports[`Repository table row component renders table row 1`] = `
 <tr
-  class="tree-item file_1"
+  class="tree-item"
 >
   <td
-    class="tree-item-file-name"
+    class="tree-item-file-name cursor-default position-relative"
   >
-    <i
-      aria-label="file"
-      class="fa fa-fw fa-file-text-o"
-      role="img"
-    />
+    <!---->
      
     <a
-      class="str-truncated"
+      class="tree-item-link str-truncated"
       data-qa-selector="file_name_link"
       href="https://test.com"
     >
-      
-      test
-    
+      <i
+        aria-label="file"
+        class="fa fa-fw mr-1 fa-file-text-o"
+        role="img"
+      />
+      <span
+        class="position-relative"
+      >
+        test
+      </span>
     </a>
      
     <!---->
@@ -31,7 +34,7 @@ exports[`Repository table row component renders table row 1`] = `
   </td>
    
   <td
-    class="d-none d-sm-table-cell tree-commit"
+    class="d-none d-sm-table-cell tree-commit cursor-default"
   >
     <gl-skeleton-loading-stub
       class="h-auto"
@@ -40,7 +43,7 @@ exports[`Repository table row component renders table row 1`] = `
   </td>
    
   <td
-    class="tree-time-ago text-right"
+    class="tree-time-ago text-right cursor-default"
   >
     <gl-skeleton-loading-stub
       class="ml-auto h-auto w-50"
@@ -52,25 +55,28 @@ exports[`Repository table row component renders table row 1`] = `
 
 exports[`Repository table row component renders table row for path with special character 1`] = `
 <tr
-  class="tree-item file_1"
+  class="tree-item"
 >
   <td
-    class="tree-item-file-name"
+    class="tree-item-file-name cursor-default position-relative"
   >
-    <i
-      aria-label="file"
-      class="fa fa-fw fa-file-text-o"
-      role="img"
-    />
+    <!---->
      
     <a
-      class="str-truncated"
+      class="tree-item-link str-truncated"
       data-qa-selector="file_name_link"
       href="https://test.com"
     >
-      
-      test
-    
+      <i
+        aria-label="file"
+        class="fa fa-fw mr-1 fa-file-text-o"
+        role="img"
+      />
+      <span
+        class="position-relative"
+      >
+        test
+      </span>
     </a>
      
     <!---->
@@ -81,7 +87,7 @@ exports[`Repository table row component renders table row for path with special
   </td>
    
   <td
-    class="d-none d-sm-table-cell tree-commit"
+    class="d-none d-sm-table-cell tree-commit cursor-default"
   >
     <gl-skeleton-loading-stub
       class="h-auto"
@@ -90,7 +96,7 @@ exports[`Repository table row component renders table row for path with special
   </td>
    
   <td
-    class="tree-time-ago text-right"
+    class="tree-time-ago text-right cursor-default"
   >
     <gl-skeleton-loading-stub
       class="ml-auto h-auto w-50"

spec/frontend/repository/components/table/row_spec.js

 import { shallowMount, RouterLinkStub } from '@vue/test-utils';
 import { GlBadge, GlLink, GlLoadingIcon } from '@gitlab/ui';
-import { visitUrl } from '~/lib/utils/url_utility';
 import TableRow from '~/repository/components/table/row.vue';
 import Icon from '~/vue_shared/components/icon.vue';
 
-jest.mock('~/lib/utils/url_utility', () => ({
-  ...jest.requireActual('~/lib/utils/url_utility'),
-  visitUrl: jest.fn(),
-}));
-
 let vm;
 let $router;
 
@@ -87,31 +81,6 @@ describe('Repository table row component', () => {
     });
   });
 
-  it.each`
-    type        | pushes
-    ${'tree'}   | ${true}
-    ${'file'}   | ${false}
-    ${'commit'} | ${false}
-  `('pushes new router if type $type is tree', ({ type, pushes }) => {
-    factory({
-      id: '1',
-      sha: '123',
-      path: 'test',
-      type,
-      currentPath: '/',
-    });
-
-    return vm.vm.$nextTick().then(() => {
-      vm.trigger('click');
-
-      if (pushes) {
-        expect($router.push).toHaveBeenCalledWith({ path: '/-/tree/master/test' });
-      } else {
-        expect($router.push).not.toHaveBeenCalled();
-      }
-    });
-  });
-
   it.each`
     path
     ${'test#'}
@@ -132,7 +101,7 @@ describe('Repository table row component', () => {
     });
   });
 
-  it('pushes new route for directory with hash', () => {
+  it('renders link for directory with hash', () => {
     factory({
       id: '1',
       sha: '123',
@@ -142,36 +111,7 @@ describe('Repository table row component', () => {
     });
 
     return vm.vm.$nextTick().then(() => {
-      vm.trigger('click');
-
-      expect($router.push).toHaveBeenCalledWith({ path: '/-/tree/master/test%23' });
-    });
-  });
-
-  it.each`
-    type        | pushes
-    ${'tree'}   | ${true}
-    ${'file'}   | ${false}
-    ${'commit'} | ${false}
-  `('calls visitUrl if $type is not tree', ({ type, pushes }) => {
-    factory({
-      id: '1',
-      sha: '123',
-      path: 'test',
-      type,
-      currentPath: '/',
-    });
-
-    return vm.vm.$nextTick().then(() => {
-      vm.trigger('click');
-
-      if (pushes) {
-        expect(visitUrl).not.toHaveBeenCalled();
-      } else {
-        const [url, external] = visitUrl.mock.calls[0];
-        expect(url).toBe('https://test.com');
-        expect(external).toBeFalsy();
-      }
+      expect(vm.find('.tree-item-link').props('to')).toEqual({ path: '/-/tree/master/test%23' });
     });
   });