From cd39c2bd2520766b9d4a7b37dbe57abe02240055 Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Thu, 12 Dec 2019 16:00:41 +0100 Subject: [PATCH] Explore: Refactor log details table (#21044) --- .../src/components/Logs/LogDetails.test.tsx | 8 +-- .../src/components/Logs/LogDetails.tsx | 44 +++++++----- .../components/Logs/LogDetailsRow.test.tsx | 2 +- .../src/components/Logs/LogDetailsRow.tsx | 71 ++++++++----------- .../src/components/Logs/LogLabelStats.tsx | 6 +- .../src/components/Logs/getLogRowStyles.ts | 48 ++++--------- 6 files changed, 79 insertions(+), 100 deletions(-) diff --git a/packages/grafana-ui/src/components/Logs/LogDetails.test.tsx b/packages/grafana-ui/src/components/Logs/LogDetails.test.tsx index 545c824d478..c0b498131e1 100644 --- a/packages/grafana-ui/src/components/Logs/LogDetails.test.tsx +++ b/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, { diff --git a/packages/grafana-ui/src/components/Logs/LogDetails.tsx b/packages/grafana-ui/src/components/Logs/LogDetails.tsx index cb8a7d0c28e..14449640df8 100644 --- a/packages/grafana-ui/src/components/Logs/LogDetails.tsx +++ b/packages/grafana-ui/src/components/Logs/LogDetails.tsx @@ -106,18 +106,20 @@ class UnThemedLogDetails extends PureComponent { 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 ( -
- {labelsAvailable && ( -
-
- Log Labels: -
+
+ + + {labelsAvailable && ( + + + + )} {Object.keys(labels).map(key => { const value = labels[key]; return ( @@ -132,14 +134,14 @@ class UnThemedLogDetails extends PureComponent { /> ); })} - - )} - {parsedFieldsAvailable && ( -
-
- Parsed fields: -
+ {parsedFieldsAvailable && ( +
+ + + )} {fields.map(field => { const { key, value, links, fieldIndex } = field; return ( @@ -156,9 +158,15 @@ class UnThemedLogDetails extends PureComponent { /> ); })} - - )} - {!parsedFieldsAvailable && !labelsAvailable &&
No details available
} + {!parsedFieldsAvailable && !labelsAvailable && ( + + + + )} + +
+ Log Labels: +
+ Parsed Fields: +
+ No details available +
); } diff --git a/packages/grafana-ui/src/components/Logs/LogDetailsRow.test.tsx b/packages/grafana-ui/src/components/Logs/LogDetailsRow.test.tsx index 038ece69090..2d1e19f6d1a 100644 --- a/packages/grafana-ui/src/components/Logs/LogDetailsRow.test.tsx +++ b/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(); }); diff --git a/packages/grafana-ui/src/components/Logs/LogDetailsRow.tsx b/packages/grafana-ui/src/components/Logs/LogDetailsRow.tsx index b6a227d882c..0636a9600df 100644 --- a/packages/grafana-ui/src/components/Logs/LogDetailsRow.tsx +++ b/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 { const styles = getStyles(theme); const style = getLogRowStyles(theme); return ( -
+ {/* Action buttons - show stats/filter results */} -
- -
- {isLabel ? ( -
this.filterLabel()} className={style.logsRowDetailsIcon}> - -
- ) : ( -
- )} - {isLabel ? ( -
this.filterOutLabel()} className={style.logsRowDetailsIcon}> - -
- ) : ( -
- )} + + + + + isLabel && this.filterLabel()} className={style.logsDetailsIcon}> + {isLabel && } + + + isLabel && this.filterOutLabel()} className={style.logsDetailsIcon}> + {isLabel && } + {/* Key - value columns */} -
- {parsedKey} -
-
- {parsedValue} + {parsedKey} + + {parsedValue} {links && links.map(link => { return ( @@ -125,18 +116,16 @@ class UnThemedLogDetailsRow extends PureComponent { ); })} {showFieldsStats && ( -
- -
+ )} -
-
+ + ); } } diff --git a/packages/grafana-ui/src/components/Logs/LogLabelStats.tsx b/packages/grafana-ui/src/components/Logs/LogLabelStats.tsx index e719b052a04..59eb362829b 100644 --- a/packages/grafana-ui/src/components/Logs/LogLabelStats.tsx +++ b/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 { const otherProportion = otherCount / total; return ( -
+
{label}: {total} of {rowCount} rows have that {isLabel ? 'label' : 'field'} @@ -97,7 +97,7 @@ class UnThemedLogLabelStats extends PureComponent { )}
-
+ ); } } diff --git a/packages/grafana-ui/src/components/Logs/getLogRowStyles.ts b/packages/grafana-ui/src/components/Logs/getLogRowStyles.ts index d19fbe804e9..2fce1717d70 100644 --- a/packages/grafana-ui/src/components/Logs/getLogRowStyles.ts +++ b/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}; }