Skip to content

Commit

Permalink
Overview heights and loading states
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@smith
smith committed Nov 16, 2020
1 parent 228387c commit 49d060e
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 126 deletions.
42 changes: 20 additions & 22 deletions x-pack/plugins/apm/public/components/app/service_overview/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import {
EuiPanel,
EuiTitle,
} from '@elastic/eui';
import theme from '@elastic/eui/dist/eui_theme_light.json';
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';
Expand All @@ -24,16 +24,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;
Expand All @@ -52,7 +47,7 @@ export function ServiceOverview({
<SearchBar />
<EuiPage>
<EuiFlexGroup direction="column" gutterSize="s">
<LatencyChartRow>
<EuiFlexItem>
<EuiPanel>
<EuiTitle size="xs">
<h2>
Expand All @@ -65,8 +60,8 @@ export function ServiceOverview({
</h2>
</EuiTitle>
</EuiPanel>
</LatencyChartRow>
<Row>
</EuiFlexItem>
<EuiFlexItem>
<EuiFlexGroup gutterSize="s">
<EuiFlexItem grow={4}>
<EuiPanel>
Expand Down Expand Up @@ -111,12 +106,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}>
Expand All @@ -125,8 +123,8 @@ export function ServiceOverview({
</EuiPanel>
</EuiFlexItem>
</EuiFlexGroup>
</Row>
<Row>
</EuiFlexItem>
<EuiFlexItem>
<EuiFlexGroup gutterSize="s">
<EuiFlexItem grow={4}>
<EuiPanel>
Expand Down Expand Up @@ -175,8 +173,8 @@ export function ServiceOverview({
</EuiPanel>
</EuiFlexItem>
</EuiFlexGroup>
</Row>
<Row>
</EuiFlexItem>
<EuiFlexItem>
<EuiFlexGroup gutterSize="s">
<EuiFlexItem grow={4}>
<EuiPanel>
Expand Down Expand Up @@ -207,7 +205,7 @@ export function ServiceOverview({
</EuiPanel>
</EuiFlexItem>
</EuiFlexGroup>
</Row>
</EuiFlexItem>
</EuiFlexGroup>
</EuiPage>
</ChartsSyncContextProvider>
Expand Down
11 changes: 2 additions & 9 deletions ...pm/public/components/app/service_overview/service_overview_errors_table/fetch_wrapper.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}</>;
}
32 changes: 17 additions & 15 deletions ...lugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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={{
Expand Down
56 changes: 56 additions & 0 deletions x-pack/plugins/apm/public/components/app/service_overview/service_overview_table.tsx
View file
Original file line number Diff line number Diff line change
@@ -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>
);
}
76 changes: 6 additions & 70 deletions x-pack/plugins/apm/public/components/shared/charts/chart_container.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,58 +9,10 @@ import { FETCH_STATUS } from '../../../hooks/useFetcher';
import { ChartContainer } from './chart_container';

describe('ChartContainer', () => {
describe('loading indicator', () => {
it('shows loading when status equals to Loading or Pending and has no data', () => {
[FETCH_STATUS.PENDING, FETCH_STATUS.LOADING].map((status) => {
const { queryAllByTestId } = render(
<ChartContainer
height={100}
status={FETCH_STATUS.LOADING}
hasData={false}
>
<div>My amazing component</div>
</ChartContainer>
);

expect(queryAllByTestId('loading')[0]).toBeInTheDocument();
});
});
it('does not show loading when status equals to Loading or Pending and has data', () => {
[FETCH_STATUS.PENDING, FETCH_STATUS.LOADING].map((status) => {
const { queryAllByText } = render(
<ChartContainer height={100} status={status} hasData={true}>
<div>My amazing component</div>
</ChartContainer>
);
expect(queryAllByText('My amazing component')[0]).toBeInTheDocument();
});
});
});

describe('failure indicator', () => {
it('shows failure message when status equals to Failure and has data', () => {
describe('when status is failure', () => {
it('shows failure message', () => {
const { getByText } = render(
<ChartContainer
height={100}
status={FETCH_STATUS.FAILURE}
hasData={true}
>
<div>My amazing component</div>
</ChartContainer>
);
expect(
getByText(
'An error happened when trying to fetch data. Please try again'
)
).toBeInTheDocument();
});
it('shows failure message when status equals to Failure and has no data', () => {
const { getByText } = render(
<ChartContainer
height={100}
status={FETCH_STATUS.FAILURE}
hasData={false}
>
<ChartContainer height={100} status={FETCH_STATUS.FAILURE}>
<div>My amazing component</div>
</ChartContainer>
);
Expand All @@ -72,26 +24,10 @@ describe('ChartContainer', () => {
});
});

describe('render component', () => {
it('shows children component when status Success and has data', () => {
const { getByText } = render(
<ChartContainer
height={100}
status={FETCH_STATUS.SUCCESS}
hasData={true}
>
<div>My amazing component</div>
</ChartContainer>
);
expect(getByText('My amazing component')).toBeInTheDocument();
});
it('shows children component when status Success and has no data', () => {
describe('when status is success', () => {
it('shows children components', () => {
const { getByText } = render(
<ChartContainer
height={100}
status={FETCH_STATUS.SUCCESS}
hasData={false}
>
<ChartContainer height={100} status={FETCH_STATUS.SUCCESS}>
<div>My amazing component</div>
</ChartContainer>
);
Expand Down
8 changes: 2 additions & 6 deletions x-pack/plugins/apm/public/components/shared/charts/chart_container.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,13 @@ import React from 'react';
import { FETCH_STATUS } from '../../../hooks/useFetcher';

interface Props {
hasData: boolean;
status: FETCH_STATUS;
height: number;
children: React.ReactNode;
}

export function ChartContainer({ children, height, status, hasData }: Props) {
if (
!hasData &&
(status === FETCH_STATUS.LOADING || status === FETCH_STATUS.PENDING)
) {
export function ChartContainer({ children, height, status }: Props) {
if (status === FETCH_STATUS.LOADING || status === FETCH_STATUS.PENDING) {
return <LoadingChartPlaceholder height={height} />;
}

Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/apm/public/components/shared/charts/line_chart/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { onBrushEnd } from '../helper/helper';
interface Props {
id: string;
fetchStatus: FETCH_STATUS;
height?: number;
onToggleLegend?: LegendItemListener;
timeseries: TimeSeries[];
/**
Expand All @@ -44,10 +45,9 @@ interface Props {
showAnnotations?: boolean;
}

const XY_HEIGHT = unit * 16;

export function LineChart({
id,
height = unit * 16,
fetchStatus,
onToggleLegend,
timeseries,
Expand Down Expand Up @@ -88,7 +88,7 @@ export function LineChart({
);

return (
<ChartContainer status={fetchStatus} hasData={!isEmpty} height={XY_HEIGHT}>
<ChartContainer status={fetchStatus} height={height}>
<Chart ref={chartRef} id={id}>
<Settings
onBrushEnd={({ x }) => onBrushEnd({ x, history })}
Expand Down
Loading

0 comments on commit 49d060e

Please sign in to comment.