Commit 3801960f authored by Takuya Noguchi's avatar Takuya Noguchi

Fix UI on global breadcrumb on Project/Group Container Registry

This fix is done by partially replacing the existing pre JavaScript
code with gitlab-ui's Breadcrumb component. A small part of the fix
is a visual-only, transitional workaround, which can be removed in
MR 48115.

https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48115Signed-off-by: default avatarTakuya Noguchi <takninnovationresearch@gmail.com>
parent 1371e8d5
<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>
......@@ -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: {
......
......@@ -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';
......
// 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;
}
......@@ -17,10 +17,6 @@
}
}
.registry-placeholder {
min-height: 60px;
}
.auto-devops-card {
margin-bottom: $gl-vert-padding;
}
......@@ -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'),
......
......@@ -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'),
......
---
title: Fix UI on global breadcrumb on Project/Group Container Registry
merge_request: 48288
author: Takuya Noguchi
type: other
// 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>
`;
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');
});
});
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment