From 60973efe23af207a35b096782eb0eba031a8783d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Rica?= Date: Tue, 10 Dec 2024 14:41:04 +0100 Subject: [PATCH] Refactor series color resolution, fix nits --- .../components/kpis/container_kpi_charts.tsx | 6 +-- .../components/kpis/host_kpi_charts.tsx | 4 +- .../hooks/use_chart_series_color.test.ts | 43 +++++++++++++++++++ .../hooks/use_chart_series_color.ts | 21 +++++++++ .../hooks/use_container_metrics_charts.ts | 5 +++ .../hooks/use_host_metrics_charts.ts | 3 ++ .../asset_details/hooks/use_log_charts.ts | 11 +++-- .../asset_details/hooks/use_page_header.tsx | 3 +- .../asset_details/tabs/overview/logs.tsx | 4 +- 9 files changed, 83 insertions(+), 17 deletions(-) create mode 100644 x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_chart_series_color.test.ts create mode 100644 x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_chart_series_color.ts diff --git a/x-pack/plugins/observability_solution/infra/public/components/asset_details/components/kpis/container_kpi_charts.tsx b/x-pack/plugins/observability_solution/infra/public/components/asset_details/components/kpis/container_kpi_charts.tsx index 9d02b374c50b0..95291d157745e 100644 --- a/x-pack/plugins/observability_solution/infra/public/components/asset_details/components/kpis/container_kpi_charts.tsx +++ b/x-pack/plugins/observability_solution/infra/public/components/asset_details/components/kpis/container_kpi_charts.tsx @@ -6,7 +6,7 @@ */ import React from 'react'; -import { EuiFlexItem, useEuiTheme } from '@elastic/eui'; +import { EuiFlexItem } from '@elastic/eui'; import type { DataView } from '@kbn/data-views-plugin/public'; import type { Filter, Query, TimeRange } from '@kbn/es-query'; import { Kpi } from './kpi'; @@ -79,10 +79,8 @@ const DockerKpiCharts = ({ searchSessionId, loading = false, }: ContainerKpiChartsProps) => { - const { euiTheme } = useEuiTheme(); const charts = useDockerContainerKpiCharts({ dataViewId: dataView?.id, - seriesColor: euiTheme.colors.backgroundLightText, }); return ( @@ -112,10 +110,8 @@ const KubernetesKpiCharts = ({ searchSessionId, loading = false, }: ContainerKpiChartsProps) => { - const { euiTheme } = useEuiTheme(); const charts = useK8sContainerKpiCharts({ dataViewId: dataView?.id, - seriesColor: euiTheme.colors.backgroundLightText, }); return ( diff --git a/x-pack/plugins/observability_solution/infra/public/components/asset_details/components/kpis/host_kpi_charts.tsx b/x-pack/plugins/observability_solution/infra/public/components/asset_details/components/kpis/host_kpi_charts.tsx index e9ee0c8d18631..64345efb1af8a 100644 --- a/x-pack/plugins/observability_solution/infra/public/components/asset_details/components/kpis/host_kpi_charts.tsx +++ b/x-pack/plugins/observability_solution/infra/public/components/asset_details/components/kpis/host_kpi_charts.tsx @@ -6,7 +6,7 @@ */ import React from 'react'; -import { EuiFlexItem, useEuiTheme } from '@elastic/eui'; +import { EuiFlexItem } from '@elastic/eui'; import type { DataView } from '@kbn/data-views-plugin/public'; import type { Filter, Query, TimeRange } from '@kbn/es-query'; import { Kpi } from './kpi'; @@ -31,11 +31,9 @@ export const HostKpiCharts = ({ searchSessionId, loading = false, }: HostKpiChartsProps) => { - const { euiTheme } = useEuiTheme(); const charts = useHostKpiCharts({ dataViewId: dataView?.id, getSubtitle, - seriesColor: euiTheme.colors.backgroundLightText, }); return ( diff --git a/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_chart_series_color.test.ts b/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_chart_series_color.test.ts new file mode 100644 index 0000000000000..7c58445a503db --- /dev/null +++ b/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_chart_series_color.test.ts @@ -0,0 +1,43 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { useEuiTheme } from '@elastic/eui'; +import { renderHook } from '@testing-library/react'; +import { useChartSeriesColor } from './use_chart_series_color'; + +describe('useChartSeriesColor', () => { + let seriesDefaultColor: string; + + beforeEach(() => { + const { result } = renderHook(() => useEuiTheme()); + + // Don't try to test a hardcoded value, just use what is provided by EUI. + // If in the future this value changes, the tests won't break. + seriesDefaultColor = result.current.euiTheme.colors.backgroundLightText; + }); + + it('returns a default color value if given no input', () => { + const { result } = renderHook(() => useChartSeriesColor()); + + expect(result.current).not.toBe(''); + expect(result.current).toBe(seriesDefaultColor); + }); + + it('returns a default color value if given an empty string', () => { + const { result } = renderHook(() => useChartSeriesColor('')); + + expect(result.current).not.toBe(''); + expect(result.current).toBe(seriesDefaultColor); + }); + + it('returns the provided color input', () => { + const { result } = renderHook(() => useChartSeriesColor('#fff')); + + expect(result.current).not.toBe(seriesDefaultColor); + expect(result.current).toBe('#fff'); + }); +}); diff --git a/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_chart_series_color.ts b/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_chart_series_color.ts new file mode 100644 index 0000000000000..d71408982226f --- /dev/null +++ b/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_chart_series_color.ts @@ -0,0 +1,21 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { useEuiTheme } from '@elastic/eui'; + +/** + * Provides either the input color, or yields the default EUI theme + * color for use as the KPI chart series color. + * @param seriesColor A user-defined color value + * @returns Either the input `seriesColor` or the default color from EUI + */ +export const useChartSeriesColor = (seriesColor?: string): string => { + const { euiTheme } = useEuiTheme(); + + // Prevent empty string being used as a valid color + return seriesColor || euiTheme.colors.backgroundLightText; +}; diff --git a/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_container_metrics_charts.ts b/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_container_metrics_charts.ts index cf1d5062a5c6c..c453725f3f527 100644 --- a/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_container_metrics_charts.ts +++ b/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_container_metrics_charts.ts @@ -9,6 +9,7 @@ import { i18n } from '@kbn/i18n'; import { findInventoryModel } from '@kbn/metrics-data-access-plugin/common'; import useAsync from 'react-use/lib/useAsync'; import { ContainerMetricTypes } from '../charts/types'; +import { useChartSeriesColor } from './use_chart_series_color'; const getSubtitleFromFormula = (value: string) => value.startsWith('max') @@ -106,6 +107,8 @@ export const useDockerContainerKpiCharts = ({ dataViewId?: string; seriesColor?: string; }) => { + seriesColor = useChartSeriesColor(seriesColor); + const { value: charts = [] } = useAsync(async () => { const model = findInventoryModel('container'); const { cpu, memory } = await model.metrics.getCharts(); @@ -134,6 +137,8 @@ export const useK8sContainerKpiCharts = ({ dataViewId?: string; seriesColor?: string; }) => { + seriesColor = useChartSeriesColor(seriesColor); + const { value: charts = [] } = useAsync(async () => { const model = findInventoryModel('container'); const { cpu, memory } = await model.metrics.getCharts(); diff --git a/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_host_metrics_charts.ts b/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_host_metrics_charts.ts index 57c9a5a0d7d42..ba3e3f973b35f 100644 --- a/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_host_metrics_charts.ts +++ b/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_host_metrics_charts.ts @@ -10,6 +10,7 @@ import { findInventoryModel } from '@kbn/metrics-data-access-plugin/common'; import { useMemo } from 'react'; import useAsync from 'react-use/lib/useAsync'; import { HostMetricTypes } from '../charts/types'; +import { useChartSeriesColor } from './use_chart_series_color'; export const useHostCharts = ({ metric, @@ -87,6 +88,8 @@ export const useHostKpiCharts = ({ seriesColor?: string; getSubtitle?: (formulaValue: string) => string; }) => { + seriesColor = useChartSeriesColor(seriesColor); + const { value: charts = [] } = useAsync(async () => { const model = findInventoryModel('host'); const { cpu, memory, disk } = await model.metrics.getCharts(); diff --git a/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_log_charts.ts b/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_log_charts.ts index fee71161a43be..e373e214a63c2 100644 --- a/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_log_charts.ts +++ b/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_log_charts.ts @@ -8,6 +8,7 @@ import { i18n } from '@kbn/i18n'; import { useMemo } from 'react'; import { LensConfig } from '@kbn/lens-embeddable-utils/config_builder'; +import { useChartSeriesColor } from './use_chart_series_color'; const LOG_RATE = i18n.translate('xpack.infra.assetDetails.charts.logRate', { defaultMessage: 'Log Rate', @@ -17,7 +18,7 @@ const LOG_ERROR_RATE = i18n.translate('xpack.infra.assetDetails.charts.logErrorR defaultMessage: 'Log Error Rate', }); -const logMetric: LensConfig & { id: string } = { +const logRateMetric: LensConfig & { id: string } = { id: 'logMetric', chartType: 'metric', title: LOG_RATE, @@ -29,7 +30,7 @@ const logMetric: LensConfig & { id: string } = { normalizeByUnit: 's', }; -const logErrorMetric: LensConfig & { id: string } = { +const logErrorRateMetric: LensConfig & { id: string } = { id: 'logErrorMetric', chartType: 'metric', title: LOG_ERROR_RATE, @@ -49,6 +50,8 @@ export const useLogsCharts = ({ dataViewId?: string; seriesColor?: string; }) => { + seriesColor = useChartSeriesColor(seriesColor); + return useMemo(() => { const dataset = dataViewId && { dataset: { @@ -59,12 +62,12 @@ export const useLogsCharts = ({ return { charts: [ { - ...logMetric, + ...logRateMetric, ...dataset, seriesColor, }, { - ...logErrorMetric, + ...logErrorRateMetric, ...dataset, seriesColor, }, diff --git a/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_page_header.tsx b/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_page_header.tsx index a90c3064793bd..aeb85617015e9 100644 --- a/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_page_header.tsx +++ b/x-pack/plugins/observability_solution/infra/public/components/asset_details/hooks/use_page_header.tsx @@ -178,8 +178,7 @@ const useTabs = (tabs: Tab[]) => { const tabEntries: TabItem[] = useMemo( () => tabs - .filter(isTabEnabled) - .filter(hasMetricsTab) + .filter((tab) => isTabEnabled(tab) && hasMetricsTab(tab)) .map(({ name, ...tab }) => { return { ...tab, diff --git a/x-pack/plugins/observability_solution/infra/public/components/asset_details/tabs/overview/logs.tsx b/x-pack/plugins/observability_solution/infra/public/components/asset_details/tabs/overview/logs.tsx index 544ed84cbd4cb..5ca334acf20ec 100644 --- a/x-pack/plugins/observability_solution/infra/public/components/asset_details/tabs/overview/logs.tsx +++ b/x-pack/plugins/observability_solution/infra/public/components/asset_details/tabs/overview/logs.tsx @@ -8,7 +8,7 @@ import React, { useMemo } from 'react'; import type { DataView } from '@kbn/data-views-plugin/public'; import type { TimeRange } from '@kbn/es-query'; -import { EuiFlexGroup, EuiFlexItem, useEuiTheme } from '@elastic/eui'; +import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; import { findInventoryFields, type InventoryItemType, @@ -38,10 +38,8 @@ export const LogsContent = ({ assetId, assetType, dataView, dateRange }: Props) ]; }, [dataView, assetId, assetType]); - const { euiTheme } = useEuiTheme(); const { charts } = useLogsCharts({ dataViewId: dataView?.id, - seriesColor: euiTheme.colors.backgroundLightText, }); return (