From c9645a96c6d8cdbb45ad6c9c614fcf2543404800 Mon Sep 17 00:00:00 2001
From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com>
Date: Thu, 21 Nov 2019 13:18:29 +0100
Subject: [PATCH] Explore: UI fixes for log details (#20485)

---
 .../src/components/Logs/LogDetailsRow.tsx     | 29 ++++++++++---
 .../grafana-ui/src/components/Logs/LogRow.tsx | 36 ++++++++++++----
 .../src/components/Logs/LogRowMessage.tsx     | 16 +------
 .../src/components/Logs/LogRows.tsx           |  8 ++--
 .../src/components/Logs/getLogRowStyles.ts    | 43 +++++++++++++++----
 public/app/plugins/panel/logs/LogsPanel.tsx   |  2 +-
 6 files changed, 92 insertions(+), 42 deletions(-)

diff --git a/packages/grafana-ui/src/components/Logs/LogDetailsRow.tsx b/packages/grafana-ui/src/components/Logs/LogDetailsRow.tsx
index f6d926d30d1..b6a227d882c 100644
--- a/packages/grafana-ui/src/components/Logs/LogDetailsRow.tsx
+++ b/packages/grafana-ui/src/components/Logs/LogDetailsRow.tsx
@@ -1,9 +1,11 @@
 import React, { PureComponent } from 'react';
-import { LogLabelStatsModel } from '@grafana/data';
+import { css, cx } from 'emotion';
+import { LogLabelStatsModel, GrafanaTheme } from '@grafana/data';
 
 import { Themeable } from '../../types/theme';
 import { withTheme } from '../../themes/index';
 import { getLogRowStyles } from './getLogRowStyles';
+import { stylesFactory } from '../../themes/stylesFactory';
 
 //Components
 import { LogLabelStats } from './LogLabelStats';
@@ -24,6 +26,17 @@ interface State {
   fieldStats: LogLabelStatsModel[] | null;
 }
 
+const getStyles = stylesFactory((theme: GrafanaTheme) => {
+  return {
+    noHoverEffect: css`
+      label: noHoverEffect;
+      :hover {
+        background-color: transparent;
+      }
+    `,
+  };
+});
+
 class UnThemedLogDetailsRow extends PureComponent<Props, State> {
   state: State = {
     showFieldsStats: false,
@@ -66,22 +79,28 @@ class UnThemedLogDetailsRow extends PureComponent<Props, State> {
   render() {
     const { theme, parsedKey, parsedValue, isLabel, links } = this.props;
     const { showFieldsStats, fieldStats, fieldCount } = this.state;
+    const styles = getStyles(theme);
     const style = getLogRowStyles(theme);
     return (
-      <div className={style.logsRowDetailsValue}>
+      <div className={cx(style.logsRowDetailsValue, { [styles.noHoverEffect]: showFieldsStats })}>
         {/* Action buttons - show stats/filter results */}
-        <div onClick={this.showStats} aria-label={'Field stats'} className={style.logsRowDetailsIcon}>
+        <div
+          title="Ad-hoc statistics"
+          onClick={this.showStats}
+          aria-label={'Field stats'}
+          className={style.logsRowDetailsIcon}
+        >
           <i className={'fa fa-signal'} />
         </div>
         {isLabel ? (
-          <div onClick={() => this.filterLabel()} className={style.logsRowDetailsIcon}>
+          <div title="Filter for value" onClick={() => this.filterLabel()} className={style.logsRowDetailsIcon}>
             <i className={'fa fa-search-plus'} />
           </div>
         ) : (
           <div className={style.logsRowDetailsIcon} />
         )}
         {isLabel ? (
-          <div onClick={() => this.filterOutLabel()} className={style.logsRowDetailsIcon}>
+          <div title="Filter out value" onClick={() => this.filterOutLabel()} className={style.logsRowDetailsIcon}>
             <i className={'fa fa-search-minus'} />
           </div>
         ) : (
diff --git a/packages/grafana-ui/src/components/Logs/LogRow.tsx b/packages/grafana-ui/src/components/Logs/LogRow.tsx
index e80828945bc..1451f9a86e6 100644
--- a/packages/grafana-ui/src/components/Logs/LogRow.tsx
+++ b/packages/grafana-ui/src/components/Logs/LogRow.tsx
@@ -1,5 +1,6 @@
 import React, { PureComponent } from 'react';
-import { Field, LinkModel, LogRowModel, TimeZone, DataQueryResponse } from '@grafana/data';
+import { Field, LinkModel, LogRowModel, TimeZone, DataQueryResponse, GrafanaTheme } from '@grafana/data';
+import { cx, css } from 'emotion';
 
 import {
   LogRowContextRows,
@@ -10,6 +11,7 @@ import {
 import { Themeable } from '../../types/theme';
 import { withTheme } from '../../themes/index';
 import { getLogRowStyles } from './getLogRowStyles';
+import { stylesFactory } from '../../themes/stylesFactory';
 
 //Components
 import { LogDetails } from './LogDetails';
@@ -21,7 +23,7 @@ interface Props extends Themeable {
   showDuplicates: boolean;
   showTime: boolean;
   timeZone: TimeZone;
-  isLogsPanel?: boolean;
+  allowDetails?: boolean;
   getRows: () => LogRowModel[];
   onClickFilterLabel?: (key: string, value: string) => void;
   onClickFilterOutLabel?: (key: string, value: string) => void;
@@ -35,6 +37,14 @@ interface State {
   showDetails: boolean;
 }
 
+const getStyles = stylesFactory((theme: GrafanaTheme) => {
+  return {
+    topVerticalAlign: css`
+      label: topVerticalAlign;
+      vertical-align: top;
+    `,
+  };
+});
 /**
  * Renders a log line.
  *
@@ -57,6 +67,9 @@ class UnThemedLogRow extends PureComponent<Props, State> {
   };
 
   toggleDetails = () => {
+    if (this.props.allowDetails) {
+      return;
+    }
     this.setState(state => {
       return {
         showDetails: !state.showDetails,
@@ -75,7 +88,7 @@ class UnThemedLogRow extends PureComponent<Props, State> {
       onClickFilterLabel,
       onClickFilterOutLabel,
       highlighterExpressions,
-      isLogsPanel,
+      allowDetails,
       row,
       showDuplicates,
       timeZone,
@@ -85,8 +98,11 @@ class UnThemedLogRow extends PureComponent<Props, State> {
     } = this.props;
     const { showDetails, showContext } = this.state;
     const style = getLogRowStyles(theme, row.logLevel);
+    const styles = getStyles(theme);
     const showUtc = timeZone === 'utc';
-
+    const showDetailsClassName = showDetails
+      ? cx(['fa fa-chevron-down', styles.topVerticalAlign])
+      : cx(['fa fa-chevron-right', styles.topVerticalAlign]);
     return (
       <div className={style.logsRow}>
         {showDuplicates && (
@@ -95,13 +111,17 @@ class UnThemedLogRow extends PureComponent<Props, State> {
           </div>
         )}
         <div className={style.logsRowLevel} />
-        {!isLogsPanel && (
-          <div title="See log details" onClick={this.toggleDetails} className={style.logsRowToggleDetails}>
-            <i className={showDetails ? 'fa fa-chevron-up' : 'fa fa-chevron-down'} />
+        {!allowDetails && (
+          <div
+            title={showDetails ? 'Hide log details' : 'See log details'}
+            onClick={this.toggleDetails}
+            className={style.logsRowToggleDetails}
+          >
+            <i className={showDetailsClassName} />
           </div>
         )}
         <div>
-          <div>
+          <div onClick={this.toggleDetails}>
             {showTime && showUtc && (
               <div className={style.logsRowLocalTime} title={`Local: ${row.timeLocal} (${row.timeFromNow})`}>
                 {row.timeUtc}
diff --git a/packages/grafana-ui/src/components/Logs/LogRowMessage.tsx b/packages/grafana-ui/src/components/Logs/LogRowMessage.tsx
index 733e3e9cd6a..47402172fb5 100644
--- a/packages/grafana-ui/src/components/Logs/LogRowMessage.tsx
+++ b/packages/grafana-ui/src/components/Logs/LogRowMessage.tsx
@@ -122,21 +122,7 @@ class UnThemedLogRowMessage extends PureComponent<Props, State> {
             )}
           </span>
           {row.searchWords && row.searchWords.length > 0 && (
-            <span
-              onClick={this.onContextToggle}
-              className={css`
-                visibility: hidden;
-                white-space: nowrap;
-                position: relative;
-                z-index: ${showContext ? 1 : 0};
-                cursor: pointer;
-                .${style.logsRow}:hover & {
-                  visibility: visible;
-                  margin-left: 10px;
-                  text-decoration: underline;
-                }
-              `}
-            >
+            <span onClick={this.onContextToggle} className={cx(style.context)}>
               {showContext ? 'Hide' : 'Show'} context
             </span>
           )}
diff --git a/packages/grafana-ui/src/components/Logs/LogRows.tsx b/packages/grafana-ui/src/components/Logs/LogRows.tsx
index 81c5d4d62a3..89a0cfef7c8 100644
--- a/packages/grafana-ui/src/components/Logs/LogRows.tsx
+++ b/packages/grafana-ui/src/components/Logs/LogRows.tsx
@@ -20,7 +20,7 @@ export interface Props extends Themeable {
   showTime: boolean;
   timeZone: TimeZone;
   rowLimit?: number;
-  isLogsPanel?: boolean;
+  allowDetails?: boolean;
   previewLimit?: number;
   onClickFilterLabel?: (key: string, value: string) => void;
   onClickFilterOutLabel?: (key: string, value: string) => void;
@@ -79,7 +79,7 @@ class UnThemedLogRows extends PureComponent<Props, State> {
       onClickFilterOutLabel,
       rowLimit,
       theme,
-      isLogsPanel,
+      allowDetails,
       previewLimit,
       getFieldLinks,
     } = this.props;
@@ -115,7 +115,7 @@ class UnThemedLogRows extends PureComponent<Props, State> {
               showDuplicates={showDuplicates}
               showTime={showTime}
               timeZone={timeZone}
-              isLogsPanel={isLogsPanel}
+              allowDetails={allowDetails}
               onClickFilterLabel={onClickFilterLabel}
               onClickFilterOutLabel={onClickFilterOutLabel}
               getFieldLinks={getFieldLinks}
@@ -132,7 +132,7 @@ class UnThemedLogRows extends PureComponent<Props, State> {
               showDuplicates={showDuplicates}
               showTime={showTime}
               timeZone={timeZone}
-              isLogsPanel={isLogsPanel}
+              allowDetails={allowDetails}
               onClickFilterLabel={onClickFilterLabel}
               onClickFilterOutLabel={onClickFilterOutLabel}
               getFieldLinks={getFieldLinks}
diff --git a/packages/grafana-ui/src/components/Logs/getLogRowStyles.ts b/packages/grafana-ui/src/components/Logs/getLogRowStyles.ts
index bfe5c8f2b7e..b117ed28477 100644
--- a/packages/grafana-ui/src/components/Logs/getLogRowStyles.ts
+++ b/packages/grafana-ui/src/components/Logs/getLogRowStyles.ts
@@ -7,7 +7,15 @@ import { stylesFactory } from '../../themes';
 
 export const getLogRowStyles = stylesFactory((theme: GrafanaTheme, logLevel?: LogLevel) => {
   let logColor = selectThemeVariant({ light: theme.colors.gray5, dark: theme.colors.gray2 }, theme.type);
-  const bgColor = selectThemeVariant({ light: theme.colors.gray5, dark: theme.colors.gray2 }, theme.type);
+  const borderColor = selectThemeVariant({ light: theme.colors.gray5, dark: theme.colors.gray2 }, theme.type);
+  const bgColor = selectThemeVariant({ light: theme.colors.gray5, dark: theme.colors.dark4 }, theme.type);
+  const context = css`
+    label: context;
+    visibility: hidden;
+    white-space: nowrap;
+    position: relative;
+  `;
+
   switch (logLevel) {
     case LogLevel.crit:
     case LogLevel.critical:
@@ -39,7 +47,6 @@ export const getLogRowStyles = stylesFactory((theme: GrafanaTheme, logLevel?: Lo
       padding: inherit;
 
       color: ${theme.colors.yellow};
-      border-bottom: ${theme.border.width.sm} solid ${theme.colors.yellow};
       background-color: rgba(${theme.colors.yellow}, 0.1);
     `,
     logsRowMatchHighLightPreview: css`
@@ -55,9 +62,22 @@ export const getLogRowStyles = stylesFactory((theme: GrafanaTheme, logLevel?: Lo
       table-layout: fixed;
       width: 100%;
     `,
+    context: context,
     logsRow: css`
       label: logs-row;
       display: table-row;
+      cursor: pointer;
+      &:hover {
+        .${context} {
+          visibility: visible;
+          z-index: 1;
+          margin-left: 10px;
+          text-decoration: underline;
+          &:hover {
+            color: ${theme.colors.yellow};
+          }
+        }
+      }
 
       > div {
         display: table-cell;
@@ -75,11 +95,13 @@ export const getLogRowStyles = stylesFactory((theme: GrafanaTheme, logLevel?: Lo
       label: logs-row__duplicates;
       text-align: right;
       width: 4em;
+      cursor: default;
     `,
     logsRowLevel: css`
       label: logs-row__level;
       position: relative;
       width: 10px;
+      cursor: default;
 
       &::after {
         content: '';
@@ -102,7 +124,6 @@ export const getLogRowStyles = stylesFactory((theme: GrafanaTheme, logLevel?: Lo
       width: 15px;
       padding-right: ${theme.spacing.sm};
       font-size: 9px;
-      cursor: pointer;
     `,
     logsRowLocalTime: css`
       label: logs-row__localtime;
@@ -123,18 +144,23 @@ export const getLogRowStyles = stylesFactory((theme: GrafanaTheme, logLevel?: Lo
     logsRowDetailsTable: css`
       label: logs-row-details-table;
       display: table;
-      border: 1px solid ${bgColor};
+      border: 1px solid ${borderColor};
       border-radius: 3px;
       margin: 20px 0;
       padding: ${theme.spacing.sm};
+      padding-top: 0;
       width: 100%;
+      cursor: default;
     `,
     logsRowDetailsSectionTable: css`
       label: logs-row-details-table__section;
       display: table;
       table-layout: fixed;
-      margin: 5px 0;
+      margin: 0;
       width: 100%;
+      &:first-of-type {
+        margin-bottom: ${theme.spacing.xs};
+      }
     `,
     logsRowDetailsIcon: css`
       label: logs-row-details__icon;
@@ -145,20 +171,19 @@ export const getLogRowStyles = stylesFactory((theme: GrafanaTheme, logLevel?: Lo
       color: ${theme.colors.gray3};
       &:hover {
         cursor: pointer;
-        color: ${theme.colors.yellow};
       }
     `,
     logsRowDetailsLabel: css`
       label: logs-row-details__label;
       display: table-cell;
       padding: 0 ${theme.spacing.md} 0 ${theme.spacing.md};
-      width: 12.5em;
+      width: 14em;
       word-break: break-all;
     `,
     logsRowDetailsHeading: css`
       label: logs-row-details__heading;
       display: table-caption;
-      margin: 5px 0 7px;
+      margin: ${theme.spacing.sm} 0 ${theme.spacing.xs};
       font-weight: ${theme.typography.weight.bold};
     `,
     logsRowDetailsValue: css`
@@ -170,7 +195,7 @@ export const getLogRowStyles = stylesFactory((theme: GrafanaTheme, logLevel?: Lo
       cursor: default;
 
       &:hover {
-        color: ${theme.colors.yellow};
+        background-color: ${bgColor};
       }
     `,
   };
diff --git a/public/app/plugins/panel/logs/LogsPanel.tsx b/public/app/plugins/panel/logs/LogsPanel.tsx
index 2cc371fa8df..bed64edde61 100644
--- a/public/app/plugins/panel/logs/LogsPanel.tsx
+++ b/public/app/plugins/panel/logs/LogsPanel.tsx
@@ -32,7 +32,7 @@ export const LogsPanel: React.FunctionComponent<LogsPanelProps> = ({
         highlighterExpressions={[]}
         showTime={showTime}
         timeZone={timeZone}
-        isLogsPanel={true}
+        allowDetails={true}
       />
     </CustomScrollbar>
   );