addressing pr comments

elastic · Nov 6, 2020 · df7c686 · df7c686
1 parent aaaa1ec
commit df7c686
Show file tree

Hide file tree

Showing 12 changed files with 168 additions and 90 deletions.
diff --git a/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/Distribution/index.tsx b/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/Distribution/index.tsx
@@ -89,13 +89,7 @@ export function ErrorDistribution({ distribution, title }: Props) {
             showOverlappingTicks
             tickFormat={xFormatter}
           />
-          <Axis
-            id="y-axis"
-            position={Position.Left}
-            ticks={2}
-            showGridLines
-            tickFormat={(value) => `${value} occ.`}
-          />
+          <Axis id="y-axis" position={Position.Left} ticks={2} showGridLines />
           <HistogramBarSeries
             minBarHeight={2}
             id="errorOccurrences"
@@ -104,7 +98,7 @@ export function ErrorDistribution({ distribution, title }: Props) {
             yScaleType={ScaleType.Linear}
             xAccessor="x0"
             yAccessors={['y']}
-            data={distribution.noHits ? [] : buckets}
+            data={buckets}
             color={theme.eui.euiColorVis1}
           />
         </Chart>

diff --git a/x-pack/plugins/apm/public/components/app/TransactionDetails/Distribution/index.tsx b/x-pack/plugins/apm/public/components/app/TransactionDetails/Distribution/index.tsx
@@ -27,10 +27,11 @@ import { ValuesType } from 'utility-types';
 import { useTheme } from '../../../../../../observability/public';
 import { getDurationFormatter } from '../../../../../common/utils/formatters';
 // eslint-disable-next-line @kbn/eslint/no-restricted-paths
-import { TransactionDistributionAPIResponse } from '../../../../../server/lib/transactions/distribution';
+import type { TransactionDistributionAPIResponse } from '../../../../../server/lib/transactions/distribution';
 // eslint-disable-next-line @kbn/eslint/no-restricted-paths
-import { DistributionBucket } from '../../../../../server/lib/transactions/distribution/get_buckets';
+import type { DistributionBucket } from '../../../../../server/lib/transactions/distribution/get_buckets';
 import { IUrlParams } from '../../../../context/UrlParamsContext/types';
+import { FETCH_STATUS } from '../../../../hooks/useFetcher';
 import { unit } from '../../../../style/variables';
 import { ChartContainer } from '../../../shared/charts/chart_container';
 import { EmptyMessage } from '../../../shared/EmptyMessage';
@@ -109,21 +110,20 @@ const getFormatYLong = (transactionType: string | undefined) => (t: number) => {
 interface Props {
   distribution?: TransactionDistributionAPIResponse;
   urlParams: IUrlParams;
-  isLoading: boolean;
+  fetchStatus: FETCH_STATUS;
   bucketIndex: number;
   onBucketClick: (
     bucket: ValuesType<TransactionDistributionAPIResponse['buckets']>
   ) => void;
 }
 
-export function TransactionDistribution(props: Props) {
-  const {
-    distribution,
-    urlParams: { transactionType },
-    isLoading,
-    bucketIndex,
-    onBucketClick,
-  } = props;
+export function TransactionDistribution({
+  distribution,
+  urlParams: { transactionType },
+  fetchStatus,
+  bucketIndex,
+  onBucketClick,
+}: Props) {
   const theme = useTheme();
 
   /* eslint-disable-next-line react-hooks/exhaustive-deps */
@@ -135,8 +135,12 @@ export function TransactionDistribution(props: Props) {
   const formatYLong = useCallback(getFormatYLong(transactionType), [
     transactionType,
   ]);
+
   // no data in response
-  if ((!distribution || distribution.noHits) && !isLoading) {
+  if (
+    (!distribution || distribution.noHits) &&
+    fetchStatus !== FETCH_STATUS.LOADING
+  ) {
     return (
       <EmptyMessage
         heading={i18n.translate('xpack.apm.transactionDetails.notFoundLabel', {
@@ -209,7 +213,8 @@ export function TransactionDistribution(props: Props) {
       </EuiTitle>
       <ChartContainer
         height={unit * 10}
-        isLoading={isLoading && (!distribution || distribution.noHits)}
+        hasData={!!(distribution && !distribution.noHits)}
+        status={fetchStatus}
       >
         <Chart>
           <Settings
@@ -247,9 +252,7 @@ export function TransactionDistribution(props: Props) {
             tickFormat={(value: number) => formatYShort(value)}
           />
           <HistogramBarSeries
-            tickFormat={(value: number) => {
-              return `${value}`;
-            }}
+            tickFormat={(value: string) => value}
             minBarHeight={2}
             id="transactionDurationDistribution"
             name={(series: XYChartSeriesIdentifier) => {

diff --git a/x-pack/plugins/apm/public/components/app/TransactionDetails/index.tsx b/x-pack/plugins/apm/public/components/app/TransactionDetails/index.tsx
@@ -125,10 +125,7 @@ export function TransactionDetails({
         <EuiFlexItem grow={7}>
           <ChartsSyncContextProvider>
             <TransactionCharts
-              isLoading={
-                transactionChartsStatus === FETCH_STATUS.LOADING ||
-                transactionChartsStatus === FETCH_STATUS.PENDING
-              }
+              fetchStatus={transactionChartsStatus}
               charts={transactionChartsData}
               urlParams={urlParams}
             />
@@ -139,7 +136,7 @@ export function TransactionDetails({
           <EuiPanel>
             <TransactionDistribution
               distribution={distributionData}
-              isLoading={distributionStatus === FETCH_STATUS.LOADING}
+              fetchStatus={distributionStatus}
               urlParams={urlParams}
               bucketIndex={bucketIndex}
               onBucketClick={(bucket) => {

diff --git a/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx b/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx
@@ -22,6 +22,7 @@ import React, { useMemo } from 'react';
 import { useLocation } from 'react-router-dom';
 import { useTrackPageview } from '../../../../../observability/public';
 import { Projection } from '../../../../common/projections';
+import { TRANSACTION_PAGE_LOAD } from '../../../../common/transaction_types';
 import { IUrlParams } from '../../../context/UrlParamsContext/types';
 import { useServiceTransactionTypes } from '../../../hooks/useServiceTransactionTypes';
 import { useTransactionCharts } from '../../../hooks/useTransactionCharts';
@@ -32,12 +33,10 @@ import { ElasticDocsLink } from '../../shared/Links/ElasticDocsLink';
 import { fromQuery, toQuery } from '../../shared/Links/url_helpers';
 import { LocalUIFilters } from '../../shared/LocalUIFilters';
 import { TransactionTypeFilter } from '../../shared/LocalUIFilters/TransactionTypeFilter';
+import { Correlations } from '../Correlations';
 import { TransactionList } from './TransactionList';
 import { useRedirect } from './useRedirect';
-import { TRANSACTION_PAGE_LOAD } from '../../../../common/transaction_types';
 import { UserExperienceCallout } from './user_experience_callout';
-import { Correlations } from '../Correlations';
-import { FETCH_STATUS } from '../../../hooks/useFetcher';
 
 function getRedirectLocation({
   urlParams,
@@ -139,10 +138,7 @@ export function TransactionOverview({ serviceName }: TransactionOverviewProps) {
             </>
           )}
           <TransactionCharts
-            isLoading={
-              transactionChartsStatus === FETCH_STATUS.LOADING ||
-              transactionChartsStatus === FETCH_STATUS.PENDING
-            }
+            fetchStatus={transactionChartsStatus}
             charts={transactionCharts}
             urlParams={urlParams}
           />

diff --git a/...ins/apm/public/components/shared/TransactionBreakdown/TransactionBreakdownGraph/index.tsx b/...ins/apm/public/components/shared/TransactionBreakdown/TransactionBreakdownGraph/index.tsx
@@ -14,12 +14,12 @@ import {
   ScaleType,
   Settings,
 } from '@elastic/charts';
-import { isEmpty } from 'lodash';
 import moment from 'moment';
 import React, { useEffect } from 'react';
 import { useHistory } from 'react-router-dom';
 import { asPercent } from '../../../../../common/utils/formatters';
 import { TimeSeries } from '../../../../../typings/timeseries';
+import { FETCH_STATUS } from '../../../../hooks/useFetcher';
 import { useUrlParams } from '../../../../hooks/useUrlParams';
 import { useChartsSync as useChartsSync2 } from '../../../../hooks/use_charts_sync';
 import { unit } from '../../../../style/variables';
@@ -30,14 +30,11 @@ import { onBrushEnd } from '../../charts/helper/helper';
 const XY_HEIGHT = unit * 16;
 
 interface Props {
-  isLoading: boolean;
+  fetchStatus: FETCH_STATUS;
   timeseries?: TimeSeries[];
 }
 
-export function TransactionBreakdownGraph({
-  isLoading,
-  timeseries = [],
-}: Props) {
+export function TransactionBreakdownGraph({ fetchStatus, timeseries }: Props) {
   const history = useHistory();
   const chartRef = React.createRef<Chart>();
   const { event, setEvent } = useChartsSync2();
@@ -56,7 +53,11 @@ export function TransactionBreakdownGraph({
   const xFormatter = niceTimeFormatter([min, max]);
 
   return (
-    <ChartContainer height={XY_HEIGHT} isLoading={isLoading}>
+    <ChartContainer
+      height={XY_HEIGHT}
+      hasData={!!timeseries}
+      status={fetchStatus}
+    >
       <Chart ref={chartRef} id="timeSpentBySpan">
         <Settings
           onBrushEnd={({ x }) => onBrushEnd({ x, history })}
@@ -87,10 +88,7 @@ export function TransactionBreakdownGraph({
 
         <Annotations />
 
-        {isEmpty(timeseries) ? (
-          // When timeseries is empty, loads an AreaSeries chart to show the default empty message.
-          <AreaSeries id="empty_chart" data={[]} />
-        ) : (
+        {timeseries?.length ? (
           timeseries.map((serie) => {
             return (
               <AreaSeries
@@ -108,6 +106,9 @@ export function TransactionBreakdownGraph({
               />
             );
           })
+        ) : (
+          // When timeseries is empty, loads an AreaSeries chart to show the default empty message.
+          <AreaSeries id="empty_chart" data={[]} />
         )}
       </Chart>
     </ChartContainer>

diff --git a/x-pack/plugins/apm/public/components/shared/TransactionBreakdown/index.tsx b/x-pack/plugins/apm/public/components/shared/TransactionBreakdown/index.tsx
@@ -6,7 +6,6 @@
 import { EuiFlexGroup, EuiFlexItem, EuiPanel, EuiTitle } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 import React from 'react';
-import { FETCH_STATUS } from '../../../hooks/useFetcher';
 import { useTransactionBreakdown } from '../../../hooks/useTransactionBreakdown';
 import { TransactionBreakdownGraph } from './TransactionBreakdownGraph';
 
@@ -29,11 +28,7 @@ function TransactionBreakdown() {
         <EuiFlexItem grow={false}>
           <TransactionBreakdownGraph
             timeseries={timeseries}
-            isLoading={
-              (status === FETCH_STATUS.LOADING ||
-                status === FETCH_STATUS.PENDING) &&
-              !data.timeseries
-            }
+            fetchStatus={status}
           />
         </EuiFlexItem>
       </EuiFlexGroup>

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
@@ -25,25 +25,26 @@ import { Coordinate } from '../../../../../typings/timeseries';
 import { ChartsSyncContextProvider } from '../../../../context/charts_sync_context';
 import { LicenseContext } from '../../../../context/LicenseContext';
 import { IUrlParams } from '../../../../context/UrlParamsContext/types';
+import { FETCH_STATUS } from '../../../../hooks/useFetcher';
 import { ITransactionChartData } from '../../../../selectors/chartSelectors';
 import { isValidCoordinateValue } from '../../../../utils/isValidCoordinateValue';
-import { TransactionErrorRateChart } from '../transaction_error_rate_chart/';
 import { TransactionBreakdown } from '../../TransactionBreakdown';
 import { LineChart } from '../line_chart';
+import { TransactionErrorRateChart } from '../transaction_error_rate_chart/';
 import { getResponseTimeTickFormatter } from './helper';
 import { MLHeader } from './ml_header';
 import { useFormatter } from './use_formatter';
 
 interface TransactionChartProps {
   charts: ITransactionChartData;
   urlParams: IUrlParams;
-  isLoading: boolean;
+  fetchStatus: FETCH_STATUS;
 }
 
 export function TransactionCharts({
   charts,
   urlParams,
-  isLoading,
+  fetchStatus,
 }: TransactionChartProps) {
   const getTPMFormatter = (t: number) => {
     return `${asDecimal(t)} ${tpmUnit(urlParams.transactionType)}`;
@@ -81,7 +82,7 @@ export function TransactionCharts({
                 </LicenseContext.Consumer>
               </EuiFlexGroup>
               <LineChart
-                isLoading={isLoading && !responseTimeSeries}
+                fetchStatus={fetchStatus}
                 id="transactionDuration"
                 timeseries={responseTimeSeries || []}
                 yLabelFormat={getResponseTimeTickFormatter(formatter)}
@@ -100,7 +101,7 @@ export function TransactionCharts({
                 <span>{tpmLabel(transactionType)}</span>
               </EuiTitle>
               <LineChart
-                isLoading={isLoading && !tpmSeries}
+                fetchStatus={fetchStatus}
                 id="requestPerMinutes"
                 timeseries={tpmSeries || []}
                 yLabelFormat={getTPMTooltipFormatter}