From c8e09cc383da3b2126b69aa8b8f64449b1cf78fb Mon Sep 17 00:00:00 2001 From: Gabriel Testault Date: Thu, 5 Jan 2023 16:16:48 +0100 Subject: [PATCH] fix(techdocs): Fixed bug in Techdocs reader where a techdocs page with a hash in the URL did not always jump to the document anchor. the scrollIntoAnchor transformer now jumps to the anchor only after css waas loaded instead of jumping after 200ms. Signed-off-by: Gabriel Testault --- .changeset/gold-masks-sleep.md | 5 + .../transformers/scrollIntoAnchor.test.ts | 92 +++++++++++++++---- .../reader/transformers/scrollIntoAnchor.ts | 23 +++-- 3 files changed, 95 insertions(+), 25 deletions(-) create mode 100644 .changeset/gold-masks-sleep.md diff --git a/.changeset/gold-masks-sleep.md b/.changeset/gold-masks-sleep.md new file mode 100644 index 0000000000..425a1abdc0 --- /dev/null +++ b/.changeset/gold-masks-sleep.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-techdocs': patch +--- + +Fixed bug in Techdocs reader where a techdocs page with a hash in the URL did not always jump to the document anchor. diff --git a/plugins/techdocs/src/reader/transformers/scrollIntoAnchor.test.ts b/plugins/techdocs/src/reader/transformers/scrollIntoAnchor.test.ts index 896f909be4..e222c24a4c 100644 --- a/plugins/techdocs/src/reader/transformers/scrollIntoAnchor.test.ts +++ b/plugins/techdocs/src/reader/transformers/scrollIntoAnchor.test.ts @@ -15,46 +15,104 @@ */ import { scrollIntoAnchor } from '../transformers'; - -jest.useFakeTimers(); +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 transformer = scrollIntoAnchor(); - const dom = { querySelector: jest.fn() }; + 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 () => { - transformer(dom as unknown as Element); - jest.advanceTimersByTime(200); - expect(dom.querySelector).not.toHaveBeenCalled(); + 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 () => { - const scrollIntoView = jest.fn(); - dom.querySelector.mockReturnValue({ scrollIntoView }); window.location.hash = '#hash'; - transformer(dom as unknown as Element); - jest.advanceTimersByTime(200); - expect(dom.querySelector).toHaveBeenCalledWith( + 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 () => { - const scrollIntoView = jest.fn(); - dom.querySelector.mockReturnValue({ scrollIntoView }); + querySelectorSpy.mockReturnValue({ scrollIntoView }); window.location.hash = '#1-hash'; - transformer(dom as unknown as Element); - jest.advanceTimersByTime(200); - expect(dom.querySelector).toHaveBeenCalledWith( + 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 index 7ba02bcdfc..0be6fefc0b 100644 --- a/plugins/techdocs/src/reader/transformers/scrollIntoAnchor.ts +++ b/plugins/techdocs/src/reader/transformers/scrollIntoAnchor.ts @@ -15,17 +15,24 @@ */ import type { Transformer } from './transformer'; +import { SHADOW_DOM_STYLE_LOAD_EVENT } from '@backstage/plugin-techdocs-react'; export const scrollIntoAnchor = (): Transformer => { return dom => { - setTimeout(() => { - // Scroll to the desired anchor on initial navigation - if (window.location.hash) { - const hash = window.location.hash.slice(1); - // fix invalid selector error for anchor starting with number - dom?.querySelector(`[id="${hash}"]`)?.scrollIntoView(); - } - }, 200); + dom.addEventListener( + SHADOW_DOM_STYLE_LOAD_EVENT, + function handleShadowDomStyleLoad() { + if (window.location.hash) { + const hash = window.location.hash.slice(1); + // fix invalid selector error for anchor starting with number + dom?.querySelector(`[id="${hash}"]`)?.scrollIntoView(); + } + dom.removeEventListener( + SHADOW_DOM_STYLE_LOAD_EVENT, + handleShadowDomStyleLoad, + ); + }, + ); return dom; }; };