chore: move attachment link back to tree item, make it flash yellow (#34353)

packages/html-reporter/src/links.css

@@ -60,11 +60,6 @@
     color: var(--color-scale-orange-6);
     border: 1px solid var(--color-scale-orange-4);
   }
-  .label-color-gray {
-    background-color: var(--color-scale-gray-0);
-    color: var(--color-scale-gray-6);
-    border: 1px solid var(--color-scale-gray-4);
-  }
 }
 
 @media(prefers-color-scheme: dark) {
@@ -98,11 +93,6 @@
     color: var(--color-scale-orange-2);
     border: 1px solid var(--color-scale-orange-4);
   }
-  .label-color-gray {
-    background-color: var(--color-scale-gray-9);
-    color: var(--color-scale-gray-2);
-    border: 1px solid var(--color-scale-gray-4);
-  }
 }
 
 .attachment-body {

packages/html-reporter/src/links.tsx

@@ -21,7 +21,7 @@ import { TreeItem } from './treeItem';
 import { CopyToClipboard } from './copyToClipboard';
 import './links.css';
 import { linkifyText } from '@web/renderUtils';
-import { clsx } from '@web/uiUtils';
+import { clsx, useFlash } from '@web/uiUtils';
 
 export function navigate(href: string | URL) {
   window.history.pushState({}, '', href);
@@ -73,7 +73,8 @@ export const AttachmentLink: React.FunctionComponent<{
   linkName?: string,
   openInNewTab?: boolean,
 }> = ({ attachment, result, href, linkName, openInNewTab }) => {
-  const isAnchored = useIsAnchored('attachment-' + result.attachments.indexOf(attachment));
+  const [flash, triggerFlash] = useFlash();
+  useAnchor('attachment-' + result.attachments.indexOf(attachment), triggerFlash);
   return <TreeItem title={<span>
     {attachment.contentType === kMissingContentType ? icons.warning() : icons.attachment()}
     {attachment.path && <a href={href || attachment.path} download={downloadFileNameForAttachment(attachment)}>{linkName || attachment.name}</a>}
@@ -84,7 +85,7 @@ export const AttachmentLink: React.FunctionComponent<{
     )}
   </span>} loadChildren={attachment.body ? () => {
     return [<div key={1} className='attachment-body'><CopyToClipboard value={attachment.body!}/>{linkifyText(attachment.body!)}</div>];
-  } : undefined} depth={0} style={{ lineHeight: '32px' }} selected={isAnchored}></TreeItem>;
+  } : undefined} depth={0} style={{ lineHeight: '32px' }} flash={flash}></TreeItem>;
 };
 
 export const SearchParamsContext = React.createContext<URLSearchParams>(new URLSearchParams(window.location.hash.slice(1)));
@@ -118,12 +119,12 @@ const kMissingContentType = 'x-playwright/missing';
 
 export type AnchorID = string | string[] | ((id: string) => boolean) | undefined;
 
