Explore: Refactor log details table (#21044)

packages/grafana-ui/src/components/Logs/LogDetails.test.tsx

@@ -37,7 +37,7 @@ describe('LogDetails', () => {
   describe('when labels are present', () => {
     it('should render heading', () => {
       const wrapper = setup(undefined, { labels: { key1: 'label1', key2: 'label2' } });
-      expect(wrapper.find({ 'aria-label': 'Log labels' })).toHaveLength(1);
+      expect(wrapper.find({ 'aria-label': 'Log Labels' })).toHaveLength(1);
     });
     it('should render labels', () => {
       const wrapper = setup(undefined, { labels: { key1: 'label1', key2: 'label2' } });
@@ -47,7 +47,7 @@ describe('LogDetails', () => {
   describe('when row entry has parsable fields', () => {
     it('should render heading ', () => {
       const wrapper = setup(undefined, { entry: 'test=successful' });
-      expect(wrapper.find({ 'aria-label': 'Parsed fields' })).toHaveLength(1);
+      expect(wrapper.find({ title: 'Ad-hoc statistics' })).toHaveLength(1);
     });
     it('should render parsed fields', () => {
       const wrapper = setup(undefined, { entry: 'test=successful' });
@@ -57,8 +57,8 @@ describe('LogDetails', () => {
   describe('when row entry have parsable fields and labels are present', () => {
     it('should render all headings', () => {
       const wrapper = setup(undefined, { entry: 'test=successful', labels: { key: 'label' } });
-      expect(wrapper.find({ 'aria-label': 'Log labels' })).toHaveLength(1);
-      expect(wrapper.find({ 'aria-label': 'Parsed fields' })).toHaveLength(1);
+      expect(wrapper.find({ 'aria-label': 'Log Labels' })).toHaveLength(1);
+      expect(wrapper.find({ 'aria-label': 'Parsed Fields' })).toHaveLength(1);
     });
     it('should render all labels and parsed fields', () => {
       const wrapper = setup(undefined, {

packages/grafana-ui/src/components/Logs/LogDetails.tsx

@@ -106,18 +106,20 @@ class UnThemedLogDetails extends PureComponent<Props> {
     const style = getLogRowStyles(theme, row.logLevel);
     const labels = row.labels ? row.labels : {};
     const labelsAvailable = Object.keys(labels).length > 0;
-
     const fields = this.getAllFields(row);
-
     const parsedFieldsAvailable = fields && fields.length > 0;
 
     return (
-      <div className={style.logsRowDetailsTable}>
-        {labelsAvailable && (
-          <div className={style.logsRowDetailsSectionTable}>
-            <div className={style.logsRowDetailsHeading} aria-label="Log labels">
-              Log Labels:
-            </div>
+      <div className={style.logDetailsContainer}>
+        <table className={style.logDetailsTable}>
+          <tbody>
+            {labelsAvailable && (
+              <tr>
+                <td colSpan={5} className={style.logDetailsHeading} aria-label="Log Labels">
+                  Log Labels:
+                </td>
+              </tr>
+            )}
             {Object.keys(labels).map(key => {
               const value = labels[key];
               return (
@@ -132,14 +134,14 @@ class UnThemedLogDetails extends PureComponent<Props> {
                 />
               );
             })}
-          </div>
-        )}
 
-        {parsedFieldsAvailable && (
-          <div className={style.logsRowDetailsSectionTable}>
-            <div className={style.logsRowDetailsHeading} aria-label="Parsed fields">
-              Parsed fields:
-            </div>
+            {parsedFieldsAvailable && (
+              <tr>
+                <td colSpan={5} className={style.logDetailsHeading} aria-label="Parsed Fields">
+                  Parsed Fields:
+                </td>
+              </tr>
+            )}
             {fields.map(field => {
               const { key, value, links, fieldIndex } = field;
               return (
@@ -156,9 +158,15 @@ class UnThemedLogDetails extends PureComponent<Props> {
                 />
               );
             })}
-          </div>
-        )}
-        {!parsedFieldsAvailable && !labelsAvailable && <div aria-label="No details">No details available</div>}
+            {!parsedFieldsAvailable && !labelsAvailable && (
+              <tr>
+                <td colSpan={5} aria-label="No details">
+                  No details available
+                </td>
+              </tr>
+            )}
+          </tbody>
+        </table>
       </div>
     );
   }

packages/grafana-ui/src/components/Logs/LogDetailsRow.test.tsx

@@ -66,7 +66,7 @@ describe('LogDetailsRow', () => {
     });
 
     expect(wrapper.find(LogLabelStats).length).toBe(0);
-    wrapper.find('[aria-label="Field stats"]').simulate('click');
+    wrapper.find({ title: 'Ad-hoc statistics' }).simulate('click');
     expect(wrapper.find(LogLabelStats).length).toBe(1);
     expect(wrapper.find(LogLabelStats).contains('another value')).toBeTruthy();
   });

packages/grafana-ui/src/components/Logs/LogDetailsRow.tsx

@@ -28,12 +28,16 @@ interface State {
 
 const getStyles = stylesFactory((theme: GrafanaTheme) => {
   return {
-    noHoverEffect: css`
-      label: noHoverEffect;
+    noHoverBackground: css`
+      label: noHoverBackground;
       :hover {
         background-color: transparent;
       }
     `,
+    hoverCursor: css`
+      label: hoverCursor;
+      cursor: pointer;
+    `,
   };
 });
 
@@ -82,37 +86,24 @@ class UnThemedLogDetailsRow extends PureComponent<Props, State> {
     const styles = getStyles(theme);
     const style = getLogRowStyles(theme);
     return (
-      <div className={cx(style.logsRowDetailsValue, { [styles.noHoverEffect]: showFieldsStats })}>
+      <tr className={cx(style.logDetailsValue, { [styles.noHoverBackground]: showFieldsStats })}>
         {/* Action buttons - show stats/filter results */}
-        <div
-          title="Ad-hoc statistics"
-          onClick={this.showStats}
-          aria-label={'Field stats'}
-          className={style.logsRowDetailsIcon}
-        >
-          <i className={'fa fa-signal'} />
-        </div>
-        {isLabel ? (
-          <div title="Filter for value" onClick={() => this.filterLabel()} className={style.logsRowDetailsIcon}>
-            <i className={'fa fa-search-plus'} />
-          </div>
-        ) : (
-          <div className={style.logsRowDetailsIcon} />
-        )}
-        {isLabel ? (
-          <div title="Filter out value" onClick={() => this.filterOutLabel()} className={style.logsRowDetailsIcon}>
-            <i className={'fa fa-search-minus'} />
-          </div>
-        ) : (
-          <div className={style.logsRowDetailsIcon} />
-        )}
+        <td title="Ad-hoc statistics" onClick={this.showStats} className={style.logsDetailsIcon}>
+          <i className={`fa fa-signal ${styles.hoverCursor}`} />
+        </td>
+
+        <td title="Filter for value" onClick={() => isLabel && this.filterLabel()} className={style.logsDetailsIcon}>
+          {isLabel && <i className={`fa fa-search-plus ${styles.hoverCursor}`} />}
+        </td>
+
+        <td title="Filter out value" onClick={() => isLabel && this.filterOutLabel()} className={style.logsDetailsIcon}>
+          {isLabel && <i className={`fa fa-search-minus ${styles.hoverCursor}`} />}
+        </td>
 
         {/* Key - value columns */}
-        <div className={style.logsRowDetailsLabel}>
-          <span>{parsedKey}</span>
-        </div>
-        <div className={style.logsRowCell}>
-          <span>{parsedValue}</span>
+        <td className={style.logDetailsLabel}>{parsedKey}</td>
+        <td className={style.logsRowCell}>
+          {parsedValue}
           {links &&
             links.map(link => {
               return (
@@ -125,18 +116,16 @@ class UnThemedLogDetailsRow extends PureComponent<Props, State> {
               );
             })}
           {showFieldsStats && (
-            <div className={style.logsRowCell}>
-              <LogLabelStats
-                stats={fieldStats!}
-                label={parsedKey}
-                value={parsedValue}
-                rowCount={fieldCount}
-                isLabel={isLabel}
-              />
-            </div>
+            <LogLabelStats
+              stats={fieldStats!}
+              label={parsedKey}
+              value={parsedValue}
+              rowCount={fieldCount}
+              isLabel={isLabel}
+            />
           )}
-        </div>
-      </div>
+        </td>
+      </tr>
     );
   }
 }

packages/grafana-ui/src/components/Logs/LogLabelStats.tsx

@@ -23,10 +23,10 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => {
   return {
     logsStats: css`
       label: logs-stats;
-      display: table-cell;
       column-span: 2;
       background: inherit;
       color: ${theme.colors.text};
+      word-break: break-all;
     `,
     logsStatsHeader: css`
       label: logs-stats__header;
@@ -82,7 +82,7 @@ class UnThemedLogLabelStats extends PureComponent<Props> {
     const otherProportion = otherCount / total;
 
     return (
-      <div className={style.logsStats}>
+      <td className={style.logsStats}>
         <div className={style.logsStatsHeader}>
           <div className={style.logsStatsTitle}>
             {label}: {total} of {rowCount} rows have that {isLabel ? 'label' : 'field'}
@@ -97,7 +97,7 @@ class UnThemedLogLabelStats extends PureComponent<Props> {
             <LogLabelStatsRow key="__OTHERS__" count={otherCount} value="Other" proportion={otherProportion} />
           )}
         </div>
-      </div>
+      </td>
     );
   }
 }

packages/grafana-ui/src/components/Logs/getLogRowStyles.ts

@@ -45,7 +45,6 @@ export const getLogRowStyles = stylesFactory((theme: GrafanaTheme, logLevel?: Lo
       label: logs-row__match-highlight;
       background: inherit;
       padding: inherit;
-
       color: ${theme.colors.yellow};
       background-color: rgba(${theme.colors.yellow}, 0.1);
     `,
@@ -119,14 +118,13 @@ export const getLogRowStyles = stylesFactory((theme: GrafanaTheme, logLevel?: Lo
     `,
     logsRowCell: css`
       label: logs-row-cell;
-      display: table-cell;
       word-break: break-all;
+      padding-right: ${theme.spacing.sm};
     `,
     logsRowToggleDetails: css`
       label: logs-row-toggle-details__level;
       position: relative;
       width: 15px;
-      padding-right: ${theme.spacing.sm};
       font-size: 9px;
     `,
     logsRowLocalTime: css`
@@ -153,59 +151,43 @@ export const getLogRowStyles = stylesFactory((theme: GrafanaTheme, logLevel?: Lo
       margin: 5px 0;
     `,
     //Log details sepcific CSS
-    logsRowDetailsTable: css`
+    logDetailsContainer: css`
       label: logs-row-details-table;
-      display: table;
       border: 1px solid ${borderColor};
+      padding: 0 ${theme.spacing.sm} ${theme.spacing.sm};
       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: 0;
+    logDetailsTable: css`
+      label: logs-row-details-table;
       width: 100%;
-      &:first-of-type {
-        margin-bottom: ${theme.spacing.xs};
-      }
     `,
-    logsRowDetailsIcon: css`
+    logsDetailsIcon: css`
       label: logs-row-details__icon;
-      display: table-cell;
       position: relative;
-      width: 22px;
       padding-right: ${theme.spacing.sm};
       color: ${theme.colors.gray3};
-      &:hover {
-        cursor: pointer;
-      }
     `,
-    logsRowDetailsLabel: css`
+    logDetailsLabel: css`
       label: logs-row-details__label;
-      display: table-cell;
-      padding: 0 ${theme.spacing.md} 0 ${theme.spacing.md};
-      width: 14em;
+      max-width: 25em;
+      min-width: 12em;
+      padding: 0 ${theme.spacing.sm};
       word-break: break-all;
     `,
-    logsRowDetailsHeading: css`
+    logDetailsHeading: css`
       label: logs-row-details__heading;
-      display: table-caption;
-      margin: ${theme.spacing.sm} 0 ${theme.spacing.xs};
       font-weight: ${theme.typography.weight.bold};
+      padding: ${theme.spacing.sm} 0 ${theme.spacing.xs};
     `,
-    logsRowDetailsValue: css`
+    logDetailsValue: css`
       label: logs-row-details__row;
-      display: table-row;
       line-height: 2;
-      padding: 0 ${theme.spacing.xl} 0 ${theme.spacing.md};
+      padding: ${theme.spacing.sm};
       position: relative;
+      vertical-align: top;
       cursor: default;
-
       &:hover {
         background-color: ${bgColor};
       }