diff --git a/.changeset/fresh-crews-promise.md b/.changeset/fresh-crews-promise.md new file mode 100644 index 0000000000..fc9859086a --- /dev/null +++ b/.changeset/fresh-crews-promise.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-techdocs-module-addons-contrib': patch +'@backstage/plugin-techdocs': patch +--- + +Fixed navigation bug that caused users to not be scrolled to the top of a new page. Fixed navigation bug where using backwards and forwards browser navigation did not scroll users to the correct place on the TechDoc page. diff --git a/plugins/techdocs-module-addons-contrib/src/setupTests.ts b/plugins/techdocs-module-addons-contrib/src/setupTests.ts index 326d2ed212..56b9b18321 100644 --- a/plugins/techdocs-module-addons-contrib/src/setupTests.ts +++ b/plugins/techdocs-module-addons-contrib/src/setupTests.ts @@ -14,3 +14,5 @@ * limitations under the License. */ import '@testing-library/jest-dom'; + +Element.prototype.scrollIntoView = jest.fn(); diff --git a/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.test.tsx b/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.test.tsx index 32fbe21714..adef2f64d0 100644 --- a/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.test.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.test.tsx @@ -33,6 +33,10 @@ jest.mock('../useReaderState', () => ({ ...jest.requireActual('../useReaderState'), useReaderState: (...args: any[]) => useReaderState(...args), })); +jest.mock('@backstage/plugin-techdocs-react', () => ({ + ...jest.requireActual('@backstage/plugin-techdocs-react'), + useShadowDomStylesLoading: jest.fn().mockReturnValue(false), +})); import { TechDocsReaderPageContent } from './TechDocsReaderPageContent'; @@ -84,6 +88,10 @@ const Wrapper = ({ ); describe('', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + it('should render techdocs page content', async () => { getEntityMetadata.mockResolvedValue(mockEntityMetadata); getTechDocsMetadata.mockResolvedValue(mockTechDocsMetadata); @@ -145,4 +153,61 @@ describe('', () => { ).toBeInTheDocument(); }); }); + + it('should scroll to hash if hash is present in url', async () => { + jest.spyOn(document, 'querySelector'); + + const mockScrollIntoView = jest.fn(); + const h2 = document.createElement('h2'); + h2.innerText = 'emojis'; + h2.id = 'emojis'; + h2.scrollIntoView = mockScrollIntoView; + const mockTechDocsPage = document.createElement('html'); + mockTechDocsPage.appendChild(h2); + + getEntityMetadata.mockResolvedValue(mockEntityMetadata); + getTechDocsMetadata.mockResolvedValue(mockTechDocsMetadata); + useTechDocsReaderDom.mockReturnValue(mockTechDocsPage); + useReaderState.mockReturnValue({ state: 'cached' }); + + window.location.hash = '#emojis'; + + const rendered = await renderInTestApp( + + + , + ); + + await waitFor(() => { + expect( + rendered.getByTestId('techdocs-native-shadowroot'), + ).toBeInTheDocument(); + expect(mockScrollIntoView).toHaveBeenCalled(); + expect(document.querySelector).not.toHaveBeenCalledWith('header'); + }); + + window.location.hash = ''; + }); + + it('should scroll to header if hash is not present in url', async () => { + jest.spyOn(document, 'querySelector'); + + getEntityMetadata.mockResolvedValue(mockEntityMetadata); + getTechDocsMetadata.mockResolvedValue(mockTechDocsMetadata); + useTechDocsReaderDom.mockReturnValue(document.createElement('html')); + useReaderState.mockReturnValue({ state: 'cached' }); + + const rendered = await renderInTestApp( + + + , + ); + + await waitFor(() => { + expect( + rendered.getByTestId('techdocs-native-shadowroot'), + ).toBeInTheDocument(); + expect(document.querySelector).toHaveBeenCalledWith('header'); + }); + }); }); diff --git a/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.tsx b/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.tsx index 1e5459fc9d..f4c5ad8745 100644 --- a/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.tsx @@ -14,12 +14,14 @@ * limitations under the License. */ -import React, { useCallback } from 'react'; +import React, { useCallback, useEffect } from 'react'; import { makeStyles, Grid } from '@material-ui/core'; import { TechDocsShadowDom, + useShadowDomStylesLoading, + useShadowRootElements, useTechDocsReaderPage, } from '@backstage/plugin-techdocs-react'; import { CompoundEntityRef } from '@backstage/catalog-model'; @@ -78,8 +80,23 @@ export const TechDocsReaderPageContent = withTechDocsReaderProvider( entityRef, setShadowRoot, } = useTechDocsReaderPage(); - const dom = useTechDocsReaderDom(entityRef); + const path = window.location.pathname; + const hash = window.location.hash; + const isStyleLoading = useShadowDomStylesLoading(dom); + const [hashElement] = useShadowRootElements([`[id="${hash.slice(1)}"]`]); + + useEffect(() => { + if (isStyleLoading) return; + + if (hash) { + if (hashElement) { + hashElement.scrollIntoView(); + } + } else { + document?.querySelector('header')?.scrollIntoView(); + } + }, [path, hash, hashElement, isStyleLoading]); const handleAppend = useCallback( (newShadowRoot: ShadowRoot) => { diff --git a/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/dom.tsx b/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/dom.tsx index cc42c46b4e..29e6945fa7 100644 --- a/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/dom.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/dom.tsx @@ -39,7 +39,6 @@ import { removeMkdocsHeader, rewriteDocLinks, simplifyMkdocsFooter, - scrollIntoAnchor, scrollIntoNavigation, transform as transformer, copyToClipboard, @@ -167,7 +166,6 @@ export const useTechDocsReaderDom = ( const postRender = useCallback( async (transformedElement: Element) => transformer(transformedElement, [ - scrollIntoAnchor(), scrollIntoNavigation(), copyToClipboard(theme), addLinkClickListener({ diff --git a/plugins/techdocs/src/reader/transformers/index.ts b/plugins/techdocs/src/reader/transformers/index.ts index ca17261572..7df98f1f26 100644 --- a/plugins/techdocs/src/reader/transformers/index.ts +++ b/plugins/techdocs/src/reader/transformers/index.ts @@ -25,6 +25,5 @@ export * from './copyToClipboard'; export * from './removeMkdocsHeader'; export * from './simplifyMkdocsFooter'; export * from './onCssReady'; -export * from './scrollIntoAnchor'; export * from './scrollIntoNavigation'; export * from './transformer'; diff --git a/plugins/techdocs/src/reader/transformers/scrollIntoAnchor.test.ts b/plugins/techdocs/src/reader/transformers/scrollIntoAnchor.test.ts deleted file mode 100644 index e222c24a4c..0000000000 --- a/plugins/techdocs/src/reader/transformers/scrollIntoAnchor.test.ts +++ /dev/null @@ -1,118 +0,0 @@ -/* - * Copyright 2021 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { scrollIntoAnchor } from '../transformers'; -import { createTestShadowDom } from '../../test-utils'; -import { Transformer } from './transformer'; -import mkdocsIndex from '../../test-utils/fixtures/mkdocs-index'; -import { SHADOW_DOM_STYLE_LOAD_EVENT } from '@backstage/plugin-techdocs-react'; - -describe('scrollIntoAnchor', () => { - const scrollIntoView = jest.fn(); - let querySelectorSpy: jest.SpyInstance; - let addEventListenerSpy: jest.SpyInstance; - let removeEventListenerSpy: jest.SpyInstance; - const applySpies: Transformer = dom => { - querySelectorSpy = jest.spyOn(dom, 'querySelector'); - querySelectorSpy.mockReturnValue({ scrollIntoView }); - addEventListenerSpy = jest.spyOn(dom, 'addEventListener'); - removeEventListenerSpy = jest.spyOn(dom, 'removeEventListener'); - return dom; - }; - - afterEach(() => { - jest.clearAllMocks(); - }); - - it('does nothing if there is no anchor element', async () => { - await createTestShadowDom(mkdocsIndex, { - preTransformers: [], - postTransformers: [applySpies, scrollIntoAnchor()], - }); - expect(querySelectorSpy).not.toHaveBeenCalled(); - expect(addEventListenerSpy).toHaveBeenCalled(); - expect(removeEventListenerSpy).toHaveBeenCalled(); - expect(addEventListenerSpy).toHaveBeenCalledWith( - SHADOW_DOM_STYLE_LOAD_EVENT, - expect.any(Function), - ); - expect(removeEventListenerSpy).toHaveBeenCalledTimes(1); - expect(removeEventListenerSpy).toHaveBeenCalledWith( - SHADOW_DOM_STYLE_LOAD_EVENT, - expect.any(Function), - ); - // check that the same function is passed to both addEventListener and removeEventListener - expect(addEventListenerSpy.mock.calls[0][1]).toBe( - removeEventListenerSpy.mock.calls[0][1], - ); - }); - - it('scroll to the hash anchor element', async () => { - window.location.hash = '#hash'; - await createTestShadowDom(mkdocsIndex, { - preTransformers: [], - postTransformers: [applySpies, scrollIntoAnchor()], - }); - expect(querySelectorSpy).toHaveBeenCalledWith( - expect.stringMatching('[id="hash"]'), - ); - expect(scrollIntoView).toHaveBeenCalledWith(); - expect(addEventListenerSpy).toHaveBeenCalledTimes(1); - expect(removeEventListenerSpy).toHaveBeenCalledTimes(1); - expect(addEventListenerSpy).toHaveBeenCalledWith( - SHADOW_DOM_STYLE_LOAD_EVENT, - expect.any(Function), - ); - expect(removeEventListenerSpy).toHaveBeenCalledTimes(1); - expect(removeEventListenerSpy).toHaveBeenCalledWith( - SHADOW_DOM_STYLE_LOAD_EVENT, - expect.any(Function), - ); - // check that the same function is passed to both addEventListener and removeEventListener - expect(addEventListenerSpy.mock.calls[0][1]).toBe( - removeEventListenerSpy.mock.calls[0][1], - ); - window.location.hash = ''; - }); - - it('works for anchor starting with number', async () => { - querySelectorSpy.mockReturnValue({ scrollIntoView }); - window.location.hash = '#1-hash'; - await createTestShadowDom(mkdocsIndex, { - preTransformers: [], - postTransformers: [applySpies, scrollIntoAnchor()], - }); - expect(querySelectorSpy).toHaveBeenCalledWith( - expect.stringMatching('[id="1-hash"]'), - ); - expect(scrollIntoView).toHaveBeenCalledWith(); - expect(addEventListenerSpy).toHaveBeenCalledTimes(1); - expect(addEventListenerSpy).toHaveBeenCalledWith( - SHADOW_DOM_STYLE_LOAD_EVENT, - expect.any(Function), - ); - expect(removeEventListenerSpy).toHaveBeenCalledTimes(1); - expect(removeEventListenerSpy).toHaveBeenCalledWith( - SHADOW_DOM_STYLE_LOAD_EVENT, - expect.any(Function), - ); - // check that the same function is passed to both addEventListener and removeEventListener - expect(addEventListenerSpy.mock.calls[0][1]).toBe( - removeEventListenerSpy.mock.calls[0][1], - ); - window.location.hash = ''; - }); -}); diff --git a/plugins/techdocs/src/reader/transformers/scrollIntoAnchor.ts b/plugins/techdocs/src/reader/transformers/scrollIntoAnchor.ts deleted file mode 100644 index 97a18b7241..0000000000 --- a/plugins/techdocs/src/reader/transformers/scrollIntoAnchor.ts +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright 2021 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import type { Transformer } from './transformer'; -import { SHADOW_DOM_STYLE_LOAD_EVENT } from '@backstage/plugin-techdocs-react'; - -export const scrollIntoAnchor = (): Transformer => { - return dom => { - dom.addEventListener( - SHADOW_DOM_STYLE_LOAD_EVENT, - function handleShadowDomStyleLoad() { - if (window.location.hash) { - const hash = window.location.hash.slice(1); - dom?.querySelector(`[id="${hash}"]`)?.scrollIntoView(); - } - dom.removeEventListener( - SHADOW_DOM_STYLE_LOAD_EVENT, - handleShadowDomStyleLoad, - ); - }, - ); - return dom; - }; -}; diff --git a/plugins/techdocs/src/setupTests.ts b/plugins/techdocs/src/setupTests.ts index 963c0f188b..6c7fc2d3e3 100644 --- a/plugins/techdocs/src/setupTests.ts +++ b/plugins/techdocs/src/setupTests.ts @@ -15,3 +15,5 @@ */ import '@testing-library/jest-dom'; + +Element.prototype.scrollIntoView = jest.fn();