chore: move attachment link back to tree item, make it flash yellow

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

@@ -74,6 +74,7 @@ export const AttachmentLink: React.FunctionComponent<{
   openInNewTab?: boolean,
 }> = ({ attachment, result, href, linkName, openInNewTab }) => {
   const isAnchored = useIsAnchored('attachment-' + result.attachments.indexOf(attachment));
+  const searchParams = React.useContext(SearchParamsContext);
   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={isAnchored ? searchParams : undefined}></TreeItem>;
 };
 
 export const SearchParamsContext = React.createContext<URLSearchParams>(new URLSearchParams(window.location.hash.slice(1)));

packages/html-reporter/src/testResultView.tsx

@@ -176,27 +176,14 @@ 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></>}
     {step.location && <span className='test-result-path'>— {step.location.file}:{step.location.line}</span>}
   </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

@@ -17,20 +17,21 @@
 import * as React from 'react';
 import './treeItem.css';
 import * as icons from './icons';
-import { clsx } from '@web/uiUtils';
+import { clsx, useFlash } from '@web/uiUtils';
 
 export const TreeItem: React.FunctionComponent<{
   title: JSX.Element,
   loadChildren?: () => JSX.Element[],
   onClick?: () => void,
   expandByDefault?: boolean,
   depth: number,
-  selected?: boolean,
   style?:  React.CSSProperties,
-}> = ({ title, loadChildren, onClick, expandByDefault, depth, selected, style }) => {
+  flash?: any
+}> = ({ title, loadChildren, onClick, expandByDefault, depth, style, flash }) => {
+  const addFlashClass = useFlash(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', addFlashClass && '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,24 +17,23 @@
 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);
@@ -47,6 +46,7 @@ const ExpandableAttachment: React.FunctionComponent<ExpandableAttachmentProps> =
     if (reveal)
       ref.current?.scrollIntoView({ behavior: 'smooth' });
   }, [reveal]);
+  const flash = useFlash(reveal);
 
   React.useEffect(() => {
     if (expanded && attachmentText === null && placeholder === null) {
@@ -66,14 +66,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 +87,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 }) => {
+}> = ({ model, revealedAttachment }) => {
   const { diffMap, screenshots, attachments } = React.useMemo(() => {
     const attachments = new Set<Attachment>();
     const screenshots = new Set<Attachment>();
@@ -153,8 +152,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)) ? revealedAttachment : undefined}
         />
       </div>;
     })}

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

@@ -148,7 +148,7 @@ export const Workbench: React.FunctionComponent<{
 
   const revealAttachment = React.useCallback((attachment: AfterActionTraceEventAttachment) => {
     selectPropertiesTab('attachments');
-    setRevealedAttachment(attachment);
+    setRevealedAttachment({ ...attachment }); // copy to force re-render
   }, [selectPropertiesTab]);
 
   React.useEffect(() => {
@@ -238,7 +238,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

@@ -224,3 +224,16 @@ 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');
+
+// flash is retriggered whenever the value changes
+export function useFlash(flash: any | undefined) {
+  const [flashState, setFlashState] = React.useState(false);
+  React.useEffect(() => {
+    if (flash) {
+      setFlashState(true);
+      const timeout = setTimeout(() => setFlashState(false), 1000);
+      return () => clearTimeout(timeout);
+    }
+  }, [flash]);
+  return flashState;
+}

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