From f7ec8110ee9ea295251d969b2b38789a772b743a Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Thu, 13 Aug 2020 11:22:31 +0200 Subject: [PATCH 1/7] changing unit when legend is disabled --- .../shared/charts/CustomPlot/index.js | 13 ++ .../TransactionLineChart/index.tsx | 19 +- .../shared/charts/TransactionCharts/index.tsx | 194 +++++++++--------- 3 files changed, 126 insertions(+), 100 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js b/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js index 7e74961e57ea1..446e8cb471794 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js +++ b/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js @@ -79,6 +79,18 @@ export class InnerCustomPlot extends PureComponent { return i === _i ? !disabledValue : !!disabledValue; }); + if (typeof this.props.onToggleLegend === 'function') { + //Filters out disabled series + const availableSeries = this.props.series + .map((serie, index) => { + if (!nextSeriesEnabledState[index]) { + return serie; + } + }) + .filter((serie) => serie); + this.props.onToggleLegend(availableSeries); + } + return { seriesEnabledState: nextSeriesEnabledState, }; @@ -235,6 +247,7 @@ InnerCustomPlot.propTypes = { }) ), noHits: PropTypes.bool, + onToggleLegend: PropTypes.func, }; InnerCustomPlot.defaultProps = { diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx index eaad883d2f9f6..1788a8b621671 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx @@ -13,14 +13,16 @@ import { useChartsSync } from '../../../../../hooks/useChartsSync'; // @ts-ignore import CustomPlot from '../../CustomPlot'; +export interface Serie { + color: string; + title: React.ReactNode; + titleShort?: React.ReactNode; + data: Array; + type: string; +} + interface Props { - series: Array<{ - color: string; - title: React.ReactNode; - titleShort?: React.ReactNode; - data: Array; - type: string; - }>; + series: Serie[]; truncateLegends?: boolean; tickFormatY: (y: number) => React.ReactNode; formatTooltipValue: (c: Coordinate) => React.ReactNode; @@ -28,6 +30,7 @@ interface Props { height?: number; stacked?: boolean; onHover?: () => void; + onToggleLegend?: (series: Serie[]) => void; } function TransactionLineChart(props: Props) { @@ -40,6 +43,7 @@ function TransactionLineChart(props: Props) { truncateLegends, stacked = false, onHover, + onToggleLegend, } = props; const syncedChartsProps = useChartsSync(); @@ -66,6 +70,7 @@ function TransactionLineChart(props: Props) { height={height} truncateLegends={truncateLegends} {...(stacked ? { stackBy: 'y' } : {})} + onToggleLegend={onToggleLegend} /> ); } diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx index 1f80dbf5f4d95..60ec345e08e6e 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx @@ -10,36 +10,36 @@ import { EuiFlexItem, EuiIconTip, EuiPanel, + EuiSpacer, EuiText, EuiTitle, - EuiSpacer, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { Location } from 'history'; -import React, { Component } from 'react'; -import { isEmpty, flatten } from 'lodash'; +import { flatten, isEmpty } from 'lodash'; +import React, { useEffect, useState } from 'react'; import styled from 'styled-components'; import { NOT_AVAILABLE_LABEL } from '../../../../../common/i18n'; +import { + TRANSACTION_PAGE_LOAD, + TRANSACTION_REQUEST, + TRANSACTION_ROUTE_CHANGE, +} from '../../../../../common/transaction_types'; import { Coordinate, TimeSeries } from '../../../../../typings/timeseries'; -import { ITransactionChartData } from '../../../../selectors/chartSelectors'; +import { LicenseContext } from '../../../../context/LicenseContext'; import { IUrlParams } from '../../../../context/UrlParamsContext/types'; +import { ITransactionChartData } from '../../../../selectors/chartSelectors'; import { - tpmUnit, - TimeFormatter, - getDurationFormatter, asDecimal, + getDurationFormatter, + TimeFormatter, + tpmUnit, } from '../../../../utils/formatters'; -import { MLJobLink } from '../../Links/MachineLearningLinks/MLJobLink'; -import { LicenseContext } from '../../../../context/LicenseContext'; -import { TransactionLineChart } from './TransactionLineChart'; import { isValidCoordinateValue } from '../../../../utils/isValidCoordinateValue'; +import { MLJobLink } from '../../Links/MachineLearningLinks/MLJobLink'; import { BrowserLineChart } from './BrowserLineChart'; import { DurationByCountryMap } from './DurationByCountryMap'; -import { - TRANSACTION_PAGE_LOAD, - TRANSACTION_ROUTE_CHANGE, - TRANSACTION_REQUEST, -} from '../../../../../common/transaction_types'; +import { Serie, TransactionLineChart } from './TransactionLineChart'; interface TransactionChartProps { charts: ITransactionChartData; @@ -81,27 +81,30 @@ export function getMaxY(responseTimeSeries: TimeSeries[]) { return Math.max(...numbers, 0); } -export class TransactionCharts extends Component { - public getTPMFormatter = (t: number) => { - const { urlParams } = this.props; +export function TransactionCharts({ + charts, + location, + urlParams, +}: TransactionChartProps) { + const getTPMFormatter = (t: number) => { const unit = tpmUnit(urlParams.transactionType); return `${asDecimal(t)} ${unit}`; }; - public getTPMTooltipFormatter = (p: Coordinate) => { + const getTPMTooltipFormatter = (p: Coordinate) => { return isValidCoordinateValue(p.y) - ? this.getTPMFormatter(p.y) + ? getTPMFormatter(p.y) : NOT_AVAILABLE_LABEL; }; - public renderMLHeader(hasValidMlLicense: boolean | undefined) { - const { mlJobId } = this.props.charts; + function renderMLHeader(hasValidMlLicense: boolean | undefined) { + const { mlJobId } = charts; if (!hasValidMlLicense || !mlJobId) { return null; } - const { serviceName, kuery, transactionType } = this.props.urlParams; + const { serviceName, kuery, transactionType } = urlParams; if (!serviceName) { return null; } @@ -150,78 +153,83 @@ export class TransactionCharts extends Component { ); } - public render() { - const { charts, urlParams } = this.props; - const { responseTimeSeries, tpmSeries } = charts; - const { transactionType } = urlParams; - const maxY = getMaxY(responseTimeSeries); - const formatter = getDurationFormatter(maxY); + const { responseTimeSeries, tpmSeries } = charts; + const { transactionType } = urlParams; + const [maxY, setMaxY] = useState(0); + useEffect(() => { + setMaxY(getMaxY(responseTimeSeries)); + }, [responseTimeSeries]); + const formatter = getDurationFormatter(maxY); - return ( - <> - - - - - - - - {responseTimeLabel(transactionType)} - - - - {(license) => - this.renderMLHeader(license?.getFeature('ml').isAvailable) - } - - - - - - + const onToggleLegend = (series: Serie[]) => { + if (!isEmpty(series)) { + setMaxY(getMaxY(series as TimeSeries[])); + } + }; - - - - - {tpmLabel(transactionType)} - - - - - - - {transactionType === TRANSACTION_PAGE_LOAD && ( - <> - - - - - - - - - - - - - - - )} - - ); - } + return ( + <> + + + + + + + + {responseTimeLabel(transactionType)} + + + + {(license) => + renderMLHeader(license?.getFeature('ml').isAvailable) + } + + + + + + + + + + + + {tpmLabel(transactionType)} + + + + + + + {transactionType === TRANSACTION_PAGE_LOAD && ( + <> + + + + + + + + + + + + + + + )} + + ); } function tpmLabel(type?: string) { From 98e9173819146fdfa95aa2ceae701f0d1ace8027 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Thu, 13 Aug 2020 13:34:36 +0200 Subject: [PATCH 2/7] changing unit when legend is disabled --- .../shared/charts/TransactionCharts/index.tsx | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx index 60ec345e08e6e..6caa90e94a416 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx @@ -16,7 +16,7 @@ import { } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { Location } from 'history'; -import { flatten, isEmpty } from 'lodash'; +import { flatten, isEmpty, mean } from 'lodash'; import React, { useEffect, useState } from 'react'; import styled from 'styled-components'; import { NOT_AVAILABLE_LABEL } from '../../../../../common/i18n'; @@ -81,6 +81,18 @@ export function getMaxY(responseTimeSeries: TimeSeries[]) { return Math.max(...numbers, 0); } +function getAverageY(responseTimeSeries: TimeSeries[]) { + const averageSeries = responseTimeSeries.find( + (serie) => serie.title === 'Avg.' + ); + if (averageSeries) { + const averageYValues = (averageSeries.data as Coordinate[]) + .map((c) => c.y) + .filter((y) => y && isFinite(y)); + return mean(averageYValues); + } +} + export function TransactionCharts({ charts, location, @@ -159,6 +171,10 @@ export function TransactionCharts({ useEffect(() => { setMaxY(getMaxY(responseTimeSeries)); }, [responseTimeSeries]); + + const averageY = getAverageY(responseTimeSeries); + + const formatTooltip = getDurationFormatter(averageY ?? maxY); const formatter = getDurationFormatter(maxY); const onToggleLegend = (series: Serie[]) => { @@ -189,7 +205,9 @@ export function TransactionCharts({ onToggleLegend={onToggleLegend} series={responseTimeSeries} tickFormatY={getResponseTimeTickFormatter(formatter)} - formatTooltipValue={getResponseTimeTooltipFormatter(formatter)} + formatTooltipValue={getResponseTimeTooltipFormatter( + formatTooltip + )} /> From a04aabac22cb0b9735600c0b2e05754b24b0eab9 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Thu, 20 Aug 2020 11:13:25 +0200 Subject: [PATCH 3/7] show individual units in the tooltip --- .../shared/charts/CustomPlot/index.js | 13 ----- .../TransactionLineChart/index.tsx | 19 +++---- .../shared/charts/TransactionCharts/index.tsx | 54 +++++-------------- 3 files changed, 21 insertions(+), 65 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js b/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js index 446e8cb471794..7e74961e57ea1 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js +++ b/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js @@ -79,18 +79,6 @@ export class InnerCustomPlot extends PureComponent { return i === _i ? !disabledValue : !!disabledValue; }); - if (typeof this.props.onToggleLegend === 'function') { - //Filters out disabled series - const availableSeries = this.props.series - .map((serie, index) => { - if (!nextSeriesEnabledState[index]) { - return serie; - } - }) - .filter((serie) => serie); - this.props.onToggleLegend(availableSeries); - } - return { seriesEnabledState: nextSeriesEnabledState, }; @@ -247,7 +235,6 @@ InnerCustomPlot.propTypes = { }) ), noHits: PropTypes.bool, - onToggleLegend: PropTypes.func, }; InnerCustomPlot.defaultProps = { diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx index 1788a8b621671..eaad883d2f9f6 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx @@ -13,16 +13,14 @@ import { useChartsSync } from '../../../../../hooks/useChartsSync'; // @ts-ignore import CustomPlot from '../../CustomPlot'; -export interface Serie { - color: string; - title: React.ReactNode; - titleShort?: React.ReactNode; - data: Array; - type: string; -} - interface Props { - series: Serie[]; + series: Array<{ + color: string; + title: React.ReactNode; + titleShort?: React.ReactNode; + data: Array; + type: string; + }>; truncateLegends?: boolean; tickFormatY: (y: number) => React.ReactNode; formatTooltipValue: (c: Coordinate) => React.ReactNode; @@ -30,7 +28,6 @@ interface Props { height?: number; stacked?: boolean; onHover?: () => void; - onToggleLegend?: (series: Serie[]) => void; } function TransactionLineChart(props: Props) { @@ -43,7 +40,6 @@ function TransactionLineChart(props: Props) { truncateLegends, stacked = false, onHover, - onToggleLegend, } = props; const syncedChartsProps = useChartsSync(); @@ -70,7 +66,6 @@ function TransactionLineChart(props: Props) { height={height} truncateLegends={truncateLegends} {...(stacked ? { stackBy: 'y' } : {})} - onToggleLegend={onToggleLegend} /> ); } diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx index 6caa90e94a416..a26272853850b 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx @@ -16,8 +16,8 @@ import { } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { Location } from 'history'; -import { flatten, isEmpty, mean } from 'lodash'; -import React, { useEffect, useState } from 'react'; +import { flatten, isEmpty } from 'lodash'; +import React from 'react'; import styled from 'styled-components'; import { NOT_AVAILABLE_LABEL } from '../../../../../common/i18n'; import { @@ -31,7 +31,7 @@ import { IUrlParams } from '../../../../context/UrlParamsContext/types'; import { ITransactionChartData } from '../../../../selectors/chartSelectors'; import { asDecimal, - getDurationFormatter, + asDuration, TimeFormatter, tpmUnit, } from '../../../../utils/formatters'; @@ -39,7 +39,7 @@ import { isValidCoordinateValue } from '../../../../utils/isValidCoordinateValue import { MLJobLink } from '../../Links/MachineLearningLinks/MLJobLink'; import { BrowserLineChart } from './BrowserLineChart'; import { DurationByCountryMap } from './DurationByCountryMap'; -import { Serie, TransactionLineChart } from './TransactionLineChart'; +import { TransactionLineChart } from './TransactionLineChart'; interface TransactionChartProps { charts: ITransactionChartData; @@ -59,14 +59,18 @@ const ShiftedEuiText = styled(EuiText)` top: 5px; `; -export function getResponseTimeTickFormatter(formatter: TimeFormatter) { - return (t: number) => formatter(t).formatted; +export function getResponseTimeTickFormatter(formatter?: TimeFormatter) { + return (t: number) => { + return formatter ? formatter(t).formatted : asDuration(t); + }; } -export function getResponseTimeTooltipFormatter(formatter: TimeFormatter) { +export function getResponseTimeTooltipFormatter(formatter?: TimeFormatter) { return (p: Coordinate) => { return isValidCoordinateValue(p.y) - ? formatter(p.y).formatted + ? formatter + ? formatter(p.y).formatted + : asDuration(p.y) : NOT_AVAILABLE_LABEL; }; } @@ -81,18 +85,6 @@ export function getMaxY(responseTimeSeries: TimeSeries[]) { return Math.max(...numbers, 0); } -function getAverageY(responseTimeSeries: TimeSeries[]) { - const averageSeries = responseTimeSeries.find( - (serie) => serie.title === 'Avg.' - ); - if (averageSeries) { - const averageYValues = (averageSeries.data as Coordinate[]) - .map((c) => c.y) - .filter((y) => y && isFinite(y)); - return mean(averageYValues); - } -} - export function TransactionCharts({ charts, location, @@ -167,21 +159,6 @@ export function TransactionCharts({ const { responseTimeSeries, tpmSeries } = charts; const { transactionType } = urlParams; - const [maxY, setMaxY] = useState(0); - useEffect(() => { - setMaxY(getMaxY(responseTimeSeries)); - }, [responseTimeSeries]); - - const averageY = getAverageY(responseTimeSeries); - - const formatTooltip = getDurationFormatter(averageY ?? maxY); - const formatter = getDurationFormatter(maxY); - - const onToggleLegend = (series: Serie[]) => { - if (!isEmpty(series)) { - setMaxY(getMaxY(series as TimeSeries[])); - } - }; return ( <> @@ -202,12 +179,9 @@ export function TransactionCharts({ From d2da9f181b4b915bff8a740f08717dec1018bde6 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Thu, 20 Aug 2020 15:07:14 +0200 Subject: [PATCH 4/7] addressing PR comment --- .../TransactionCharts/BrowserLineChart.tsx | 24 ++++++++++++---- .../shared/charts/TransactionCharts/index.tsx | 28 ++++--------------- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/BrowserLineChart.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/BrowserLineChart.tsx index 0e19c63775d31..a3fa2da40619b 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/BrowserLineChart.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/BrowserLineChart.tsx @@ -7,14 +7,28 @@ import React from 'react'; import { i18n } from '@kbn/i18n'; import { EuiTitle } from '@elastic/eui'; +import { NOT_AVAILABLE_LABEL } from '../../../../../common/i18n'; +import { isValidCoordinateValue } from '../../../../utils/isValidCoordinateValue'; import { TransactionLineChart } from './TransactionLineChart'; +import { getMaxY } from '.'; import { - getMaxY, - getResponseTimeTickFormatter, - getResponseTimeTooltipFormatter, -} from '.'; -import { getDurationFormatter } from '../../../../utils/formatters'; + getDurationFormatter, + TimeFormatter, +} from '../../../../utils/formatters'; import { useAvgDurationByBrowser } from '../../../../hooks/useAvgDurationByBrowser'; +import { Coordinate } from '../../../../../typings/timeseries'; + +function getResponseTimeTickFormatter(formatter: TimeFormatter) { + return (t: number) => formatter(t).formatted; +} + +function getResponseTimeTooltipFormatter(formatter: TimeFormatter) { + return (p: Coordinate) => { + return isValidCoordinateValue(p.y) + ? formatter(p.y).formatted + : NOT_AVAILABLE_LABEL; + }; +} export function BrowserLineChart() { const { data } = useAvgDurationByBrowser(); diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx index a26272853850b..583a16281edc4 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx @@ -29,12 +29,7 @@ import { Coordinate, TimeSeries } from '../../../../../typings/timeseries'; import { LicenseContext } from '../../../../context/LicenseContext'; import { IUrlParams } from '../../../../context/UrlParamsContext/types'; import { ITransactionChartData } from '../../../../selectors/chartSelectors'; -import { - asDecimal, - asDuration, - TimeFormatter, - tpmUnit, -} from '../../../../utils/formatters'; +import { asDecimal, asDuration, tpmUnit } from '../../../../utils/formatters'; import { isValidCoordinateValue } from '../../../../utils/isValidCoordinateValue'; import { MLJobLink } from '../../Links/MachineLearningLinks/MLJobLink'; import { BrowserLineChart } from './BrowserLineChart'; @@ -59,21 +54,10 @@ const ShiftedEuiText = styled(EuiText)` top: 5px; `; -export function getResponseTimeTickFormatter(formatter?: TimeFormatter) { - return (t: number) => { - return formatter ? formatter(t).formatted : asDuration(t); - }; -} +const getResponseTimeTickFormatter = (t: number) => asDuration(t); -export function getResponseTimeTooltipFormatter(formatter?: TimeFormatter) { - return (p: Coordinate) => { - return isValidCoordinateValue(p.y) - ? formatter - ? formatter(p.y).formatted - : asDuration(p.y) - : NOT_AVAILABLE_LABEL; - }; -} +const getResponseTimeTooltipFormatter = (p: Coordinate) => + isValidCoordinateValue(p.y) ? asDuration(p.y) : NOT_AVAILABLE_LABEL; export function getMaxY(responseTimeSeries: TimeSeries[]) { const coordinates = flatten( @@ -180,8 +164,8 @@ export function TransactionCharts({ From 294879e1a7936aac9542eb32e53e4374cca7a517 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Wed, 26 Aug 2020 13:30:33 +0200 Subject: [PATCH 5/7] increasing duration threshold --- .../shared/charts/CustomPlot/index.js | 9 +++++ .../TransactionLineChart/index.tsx | 16 +++------ .../shared/charts/TransactionCharts/index.tsx | 33 +++++++++++++++---- .../formatters/__test__/duration.test.ts | 7 ++-- .../apm/public/utils/formatters/duration.ts | 4 +-- 5 files changed, 47 insertions(+), 22 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js b/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js index 7e74961e57ea1..b072c22c09c19 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js +++ b/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js @@ -79,6 +79,14 @@ export class InnerCustomPlot extends PureComponent { return i === _i ? !disabledValue : !!disabledValue; }); + if (typeof this.props.onToggleLegend === 'function') { + //Filters out disabled series + const availableSeries = this.props.series.filter( + (serie, index) => !nextSeriesEnabledState[index] + ); + this.props.onToggleLegend(availableSeries); + } + return { seriesEnabledState: nextSeriesEnabledState, }; @@ -235,6 +243,7 @@ InnerCustomPlot.propTypes = { }) ), noHits: PropTypes.bool, + onToggleLegend: PropTypes.func, }; InnerCustomPlot.defaultProps = { diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx index eaad883d2f9f6..3657a02b1589c 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx @@ -5,22 +5,13 @@ */ import React, { useCallback } from 'react'; -import { - Coordinate, - RectCoordinate, -} from '../../../../../../typings/timeseries'; +import { Coordinate, TimeSeries } from '../../../../../../typings/timeseries'; import { useChartsSync } from '../../../../../hooks/useChartsSync'; // @ts-ignore import CustomPlot from '../../CustomPlot'; interface Props { - series: Array<{ - color: string; - title: React.ReactNode; - titleShort?: React.ReactNode; - data: Array; - type: string; - }>; + series: TimeSeries[]; truncateLegends?: boolean; tickFormatY: (y: number) => React.ReactNode; formatTooltipValue: (c: Coordinate) => React.ReactNode; @@ -28,6 +19,7 @@ interface Props { height?: number; stacked?: boolean; onHover?: () => void; + onToggleLegend?: (visibleSeries: TimeSeries[]) => void; } function TransactionLineChart(props: Props) { @@ -40,6 +32,7 @@ function TransactionLineChart(props: Props) { truncateLegends, stacked = false, onHover, + onToggleLegend, } = props; const syncedChartsProps = useChartsSync(); @@ -66,6 +59,7 @@ function TransactionLineChart(props: Props) { height={height} truncateLegends={truncateLegends} {...(stacked ? { stackBy: 'y' } : {})} + onToggleLegend={onToggleLegend} /> ); } diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx index 583a16281edc4..e55f86bd3abf3 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx @@ -29,7 +29,11 @@ import { Coordinate, TimeSeries } from '../../../../../typings/timeseries'; import { LicenseContext } from '../../../../context/LicenseContext'; import { IUrlParams } from '../../../../context/UrlParamsContext/types'; import { ITransactionChartData } from '../../../../selectors/chartSelectors'; -import { asDecimal, asDuration, tpmUnit } from '../../../../utils/formatters'; +import { + asDecimal, + getDurationFormatter, + tpmUnit, +} from '../../../../utils/formatters'; import { isValidCoordinateValue } from '../../../../utils/isValidCoordinateValue'; import { MLJobLink } from '../../Links/MachineLearningLinks/MLJobLink'; import { BrowserLineChart } from './BrowserLineChart'; @@ -54,11 +58,6 @@ const ShiftedEuiText = styled(EuiText)` top: 5px; `; -const getResponseTimeTickFormatter = (t: number) => asDuration(t); - -const getResponseTimeTooltipFormatter = (p: Coordinate) => - isValidCoordinateValue(p.y) ? asDuration(p.y) : NOT_AVAILABLE_LABEL; - export function getMaxY(responseTimeSeries: TimeSeries[]) { const coordinates = flatten( responseTimeSeries.map((serie: TimeSeries) => serie.data as Coordinate[]) @@ -140,9 +139,28 @@ export function TransactionCharts({ ); } - const { responseTimeSeries, tpmSeries } = charts; const { transactionType } = urlParams; + const maxY = getMaxY(responseTimeSeries); + let formatter = getDurationFormatter(maxY); + + function onToggleLegend(visibleSeries: TimeSeries[]) { + if (!isEmpty(visibleSeries)) { + // recalculate the formatter based on the max Y from the visible series + const maxVisibleY = getMaxY(visibleSeries); + formatter = getDurationFormatter(maxVisibleY); + } + } + + function getResponseTimeTickFormatter(t: number) { + return formatter(t).formatted; + } + + function getResponseTimeTooltipFormatter(coordinate: Coordinate) { + return isValidCoordinateValue(coordinate.y) + ? formatter(coordinate.y).formatted + : NOT_AVAILABLE_LABEL; + } return ( <> @@ -166,6 +184,7 @@ export function TransactionCharts({ series={responseTimeSeries} tickFormatY={getResponseTimeTickFormatter} formatTooltipValue={getResponseTimeTooltipFormatter} + onToggleLegend={onToggleLegend} /> diff --git a/x-pack/plugins/apm/public/utils/formatters/__test__/duration.test.ts b/x-pack/plugins/apm/public/utils/formatters/__test__/duration.test.ts index c4d59beb4b7a2..ca8a4919dd216 100644 --- a/x-pack/plugins/apm/public/utils/formatters/__test__/duration.test.ts +++ b/x-pack/plugins/apm/public/utils/formatters/__test__/duration.test.ts @@ -20,9 +20,12 @@ describe('duration formatters', () => { '10,000 ms' ); expect(asDuration(toMicroseconds(20, 'seconds'))).toEqual('20 s'); - expect(asDuration(toMicroseconds(10, 'minutes'))).toEqual('10 min'); + expect(asDuration(toMicroseconds(10, 'minutes'))).toEqual('600 s'); + expect(asDuration(toMicroseconds(11, 'minutes'))).toEqual('11 min'); expect(asDuration(toMicroseconds(1, 'hours'))).toEqual('60 min'); - expect(asDuration(toMicroseconds(1.5, 'hours'))).toEqual('1.5 h'); + expect(asDuration(toMicroseconds(1.5, 'hours'))).toEqual('90 min'); + expect(asDuration(toMicroseconds(10, 'hours'))).toEqual('600 min'); + expect(asDuration(toMicroseconds(11, 'hours'))).toEqual('11 h'); }); it('falls back to default value', () => { diff --git a/x-pack/plugins/apm/public/utils/formatters/duration.ts b/x-pack/plugins/apm/public/utils/formatters/duration.ts index 64a9e3b952b98..8381b0afb5f07 100644 --- a/x-pack/plugins/apm/public/utils/formatters/duration.ts +++ b/x-pack/plugins/apm/public/utils/formatters/duration.ts @@ -127,10 +127,10 @@ export const toMicroseconds = (value: number, timeUnit: TimeUnit) => moment.duration(value, timeUnit).asMilliseconds() * 1000; function getDurationUnitKey(max: number): DurationTimeUnit { - if (max > toMicroseconds(1, 'hours')) { + if (max > toMicroseconds(10, 'hours')) { return 'hours'; } - if (max > toMicroseconds(1, 'minutes')) { + if (max > toMicroseconds(10, 'minutes')) { return 'minutes'; } if (max > toMicroseconds(10, 'seconds')) { From ee533a29751f97181ebb1cee5d3f09ac9b45eeb1 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Thu, 27 Aug 2020 14:25:56 +0200 Subject: [PATCH 6/7] change formatter based on available legends --- .../shared/charts/CustomPlot/index.js | 6 +- .../TransactionCharts/BrowserLineChart.tsx | 32 ++--- .../TransactionLineChart/index.tsx | 2 +- .../charts/TransactionCharts/helper.test.ts | 69 +++++++++ .../charts/TransactionCharts/helper.tsx | 35 +++++ .../shared/charts/TransactionCharts/index.tsx | 132 +++--------------- .../charts/TransactionCharts/ml_header.tsx | 90 ++++++++++++ .../TransactionCharts/use_formatter.test.tsx | 109 +++++++++++++++ .../charts/TransactionCharts/use_formatter.ts | 30 ++++ 9 files changed, 366 insertions(+), 139 deletions(-) create mode 100644 x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/helper.test.ts create mode 100644 x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/helper.tsx create mode 100644 x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/ml_header.tsx create mode 100644 x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/use_formatter.test.tsx create mode 100644 x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/use_formatter.ts diff --git a/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js b/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js index b072c22c09c19..41925d651e361 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js +++ b/x-pack/plugins/apm/public/components/shared/charts/CustomPlot/index.js @@ -80,11 +80,7 @@ export class InnerCustomPlot extends PureComponent { }); if (typeof this.props.onToggleLegend === 'function') { - //Filters out disabled series - const availableSeries = this.props.series.filter( - (serie, index) => !nextSeriesEnabledState[index] - ); - this.props.onToggleLegend(availableSeries); + this.props.onToggleLegend(nextSeriesEnabledState); } return { diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/BrowserLineChart.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/BrowserLineChart.tsx index a3fa2da40619b..40caf35155918 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/BrowserLineChart.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/BrowserLineChart.tsx @@ -4,31 +4,17 @@ * you may not use this file except in compliance with the Elastic License. */ -import React from 'react'; -import { i18n } from '@kbn/i18n'; import { EuiTitle } from '@elastic/eui'; -import { NOT_AVAILABLE_LABEL } from '../../../../../common/i18n'; -import { isValidCoordinateValue } from '../../../../utils/isValidCoordinateValue'; -import { TransactionLineChart } from './TransactionLineChart'; -import { getMaxY } from '.'; -import { - getDurationFormatter, - TimeFormatter, -} from '../../../../utils/formatters'; +import { i18n } from '@kbn/i18n'; +import React from 'react'; import { useAvgDurationByBrowser } from '../../../../hooks/useAvgDurationByBrowser'; -import { Coordinate } from '../../../../../typings/timeseries'; - -function getResponseTimeTickFormatter(formatter: TimeFormatter) { - return (t: number) => formatter(t).formatted; -} - -function getResponseTimeTooltipFormatter(formatter: TimeFormatter) { - return (p: Coordinate) => { - return isValidCoordinateValue(p.y) - ? formatter(p.y).formatted - : NOT_AVAILABLE_LABEL; - }; -} +import { getDurationFormatter } from '../../../../utils/formatters'; +import { + getResponseTimeTickFormatter, + getResponseTimeTooltipFormatter, + getMaxY, +} from './helper'; +import { TransactionLineChart } from './TransactionLineChart'; export function BrowserLineChart() { const { data } = useAvgDurationByBrowser(); diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx index 3657a02b1589c..07b7f01194d5c 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/TransactionLineChart/index.tsx @@ -19,7 +19,7 @@ interface Props { height?: number; stacked?: boolean; onHover?: () => void; - onToggleLegend?: (visibleSeries: TimeSeries[]) => void; + onToggleLegend?: (disabledSeriesState: boolean[]) => void; } function TransactionLineChart(props: Props) { diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/helper.test.ts b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/helper.test.ts new file mode 100644 index 0000000000000..a476892fa4a3f --- /dev/null +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/helper.test.ts @@ -0,0 +1,69 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { + getResponseTimeTickFormatter, + getResponseTimeTooltipFormatter, + getMaxY, +} from './helper'; +import { + getDurationFormatter, + toMicroseconds, +} from '../../../../utils/formatters'; +import { TimeSeries } from '../../../../../typings/timeseries'; + +describe('transaction chart helper', () => { + describe('getResponseTimeTickFormatter', () => { + it('formattes time tick in minutes', () => { + const formatter = getDurationFormatter(toMicroseconds(11, 'minutes')); + const timeTickFormatter = getResponseTimeTickFormatter(formatter); + expect(timeTickFormatter(toMicroseconds(60, 'seconds'))).toEqual( + '1.0 min' + ); + }); + it('formattes time tick in seconds', () => { + const formatter = getDurationFormatter(toMicroseconds(11, 'seconds')); + const timeTickFormatter = getResponseTimeTickFormatter(formatter); + expect(timeTickFormatter(toMicroseconds(6, 'seconds'))).toEqual('6.0 s'); + }); + }); + describe('getResponseTimeTooltipFormatter', () => { + const formatter = getDurationFormatter(toMicroseconds(11, 'minutes')); + const tooltipFormatter = getResponseTimeTooltipFormatter(formatter); + it("doesn't format invalid y coordinate", () => { + expect(tooltipFormatter({ x: 1, y: undefined })).toEqual('N/A'); + expect(tooltipFormatter({ x: 1, y: null })).toEqual('N/A'); + }); + it('formattes tooltip in minutes', () => { + expect( + tooltipFormatter({ x: 1, y: toMicroseconds(60, 'seconds') }) + ).toEqual('1.0 min'); + }); + }); + describe('getMaxY', () => { + it('returns zero when empty time series', () => { + expect(getMaxY([])).toEqual(0); + }); + it('returns zero for invalid y coordinate', () => { + const timeSeries = ([ + { data: [{ x: 1 }, { x: 2 }, { x: 3, y: -1 }] }, + ] as unknown) as TimeSeries[]; + expect(getMaxY(timeSeries)).toEqual(0); + }); + it('returns the max y coordinate', () => { + const timeSeries = ([ + { + data: [ + { x: 1, y: 10 }, + { x: 2, y: 5 }, + { x: 3, y: 1 }, + ], + }, + ] as unknown) as TimeSeries[]; + expect(getMaxY(timeSeries)).toEqual(10); + }); + }); +}); diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/helper.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/helper.tsx new file mode 100644 index 0000000000000..f11a33f932553 --- /dev/null +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/helper.tsx @@ -0,0 +1,35 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { flatten } from 'lodash'; +import { NOT_AVAILABLE_LABEL } from '../../../../../common/i18n'; +import { isValidCoordinateValue } from '../../../../utils/isValidCoordinateValue'; +import { TimeSeries, Coordinate } from '../../../../../typings/timeseries'; +import { TimeFormatter } from '../../../../utils/formatters'; + +export function getResponseTimeTickFormatter(formatter: TimeFormatter) { + return (t: number) => { + return formatter(t).formatted; + }; +} + +export function getResponseTimeTooltipFormatter(formatter: TimeFormatter) { + return (coordinate: Coordinate) => { + return isValidCoordinateValue(coordinate.y) + ? formatter(coordinate.y).formatted + : NOT_AVAILABLE_LABEL; + }; +} + +export function getMaxY(timeSeries: TimeSeries[]) { + const coordinates = flatten( + timeSeries.map((serie: TimeSeries) => serie.data as Coordinate[]) + ); + + const numbers: number[] = coordinates.map((c: Coordinate) => (c.y ? c.y : 0)); + + return Math.max(...numbers, 0); +} diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx index e55f86bd3abf3..2902b907ba691 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx @@ -8,37 +8,34 @@ import { EuiFlexGrid, EuiFlexGroup, EuiFlexItem, - EuiIconTip, EuiPanel, EuiSpacer, - EuiText, EuiTitle, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { Location } from 'history'; -import { flatten, isEmpty } from 'lodash'; import React from 'react'; -import styled from 'styled-components'; import { NOT_AVAILABLE_LABEL } from '../../../../../common/i18n'; import { TRANSACTION_PAGE_LOAD, TRANSACTION_REQUEST, TRANSACTION_ROUTE_CHANGE, } from '../../../../../common/transaction_types'; -import { Coordinate, TimeSeries } from '../../../../../typings/timeseries'; +import { Coordinate } from '../../../../../typings/timeseries'; import { LicenseContext } from '../../../../context/LicenseContext'; import { IUrlParams } from '../../../../context/UrlParamsContext/types'; import { ITransactionChartData } from '../../../../selectors/chartSelectors'; -import { - asDecimal, - getDurationFormatter, - tpmUnit, -} from '../../../../utils/formatters'; +import { asDecimal, tpmUnit } from '../../../../utils/formatters'; import { isValidCoordinateValue } from '../../../../utils/isValidCoordinateValue'; -import { MLJobLink } from '../../Links/MachineLearningLinks/MLJobLink'; import { BrowserLineChart } from './BrowserLineChart'; import { DurationByCountryMap } from './DurationByCountryMap'; +import { + getResponseTimeTickFormatter, + getResponseTimeTooltipFormatter, +} from './helper'; +import { MLHeader } from './ml_header'; import { TransactionLineChart } from './TransactionLineChart'; +import { useFormatter } from './use_formatter'; interface TransactionChartProps { charts: ITransactionChartData; @@ -46,28 +43,6 @@ interface TransactionChartProps { urlParams: IUrlParams; } -const ShiftedIconWrapper = styled.span` - padding-right: 5px; - position: relative; - top: -1px; - display: inline-block; -`; - -const ShiftedEuiText = styled(EuiText)` - position: relative; - top: 5px; -`; - -export function getMaxY(responseTimeSeries: TimeSeries[]) { - const coordinates = flatten( - responseTimeSeries.map((serie: TimeSeries) => serie.data as Coordinate[]) - ); - - const numbers: number[] = coordinates.map((c: Coordinate) => (c.y ? c.y : 0)); - - return Math.max(...numbers, 0); -} - export function TransactionCharts({ charts, location, @@ -84,82 +59,16 @@ export function TransactionCharts({ : NOT_AVAILABLE_LABEL; }; - function renderMLHeader(hasValidMlLicense: boolean | undefined) { - const { mlJobId } = charts; - - if (!hasValidMlLicense || !mlJobId) { - return null; - } - - const { serviceName, kuery, transactionType } = urlParams; - if (!serviceName) { - return null; - } - - const hasKuery = !isEmpty(kuery); - const icon = hasKuery ? ( - - ) : ( - - ); - - return ( - - - {icon} - - {i18n.translate( - 'xpack.apm.metrics.transactionChart.machineLearningLabel', - { - defaultMessage: 'Machine learning:', - } - )}{' '} - - - View Job - - - - ); - } - const { responseTimeSeries, tpmSeries } = charts; const { transactionType } = urlParams; - const maxY = getMaxY(responseTimeSeries); - let formatter = getDurationFormatter(maxY); - function onToggleLegend(visibleSeries: TimeSeries[]) { - if (!isEmpty(visibleSeries)) { - // recalculate the formatter based on the max Y from the visible series - const maxVisibleY = getMaxY(visibleSeries); - formatter = getDurationFormatter(maxVisibleY); - } - } + const { responseTimeSeries, tpmSeries } = charts; - function getResponseTimeTickFormatter(t: number) { - return formatter(t).formatted; - } + const { formatter, setDisabledSeriesState } = useFormatter( + responseTimeSeries + ); - function getResponseTimeTooltipFormatter(coordinate: Coordinate) { - return isValidCoordinateValue(coordinate.y) - ? formatter(coordinate.y).formatted - : NOT_AVAILABLE_LABEL; + function onToggleLegend(disabledSeriesStates: boolean[]) { + setDisabledSeriesState(disabledSeriesStates); } return ( @@ -175,15 +84,18 @@ export function TransactionCharts({ - {(license) => - renderMLHeader(license?.getFeature('ml').isAvailable) - } + {(license) => ( + + )} diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/ml_header.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/ml_header.tsx new file mode 100644 index 0000000000000..e65c4ded4003b --- /dev/null +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/ml_header.tsx @@ -0,0 +1,90 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { EuiIconTip } from '@elastic/eui'; +import { isEmpty } from 'lodash'; +import React from 'react'; +import { EuiFlexItem } from '@elastic/eui'; +import styled from 'styled-components'; +import { i18n } from '@kbn/i18n'; +import { EuiText } from '@elastic/eui'; +import { useUrlParams } from '../../../../hooks/useUrlParams'; +import { MLJobLink } from '../../Links/MachineLearningLinks/MLJobLink'; + +interface Props { + hasValidMlLicense?: boolean; + mlJobId?: string; +} + +const ShiftedIconWrapper = styled.span` + padding-right: 5px; + position: relative; + top: -1px; + display: inline-block; +`; + +const ShiftedEuiText = styled(EuiText)` + position: relative; + top: 5px; +`; + +export function MLHeader({ hasValidMlLicense, mlJobId }: Props) { + const { urlParams } = useUrlParams(); + + if (!hasValidMlLicense || !mlJobId) { + return null; + } + + const { serviceName, kuery, transactionType } = urlParams; + if (!serviceName) { + return null; + } + + const hasKuery = !isEmpty(kuery); + const icon = hasKuery ? ( + + ) : ( + + ); + + return ( + + + {icon} + + {i18n.translate( + 'xpack.apm.metrics.transactionChart.machineLearningLabel', + { + defaultMessage: 'Machine learning:', + } + )}{' '} + + + {i18n.translate('xpack.apm.metrics.transactionChart.viewJob', { + defaultMessage: 'View Job:', + })} + + + + ); +} diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/use_formatter.test.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/use_formatter.test.tsx new file mode 100644 index 0000000000000..78ff9a398b2e7 --- /dev/null +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/use_formatter.test.tsx @@ -0,0 +1,109 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import React from 'react'; +import { TimeSeries } from '../../../../../typings/timeseries'; +import { toMicroseconds } from '../../../../utils/formatters'; +import { useFormatter } from './use_formatter'; +import { render, fireEvent, act } from '@testing-library/react'; + +function MockComponent({ + timeSeries, + disabledSeries, + value, +}: { + timeSeries: TimeSeries[]; + disabledSeries: boolean[]; + value: number; +}) { + const { formatter, setDisabledSeriesState } = useFormatter(timeSeries); + + const onDisableSeries = () => { + setDisabledSeriesState(disabledSeries); + }; + + return ( +
+ + {formatter(value).formatted} +
+ ); +} + +describe('useFormatter', () => { + const timeSeries = ([ + { + data: [ + { x: 1, y: toMicroseconds(11, 'minutes') }, + { x: 2, y: toMicroseconds(1, 'minutes') }, + { x: 3, y: toMicroseconds(60, 'seconds') }, + ], + }, + { + data: [ + { x: 1, y: toMicroseconds(120, 'seconds') }, + { x: 2, y: toMicroseconds(1, 'minutes') }, + { x: 3, y: toMicroseconds(60, 'seconds') }, + ], + }, + { + data: [ + { x: 1, y: toMicroseconds(60, 'seconds') }, + { x: 2, y: toMicroseconds(5, 'minutes') }, + { x: 3, y: toMicroseconds(100, 'seconds') }, + ], + }, + ] as unknown) as TimeSeries[]; + it('returns new formatter when disabled series state changes', () => { + const { getByText } = render( + + ); + expect(getByText('2.0 min')).toBeInTheDocument(); + act(() => { + fireEvent.click(getByText('disable series')); + }); + expect(getByText('120 s')).toBeInTheDocument(); + }); + it('falls back to the first formatter when disabled series is empty', () => { + const { getByText } = render( + + ); + expect(getByText('2.0 min')).toBeInTheDocument(); + act(() => { + fireEvent.click(getByText('disable series')); + }); + expect(getByText('2.0 min')).toBeInTheDocument(); + // const { formatter, setDisabledSeriesState } = useFormatter(timeSeries); + // expect(formatter(toMicroseconds(120, 'seconds'))).toEqual('2.0 min'); + // setDisabledSeriesState([true, true, false]); + // expect(formatter(toMicroseconds(120, 'seconds'))).toEqual('2.0 min'); + }); + it('falls back to the first formatter when disabled series is all true', () => { + const { getByText } = render( + + ); + expect(getByText('2.0 min')).toBeInTheDocument(); + act(() => { + fireEvent.click(getByText('disable series')); + }); + expect(getByText('2.0 min')).toBeInTheDocument(); + // const { formatter, setDisabledSeriesState } = useFormatter(timeSeries); + // expect(formatter(toMicroseconds(120, 'seconds'))).toEqual('2.0 min'); + // setDisabledSeriesState([true, true, false]); + // expect(formatter(toMicroseconds(120, 'seconds'))).toEqual('2.0 min'); + }); +}); diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/use_formatter.ts b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/use_formatter.ts new file mode 100644 index 0000000000000..8cd8929c89960 --- /dev/null +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/use_formatter.ts @@ -0,0 +1,30 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { useState, Dispatch, SetStateAction } from 'react'; +import { isEmpty } from 'lodash'; +import { + getDurationFormatter, + TimeFormatter, +} from '../../../../utils/formatters'; +import { TimeSeries } from '../../../../../typings/timeseries'; +import { getMaxY } from './helper'; + +export const useFormatter = ( + series: TimeSeries[] +): { + formatter: TimeFormatter; + setDisabledSeriesState: Dispatch>; +} => { + const [disabledSeriesState, setDisabledSeriesState] = useState([]); + const visibleSeries = series.filter( + (serie, index) => disabledSeriesState[index] !== true + ); + const maxY = getMaxY(isEmpty(visibleSeries) ? series : visibleSeries); + const formatter = getDurationFormatter(maxY); + + return { formatter, setDisabledSeriesState }; +}; From 839480034723c30c18ac6caf80eb48908b82e5fc Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 31 Aug 2020 09:39:52 +0200 Subject: [PATCH 7/7] addressing PR comment --- .../components/shared/charts/TransactionCharts/index.tsx | 6 +----- .../shared/charts/TransactionCharts/ml_header.tsx | 8 +++++++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx index 2902b907ba691..d11925dc0303d 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx @@ -67,10 +67,6 @@ export function TransactionCharts({ responseTimeSeries ); - function onToggleLegend(disabledSeriesStates: boolean[]) { - setDisabledSeriesState(disabledSeriesStates); - } - return ( <> @@ -96,7 +92,7 @@ export function TransactionCharts({ series={responseTimeSeries} tickFormatY={getResponseTimeTickFormatter(formatter)} formatTooltipValue={getResponseTimeTooltipFormatter(formatter)} - onToggleLegend={onToggleLegend} + onToggleLegend={setDisabledSeriesState} /> diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/ml_header.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/ml_header.tsx index e65c4ded4003b..f829b5841efa9 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/ml_header.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/ml_header.tsx @@ -49,7 +49,13 @@ export function MLHeader({ hasValidMlLicense, mlJobId }: Props) { aria-label="Warning" type="alert" color="warning" - content="The Machine learning results are hidden when the search bar is used for filtering" + content={i18n.translate( + 'xpack.apm.metrics.transactionChart.machineLearningTooltip.withKuery', + { + defaultMessage: + 'The Machine learning results are hidden when the search bar is used for filtering', + } + )} /> ) : (