report(flow): fix report anchors

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<HTMLDivElement>(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 (
     <div ref={ref} className="Content">
       {
-        currentLhr ?
+        hashState ?
           <>
-            <Header currentLhr={currentLhr}/>
-            <Report currentLhr={currentLhr}/>
+            <Header hashState={hashState}/>
+            <Report hashState={hashState}/>
           </> :
           <Summary/>
       }

flow-report/src/header.tsx

@@ -30,10 +30,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];

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 (
     <div className="SidebarFlow">
@@ -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)}
             />
           </>;
         })

flow-report/src/sidebar/sidebar.tsx

@@ -9,19 +9,19 @@ 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);
   url.hash = '#';
   return (
     <a
       href={url.href}
-      className={classNames('SidebarSummary', {'Sidebar--current': currentLhr === null})}
+      className={classNames('SidebarSummary', {'Sidebar--current': hashState === null})}
       data-testid="SidebarSummary"
     >
       <div className="SidebarSummary__icon">

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]);
 }

flow-report/src/wrappers/report.tsx

@@ -28,27 +28,32 @@ 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<HTMLDivElement>(null);
 
   useLayoutEffect(() => {
     if (ref.current) {
       dom.clearComponentCache();
-      reportRenderer.renderReport(currentLhr.value, ref.current);
-      convertChildAnchors(ref.current, currentLhr.index);
+      reportRenderer.renderReport(hashState.currentLhr, ref.current);
+      convertChildAnchors(ref.current, hashState.index);
       const topbar = ref.current.querySelector('.lh-topbar');
       if (topbar) topbar.remove();
     }
 
     return () => {
       if (ref.current) ref.current.textContent = '';
     };
-  }, [reportRenderer, currentLhr]);
+  }, [reportRenderer, hashState]);
 
   return (
     <div ref={ref} className="lh-root" data-testid="Report"/>

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(<Header currentLhr={currentLhr}/>, {wrapper});
+  const hashState = {index: 1} as any;
+  const root = render(<Header hashState={hashState}/>, {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(<Header currentLhr={currentLhr}/>, {wrapper});
+  const hashState = {index: 0} as any;
+  const root = render(<Header hashState={hashState}/>, {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(<Header currentLhr={currentLhr}/>, {wrapper});
+  const hashState = {index: 3} as any;
+  const root = render(<Header hashState={hashState}/>, {wrapper});
 
   expect(root.baseElement.querySelector('.Header__prev-thumbnail')).toBeTruthy();
   expect(root.baseElement.querySelector('.Header__prev-title')).toBeTruthy();

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();
   });

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;
   }
 }