-export function useAnchor(id: AnchorID, onReveal: () => void) {
+export function useAnchor(id: AnchorID, onReveal: React.EffectCallback) {
   const searchParams = React.useContext(SearchParamsContext);
   const isAnchored = useIsAnchored(id);
   React.useEffect(() => {
     if (isAnchored)
-      onReveal();
+      return onReveal();
   }, [isAnchored, onReveal, searchParams]);
 }
 

packages/html-reporter/src/testResultView.tsx

@@ -176,6 +176,7 @@ const StepTreeItem: React.FC<{
 }> = ({ test, step, result, depth }) => {
   return <TreeItem title={<span aria-label={step.title}>
     <span style={{ float: 'right' }}>{msToString(step.duration)}</span>
+    {step.attachments.length > 0 && <a style={{ float: 'right' }} title={`reveal attachment`} href={testResultHref({ test, result, anchor: `attachment-${step.attachments[0]}` })} onClick={evt => { evt.stopPropagation(); }}>{icons.attachment()}</a>}
     {statusIcon(step.error || step.duration === -1 ? 'failed' : (step.skipped ? 'skipped' : 'passed'))}
     <span>{step.title}</span>
     {step.count > 1 && <> ✕ <span className='test-result-counter'>{step.count}</span></>}
@@ -183,20 +184,6 @@ const StepTreeItem: React.FC<{
   </span>} loadChildren={step.steps.length || step.snippet ? () => {
     const snippet = step.snippet ? [<TestErrorView testId='test-snippet' key='line' error={step.snippet}/>] : [];
     const steps = step.steps.map((s, i) => <StepTreeItem key={i} step={s} depth={depth + 1} result={result} test={test} />);
-    const attachments = step.attachments.map(attachmentIndex => (
-      <a key={'' + attachmentIndex}
-        href={testResultHref({ test, result, anchor: `attachment-${attachmentIndex}` })}
-        style={{ paddingLeft: depth * 22 + 4, textDecoration: 'none' }}
-      >
-        <span
-          style={{ margin: '8px 0 0 8px', padding: '2px 10px', cursor: 'pointer' }}
-          className='label label-color-gray'
-          title={`see "${result.attachments[attachmentIndex].name}"`}
-        >
-          {icons.attachment()}{result.attachments[attachmentIndex].name}
-        </span>
-      </a>
-    ));
-    return snippet.concat(steps, attachments);
+    return snippet.concat(steps);
   } : undefined} depth={depth}/>;
 };

packages/html-reporter/src/treeItem.css

@@ -25,11 +25,14 @@
   cursor: pointer;
 }
 
-.tree-item-title.selected {
-  text-decoration: underline var(--color-underlinenav-icon);
-  text-decoration-thickness: 1.5px;
-}
-
 .tree-item-body {
   min-height: 18px;
 }
+
+.yellow-flash {
+  animation: yellowflash-bg 2s;
+}
+@keyframes yellowflash-bg {
+  from { background: var(--color-attention-subtle); }
+  to   { background: transparent; }
+}

packages/html-reporter/src/treeItem.tsx

@@ -25,12 +25,12 @@ export const TreeItem: React.FunctionComponent<{
   onClick?: () => void,
   expandByDefault?: boolean,
   depth: number,
-  selected?: boolean,
   style?:  React.CSSProperties,
-}> = ({ title, loadChildren, onClick, expandByDefault, depth, selected, style }) => {
+  flash?: boolean
+}> = ({ title, loadChildren, onClick, expandByDefault, depth, style, flash }) => {
   const [expanded, setExpanded] = React.useState(expandByDefault || false);
-  return <div className={'tree-item'} style={style}>
-    <span className={clsx('tree-item-title', selected && 'selected')} style={{ whiteSpace: 'nowrap', paddingLeft: depth * 22 + 4 }} onClick={() => { onClick?.(); setExpanded(!expanded); }} >
+  return <div className={clsx('tree-item', flash && 'yellow-flash')} style={style}>
+    <span className='tree-item-title' style={{ whiteSpace: 'nowrap', paddingLeft: depth * 22 + 4 }} onClick={() => { onClick?.(); setExpanded(!expanded); }} >
       {loadChildren && !!expanded && icons.downArrow()}
       {loadChildren && !expanded && icons.rightArrow()}
       {!loadChildren && <span style={{ visibility: 'hidden' }}>{icons.rightArrow()}</span>}

packages/trace-viewer/src/ui/attachmentsTab.css

@@ -55,3 +55,11 @@
 a.codicon-cloud-download:hover{
   background-color: var(--vscode-list-inactiveSelectionBackground)
 }
+
+.yellow-flash {
+  animation: yellowflash-bg 2s;
+}
+@keyframes yellowflash-bg {
+  from { background: var(--vscode-peekViewEditor-matchHighlightBackground); }
+  to   { background: transparent; }
+}

packages/trace-viewer/src/ui/attachmentsTab.tsx

@@ -17,36 +17,38 @@
 import * as React from 'react';
 import './attachmentsTab.css';
 import { ImageDiffView } from '@web/shared/imageDiffView';
-import type { ActionTraceEventInContext, MultiTraceModel } from './modelUtil';
+import type { MultiTraceModel } from './modelUtil';
 import { PlaceholderPanel } from './placeholderPanel';
 import type { AfterActionTraceEventAttachment } from '@trace/trace';
 import { CodeMirrorWrapper, lineHeight } from '@web/components/codeMirrorWrapper';
 import { isTextualMimeType } from '@isomorphic/mimeType';
 import { Expandable } from '@web/components/expandable';
 import { linkifyText } from '@web/renderUtils';
-import { clsx } from '@web/uiUtils';
+import { clsx, useFlash } from '@web/uiUtils';
 
 type Attachment = AfterActionTraceEventAttachment & { traceUrl: string };
 
 type ExpandableAttachmentProps = {
   attachment: Attachment;
-  reveal: boolean;
-  highlight: boolean;
+  reveal?: any;
 };
 
-const ExpandableAttachment: React.FunctionComponent<ExpandableAttachmentProps> = ({ attachment, reveal, highlight }) => {
+const ExpandableAttachment: React.FunctionComponent<ExpandableAttachmentProps> = ({ attachment, reveal }) => {
   const [expanded, setExpanded] = React.useState(false);
   const [attachmentText, setAttachmentText] = React.useState<string | null>(null);
   const [placeholder, setPlaceholder] = React.useState<string | null>(null);
+  const [flash, triggerFlash] = useFlash();
   const ref = React.useRef<HTMLSpanElement>(null);
 
   const isTextAttachment = isTextualMimeType(attachment.contentType);
   const hasContent = !!attachment.sha1 || !!attachment.path;
 
   React.useEffect(() => {
-    if (reveal)
+    if (reveal) {
       ref.current?.scrollIntoView({ behavior: 'smooth' });
-  }, [reveal]);
+      return triggerFlash();
+    }
+  }, [reveal, triggerFlash]);
 
   React.useEffect(() => {
     if (expanded && attachmentText === null && placeholder === null) {
@@ -66,14 +68,14 @@ const ExpandableAttachment: React.FunctionComponent<ExpandableAttachmentProps> =
   }, [attachmentText]);
 
   const title = <span style={{ marginLeft: 5 }} ref={ref} aria-label={attachment.name}>
-    <span className={clsx(highlight && 'attachment-title-highlight')}>{linkifyText(attachment.name)}</span>
+    <span>{linkifyText(attachment.name)}</span>
     {hasContent && <a style={{ marginLeft: 5 }} href={downloadURL(attachment)}>download</a>}
   </span>;
 
   if (!isTextAttachment || !hasContent)
     return <div style={{ marginLeft: 20 }}>{title}</div>;
 
-  return <>
+  return <div className={clsx(flash && 'yellow-flash')}>
     <Expandable title={title} expanded={expanded} setExpanded={setExpanded} expandOnTitleClick={true}>
       {placeholder && <i>{placeholder}</i>}
     </Expandable>
@@ -87,14 +89,13 @@ const ExpandableAttachment: React.FunctionComponent<ExpandableAttachmentProps> =
         wrapLines={false}>
       </CodeMirrorWrapper>
     </div>}
-  </>;
+  </div>;
 };
 
 export const AttachmentsTab: React.FunctionComponent<{
   model: MultiTraceModel | undefined,
-  selectedAction: ActionTraceEventInContext | undefined,
-  revealedAttachment?: AfterActionTraceEventAttachment,
-}> = ({ model, selectedAction, revealedAttachment }) => {
+  revealedAttachment?: [AfterActionTraceEventAttachment, number],
+}> = ({ model, revealedAttachment }) => {
   const { diffMap, screenshots, attachments } = React.useMemo(() => {
     const attachments = new Set<Attachment>();
     const screenshots = new Set<Attachment>();
@@ -153,8 +154,7 @@ export const AttachmentsTab: React.FunctionComponent<{
       return <div className='attachment-item' key={attachmentKey(a, i)}>
         <ExpandableAttachment
           attachment={a}
-          highlight={selectedAction?.attachments?.some(selected => isEqualAttachment(a, selected)) ?? false}
-          reveal={!!revealedAttachment && isEqualAttachment(a, revealedAttachment)}
+          reveal={(!!revealedAttachment && isEqualAttachment(a, revealedAttachment[0])) ? revealedAttachment : undefined}
         />
       </div>;
     })}

packages/trace-viewer/src/ui/workbench.tsx

@@ -59,7 +59,7 @@ export const Workbench: React.FunctionComponent<{
 }> = ({ model, showSourcesFirst, rootDir, fallbackLocation, isLive, hideTimeline, status, annotations, inert, onOpenExternally, revealSource }) => {
   const [selectedCallId, setSelectedCallId] = React.useState<string | undefined>(undefined);
   const [revealedError, setRevealedError] = React.useState<ErrorDescription | undefined>(undefined);
-  const [revealedAttachment, setRevealedAttachment] = React.useState<AfterActionTraceEventAttachment | undefined>(undefined);
+  const [revealedAttachment, setRevealedAttachment] = React.useState<[attachment: AfterActionTraceEventAttachment, renderCounter: number] | undefined>(undefined);
   const [highlightedCallId, setHighlightedCallId] = React.useState<string | undefined>();
   const [highlightedEntry, setHighlightedEntry] = React.useState<Entry | undefined>();
   const [highlightedConsoleMessage, setHighlightedConsoleMessage] = React.useState<ConsoleEntry | undefined>();
@@ -148,7 +148,12 @@ export const Workbench: React.FunctionComponent<{
 
   const revealAttachment = React.useCallback((attachment: AfterActionTraceEventAttachment) => {
     selectPropertiesTab('attachments');
-    setRevealedAttachment(attachment);
+    setRevealedAttachment(currentValue => {
+      if (!currentValue)
+        return [attachment, 0];
+      const revealCounter = currentValue[1];
+      return [attachment, revealCounter + 1];
+    });
   }, [selectPropertiesTab]);
 
   React.useEffect(() => {
@@ -238,7 +243,7 @@ export const Workbench: React.FunctionComponent<{
     id: 'attachments',
     title: 'Attachments',
     count: attachments.length,
-    render: () => <AttachmentsTab model={model} selectedAction={selectedAction} revealedAttachment={revealedAttachment} />
+    render: () => <AttachmentsTab model={model} revealedAttachment={revealedAttachment} />
   };
 
   const tabs: TabbedPaneTabModel[] = [

packages/web/src/uiUtils.ts

@@ -14,6 +14,7 @@
   limitations under the License.
 */
 
+import type { EffectCallback } from 'react';
 import React from 'react';
 
 // Recalculates the value when dependencies change.
@@ -224,3 +225,26 @@ export function scrollIntoViewIfNeeded(element: Element | undefined) {
 
 const kControlCodesRe = '\\u0000-\\u0020\\u007f-\\u009f';
 export const kWebLinkRe = new RegExp('(?:[a-zA-Z][a-zA-Z0-9+.-]{2,}:\\/\\/|www\\.)[^\\s' + kControlCodesRe + '"]{2,}[^\\s' + kControlCodesRe + '"\')}\\],:;.!?]', 'ug');
+
+/**
+ * Manages flash animation state.
+ * Calling `trigger` will turn `flash` to true for a second, and then back to false.
+ * If `trigger` is called while a flash is ongoing, the ongoing flash will be cancelled and after 50ms a new flash is started.
+ * @returns [flash, trigger]
+ */
+export function useFlash(): [boolean, EffectCallback] {
+  const [flash, setFlash] = React.useState(false);
+  const trigger = React.useCallback<React.EffectCallback>(() => {
+    const timeouts: any[] = [];
+    setFlash(currentlyFlashing => {
+      timeouts.push(setTimeout(() => setFlash(false), 1000));
+      if (!currentlyFlashing)
+        return true;
+
+      timeouts.push(setTimeout(() => setFlash(true), 50));
+      return false;
+    });
+    return () => timeouts.forEach(clearTimeout);
+  }, [setFlash]);
+  return [flash, trigger];
+}

tests/playwright-test/reporter-html.spec.ts

@@ -959,10 +959,9 @@ for (const useIntermediateMergeReport of [true, false] as const) {
       await showReport();
       await page.getByRole('link', { name: 'passing' }).click();
 
-      const attachment = page.getByTestId('attachments').getByText('foo-2', { exact: true });
+      const attachment = page.getByText('foo-2', { exact: true });
       await expect(attachment).not.toBeInViewport();
-      await page.getByLabel('attach "foo-2"').click();
-      await page.getByTitle('see "foo-2"').click();
+      await page.getByLabel(`attach "foo-2"`).getByTitle('reveal attachment').click();
       await expect(attachment).toBeInViewport();
 
       await page.reload();
@@ -989,10 +988,9 @@ for (const useIntermediateMergeReport of [true, false] as const) {
       await showReport();
       await page.getByRole('link', { name: 'passing' }).click();
 
-      const attachment = page.getByTestId('attachments').getByText('attachment', { exact: true });
+      const attachment = page.getByText('attachment', { exact: true });
       await expect(attachment).not.toBeInViewport();
-      await page.getByLabel('step').click();
-      await page.getByTitle('see "attachment"').click();
+      await page.getByLabel('step').getByTitle('reveal attachment').click();
       await expect(attachment).toBeInViewport();
     });