From 4a2f73a302c83c70c88905c1f462694fdf4b3a00 Mon Sep 17 00:00:00 2001 From: Thomas Cardonne Date: Thu, 17 Oct 2024 14:04:39 +0200 Subject: [PATCH] fix(techdocs): avoid rerender current page when navigating to another (#26944) Signed-off-by: Thomas Cardonne --- .changeset/slimy-jobs-scream.md | 7 ++++ plugins/techdocs-react/src/component.test.tsx | 26 ------------- plugins/techdocs-react/src/component.tsx | 4 -- .../TechDocsReaderPageContent.test.tsx | 39 ++++++++++++++++++- .../TechDocsReaderPageContent.tsx | 10 ++++- .../TechDocsReaderPageContent/dom.tsx | 7 ++++ 6 files changed, 60 insertions(+), 33 deletions(-) create mode 100644 .changeset/slimy-jobs-scream.md diff --git a/.changeset/slimy-jobs-scream.md b/.changeset/slimy-jobs-scream.md new file mode 100644 index 0000000000..e20119fed4 --- /dev/null +++ b/.changeset/slimy-jobs-scream.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-techdocs-react': patch +'@backstage/plugin-techdocs': patch +--- + +Fix an issue that caused the current documentation page to be re-rendered when navigating to +another one. diff --git a/plugins/techdocs-react/src/component.test.tsx b/plugins/techdocs-react/src/component.test.tsx index 819f8fdaad..4463932f05 100644 --- a/plugins/techdocs-react/src/component.test.tsx +++ b/plugins/techdocs-react/src/component.test.tsx @@ -64,24 +64,6 @@ describe('TechDocsShadowDom', () => { expect(onAppend).toHaveBeenCalledTimes(2); }); - it('Should show progress bar while styles are being loaded', async () => { - const dom = createDom( - '

Title

', - ); - const onAppend = jest.fn(); - dom.querySelector('link[rel="stylesheet"]')!.addEventListener = () => {}; - - render( - - Children - , - ); - - await await waitFor(() => { - expect(screen.getByRole('progressbar')).toBeInTheDocument(); - }); - }); - it('Should dispatch an event after all styles are loaded', async () => { const dom = createDom( '

Title

', @@ -98,16 +80,8 @@ describe('TechDocsShadowDom', () => { render(Children); - await await waitFor(() => { - expect(screen.getByRole('progressbar')).toBeInTheDocument(); - }); - listener({} as Event); - await waitFor(() => { - expect(screen.queryByRole('progressbar')).not.toBeInTheDocument(); - }); - expect(handleStylesLoad).toHaveBeenCalledTimes(1); }); }); diff --git a/plugins/techdocs-react/src/component.tsx b/plugins/techdocs-react/src/component.tsx index 0388aafed8..7d20565d60 100644 --- a/plugins/techdocs-react/src/component.tsx +++ b/plugins/techdocs-react/src/component.tsx @@ -25,8 +25,6 @@ import { create } from 'jss'; import StylesProvider from '@material-ui/styles/StylesProvider'; import jssPreset from '@material-ui/styles/jssPreset'; -import { Progress } from '@backstage/core-components'; - /** * Name for the event dispatched when ShadowRoot styles are loaded. * @public @@ -216,7 +214,6 @@ export const TechDocsShadowDom = (props: TechDocsShadowDomProps) => { ); useShadowDomStylesEvents(element); - const loading = useShadowDomStylesLoading(element); const ref = useCallback( (shadowHost: HTMLDivElement) => { @@ -246,7 +243,6 @@ export const TechDocsShadowDom = (props: TechDocsShadowDomProps) => { return ( <> - {loading && } {/* The sheetsManager={new Map()} is needed in order to deduplicate the injection of CSS in the page. */}
diff --git a/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.test.tsx b/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.test.tsx index 322ddf5592..3c6756e058 100644 --- a/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.test.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.test.tsx @@ -34,9 +34,11 @@ jest.mock('../useReaderState', () => ({ ...jest.requireActual('../useReaderState'), useReaderState: (...args: any[]) => useReaderState(...args), })); +const useShadowDomStylesLoading = jest.fn().mockReturnValue(false); jest.mock('@backstage/plugin-techdocs-react', () => ({ ...jest.requireActual('@backstage/plugin-techdocs-react'), - useShadowDomStylesLoading: jest.fn().mockReturnValue(false), + useShadowDomStylesLoading: (...args: any[]) => + useShadowDomStylesLoading(...args), useShadowRootElements: jest.fn(), })); @@ -220,4 +222,39 @@ describe('', () => { window.location.hash = ''; }); + + it('should render progress bar when content is loading', async () => { + getEntityMetadata.mockResolvedValue(mockEntityMetadata); + getTechDocsMetadata.mockResolvedValue(mockTechDocsMetadata); + useTechDocsReaderDom.mockReturnValue(document.createElement('html')); + useReaderState.mockReturnValue({ state: 'CHECKING' }); + + const rendered = await renderInTestApp( + + + , + ); + + await waitFor(() => { + expect(rendered.queryByRole('progressbar')).toBeInTheDocument(); + }); + }); + + it('should render progress bar when styles are loading', async () => { + getEntityMetadata.mockResolvedValue(mockEntityMetadata); + getTechDocsMetadata.mockResolvedValue(mockTechDocsMetadata); + useTechDocsReaderDom.mockReturnValue(document.createElement('html')); + useReaderState.mockReturnValue({ state: 'cached' }); + useShadowDomStylesLoading.mockReturnValue(true); + + const rendered = await renderInTestApp( + + + , + ); + + await waitFor(() => { + expect(rendered.queryByRole('progressbar')).toBeInTheDocument(); + }); + }); }); diff --git a/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.tsx b/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.tsx index 3692ed22d3..21418a716e 100644 --- a/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.tsx @@ -26,13 +26,16 @@ import { useTechDocsReaderPage, } from '@backstage/plugin-techdocs-react'; import { CompoundEntityRef } from '@backstage/catalog-model'; -import { Content, ErrorPage } from '@backstage/core-components'; +import { Content, ErrorPage, Progress } from '@backstage/core-components'; import { TechDocsSearch } from '../../../search'; import { TechDocsStateIndicator } from '../TechDocsStateIndicator'; import { useTechDocsReaderDom } from './dom'; -import { withTechDocsReaderProvider } from '../TechDocsReaderProvider'; +import { + useTechDocsReader, + withTechDocsReaderProvider, +} from '../TechDocsReaderProvider'; import { TechDocsReaderPageContentAddons } from './TechDocsReaderPageContentAddons'; const useStyles = makeStyles({ @@ -81,6 +84,7 @@ export const TechDocsReaderPageContent = withTechDocsReaderProvider( entityRef, setShadowRoot, } = useTechDocsReaderPage(); + const { state } = useTechDocsReader(); const dom = useTechDocsReaderDom(entityRef); const path = window.location.pathname; const hash = window.location.hash; @@ -143,6 +147,8 @@ export const TechDocsReaderPageContent = withTechDocsReaderProvider( )} {/* Centers the styles loaded event to avoid having multiple locations setting the opacity style in Shadow Dom causing the screen to flash multiple times */} + {(state === 'CHECKING' || isStyleLoading) && } + diff --git a/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/dom.tsx b/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/dom.tsx index 141f869ecf..3656d22326 100644 --- a/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/dom.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/dom.tsx @@ -276,6 +276,12 @@ export const useTechDocsReaderDom = ( return; } + // Skip this update if the location's path has changed but the state + // contains a page for another page that isn't loaded yet. + if (!window.location.pathname.endsWith(path)) { + return; + } + // Scroll to top after render window.scroll({ top: 0 }); @@ -283,6 +289,7 @@ export const useTechDocsReaderDom = ( const postTransformedDomElement = await postRender( preTransformedDomElement, ); + setDom(postTransformedDomElement as HTMLElement); });