diff --git a/flow-report/src/app.tsx b/flow-report/src/app.tsx index 280caadc540f..b199422e4f2c 100644 --- a/flow-report/src/app.tsx +++ b/flow-report/src/app.tsx @@ -5,25 +5,24 @@ */ import {FunctionComponent} from 'preact'; -import {useEffect, useRef, useState} from 'preact/hooks'; +import {useLayoutEffect, useRef, useState} from 'preact/hooks'; import {ReportRendererProvider} from './wrappers/report-renderer'; import {Sidebar} from './sidebar/sidebar'; import {Summary} from './summary/summary'; -import {classNames, FlowResultContext, useCurrentLhr, useHashParam} from './util'; +import {classNames, FlowResultContext, useHashState} from './util'; import {Report} from './wrappers/report'; import {Topbar} from './topbar'; import {Header} from './header'; import {I18nProvider} from './i18n/i18n'; const Content: FunctionComponent = () => { - const currentLhr = useCurrentLhr(); - const anchor = useHashParam('anchor'); + const hashState = useHashState(); const ref = useRef(null); - useEffect(() => { - if (anchor) { - const el = document.getElementById(anchor); + useLayoutEffect(() => { + if (hashState && hashState.anchor) { + const el = document.getElementById(hashState.anchor); if (el) { el.scrollIntoView({behavior: 'smooth'}); return; @@ -32,15 +31,15 @@ const Content: FunctionComponent = () => { // Scroll to top no anchor is found. if (ref.current) ref.current.scrollTop = 0; - }, [anchor, currentLhr]); + }, [hashState]); return (
{ - currentLhr ? + hashState ? <> -
- +
+ : } diff --git a/flow-report/src/header.tsx b/flow-report/src/header.tsx index 54f81d08de0c..ef69be451352 100644 --- a/flow-report/src/header.tsx +++ b/flow-report/src/header.tsx @@ -29,10 +29,10 @@ const HeaderThumbnail: FunctionComponent<{ ); }; -export const Header: FunctionComponent<{currentLhr: LH.FlowResult.LhrRef}> = -({currentLhr}) => { +export const Header: FunctionComponent<{hashState: LH.FlowResult.HashState}> = +({hashState}) => { const flowResult = useFlowResult(); - const {index} = currentLhr; + const {index} = hashState; const step = flowResult.steps[index]; const prevStep = flowResult.steps[index - 1]; diff --git a/flow-report/src/sidebar/flow.tsx b/flow-report/src/sidebar/flow.tsx index 395f30b88d81..f7b2e919e6f9 100644 --- a/flow-report/src/sidebar/flow.tsx +++ b/flow-report/src/sidebar/flow.tsx @@ -7,7 +7,7 @@ import {FunctionComponent} from 'preact'; import {FlowSegment} from '../common'; -import {classNames, useCurrentLhr, useFlowResult} from '../util'; +import {classNames, useHashState, useFlowResult} from '../util'; import {Separator} from '../common'; const SidebarFlowStep: FunctionComponent<{ @@ -41,7 +41,7 @@ const SidebarFlowSeparator: FunctionComponent = () => { export const SidebarFlow: FunctionComponent = () => { const flowResult = useFlowResult(); - const currentLhr = useCurrentLhr(); + const hashState = useHashState(); return (
@@ -61,7 +61,7 @@ export const SidebarFlow: FunctionComponent = () => { mode={lhr.gatherMode} href={url.href} label={name} - isCurrent={index === (currentLhr && currentLhr.index)} + isCurrent={index === (hashState && hashState.index)} /> ; }) diff --git a/flow-report/src/sidebar/sidebar.tsx b/flow-report/src/sidebar/sidebar.tsx index 8ddd5e13b3a6..0fcbf13930ba 100644 --- a/flow-report/src/sidebar/sidebar.tsx +++ b/flow-report/src/sidebar/sidebar.tsx @@ -9,11 +9,11 @@ import {FunctionComponent} from 'preact'; import {Separator} from '../common'; import {useI18n, useLocalizedStrings} from '../i18n/i18n'; import {CpuIcon, EnvIcon, SummaryIcon} from '../icons'; -import {classNames, useCurrentLhr, useFlowResult} from '../util'; +import {classNames, useHashState, useFlowResult} from '../util'; import {SidebarFlow} from './flow'; export const SidebarSummary: FunctionComponent = () => { - const currentLhr = useCurrentLhr(); + const hashState = useHashState(); const strings = useLocalizedStrings(); const url = new URL(location.href); @@ -21,7 +21,7 @@ export const SidebarSummary: FunctionComponent = () => { return (
diff --git a/flow-report/src/util.ts b/flow-report/src/util.ts index b63f7cdabc7c..32833938f1a9 100644 --- a/flow-report/src/util.ts +++ b/flow-report/src/util.ts @@ -79,27 +79,27 @@ export function useFlowResult(): LH.FlowResult { return flowResult; } -export function useHashParam(param: string) { - const [paramValue, setParamValue] = useState(getHashParam(param)); +export function useHashParams(...params: string[]) { + const [paramValues, setParamValues] = useState(params.map(getHashParam)); // Use two-way-binding on the URL hash. - // Triggers a re-render if the param changes. + // Triggers a re-render if any param changes. useEffect(() => { function hashListener() { - const newIndexString = getHashParam(param); - if (newIndexString === paramValue) return; - setParamValue(newIndexString); + const newParamValues = params.map(getHashParam); + if (newParamValues.every((value, i) => value === paramValues[i])) return; + setParamValues(newParamValues); } window.addEventListener('hashchange', hashListener); return () => window.removeEventListener('hashchange', hashListener); - }, [paramValue]); + }, [paramValues]); - return paramValue; + return paramValues; } -export function useCurrentLhr(): LH.FlowResult.LhrRef|null { +export function useHashState(): LH.FlowResult.HashState|null { const flowResult = useFlowResult(); - const indexString = useHashParam('index'); + const [indexString, anchor] = useHashParams('index', 'anchor'); // Memoize result so a new object is not created on every call. return useMemo(() => { @@ -117,6 +117,6 @@ export function useCurrentLhr(): LH.FlowResult.LhrRef|null { return null; } - return {value: step.lhr, index}; - }, [indexString, flowResult]); + return {currentLhr: step.lhr, index, anchor}; + }, [indexString, flowResult, anchor]); } diff --git a/flow-report/src/wrappers/report.tsx b/flow-report/src/wrappers/report.tsx index 5e976a223a96..eaa259b222dc 100644 --- a/flow-report/src/wrappers/report.tsx +++ b/flow-report/src/wrappers/report.tsx @@ -30,20 +30,25 @@ export function convertChildAnchors(element: HTMLElement, index: number) { const nodeId = link.hash.substr(1); link.hash = `#index=${index}&anchor=${nodeId}`; + link.onclick = e => { + e.preventDefault(); + const el = document.getElementById(nodeId); + if (el) el.scrollIntoView(); + }; } } -export const Report: FunctionComponent<{currentLhr: LH.FlowResult.LhrRef}> = -({currentLhr}) => { +export const Report: FunctionComponent<{hashState: LH.FlowResult.HashState}> = +({hashState}) => { const {dom, reportRenderer} = useReportRenderer(); const ref = useRef(null); useLayoutEffect(() => { if (ref.current) { dom.clearComponentCache(); - reportRenderer.renderReport(currentLhr.value, ref.current); - convertChildAnchors(ref.current, currentLhr.index); - const fullPageScreenshot = getFullPageScreenshot(currentLhr.value); + reportRenderer.renderReport(hashState.currentLhr, ref.current); + convertChildAnchors(ref.current, hashState.index); + const fullPageScreenshot = getFullPageScreenshot(hashState.currentLhr); if (fullPageScreenshot) { const container = dom.find('.lh-container', ref.current); ElementScreenshotRenderer.installOverlayFeature({ @@ -60,7 +65,7 @@ export const Report: FunctionComponent<{currentLhr: LH.FlowResult.LhrRef}> = return () => { if (ref.current) ref.current.textContent = ''; }; - }, [reportRenderer, currentLhr]); + }, [reportRenderer, hashState]); return (
diff --git a/flow-report/test/header-test.tsx b/flow-report/test/header-test.tsx index 3a061d7a6c37..86ee5df5eef2 100644 --- a/flow-report/test/header-test.tsx +++ b/flow-report/test/header-test.tsx @@ -25,8 +25,8 @@ beforeEach(() => { }); it('renders all sections for a middle step', () => { - const currentLhr = {index: 1} as any; - const root = render(
, {wrapper}); + const hashState = {index: 1} as any; + const root = render(
, {wrapper}); expect(root.baseElement.querySelector('.Header__prev-thumbnail')).toBeTruthy(); expect(root.baseElement.querySelector('.Header__prev-title')).toBeTruthy(); @@ -35,8 +35,8 @@ it('renders all sections for a middle step', () => { }); it('renders only next section for first step', () => { - const currentLhr = {index: 0} as any; - const root = render(
, {wrapper}); + const hashState = {index: 0} as any; + const root = render(
, {wrapper}); expect(root.baseElement.querySelector('.Header__prev-thumbnail')).toBeFalsy(); expect(root.baseElement.querySelector('.Header__prev-title')).toBeFalsy(); @@ -45,8 +45,8 @@ it('renders only next section for first step', () => { }); it('renders only previous section for last step', () => { - const currentLhr = {index: 3} as any; - const root = render(
, {wrapper}); + const hashState = {index: 3} as any; + const root = render(
, {wrapper}); expect(root.baseElement.querySelector('.Header__prev-thumbnail')).toBeTruthy(); expect(root.baseElement.querySelector('.Header__prev-title')).toBeTruthy(); diff --git a/flow-report/test/util-test.tsx b/flow-report/test/util-test.tsx index 8c400ebedc34..1dbf2c632f7c 100644 --- a/flow-report/test/util-test.tsx +++ b/flow-report/test/util-test.tsx @@ -9,7 +9,7 @@ import {renderHook} from '@testing-library/preact-hooks'; import {FunctionComponent} from 'preact'; import {act} from 'preact/test-utils'; -import {FlowResultContext, useCurrentLhr} from '../src/util'; +import {FlowResultContext, useHashState} from '../src/util'; import {flowResult} from './sample-flow'; let wrapper: FunctionComponent; @@ -21,54 +21,75 @@ beforeEach(() => { ); }); -describe('useCurrentLhr', () => { +describe('useHashState', () => { it('gets current lhr index from url hash', () => { global.location.hash = '#index=1'; - const {result} = renderHook(() => useCurrentLhr(), {wrapper}); + const {result} = renderHook(() => useHashState(), {wrapper}); expect(console.warn).not.toHaveBeenCalled(); expect(result.current).toEqual({ index: 1, - value: flowResult.steps[1].lhr, + currentLhr: flowResult.steps[1].lhr, + anchor: null, + }); + }); + + it('gets anchor id from url hash', () => { + global.location.hash = '#index=1&anchor=seo'; + const {result} = renderHook(() => useHashState(), {wrapper}); + expect(console.warn).not.toHaveBeenCalled(); + expect(result.current).toEqual({ + index: 1, + currentLhr: flowResult.steps[1].lhr, + anchor: 'seo', }); }); it('changes on navigation', async () => { global.location.hash = '#index=1'; - const render = renderHook(() => useCurrentLhr(), {wrapper}); + const render = renderHook(() => useHashState(), {wrapper}); expect(render.result.current).toEqual({ index: 1, - value: flowResult.steps[1].lhr, + currentLhr: flowResult.steps[1].lhr, + anchor: null, }); await act(() => { - global.location.hash = '#index=2'; + global.location.hash = '#index=2&anchor=seo'; }); await render.waitForNextUpdate(); expect(console.warn).not.toHaveBeenCalled(); expect(render.result.current).toEqual({ index: 2, - value: flowResult.steps[2].lhr, + currentLhr: flowResult.steps[2].lhr, + anchor: 'seo', }); }); it('return null if lhr index is unset', () => { - const {result} = renderHook(() => useCurrentLhr(), {wrapper}); + const {result} = renderHook(() => useHashState(), {wrapper}); expect(console.warn).not.toHaveBeenCalled(); expect(result.current).toBeNull(); }); it('return null if lhr index is out of bounds', () => { global.location.hash = '#index=5'; - const {result} = renderHook(() => useCurrentLhr(), {wrapper}); + const {result} = renderHook(() => useHashState(), {wrapper}); expect(console.warn).toHaveBeenCalled(); expect(result.current).toBeNull(); }); it('returns null for invalid value', () => { global.location.hash = '#index=OHNO'; - const {result} = renderHook(() => useCurrentLhr(), {wrapper}); + const {result} = renderHook(() => useHashState(), {wrapper}); + expect(console.warn).toHaveBeenCalled(); + expect(result.current).toBeNull(); + }); + + it('returns null for invalid value with valid anchor', () => { + global.location.hash = '#index=OHNO&anchor=seo'; + const {result} = renderHook(() => useHashState(), {wrapper}); expect(console.warn).toHaveBeenCalled(); expect(result.current).toBeNull(); }); diff --git a/types/lhr/flow.d.ts b/types/lhr/flow.d.ts index 74a4b2a295a4..451680e2c717 100644 --- a/types/lhr/flow.d.ts +++ b/types/lhr/flow.d.ts @@ -12,9 +12,10 @@ declare module FlowResult { name: string; } - interface LhrRef { - value: Result; + interface HashState { + currentLhr: Result; index: number; + anchor: string|null; } }