From 85786b1a11108ba4e38fa24cb1080afb07971bc9 Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Thu, 3 Jun 2021 21:52:29 -0700
Subject: [PATCH] feat(trace viewer): fix UI issues (#6890)

---
 src/web/traceViewer/ui/actionList.css |  1 +
 src/web/traceViewer/ui/filmStrip.tsx  |  1 -
 src/web/traceViewer/ui/logsTab.css    | 12 +++++++
 src/web/traceViewer/ui/logsTab.tsx    | 12 +++----
 src/web/traceViewer/ui/timeline.css   | 10 ++----
 src/web/traceViewer/ui/timeline.tsx   | 52 +++++++++++++++++++--------
 6 files changed, 58 insertions(+), 30 deletions(-)

diff --git a/src/web/traceViewer/ui/actionList.css b/src/web/traceViewer/ui/actionList.css
index d6c9f3c7c3..34f0231a12 100644
--- a/src/web/traceViewer/ui/actionList.css
+++ b/src/web/traceViewer/ui/actionList.css
@@ -69,6 +69,7 @@
   color: red;
   position: relative;
   margin-right: 2px;
+  flex: none;
 }
 
 .action-selector {
diff --git a/src/web/traceViewer/ui/filmStrip.tsx b/src/web/traceViewer/ui/filmStrip.tsx
index 7725375544..3af5e4fd77 100644
--- a/src/web/traceViewer/ui/filmStrip.tsx
+++ b/src/web/traceViewer/ui/filmStrip.tsx
@@ -37,7 +37,6 @@ export const FilmStrip: React.FunctionComponent<{
   }
 
   const screencastFrames = context.pages[pageIndex]?.screencastFrames;
-  // TODO: pick file from the Y position.
   let previewImage = undefined;
   if (previewPoint !== undefined && screencastFrames) {
     const previewTime = boundaries.minimum + (boundaries.maximum - boundaries.minimum) * previewPoint.x / measure.width;
diff --git a/src/web/traceViewer/ui/logsTab.css b/src/web/traceViewer/ui/logsTab.css
index 01b3c9dc37..67bf64f505 100644
--- a/src/web/traceViewer/ui/logsTab.css
+++ b/src/web/traceViewer/ui/logsTab.css
@@ -23,6 +23,18 @@
   user-select: text;
 }
 
+.logs-error {
+  border-bottom: 1px solid var(--background);
+  padding: 3px 0 3px 12px;
+}
+
+.logs-error .codicon {
+  color: red;
+  position: relative;
+  top: 2px;
+  margin-right: 2px;
+}
+
 .log-line {
   flex: none;
   padding: 3px 0 3px 12px;
diff --git a/src/web/traceViewer/ui/logsTab.tsx b/src/web/traceViewer/ui/logsTab.tsx
index 596f43bcfb..91b13464c3 100644
--- a/src/web/traceViewer/ui/logsTab.tsx
+++ b/src/web/traceViewer/ui/logsTab.tsx
@@ -21,13 +21,13 @@ import { ActionTraceEvent } from '../../../server/trace/common/traceEvents';
 export const LogsTab: React.FunctionComponent<{
   action: ActionTraceEvent | undefined,
 }> = ({ action }) => {
-  let logs: string[] = [];
-  if (action) {
-    logs = action.metadata.log || [];
-    if (action.metadata.error)
-      logs = [action.metadata.error, ...logs];
-  }
+  const logs = action?.metadata.log || [];
+  const error = action?.metadata.error;
   return <div className='logs-tab'>{
+    <div className='logs-error' key='error' hidden={!error}>
+      <div className='codicon codicon-issues'/>
+      {error}
+    </div>}{
     logs.map((logLine, index) => {
       return <div key={index} className='log-line'>
         {logLine}
diff --git a/src/web/traceViewer/ui/timeline.css b/src/web/traceViewer/ui/timeline.css
index c64e1ab252..6c35c8c092 100644
--- a/src/web/traceViewer/ui/timeline.css
+++ b/src/web/traceViewer/ui/timeline.css
@@ -70,7 +70,6 @@
 .timeline-bar {
   position: absolute;
   height: 9px;
-  top: 11px;
   border-radius: 2px;
   min-width: 3px;
   --action-color: gray;
@@ -125,13 +124,8 @@
   --action-color: var(--blue);
 }
 
-.timeline-bar.event {
-  top: 22px;
-}
-
-.timeline-bar.frame_waitforeventinfo {
-  bottom: inherit;
-  top: 0;
+.timeline-bar.frame_waitforeventinfo,
+.timeline-bar.page_waitforeventinfo {
   --action-color: var(--gray);
 }
 
diff --git a/src/web/traceViewer/ui/timeline.tsx b/src/web/traceViewer/ui/timeline.tsx
index 376c79ff04..ad8651a163 100644
--- a/src/web/traceViewer/ui/timeline.tsx
+++ b/src/web/traceViewer/ui/timeline.tsx
@@ -45,6 +45,8 @@ export const Timeline: React.FunctionComponent<{
   onHighlighted: (action: ActionTraceEvent | undefined) => void,
 }> = ({ context, boundaries, selectedAction, highlightedAction, onSelected, onHighlighted }) => {
   const [measure, ref] = useMeasure<HTMLDivElement>();
+  const barsRef = React.useRef<HTMLDivElement | null>(null);
+
   const [previewPoint, setPreviewPoint] = React.useState<{ x: number, clientY: number } | undefined>();
   const [hoveredBarIndex, setHoveredBarIndex] = React.useState<number | undefined>();
 
@@ -58,9 +60,9 @@ export const Timeline: React.FunctionComponent<{
       for (const entry of page.actions) {
         if (!entry.metadata.params)
           console.log(entry);
-        let detail = entry.metadata.params.selector || '';
+        let detail = trimRight(entry.metadata.params.selector || '', 50);
         if (entry.metadata.method === 'goto')
-          detail = entry.metadata.params.url || '';
+          detail = trimRight(entry.metadata.params.url || '', 50);
         bars.push({
           action: entry,
           leftTime: entry.metadata.startTime,
@@ -94,32 +96,41 @@ export const Timeline: React.FunctionComponent<{
   let targetBar: TimelineBar | undefined = bars.find(bar => bar.action === (highlightedAction || selectedAction));
   targetBar = hoveredBar || targetBar;
 
-  const findHoveredBarIndex = (x: number) => {
+  const findHoveredBarIndex = (x: number, y: number) => {
     const time = positionToTime(measure.width, boundaries, x);
     const time1 = positionToTime(measure.width, boundaries, x - 5);
     const time2 = positionToTime(measure.width, boundaries, x + 5);
     let index: number | undefined;
-    let distance: number | undefined;
+    let yDistance: number | undefined;
+    let xDistance: number | undefined;
     for (let i = 0; i < bars.length; i++) {
       const bar = bars[i];
+      const yMiddle = kBarHeight / 2 + barTop(bar);
       const left = Math.max(bar.leftTime, time1);
       const right = Math.min(bar.rightTime, time2);
-      const middle = (bar.leftTime + bar.rightTime) / 2;
-      const d = Math.abs(time - middle);
-      if (left <= right && (index === undefined || d < distance!)) {
+      const xMiddle = (bar.leftTime + bar.rightTime) / 2;
+      const xd = Math.abs(time - xMiddle);
+      const yd = Math.abs(y - yMiddle);
+      if (left > right)
+        continue;
+      // Prefer closest yDistance (the same bar), among those prefer the closest xDistance.
+      if (index === undefined ||
+          (yd < yDistance!) ||
+          (Math.abs(yd - yDistance!) < 1e-2 && xd < xDistance!)) {
         index = i;
-        distance = d;
+        xDistance = xd;
+        yDistance = yd;
       }
     }
     return index;
   };
 
   const onMouseMove = (event: React.MouseEvent) => {
-    if (!ref.current)
+    if (!ref.current || !barsRef.current)
       return;
-    const bounds = ref.current.getBoundingClientRect();
-    const x = event.clientX - bounds.left;
-    const index = findHoveredBarIndex(x);
+    const x = event.clientX - ref.current.getBoundingClientRect().left;
+    const y = event.clientY - barsRef.current.getBoundingClientRect().top;
+    const index = findHoveredBarIndex(x, y);
     setPreviewPoint({ x, clientY: event.clientY });
     setHoveredBarIndex(index);
     if (typeof index === 'number')
@@ -134,10 +145,11 @@ export const Timeline: React.FunctionComponent<{
 
   const onClick = (event: React.MouseEvent) => {
     setPreviewPoint(undefined);
-    if (!ref.current)
+    if (!ref.current || !barsRef.current)
       return;
     const x = event.clientX - ref.current.getBoundingClientRect().left;
-    const index = findHoveredBarIndex(x);
+    const y = event.clientY - barsRef.current.getBoundingClientRect().top;
+    const index = findHoveredBarIndex(x, y);
     if (index === undefined)
       return;
     const entry = bars[index].action;
@@ -166,13 +178,14 @@ export const Timeline: React.FunctionComponent<{
         </div>;
       })
     }</div>
-    <div className='timeline-lane timeline-bars'>{
+    <div className='timeline-lane timeline-bars' ref={barsRef}>{
       bars.map((bar, index) => {
         return <div key={index}
           className={'timeline-bar ' + (bar.action ? 'action ' : '') + (bar.event ? 'event ' : '') + bar.className + (targetBar === bar ? ' selected' : '')}
           style={{
             left: bar.leftPosition + 'px',
             width: Math.max(1, bar.rightPosition - bar.leftPosition) + 'px',
+            top: barTop(bar) + 'px',
           }}
         ></div>;
       })
@@ -222,3 +235,12 @@ function timeToPosition(clientWidth: number, boundaries: Boundaries, time: numbe
 function positionToTime(clientWidth: number, boundaries: Boundaries, x: number): number {
   return x / clientWidth * (boundaries.maximum - boundaries.minimum) + boundaries.minimum;
 }
+
+function trimRight(s: string, maxLength: number): string {
+  return s.length <= maxLength ? s : s.substring(0, maxLength - 1) + '\u2026';
+}
+
+const kBarHeight = 11;
+function barTop(bar: TimelineBar): number {
+  return bar.event ? 22 : (bar.action?.metadata.method === 'waitForEventInfo' ? 0 : 11);
+}