Merge branch '285509-global-breadcrumb-on-container-girestry-explorer' into 'master'

Fix UI on global breadcrumb on Project/Group Container Registry

See merge request gitlab-org/gitlab!48288

app/assets/javascripts/registry/explorer/components/registry_breadcrumb.vue

 <script>
-/* eslint-disable vue/no-v-html */
-// We are forced to use `v-html` untill this gitlab-ui issue is resolved: https://gitlab.com/gitlab-org/gitlab-ui/-/issues/1079
-//  then we can re-write this to use gl-breadcrumb
-import { initial, first, last } from 'lodash';
-import { sanitize } from '~/lib/dompurify';
+// We are using gl-breadcrumb only at the last child of the handwritten breadcrumb
+// until this gitlab-ui issue is resolved: https://gitlab.com/gitlab-org/gitlab-ui/-/issues/1079
+//
+// See the CSS workaround in app/assets/stylesheets/pages/registry.scss when this file is changed.
+import { GlBreadcrumb, GlIcon } from '@gitlab/ui';
 
 export default {
-  props: {
-    crumbs: {
-      type: Array,
-      required: true,
-    },
+  components: {
+    GlBreadcrumb,
+    GlIcon,
   },
   computed: {
-    parsedCrumbs() {
-      return this.crumbs.map((c) => ({ ...c, innerHTML: sanitize(c.innerHTML) }));
-    },
     rootRoute() {
       return this.$router.options.routes.find((r) => r.meta.root);
     },
+    detailsRoute() {
+      return this.$router.options.routes.find((r) => r.name === 'details');
+    },
     isRootRoute() {
       return this.$route.name === this.rootRoute.name;
     },
-    rootCrumbs() {
-      return initial(this.parsedCrumbs);
+    isLoaded() {
+      return this.isRootRoute || this.$store?.state.imageDetails?.name;
     },
-    divider() {
-      const { classList, tagName, innerHTML } = first(this.crumbs).querySelector('svg');
-      return { classList: [...classList], tagName, innerHTML: sanitize(innerHTML) };
+    allCrumbs() {
+      const crumbs = [
+        {
+          text: this.rootRoute.meta.nameGenerator(),
+          to: this.rootRoute.path,
         },
-    lastCrumb() {
-      const { children } = last(this.crumbs);
-      const { tagName, className } = first(children);
-      return {
-        tagName,
-        className,
-        text: this.$route.meta.nameGenerator(),
-        path: { to: this.$route.name },
-      };
+      ];
+      if (!this.isRootRoute) {
+        crumbs.push({
+          text: this.detailsRoute.meta.nameGenerator(),
+          href: this.detailsRoute.meta.path,
+        });
+      }
+      return crumbs;
     },
   },
 };
 </script>
 
 <template>
-  <ul>
-    <li
-      v-for="(crumb, index) in rootCrumbs"
-      :key="index"
-      :class="crumb.className"
-      v-html="crumb.innerHTML"
-    ></li>
-    <li v-if="!isRootRoute">
-      <router-link ref="rootRouteLink" :to="rootRoute.path">
-        {{ rootRoute.meta.nameGenerator() }}
-      </router-link>
-      <component :is="divider.tagName" :class="divider.classList" v-html="divider.innerHTML" />
-    </li>
-    <li>
-      <component :is="lastCrumb.tagName" ref="lastCrumb" :class="lastCrumb.className">
-        <router-link ref="childRouteLink" :to="lastCrumb.path">{{ lastCrumb.text }}</router-link>
-      </component>
-    </li>
-  </ul>
+  <gl-breadcrumb :key="isLoaded" :items="allCrumbs">
+    <template #separator>
+      <gl-icon name="angle-right" :size="8" />
+    </template>
+  </gl-breadcrumb>
 </template>

app/assets/javascripts/registry/explorer/index.js

@@ -71,16 +71,28 @@ export default () => {
     });
 
   const attachBreadcrumb = () => {
-    const breadCrumbEl = document.querySelector('nav .js-breadcrumbs-list');
-    const crumbs = [...document.querySelectorAll('.js-breadcrumbs-list li')];
+    const breadCrumbEls = document.querySelectorAll('nav .js-breadcrumbs-list li');
+    const breadCrumbEl = breadCrumbEls[breadCrumbEls.length - 1];
+    const crumbs = [breadCrumbEl.querySelector('h2')];
+    const nestedBreadcrumbEl = document.createElement('div');
+    breadCrumbEl.replaceChild(nestedBreadcrumbEl, breadCrumbEl.querySelector('h2'));
     return new Vue({
-      el: breadCrumbEl,
+      el: nestedBreadcrumbEl,
       router,
       apolloProvider,
       components: {
         RegistryBreadcrumb,
       },
       render(createElement) {
+        // FIXME(@tnir): this is a workaround until the MR gets merged:
+        // https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48115
+        const parentEl = breadCrumbEl.parentElement.parentElement;
+        if (parentEl) {
+          parentEl.classList.remove('breadcrumbs-container');
+          parentEl.classList.add('gl-display-flex');
+          parentEl.classList.add('w-100');
+        }
+        // End of FIXME(@tnir)
         return createElement('registry-breadcrumb', {
           class: breadCrumbEl.className,
           props: {

app/assets/stylesheets/_page_specific_files.scss

@@ -28,6 +28,7 @@
 @import './pages/profiles/preferences';
 @import './pages/projects';
 @import './pages/prometheus';
+@import './pages/registry';
 @import './pages/runners';
 @import './pages/search';
 @import './pages/service_desk';

app/assets/stylesheets/pages/registry.scss

+// Workaround for gl-breadcrumb at the last child of the handwritten breadcrumb
+// until this gitlab-ui issue is resolved: https://gitlab.com/gitlab-org/gitlab-ui/-/issues/1079
+//
+// See app/assets/javascripts/registry/explorer/components/registry_breadcrumb.vue when this is changed.
+.breadcrumbs-container .gl-breadcrumbs {
+  padding: 0;
+  box-shadow: none;
+}

app/assets/stylesheets/pages/settings_ci_cd.scss

@@ -17,10 +17,6 @@
   }
 }
 
-.registry-placeholder {
-  min-height: 60px;
-}
-
 .auto-devops-card {
   margin-bottom: $gl-vert-padding;
 }

app/views/groups/registry/repositories/index.html.haml

@@ -2,8 +2,6 @@
 - @content_class = "limit-container-width" unless fluid_layout
 
 %section
-  .row.registry-placeholder.prepend-bottom-10
-    .col-12
   #js-container-registry{ data: { endpoint: group_container_registries_path(@group),
     "help_page_path" => help_page_path('user/packages/container_registry/index'),
     "two_factor_auth_help_link" => help_page_path('user/profile/account/two_factor_authentication'),

app/views/projects/registry/repositories/index.html.haml

@@ -2,8 +2,6 @@
 - @content_class = "limit-container-width" unless fluid_layout
 
 %section
-  .row.registry-placeholder.prepend-bottom-10
-    .col-12
   #js-container-registry{ data: { endpoint: project_container_registry_index_path(@project),
     expiration_policy: @project.container_expiration_policy.to_json,
     "help_page_path" => help_page_path('user/packages/container_registry/index'),

changelogs/unreleased/285509-global-breadcrumb-on-container-girestry-explorer.yml

+---
+title: Fix UI on global breadcrumb on Project/Group Container Registry
+merge_request: 48288
+author: Takuya Noguchi
+type: other

spec/frontend/registry/explorer/components/__snapshots__/registry_breadcrumb_spec.js.snap

 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Registry Breadcrumb when is rootRoute renders 1`] = `
-<ul>
+exports[`Registry Breadcrumb when is not rootRoute renders 1`] = `
+<div
+  class="gl-breadcrumbs"
+>
+  <ol
+    class="breadcrumb gl-breadcrumb-list"
+  >
+     
     <li
-    class="foo bar"
+      class="breadcrumb-item gl-breadcrumb-item"
     >
-    baz
+      <a
+        class=""
+        href="/"
+        target="_self"
+      />
     </li>
+     
+    <span
+      class="gl-breadcrumb-separator"
+      data-testid="separator"
+    >
+      <svg
+        aria-hidden="true"
+        class="gl-icon s8"
+        data-testid="angle-right-icon"
+      >
+        <use
+          href="#angle-right"
+        />
+      </svg>
+    </span>
     <li
-    class="foo bar"
+      class="breadcrumb-item gl-breadcrumb-item"
     >
-    foo
+      <a
+        class=""
+        href="#"
+        target="_self"
+      />
     </li>
      
     <!---->
+  </ol>
+</div>
+`;
 
-  <li>
-    <a
-      class="foo"
+exports[`Registry Breadcrumb when is rootRoute renders 1`] = `
+<div
+  class="gl-breadcrumbs"
+>
+  <ol
+    class="breadcrumb gl-breadcrumb-list"
   >
-      <a>
      
-      </a>
-    </a>
+    <li
+      class="breadcrumb-item gl-breadcrumb-item"
+    >
+      <a
+        class=""
+        href="/"
+        target="_self"
+      />
     </li>
-</ul>
+     
+    <!---->
+  </ol>
+</div>
 `;

spec/frontend/registry/explorer/components/registry_breadcrumb_spec.js

-import { shallowMount } from '@vue/test-utils';
+import { mount } from '@vue/test-utils';
 
 import component from '~/registry/explorer/components/registry_breadcrumb.vue';
 
@@ -6,45 +6,13 @@ describe('Registry Breadcrumb', () => {
   let wrapper;
   const nameGenerator = jest.fn();
 
-  const crumb = {
-    className: 'foo bar',
-    tagName: 'div',
-    innerHTML: 'baz',
-    querySelector: jest.fn(),
-    children: [
-      {
-        tagName: 'a',
-        className: 'foo',
-      },
-    ],
-  };
-
-  const querySelectorReturnValue = {
-    classList: ['js-divider'],
-    tagName: 'svg',
-    innerHTML: 'foo',
-  };
-
-  const crumbs = [crumb, { ...crumb, innerHTML: 'foo' }, { ...crumb, className: 'baz' }];
-
   const routes = [
-    { name: 'foo', meta: { nameGenerator, root: true } },
-    { name: 'baz', meta: { nameGenerator } },
+    { name: 'list', path: '/', meta: { nameGenerator, root: true } },
+    { name: 'details', path: '/:id', meta: { nameGenerator } },
   ];
 
-  const findDivider = () => wrapper.find('.js-divider');
-  const findRootRoute = () => wrapper.find({ ref: 'rootRouteLink' });
-  const findChildRoute = () => wrapper.find({ ref: 'childRouteLink' });
-  const findLastCrumb = () => wrapper.find({ ref: 'lastCrumb' });
-
   const mountComponent = ($route) => {
-    wrapper = shallowMount(component, {
-      propsData: {
-        crumbs,
-      },
-      stubs: {
-        'router-link': { name: 'router-link', template: '<a><slot></slot></a>', props: ['to'] },
-      },
+    wrapper = mount(component, {
       mocks: {
         $route,
         $router: {
@@ -58,7 +26,6 @@ describe('Registry Breadcrumb', () => {
 
   beforeEach(() => {
     nameGenerator.mockClear();
-    crumb.querySelector = jest.fn();
   });
 
   afterEach(() => {
@@ -75,8 +42,11 @@ describe('Registry Breadcrumb', () => {
       expect(wrapper.element).toMatchSnapshot();
     });
 
-    it('contains a router-link for the child route', () => {
-      expect(findChildRoute().exists()).toBe(true);
+    it('contains only a single router-link to list', () => {
+      const links = wrapper.findAll('a');
+
+      expect(links).toHaveLength(1);
+      expect(links.at(0).attributes('href')).toBe('/');
     });
 
     it('the link text is calculated by nameGenerator', () => {
@@ -86,48 +56,23 @@ describe('Registry Breadcrumb', () => {
 
   describe('when is not rootRoute', () => {
     beforeEach(() => {
-      crumb.querySelector.mockReturnValue(querySelectorReturnValue);
       mountComponent(routes[1]);
     });
 
-    it('renders a divider', () => {
-      expect(findDivider().exists()).toBe(true);
+    it('renders', () => {
+      expect(wrapper.element).toMatchSnapshot();
     });
 
-    it('contains a router-link for the root route', () => {
-      expect(findRootRoute().exists()).toBe(true);
-    });
+    it('contains two router-links to list and details', () => {
+      const links = wrapper.findAll('a');
 
-    it('contains a router-link for the child route', () => {
-      expect(findChildRoute().exists()).toBe(true);
+      expect(links).toHaveLength(2);
+      expect(links.at(0).attributes('href')).toBe('/');
+      expect(links.at(1).attributes('href')).toBe('#');
     });
 
     it('the link text is calculated by nameGenerator', () => {
       expect(nameGenerator).toHaveBeenCalledTimes(2);
     });
   });
-
-  describe('last crumb', () => {
-    const lastChildren = crumb.children[0];
-    beforeEach(() => {
-      nameGenerator.mockReturnValue('foo');
-      mountComponent(routes[0]);
-    });
-
-    it('has the same tag as the last children of the crumbs', () => {
-      expect(findLastCrumb().element.tagName).toBe(lastChildren.tagName.toUpperCase());
-    });
-
-    it('has the same classes as the last children of the crumbs', () => {
-      expect(findLastCrumb().classes().join(' ')).toEqual(lastChildren.className);
-    });
-
-    it('has a link to the current route', () => {
-      expect(findChildRoute().props('to')).toEqual({ to: routes[0].name });
-    });
-
-    it('the link has the correct text', () => {
-      expect(findChildRoute().text()).toEqual('foo');
-    });
-  });
 });