From 4979ce2b5d0d0a9a210c0549f66c2a878c8d7dff Mon Sep 17 00:00:00 2001
From: Simon Knott <info@simonknott.de>
Date: Tue, 19 Nov 2024 16:40:02 +0100
Subject: [PATCH] chore(html): Reveal elements with `Anchor` abstraction
 (#33537)

---
 packages/html-reporter/src/chip.tsx           | 15 ++++----
 packages/html-reporter/src/links.tsx          | 29 +++++++++++++++
 packages/html-reporter/src/reportView.tsx     |  2 --
 .../html-reporter/src/testCaseView.spec.tsx   | 12 +++----
 packages/html-reporter/src/testCaseView.tsx   |  5 ++-
 packages/html-reporter/src/testFileView.tsx   |  4 +--
 packages/html-reporter/src/testResultView.tsx | 36 ++++++-------------
 7 files changed, 58 insertions(+), 45 deletions(-)

diff --git a/packages/html-reporter/src/chip.tsx b/packages/html-reporter/src/chip.tsx
index 0965a408886bd..f94dcbc6d680f 100644
--- a/packages/html-reporter/src/chip.tsx
+++ b/packages/html-reporter/src/chip.tsx
@@ -20,6 +20,7 @@ import './colors.css';
 import './common.css';
 import * as icons from './icons';
 import { clsx } from '@web/uiUtils';
+import { useAnchor } from './links';
 
 export const Chip: React.FC<{
   header: JSX.Element | string,
@@ -28,10 +29,9 @@ export const Chip: React.FC<{
   setExpanded?: (expanded: boolean) => void,
   children?: any,
   dataTestId?: string,
-  targetRef?: React.RefObject<HTMLDivElement>,
-}> = ({ header, expanded, setExpanded, children, noInsets, dataTestId, targetRef }) => {
+}> = ({ header, expanded, setExpanded, children, noInsets, dataTestId }) => {
   const id = React.useId();
-  return <div className='chip' data-testid={dataTestId} ref={targetRef}>
+  return <div className='chip' data-testid={dataTestId}>
     <div
       role='button'
       aria-expanded={!!expanded}
@@ -53,16 +53,17 @@ export const AutoChip: React.FC<{
   noInsets?: boolean,
   children?: any,
   dataTestId?: string,
-  targetRef?: React.RefObject<HTMLDivElement>,
-}> = ({ header, initialExpanded, noInsets, children, dataTestId, targetRef }) => {
-  const [expanded, setExpanded] = React.useState(initialExpanded || initialExpanded === undefined);
+  revealOnAnchorId?: string,
+}> = ({ header, initialExpanded, noInsets, children, dataTestId, revealOnAnchorId }) => {
+  const [expanded, setExpanded] = React.useState(initialExpanded ?? true);
+  const onReveal = React.useCallback(() => setExpanded(true), []);
+  useAnchor(revealOnAnchorId, onReveal);
   return <Chip
     header={header}
     expanded={expanded}
     setExpanded={setExpanded}
     noInsets={noInsets}
     dataTestId={dataTestId}
-    targetRef={targetRef}
   >
     {children}
   </Chip>;
diff --git a/packages/html-reporter/src/links.tsx b/packages/html-reporter/src/links.tsx
index 4b48090e0a5f3..4beb7b65e049c 100644
--- a/packages/html-reporter/src/links.tsx
+++ b/packages/html-reporter/src/links.tsx
@@ -113,3 +113,32 @@ export function generateTraceUrl(traces: TestAttachment[]) {
 }
 
 const kMissingContentType = 'x-playwright/missing';
+
+type AnchorID = string | ((id: string | null) => boolean) | undefined;
+
+export function useAnchor(id: AnchorID, onReveal: () => void) {
+  React.useEffect(() => {
+    if (typeof id === 'undefined')
+      return;
+
+    const listener = () => {
+      const params = new URLSearchParams(window.location.hash.slice(1));
+      const anchor = params.get('anchor');
+      const isRevealed = typeof id === 'function' ? id(anchor) : anchor === id;
+      if (isRevealed)
+        onReveal();
+    };
+    window.addEventListener('popstate', listener);
+    return () => window.removeEventListener('popstate', listener);
+  }, [id, onReveal]);
+}
+
+export function Anchor({ id, children }: React.PropsWithChildren<{ id: AnchorID }>) {
+  const ref = React.useRef<HTMLDivElement>(null);
+  const onAnchorReveal = React.useCallback(() => {
+    requestAnimationFrame(() => ref.current?.scrollIntoView({ block: 'start', inline: 'start' }));
+  }, []);
+  useAnchor(id, onAnchorReveal);
+
+  return <div ref={ref}>{children}</div>;
+}
diff --git a/packages/html-reporter/src/reportView.tsx b/packages/html-reporter/src/reportView.tsx
index e8a5c3b250d4f..cf0f5e5e56794 100644
--- a/packages/html-reporter/src/reportView.tsx
+++ b/packages/html-reporter/src/reportView.tsx
@@ -101,7 +101,6 @@ const TestCaseViewLoader: React.FC<{
   const searchParams = React.useContext(SearchParamsContext);
   const [test, setTest] = React.useState<TestCase | undefined>();
   const testId = searchParams.get('testId');
-  const anchor = (searchParams.get('anchor') || '') as 'video' | 'diff' | '';
   const run = +(searchParams.get('run') || '0');
 
   const { prev, next } = React.useMemo(() => {
@@ -133,7 +132,6 @@ const TestCaseViewLoader: React.FC<{
     next={next}
     prev={prev}
     test={test}
-    anchor={anchor}
     run={run}
   />;
 };
diff --git a/packages/html-reporter/src/testCaseView.spec.tsx b/packages/html-reporter/src/testCaseView.spec.tsx
index 892ad51b7f911..b7a9f9405b8eb 100644
--- a/packages/html-reporter/src/testCaseView.spec.tsx
+++ b/packages/html-reporter/src/testCaseView.spec.tsx
@@ -63,7 +63,7 @@ const testCase: TestCase = {
 };
 
 test('should render test case', async ({ mount }) => {
-  const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={testCase} prev={undefined} next={undefined} run={0} anchor=''></TestCaseView>);
+  const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={testCase} prev={undefined} next={undefined} run={0}></TestCaseView>);
   await expect(component.getByText('Annotation text', { exact: false }).first()).toBeVisible();
   await expect(component.getByText('Hidden annotation')).toBeHidden();
   await component.getByText('Annotations').click();
@@ -79,7 +79,7 @@ test('should render test case', async ({ mount }) => {
 test('should render copy buttons for annotations', async ({ mount, page, context }) => {
   await context.grantPermissions(['clipboard-read', 'clipboard-write']);
 
-  const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={testCase} prev={undefined} next={undefined} run={0} anchor=''></TestCaseView>);
+  const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={testCase} prev={undefined} next={undefined} run={0}></TestCaseView>);
   await expect(component.getByText('Annotation text', { exact: false }).first()).toBeVisible();
   await component.getByText('Annotation text', { exact: false }).first().hover();
   await expect(component.locator('.test-case-annotation').getByLabel('Copy to clipboard').first()).toBeVisible();
@@ -108,7 +108,7 @@ const annotationLinkRenderingTestCase: TestCase = {
 };
 
 test('should correctly render links in annotations', async ({ mount }) => {
-  const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={annotationLinkRenderingTestCase} prev={undefined} next={undefined} run={0} anchor=''></TestCaseView>);
+  const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={annotationLinkRenderingTestCase} prev={undefined} next={undefined} run={0}></TestCaseView>);
 
   const firstLink = await component.getByText('https://playwright.dev/docs/intro').first();
   await expect(firstLink).toBeVisible();
@@ -181,7 +181,7 @@ const testCaseSummary: TestCaseSummary = {
 
 
 test('should correctly render links in attachments', async ({ mount }) => {
-  const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={attachmentLinkRenderingTestCase} prev={undefined} next={undefined} run={0} anchor=''></TestCaseView>);
+  const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={attachmentLinkRenderingTestCase} prev={undefined} next={undefined} run={0}></TestCaseView>);
   await component.getByText('first attachment').click();
   const body = await component.getByText('The body with https://playwright.dev/docs/intro link');
   await expect(body).toBeVisible();
@@ -194,7 +194,7 @@ test('should correctly render links in attachments', async ({ mount }) => {
 });
 
 test('should correctly render links in attachment name', async ({ mount }) => {
-  const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={attachmentLinkRenderingTestCase} prev={undefined} next={undefined} run={0} anchor=''></TestCaseView>);
+  const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={attachmentLinkRenderingTestCase} prev={undefined} next={undefined} run={0}></TestCaseView>);
   const link = component.getByText('attachment with inline link').locator('a');
   await expect(link).toHaveAttribute('href', 'https://github.com/microsoft/playwright/issues/31284');
   await expect(link).toHaveText('https://github.com/microsoft/playwright/issues/31284');
@@ -204,7 +204,7 @@ test('should correctly render links in attachment name', async ({ mount }) => {
 });
 
 test('should correctly render prev and next', async ({ mount }) => {
-  const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={attachmentLinkRenderingTestCase} prev={testCaseSummary} next={testCaseSummary} run={0} anchor=''></TestCaseView>);
+  const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={attachmentLinkRenderingTestCase} prev={testCaseSummary} next={testCaseSummary} run={0}></TestCaseView>);
   await expect(component).toMatchAriaSnapshot(`
     - text: group
     - link "« previous"
diff --git a/packages/html-reporter/src/testCaseView.tsx b/packages/html-reporter/src/testCaseView.tsx
index 320722fa9bed3..4e9785ad8ae35 100644
--- a/packages/html-reporter/src/testCaseView.tsx
+++ b/packages/html-reporter/src/testCaseView.tsx
@@ -33,9 +33,8 @@ export const TestCaseView: React.FC<{
   test: TestCase | undefined,
   next: TestCaseSummary | undefined,
   prev: TestCaseSummary | undefined,
-  anchor: 'video' | 'diff' | '',
   run: number,
-}> = ({ projectNames, test, run, anchor, next, prev }) => {
+}> = ({ projectNames, test, run, next, prev }) => {
   const [selectedResultIndex, setSelectedResultIndex] = React.useState(run);
   const searchParams = React.useContext(SearchParamsContext);
   const filterParam = searchParams.has('q') ? '&q=' + searchParams.get('q') : '';
@@ -79,7 +78,7 @@ export const TestCaseView: React.FC<{
       test.results.map((result, index) => ({
         id: String(index),
         title: <div style={{ display: 'flex', alignItems: 'center' }}>{statusIcon(result.status)} {retryLabel(index)}</div>,
-        render: () => <TestResultView test={test!} result={result} anchor={anchor}></TestResultView>
+        render: () => <TestResultView test={test!} result={result} />
       })) || []} selectedTab={String(selectedResultIndex)} setSelectedTab={id => setSelectedResultIndex(+id)} />}
   </div>;
 };
diff --git a/packages/html-reporter/src/testFileView.tsx b/packages/html-reporter/src/testFileView.tsx
index 4d6890ad33393..6b31d2ebe2d53 100644
--- a/packages/html-reporter/src/testFileView.tsx
+++ b/packages/html-reporter/src/testFileView.tsx
@@ -75,12 +75,12 @@ function imageDiffBadge(test: TestCaseSummary): JSX.Element | undefined {
   const resultWithImageDiff = test.results.find(result => result.attachments.some(attachment => {
     return attachment.contentType.startsWith('image/') && !!attachment.name.match(/-(expected|actual|diff)/);
   }));
-  return resultWithImageDiff ? <Link href={`#?testId=${test.testId}&anchor=diff&run=${test.results.indexOf(resultWithImageDiff)}`} title='View images' className='test-file-badge'>{image()}</Link> : undefined;
+  return resultWithImageDiff ? <Link href={`#?testId=${test.testId}&anchor=diff-0&run=${test.results.indexOf(resultWithImageDiff)}`} title='View images' className='test-file-badge'>{image()}</Link> : undefined;
 }
 
 function videoBadge(test: TestCaseSummary): JSX.Element | undefined {
   const resultWithVideo = test.results.find(result => result.attachments.some(attachment => attachment.name === 'video'));
-  return resultWithVideo ? <Link href={`#?testId=${test.testId}&anchor=video&run=${test.results.indexOf(resultWithVideo)}`} title='View video' className='test-file-badge'>{video()}</Link> : undefined;
+  return resultWithVideo ? <Link href={`#?testId=${test.testId}&anchor=videos&run=${test.results.indexOf(resultWithVideo)}`} title='View video' className='test-file-badge'>{video()}</Link> : undefined;
 }
 
 function traceBadge(test: TestCaseSummary): JSX.Element | undefined {
diff --git a/packages/html-reporter/src/testResultView.tsx b/packages/html-reporter/src/testResultView.tsx
index 3a562f3fcf789..9170f2023dceb 100644
--- a/packages/html-reporter/src/testResultView.tsx
+++ b/packages/html-reporter/src/testResultView.tsx
@@ -20,7 +20,7 @@ import { TreeItem } from './treeItem';
 import { msToString } from './utils';
 import { AutoChip } from './chip';
 import { traceImage } from './images';
-import { AttachmentLink, generateTraceUrl } from './links';
+import { Anchor, AttachmentLink, generateTraceUrl } from './links';
 import { statusIcon } from './statusIcon';
 import type { ImageDiff } from '@web/shared/imageDiffView';
 import { ImageDiffView } from '@web/shared/imageDiffView';
@@ -64,9 +64,7 @@ function groupImageDiffs(screenshots: Set<TestAttachment>): ImageDiff[] {
 export const TestResultView: React.FC<{
   test: TestCase,
   result: TestResult,
-  anchor: 'video' | 'diff' | '',
-}> = ({ result, anchor }) => {
-
+}> = ({ result }) => {
   const { screenshots, videos, traces, otherAttachments, diffs, errors, htmls } = React.useMemo(() => {
     const attachments = result?.attachments || [];
     const screenshots = new Set(attachments.filter(a => a.contentType.startsWith('image/')));
@@ -80,20 +78,6 @@ export const TestResultView: React.FC<{
     return { screenshots: [...screenshots], videos, traces, otherAttachments, diffs, errors, htmls };
   }, [result]);
 
-  const videoRef = React.useRef<HTMLDivElement>(null);
-  const imageDiffRef = React.useRef<HTMLDivElement>(null);
-
-  const [scrolled, setScrolled] = React.useState(false);
-  React.useEffect(() => {
-    if (scrolled)
-      return;
-    setScrolled(true);
-    if (anchor === 'video')
-      videoRef.current?.scrollIntoView({ block: 'start', inline: 'start' });
-    if (anchor === 'diff')
-      imageDiffRef.current?.scrollIntoView({ block: 'start', inline: 'start' });
-  }, [scrolled, anchor, setScrolled, videoRef]);
-
   return <div className='test-result'>
     {!!errors.length && <AutoChip header='Errors'>
       {errors.map((error, index) => {
@@ -107,9 +91,11 @@ export const TestResultView: React.FC<{
     </AutoChip>}
 
     {diffs.map((diff, index) =>
-      <AutoChip key={`diff-${index}`} dataTestId='test-results-image-diff' header={`Image mismatch: ${diff.name}`} targetRef={imageDiffRef}>
-        <ImageDiffView key='image-diff' diff={diff}></ImageDiffView>
-      </AutoChip>
+      <Anchor key={`diff-${index}`} id={`diff-${index}`}>
+        <AutoChip dataTestId='test-results-image-diff' header={`Image mismatch: ${diff.name}`} revealOnAnchorId={`diff-${index}`}>
+          <ImageDiffView diff={diff}/>
+        </AutoChip>
+      </Anchor>
     )}
 
     {!!screenshots.length && <AutoChip header='Screenshots'>
@@ -123,23 +109,23 @@ export const TestResultView: React.FC<{
       })}
     </AutoChip>}
 
-    {!!traces.length && <AutoChip header='Traces'>
+    {!!traces.length && <Anchor id='traces'><AutoChip header='Traces' revealOnAnchorId='traces'>
       {<div>
         <a href={generateTraceUrl(traces)}>
           <img className='screenshot' src={traceImage} style={{ width: 192, height: 117, marginLeft: 20 }} />
         </a>
         {traces.map((a, i) => <AttachmentLink key={`trace-${i}`} attachment={a} linkName={traces.length === 1 ? 'trace' : `trace-${i + 1}`}></AttachmentLink>)}
       </div>}
-    </AutoChip>}
+    </AutoChip></Anchor>}
 
-    {!!videos.length && <AutoChip header='Videos' targetRef={videoRef}>
+    {!!videos.length && <Anchor id='videos'><AutoChip header='Videos' revealOnAnchorId='videos'>
       {videos.map((a, i) => <div key={`video-${i}`}>
         <video controls>
           <source src={a.path} type={a.contentType}/>
         </video>
         <AttachmentLink attachment={a}></AttachmentLink>
       </div>)}
-    </AutoChip>}
+    </AutoChip></Anchor>}
 
     {!!(otherAttachments.size + htmls.length) && <AutoChip header='Attachments'>
       {[...htmls].map((a, i) => (