From 99e41df6f4bcc85fb1846e969c3eef0112941cad Mon Sep 17 00:00:00 2001 From: Chad Lavimoniere Date: Thu, 3 Apr 2025 17:48:04 -0400 Subject: [PATCH] Alternative Vue breadcrumb injection method We have various problems with breadcrumbs on Vue pages, all stemming from our injection of a new `GlBreadcrumb` next to the existing one used for static pages. This change takes the static breadcrumb items and adds them to the injected Vue breadcrumb, while removing the static one from the dom. Changelog: fixed --- .../javascripts/lib/utils/breadcrumbs.js | 32 ++++++++++--- .../lib/utils/breadcrumbs_state.js | 3 ++ .../super_sidebar_breadcrumbs.js | 35 ++++++++++++++ .../super_sidebar/super_sidebar_bundle.js | 23 ++------- .../components/work_item_breadcrumb.vue | 11 ++++- app/assets/javascripts/work_items/index.js | 5 +- spec/frontend/lib/utils/breadcrumbs_spec.js | 48 ++++++++++++++++++- .../components/work_item_breadcrumb_spec.js | 17 +++++++ 8 files changed, 145 insertions(+), 29 deletions(-) create mode 100644 app/assets/javascripts/lib/utils/breadcrumbs_state.js create mode 100644 app/assets/javascripts/super_sidebar/super_sidebar_breadcrumbs.js diff --git a/app/assets/javascripts/lib/utils/breadcrumbs.js b/app/assets/javascripts/lib/utils/breadcrumbs.js index 4a34b9b3c4d257..70a03b27b91b36 100644 --- a/app/assets/javascripts/lib/utils/breadcrumbs.js +++ b/app/assets/javascripts/lib/utils/breadcrumbs.js @@ -1,12 +1,17 @@ import Vue from 'vue'; - -export const staticBreadcrumbs = Vue.observable({}); +import { destroySuperSidebarBreadcrumbs } from '~/super_sidebar/super_sidebar_breadcrumbs'; +import { staticBreadcrumbs } from './breadcrumbs_state'; export const injectVueAppBreadcrumbs = ( router, BreadcrumbsComponent, apolloProvider = null, provide = {}, + // this is intended to be a temporary option. Once all uses of + // injectVueAppBreadcrumbs use it, the option should be removed and its + // behavior should be the default. + // Cf. https://gitlab.com/gitlab-org/gitlab/-/merge_requests/186906 + { singleNavOptIn = false } = {}, // eslint-disable-next-line max-params ) => { const injectBreadcrumbEl = document.querySelector('#js-injected-page-breadcrumbs'); @@ -15,10 +20,22 @@ export const injectVueAppBreadcrumbs = ( return false; } - // Hide the last of the static breadcrumbs by nulling its values. - // This way, the separator "/" stays visible and also the new "last" static item isn't displayed in bold font. - staticBreadcrumbs.items[staticBreadcrumbs.items.length - 1].text = ''; - staticBreadcrumbs.items[staticBreadcrumbs.items.length - 1].href = ''; + if (singleNavOptIn) { + destroySuperSidebarBreadcrumbs(); + // After singleNavOptIn is turned on for all Vue apps, we can stop + // changing the content of staticBreadcrumbs and instead pass a mutated + // copy of it to the CustomBreadcrumbsRoot component. For now, we need + // to conditionally mutate the staticBreadcrumbs object so that the last + // breadcrumb is hidden for Vue apps that have not opted in to the + // singleNavOptIn. + // Cf. https://gitlab.com/gitlab-org/gitlab/-/merge_requests/186906 + staticBreadcrumbs.items = staticBreadcrumbs.items.slice(0, -1); + } else { + // Hide the last of the static breadcrumbs by nulling its values. + // This way, the separator "/" stays visible and also the new "last" static item isn't displayed in bold font. + staticBreadcrumbs.items[staticBreadcrumbs.items.length - 1].text = ''; + staticBreadcrumbs.items[staticBreadcrumbs.items.length - 1].href = ''; + } return new Vue({ el: injectBreadcrumbEl, @@ -29,6 +46,9 @@ export const injectVueAppBreadcrumbs = ( render(createElement) { return createElement(BreadcrumbsComponent, { class: injectBreadcrumbEl.className, + props: { + staticBreadcrumbs, + }, }); }, }); diff --git a/app/assets/javascripts/lib/utils/breadcrumbs_state.js b/app/assets/javascripts/lib/utils/breadcrumbs_state.js new file mode 100644 index 00000000000000..dc7272373a0f1d --- /dev/null +++ b/app/assets/javascripts/lib/utils/breadcrumbs_state.js @@ -0,0 +1,3 @@ +import Vue from 'vue'; + +export const staticBreadcrumbs = Vue.observable({ items: [] }); diff --git a/app/assets/javascripts/super_sidebar/super_sidebar_breadcrumbs.js b/app/assets/javascripts/super_sidebar/super_sidebar_breadcrumbs.js new file mode 100644 index 00000000000000..7ab0b87a2994ff --- /dev/null +++ b/app/assets/javascripts/super_sidebar/super_sidebar_breadcrumbs.js @@ -0,0 +1,35 @@ +import Vue from 'vue'; +import { GlBreadcrumb } from '@gitlab/ui'; +import { staticBreadcrumbs } from '~/lib/utils/breadcrumbs_state'; + +let superSidebarBreadcrumbsApp = null; + +export function initPageBreadcrumbs() { + const el = document.querySelector('#js-vue-page-breadcrumbs'); + if (!el) return false; + const { breadcrumbsJson } = el.dataset; + + staticBreadcrumbs.items = JSON.parse(breadcrumbsJson); + + superSidebarBreadcrumbsApp = new Vue({ + el, + name: 'SuperSidebarBreadcrumbs', + destroyed() { + this.$el?.remove(); + superSidebarBreadcrumbsApp = null; + }, + render(h) { + return h(GlBreadcrumb, { + props: staticBreadcrumbs, + }); + }, + }); + + return superSidebarBreadcrumbsApp; +} + +export function destroySuperSidebarBreadcrumbs() { + if (superSidebarBreadcrumbsApp) { + superSidebarBreadcrumbsApp.$destroy(); + } +} diff --git a/app/assets/javascripts/super_sidebar/super_sidebar_bundle.js b/app/assets/javascripts/super_sidebar/super_sidebar_bundle.js index 543e91583d1811..e7b08dfcb3db52 100644 --- a/app/assets/javascripts/super_sidebar/super_sidebar_bundle.js +++ b/app/assets/javascripts/super_sidebar/super_sidebar_bundle.js @@ -1,9 +1,8 @@ import Vue from 'vue'; -import { GlBreadcrumb, GlToast } from '@gitlab/ui'; +import { GlToast } from '@gitlab/ui'; import VueApollo from 'vue-apollo'; import { convertObjectPropsToCamelCase, parseBoolean } from '~/lib/utils/common_utils'; import { apolloProvider } from '~/graphql_shared/issuable_client'; -import { staticBreadcrumbs } from '~/lib/utils/breadcrumbs'; import { JS_TOGGLE_EXPAND_CLASS, CONTEXT_NAMESPACE_GROUPS } from './constants'; import createStore from './components/global_search/store'; import { @@ -14,6 +13,8 @@ import SuperSidebar from './components/super_sidebar.vue'; import SuperSidebarToggle from './components/super_sidebar_toggle.vue'; import AdvancedSearchModal from './components/global_search/components/global_search_header_app.vue'; +export { initPageBreadcrumbs } from './super_sidebar_breadcrumbs'; + Vue.use(GlToast); Vue.use(VueApollo); @@ -197,24 +198,6 @@ export const initSuperSidebarToggle = () => { }); }; -export function initPageBreadcrumbs() { - const el = document.querySelector('#js-vue-page-breadcrumbs'); - if (!el) return false; - const { breadcrumbsJson } = el.dataset; - - staticBreadcrumbs.items = JSON.parse(breadcrumbsJson); - - return new Vue({ - el, - name: 'SuperSidebarBreadcrumbs', - render(h) { - return h(GlBreadcrumb, { - props: staticBreadcrumbs, - }); - }, - }); -} - export function initAdvancedSearchModal({ rootPath, isSaas, diff --git a/app/assets/javascripts/work_items/components/work_item_breadcrumb.vue b/app/assets/javascripts/work_items/components/work_item_breadcrumb.vue index abead216460a4b..4062a0e3efd941 100644 --- a/app/assets/javascripts/work_items/components/work_item_breadcrumb.vue +++ b/app/assets/javascripts/work_items/components/work_item_breadcrumb.vue @@ -25,6 +25,13 @@ export default { default: false, }, }, + props: { + staticBreadcrumbs: { + type: Object, + required: false, + default: () => ({ items: [] }), + }, + }, computed: { isWorkItemOnly() { return this.glFeatures.workItemPlanningView; @@ -62,7 +69,9 @@ export default { indexCrumb.href = this.listPath; } - const crumbs = [indexCrumb]; + const staticCrumbs = this.staticBreadcrumbs.items; + + const crumbs = [...staticCrumbs, indexCrumb]; if (this.$route.name === ROUTES.new) { crumbs.push({ diff --git a/app/assets/javascripts/work_items/index.js b/app/assets/javascripts/work_items/index.js index 34d41525bc1910..529a778adcf61c 100644 --- a/app/assets/javascripts/work_items/index.js +++ b/app/assets/javascripts/work_items/index.js @@ -77,7 +77,10 @@ export const initWorkItemsRoot = ({ workItemType, workspaceType, withTabs } = {} breadcrumbParams.listPath = issuesListPath; } - injectVueAppBreadcrumbs(router, WorkItemBreadcrumb, apolloProvider, breadcrumbParams); + injectVueAppBreadcrumbs(router, WorkItemBreadcrumb, apolloProvider, breadcrumbParams, { + // Cf. https://gitlab.com/gitlab-org/gitlab/-/merge_requests/186906 + singleNavOptIn: true, + }); apolloProvider.clients.defaultClient.cache.writeQuery({ query: activeDiscussionQuery, diff --git a/spec/frontend/lib/utils/breadcrumbs_spec.js b/spec/frontend/lib/utils/breadcrumbs_spec.js index f1876b18a4dd8d..874dcd4b0be7bb 100644 --- a/spec/frontend/lib/utils/breadcrumbs_spec.js +++ b/spec/frontend/lib/utils/breadcrumbs_spec.js @@ -1,6 +1,7 @@ import { createWrapper } from '@vue/test-utils'; import Vue from 'vue'; -import { injectVueAppBreadcrumbs, staticBreadcrumbs } from '~/lib/utils/breadcrumbs'; +import { injectVueAppBreadcrumbs } from '~/lib/utils/breadcrumbs'; +import { staticBreadcrumbs } from '~/lib/utils/breadcrumbs_state'; import { resetHTMLFixture, setHTMLFixture } from 'helpers/fixtures'; import createMockApollo from 'helpers/mock_apollo_helper'; @@ -8,6 +9,13 @@ describe('Breadcrumbs utils', () => { const mockRouter = jest.fn(); const MockComponent = Vue.component('MockComponent', { + props: { + staticBreadcrumbs: { + type: Object, + required: false, + default: () => ({ items: [] }), + }, + }, render: (createElement) => createElement('span', { attrs: { @@ -69,5 +77,43 @@ describe('Breadcrumbs utils', () => { ).toHaveLength(1); }); }); + + describe('when singleNavOptIn is enabled', () => { + const breadcrumbsHTML = ` +
+ +
+
+ `; + + beforeEach(() => { + setHTMLFixture(breadcrumbsHTML); + staticBreadcrumbs.items = [ + { text: 'First', href: '/first' }, + { text: 'Last', href: '/last' }, + ]; + }); + + it('removes the last item from staticBreadcrumbs.items and passes it to the component', () => { + const wrapper = createWrapper( + injectVueAppBreadcrumbs( + mockRouter, + MockComponent, + mockApolloProvider, + {}, + { singleNavOptIn: true }, + ), + ); + + expect(staticBreadcrumbs.items).toHaveLength(1); + expect(staticBreadcrumbs.items[0].text).toBe('First'); + expect(staticBreadcrumbs.items[0].href).toBe('/first'); + + const component = wrapper.findComponent(MockComponent); + expect(component.props('staticBreadcrumbs')).toEqual({ + items: [{ text: 'First', href: '/first' }], + }); + }); + }); }); }); diff --git a/spec/frontend/work_items/components/work_item_breadcrumb_spec.js b/spec/frontend/work_items/components/work_item_breadcrumb_spec.js index 88195978e9f505..d42d7538c61ff8 100644 --- a/spec/frontend/work_items/components/work_item_breadcrumb_spec.js +++ b/spec/frontend/work_items/components/work_item_breadcrumb_spec.js @@ -16,6 +16,7 @@ describe('WorkItemBreadcrumb', () => { isGroup = true, workItemsViewPreference = false, workItemsAlpha = false, + props = {}, } = {}) => { wrapper = shallowMount(WorkItemBreadcrumb, { provide: { @@ -31,6 +32,7 @@ describe('WorkItemBreadcrumb', () => { mocks: { $route, }, + propsData: props, }); }; @@ -157,6 +159,21 @@ describe('WorkItemBreadcrumb', () => { ); }); + it('combines static and dynamic breadcrumbs', () => { + createComponent({ + $route: { name: 'workItem', params: { iid: '1' }, path: '/1' }, + props: { + staticBreadcrumbs: { items: [{ text: 'Static', href: '/static' }] }, + }, + }); + + expect(findBreadcrumb().props('items')).toEqual([ + { text: 'Static', href: '/static' }, + { text: 'Work items', to: { name: 'workItemList', query: undefined } }, + { text: '#1', to: '/1' }, + ]); + }); + it('renders work item iid breadcrumb on work item detail page', () => { createComponent({ $route: { name: 'workItem', params: { iid: '1' }, path: '/1' } }); -- GitLab