From c1b42f6ca8c0fd43aac919142872c179b60b4e2f Mon Sep 17 00:00:00 2001
From: Nathan L Smith <nathan.smith@elastic.co>
Date: Mon, 16 Nov 2020 21:27:19 -0600
Subject: [PATCH] Overview heights and loading states (#83360) (#83508)

* Set the chart height to fill the whole container
* Remove the initial loading spinner for the tables and always show the progress bar
* Make the last seen column on the errors table a bit wider so it doesn't wrap
* Make a `ServiceOverviewTable` component that pins the pagination to the bottom of the panel
* Show the loading spinner on charts when doing updates
---
 .../components/app/service_overview/index.tsx | 41 +++++++-------
 .../fetch_wrapper.tsx                         | 11 +---
 .../service_overview_errors_table/index.tsx   | 32 ++++++-----
 .../service_overview_table.tsx                | 56 +++++++++++++++++++
 .../shared/charts/line_chart/index.tsx        |  6 +-
 .../transaction_error_rate_chart/index.tsx    |  7 ++-
 6 files changed, 103 insertions(+), 50 deletions(-)
 create mode 100644 x-pack/plugins/apm/public/components/app/service_overview/service_overview_table.tsx

diff --git a/x-pack/plugins/apm/public/components/app/service_overview/index.tsx b/x-pack/plugins/apm/public/components/app/service_overview/index.tsx
index 44bd7d6c73d8ef..50667d3135f1ad 100644
--- a/x-pack/plugins/apm/public/components/app/service_overview/index.tsx
+++ b/x-pack/plugins/apm/public/components/app/service_overview/index.tsx
@@ -13,7 +13,6 @@ import {
 } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 import React from 'react';
-import styled from 'styled-components';
 import { useTrackPageview } from '../../../../../observability/public';
 import { isRumAgentName } from '../../../../common/agent_name';
 import { ChartsSyncContextProvider } from '../../../context/charts_sync_context';
@@ -24,16 +23,11 @@ import { SearchBar } from '../../shared/search_bar';
 import { ServiceOverviewErrorsTable } from './service_overview_errors_table';
 import { TableLinkFlexItem } from './table_link_flex_item';
 
-const rowHeight = 310;
-const latencyChartRowHeight = 230;
-
-const Row = styled(EuiFlexItem)`
-  height: ${rowHeight}px;
-`;
-
-const LatencyChartRow = styled(EuiFlexItem)`
-  height: ${latencyChartRowHeight}px;
-`;
+/**
+ * The height a chart should be if it's next to a table with 5 rows and a title.
+ * Add the height of the pagination row.
+ */
+export const chartHeight = 322;
 
 interface ServiceOverviewProps {
   agentName?: string;
@@ -52,7 +46,7 @@ export function ServiceOverview({
       <SearchBar />
       <EuiPage>
         <EuiFlexGroup direction="column" gutterSize="s">
-          <LatencyChartRow>
+          <EuiFlexItem>
             <EuiPanel>
               <EuiTitle size="xs">
                 <h2>
@@ -65,8 +59,8 @@ export function ServiceOverview({
                 </h2>
               </EuiTitle>
             </EuiPanel>
-          </LatencyChartRow>
-          <Row>
+          </EuiFlexItem>
+          <EuiFlexItem>
             <EuiFlexGroup gutterSize="s">
               <EuiFlexItem grow={4}>
                 <EuiPanel>
@@ -111,12 +105,15 @@ export function ServiceOverview({
                 </EuiPanel>
               </EuiFlexItem>
             </EuiFlexGroup>
-          </Row>
-          <Row>
+          </EuiFlexItem>
+          <EuiFlexItem>
             <EuiFlexGroup gutterSize="s">
               {!isRumAgentName(agentName) && (
                 <EuiFlexItem grow={4}>
-                  <TransactionErrorRateChart showAnnotations={false} />
+                  <TransactionErrorRateChart
+                    height={chartHeight}
+                    showAnnotations={false}
+                  />
                 </EuiFlexItem>
               )}
               <EuiFlexItem grow={6}>
@@ -125,8 +122,8 @@ export function ServiceOverview({
                 </EuiPanel>
               </EuiFlexItem>
             </EuiFlexGroup>
-          </Row>
-          <Row>
+          </EuiFlexItem>
+          <EuiFlexItem>
             <EuiFlexGroup gutterSize="s">
               <EuiFlexItem grow={4}>
                 <EuiPanel>
@@ -175,8 +172,8 @@ export function ServiceOverview({
                 </EuiPanel>
               </EuiFlexItem>
             </EuiFlexGroup>
-          </Row>
-          <Row>
+          </EuiFlexItem>
+          <EuiFlexItem>
             <EuiFlexGroup gutterSize="s">
               <EuiFlexItem grow={4}>
                 <EuiPanel>
@@ -207,7 +204,7 @@ export function ServiceOverview({
                 </EuiPanel>
               </EuiFlexItem>
             </EuiFlexGroup>
-          </Row>
+          </EuiFlexItem>
         </EuiFlexGroup>
       </EuiPage>
     </ChartsSyncContextProvider>
diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/fetch_wrapper.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/fetch_wrapper.tsx
index 4c8d368811a0c1..912490d866e88c 100644
--- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/fetch_wrapper.tsx
+++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/fetch_wrapper.tsx
@@ -4,27 +4,20 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import React from 'react';
+import React, { ReactNode } from 'react';
 import { FETCH_STATUS } from '../../../../hooks/useFetcher';
 import { ErrorStatePrompt } from '../../../shared/ErrorStatePrompt';
-import { LoadingStatePrompt } from '../../../shared/LoadingStatePrompt';
 
 export function FetchWrapper({
-  hasData,
   status,
   children,
 }: {
-  hasData: boolean;
   status: FETCH_STATUS;
-  children: React.ReactNode;
+  children: ReactNode;
 }) {
   if (status === FETCH_STATUS.FAILURE) {
     return <ErrorStatePrompt />;
   }
 
-  if (!hasData && status !== FETCH_STATUS.SUCCESS) {
-    return <LoadingStatePrompt />;
-  }
-
   return <>{children}</>;
 }
diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx
index a5a002cf3aca49..34b934c41cca3f 100644
--- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx
+++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx
@@ -3,25 +3,27 @@
  * 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, { useState } from 'react';
-import { EuiTitle } from '@elastic/eui';
-import { EuiFlexItem } from '@elastic/eui';
-import { EuiFlexGroup } from '@elastic/eui';
+import {
+  EuiBasicTableColumn,
+  EuiFlexGroup,
+  EuiFlexItem,
+  EuiTitle,
+  EuiToolTip,
+} from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
-import { EuiBasicTable } from '@elastic/eui';
-import { EuiBasicTableColumn } from '@elastic/eui';
+import React, { useState } from 'react';
 import styled from 'styled-components';
-import { EuiToolTip } from '@elastic/eui';
 import { asInteger } from '../../../../../common/utils/formatters';
 import { FETCH_STATUS, useFetcher } from '../../../../hooks/useFetcher';
 import { useUrlParams } from '../../../../hooks/useUrlParams';
-import { ErrorOverviewLink } from '../../../shared/Links/apm/ErrorOverviewLink';
-import { TableLinkFlexItem } from '../table_link_flex_item';
-import { SparkPlotWithValueLabel } from '../../../shared/charts/spark_plot/spark_plot_with_value_label';
 import { callApmApi } from '../../../../services/rest/createCallApmApi';
-import { TimestampTooltip } from '../../../shared/TimestampTooltip';
-import { ErrorDetailLink } from '../../../shared/Links/apm/ErrorDetailLink';
 import { px, truncate, unit } from '../../../../style/variables';
+import { SparkPlotWithValueLabel } from '../../../shared/charts/spark_plot/spark_plot_with_value_label';
+import { ErrorDetailLink } from '../../../shared/Links/apm/ErrorDetailLink';
+import { ErrorOverviewLink } from '../../../shared/Links/apm/ErrorOverviewLink';
+import { TimestampTooltip } from '../../../shared/TimestampTooltip';
+import { ServiceOverviewTable } from '../service_overview_table';
+import { TableLinkFlexItem } from '../table_link_flex_item';
 import { FetchWrapper } from './fetch_wrapper';
 
 interface Props {
@@ -108,7 +110,7 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) {
       render: (_, { last_seen: lastSeen }) => {
         return <TimestampTooltip time={lastSeen} timeUnit="minutes" />;
       },
-      width: px(unit * 8),
+      width: px(unit * 9),
     },
     {
       field: 'occurrences',
@@ -223,8 +225,8 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) {
         </EuiFlexGroup>
       </EuiFlexItem>
       <EuiFlexItem>
-        <FetchWrapper hasData={!!items.length} status={status}>
-          <EuiBasicTable
+        <FetchWrapper status={status}>
+          <ServiceOverviewTable
             columns={columns}
             items={items}
             pagination={{
diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_table.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_table.tsx
new file mode 100644
index 00000000000000..b54458e4555f74
--- /dev/null
+++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_table.tsx
@@ -0,0 +1,56 @@
+/*
+ * 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 { EuiBasicTable, EuiBasicTableProps } from '@elastic/eui';
+import React from 'react';
+import styled from 'styled-components';
+
+/**
+ * The height for a table on the overview page. Is the height of a 5-row basic
+ * table.
+ */
+const tableHeight = 298;
+
+/**
+ * A container for the table. Sets height and flex properties on the EUI Basic
+ * Table contained within and the first child div of that. This makes it so the
+ * pagination controls always stay fixed at the bottom in the same position.
+ *
+ * Hide the empty message when we don't yet have any items and are still loading.
+ */
+const ServiceOverviewTableContainer = styled.div<{
+  isEmptyAndLoading: boolean;
+}>`
+  height: ${tableHeight}px;
+  display: flex;
+  flex-direction: column;
+
+  .euiBasicTable {
+    display: flex;
+    flex-direction: column;
+    flex-grow: 1;
+
+    > :first-child {
+      flex-grow: 1;
+    }
+  }
+
+  .euiTableRowCell {
+    visibility: ${({ isEmptyAndLoading }) =>
+      isEmptyAndLoading ? 'hidden' : 'visible'};
+  }
+`;
+
+export function ServiceOverviewTable<T>(props: EuiBasicTableProps<T>) {
+  const { items, loading } = props;
+  const isEmptyAndLoading = !!(items.length === 0 && loading);
+
+  return (
+    <ServiceOverviewTableContainer isEmptyAndLoading={isEmptyAndLoading}>
+      <EuiBasicTable {...props} />
+    </ServiceOverviewTableContainer>
+  );
+}
diff --git a/x-pack/plugins/apm/public/components/shared/charts/line_chart/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/line_chart/index.tsx
index 507acc49d89dbd..b40df89a22c33d 100644
--- a/x-pack/plugins/apm/public/components/shared/charts/line_chart/index.tsx
+++ b/x-pack/plugins/apm/public/components/shared/charts/line_chart/index.tsx
@@ -31,6 +31,7 @@ import { onBrushEnd } from '../helper/helper';
 interface Props {
   id: string;
   fetchStatus: FETCH_STATUS;
+  height?: number;
   onToggleLegend?: LegendItemListener;
   timeseries: TimeSeries[];
   /**
@@ -44,10 +45,9 @@ interface Props {
   showAnnotations?: boolean;
 }
 
-const XY_HEIGHT = unit * 16;
-
 export function LineChart({
   id,
+  height = unit * 16,
   fetchStatus,
   onToggleLegend,
   timeseries,
@@ -88,7 +88,7 @@ export function LineChart({
     );
 
   return (
-    <ChartContainer status={fetchStatus} hasData={!isEmpty} height={XY_HEIGHT}>
+    <ChartContainer hasData={!isEmpty} height={height} status={fetchStatus}>
       <Chart ref={chartRef} id={id}>
         <Settings
           onBrushEnd={({ x }) => onBrushEnd({ x, history })}
diff --git a/x-pack/plugins/apm/public/components/shared/charts/transaction_error_rate_chart/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/transaction_error_rate_chart/index.tsx
index 2743d12a3eb046..5b977b69916129 100644
--- a/x-pack/plugins/apm/public/components/shared/charts/transaction_error_rate_chart/index.tsx
+++ b/x-pack/plugins/apm/public/components/shared/charts/transaction_error_rate_chart/index.tsx
@@ -27,10 +27,14 @@ function yTickFormat(y?: number | null) {
 }
 
 interface Props {
+  height?: number;
   showAnnotations?: boolean;
 }
 
-export function TransactionErrorRateChart({ showAnnotations = true }: Props) {
+export function TransactionErrorRateChart({
+  height,
+  showAnnotations = true,
+}: Props) {
   const theme = useTheme();
   const { serviceName } = useParams<{ serviceName?: string }>();
   const { urlParams, uiFilters } = useUrlParams();
@@ -71,6 +75,7 @@ export function TransactionErrorRateChart({ showAnnotations = true }: Props) {
       </EuiTitle>
       <LineChart
         id="errorRate"
+        height={height}
         showAnnotations={showAnnotations}
         fetchStatus={status}
         timeseries={[