diff --git a/.changeset/chart-loading-spinner.md b/.changeset/chart-loading-spinner.md new file mode 100644 index 000000000..d652952a5 --- /dev/null +++ b/.changeset/chart-loading-spinner.md @@ -0,0 +1,10 @@ +--- +'@hyperdx/app': patch +--- + +fix: show a neutral spinner while charts load + +Chart tiles now show a spinner with "Loading Chart Data..." while a query is in +flight, including when the previous result was empty. Before, they kept showing +"No data found within time range." until the new query finished. Loaders now +default to a neutral gray instead of yellow. diff --git a/packages/app/src/components/DBBarChart.tsx b/packages/app/src/components/DBBarChart.tsx index 6bdfbf008..a5bfb4013 100644 --- a/packages/app/src/components/DBBarChart.tsx +++ b/packages/app/src/components/DBBarChart.tsx @@ -1,5 +1,4 @@ import { memo } from 'react'; -import cx from 'classnames'; import { Bar, BarChart, @@ -20,6 +19,7 @@ import { useCategoricalChart, } from './charts/CategoricalChart'; import ChartContainer from './charts/ChartContainer'; +import ChartEmptyState from './charts/ChartEmptyState'; import ChartErrorState from './charts/ChartErrorState'; import { ChartTooltipContainer, ChartTooltipItem } from './charts/ChartTooltip'; @@ -85,9 +85,7 @@ export const DBBarChart = (props: CategoricalChartProps) => { return ( {isLoading && !data ? ( -
- Loading Chart Data... -
+ ) : isError && error ? ( ) : responseFormatError ? ( @@ -96,14 +94,7 @@ export const DBBarChart = (props: CategoricalChartProps) => { variant={props.errorVariant} /> ) : data?.data.length === 0 ? ( -
- No data found within time range. -
+ ) : ( {isLoading && !data ? ( -
- Loading Chart Data... -
+ ) : isError ? ( ) : data?.data.length === 0 ? ( -
- No data found within time range. -
+ ) : ( {isLoading && !data ? ( -
- Loading Chart Data... -
+ ) : isError ? ( ) : data?.data.length === 0 ? ( -
- No data found within time range. -
+ ) : (
{isLoading && !data ? ( -
- Loading Chart Data... -
+ ) : isError ? ( ) : resultError ? ( ) : data?.data.length === 0 ? ( -
- No data found within time range. -
+ ) : (
{ return ( {isLoading && !data ? ( -
- Loading Chart Data... -
+ ) : isError && error ? ( ) : responseFormatError ? ( ) : data?.data.length === 0 ? ( -
- No data found within time range. -
+ ) : ( {isLoading && !data ? ( -
- Loading Chart Data... -
+ ) : isError && error ? ( ) : data?.data.length === 0 ? ( -
- No data found within time range. -
+ ) : ( prev, - queryKey: [ - queryKeyPrefix, - queriedConfig, - 'chunked', - { - disableQueryChunking, - enableParallelQueries, - parallelizeWhenPossible: me?.team?.parallelizeWhenPossible, - }, - ], - enabled: enabled && !isLoadingMe, - enableQueryChunking: !disableQueryChunking, - enableParallelQueries: - enableParallelQueries && me?.team?.parallelizeWhenPossible, - }); + const { + data, + isLoading, + isFetching, + isError, + error, + isPlaceholderData, + isSuccess, + } = useQueriedChartConfig(queriedConfig, { + placeholderData: (prev: any) => prev, + queryKey: [ + queryKeyPrefix, + queriedConfig, + 'chunked', + { + disableQueryChunking, + enableParallelQueries, + parallelizeWhenPossible: me?.team?.parallelizeWhenPossible, + }, + ], + enabled: enabled && !isLoadingMe, + enableQueryChunking: !disableQueryChunking, + enableParallelQueries: + enableParallelQueries && me?.team?.parallelizeWhenPossible, + }); const previousPeriodChartConfig: ChartConfigWithDateRange = useMemo(() => { const previousPeriodDateRange = @@ -599,13 +607,16 @@ function DBTimeChartComponent({ previousPeriodChartConfig.dateRange, ]); - const { data: previousPeriodData, isLoading: isPreviousPeriodLoading } = - useQueriedChartConfig(previousPeriodChartConfig, { - placeholderData: (prev: any) => prev, - queryKey: [queryKeyPrefix, previousPeriodChartConfig, 'chunked'], - enabled: !!(enabled && config.compareToPreviousPeriod), - enableQueryChunking: true, - }); + const { + data: previousPeriodData, + isLoading: isPreviousPeriodLoading, + isFetching: isPreviousPeriodFetching, + } = useQueriedChartConfig(previousPeriodChartConfig, { + placeholderData: (prev: any) => prev, + queryKey: [queryKeyPrefix, previousPeriodChartConfig, 'chunked'], + enabled: !!(enabled && config.compareToPreviousPeriod), + enableQueryChunking: true, + }); const isLoadingOrPlaceholder = isLoading || @@ -613,6 +624,9 @@ function DBTimeChartComponent({ !data?.isComplete || (config.compareToPreviousPeriod && !previousPeriodData?.isComplete) || isPlaceholderData; + // Unlike isLoadingOrPlaceholder, this goes false once both queries settle, + // even if one errors, so the empty state can't get stuck loading. + const isFetchingAnyPeriod = isFetching || isPreviousPeriodFetching; const { formatByColumn, chartFormat: axisNumberFormat } = useChartNumberFormats(queriedConfig, data?.meta); @@ -1087,9 +1101,7 @@ function DBTimeChartComponent({ return ( {isLoading && !data ? ( -
- Loading Chart Data... -
+ ) : isError ? ( ) : resultFormattingError ? ( @@ -1102,9 +1114,7 @@ function DBTimeChartComponent({ } /> ) : graphResults.length === 0 ? ( -
- No data found within time range. -
+ ) : ( <> {/* Pinned (click-locked) tooltip; hover is handled in MemoChart. */} diff --git a/packages/app/src/components/ServiceDashboardSlowestEventsTile.tsx b/packages/app/src/components/ServiceDashboardSlowestEventsTile.tsx index be0910712..1c2638c05 100644 --- a/packages/app/src/components/ServiceDashboardSlowestEventsTile.tsx +++ b/packages/app/src/components/ServiceDashboardSlowestEventsTile.tsx @@ -11,6 +11,7 @@ import { useQueriedChartConfig } from '@/hooks/useChartConfig'; import { useServiceDashboardExpressions } from '@/serviceDashboard'; import ChartContainer from './charts/ChartContainer'; +import ChartEmptyState from './charts/ChartEmptyState'; import ChartErrorState from './charts/ChartErrorState'; import DBSqlRowTableWithSideBar from './DBSqlRowTableWithSidebar'; @@ -77,15 +78,11 @@ export default function SlowestEventsTile({ ]} > {isLoading && !data ? ( -
- Loading Chart Data... -
+ ) : isError ? ( ) : data?.data.length === 0 ? ( -
- No data found within time range. -
+ ) : ( source && expressions && ( diff --git a/packages/app/src/components/__tests__/DBBarChart.test.tsx b/packages/app/src/components/__tests__/DBBarChart.test.tsx index 6250a9a96..581349c49 100644 --- a/packages/app/src/components/__tests__/DBBarChart.test.tsx +++ b/packages/app/src/components/__tests__/DBBarChart.test.tsx @@ -114,7 +114,7 @@ describe('DBBarChart', () => { ).toBeNull(); }); - it('pulses the empty state while a refetch is running', () => { + it('shows loading instead of the stale empty result during a refetch', () => { mockUseQueriedChartConfig.mockReturnValue({ data: { data: [], meta: groupedData.meta }, isLoading: false, @@ -123,9 +123,11 @@ describe('DBBarChart', () => { }); renderWithMantine(); - expect(screen.getByText('No data found within time range.')).toHaveClass( - 'effect-pulse', - ); + const loadingState = screen.getByText('Loading Chart Data...'); + expect( + loadingState.querySelector('.mantine-Loader-root'), + ).toBeInTheDocument(); + expect(screen.queryByText('No data found within time range.')).toBeNull(); }); }); diff --git a/packages/app/src/components/__tests__/DBHistogramChart.test.tsx b/packages/app/src/components/__tests__/DBHistogramChart.test.tsx index 1b242fa12..2ca9c63e1 100644 --- a/packages/app/src/components/__tests__/DBHistogramChart.test.tsx +++ b/packages/app/src/components/__tests__/DBHistogramChart.test.tsx @@ -192,7 +192,7 @@ describe('DBHistogramChart', () => { expect(chart()).not.toHaveClass('effect-pulse'); }); - it('pulses the empty state while a refresh loads', () => { + it('shows loading instead of the stale empty result during a refetch', () => { mockUseQueriedChartConfig.mockReturnValue({ data: { data: [], meta: [] }, isLoading: false, @@ -202,9 +202,8 @@ describe('DBHistogramChart', () => { renderWithMantine(); - expect(screen.getByText('No data found within time range.')).toHaveClass( - 'effect-pulse', - ); + expect(screen.getByText('Loading Chart Data...')).toBeInTheDocument(); + expect(screen.queryByText('No data found within time range.')).toBeNull(); }); it('does not pulse a fresh empty result', () => { diff --git a/packages/app/src/components/__tests__/DBListBarChart.test.tsx b/packages/app/src/components/__tests__/DBListBarChart.test.tsx index 9fbb6c218..4d3548675 100644 --- a/packages/app/src/components/__tests__/DBListBarChart.test.tsx +++ b/packages/app/src/components/__tests__/DBListBarChart.test.tsx @@ -209,7 +209,7 @@ describe('DBListBarChart', () => { expect(screen.getByText('checkout').closest('.effect-pulse')).toBeNull(); }); - it('pulses the empty state while a refresh loads', () => { + it('shows loading instead of the stale empty result during a refetch', () => { mockUseQueriedChartConfig.mockReturnValue({ data: { data: [] }, isLoading: false, @@ -219,9 +219,8 @@ describe('DBListBarChart', () => { renderChart(); - expect(screen.getByText('No data found within time range.')).toHaveClass( - 'effect-pulse', - ); + expect(screen.getByText('Loading Chart Data...')).toBeInTheDocument(); + expect(screen.queryByText('No data found within time range.')).toBeNull(); }); it('does not pulse a fresh empty result', () => { diff --git a/packages/app/src/components/__tests__/DBNumberChart.test.tsx b/packages/app/src/components/__tests__/DBNumberChart.test.tsx index 556e78610..f25156d73 100644 --- a/packages/app/src/components/__tests__/DBNumberChart.test.tsx +++ b/packages/app/src/components/__tests__/DBNumberChart.test.tsx @@ -259,7 +259,7 @@ describe('DBNumberChart', () => { ).toBeNull(); }); - it('pulses the empty state while a refetch is running', () => { + it('shows loading instead of the stale empty result during a refetch', () => { mockUseQueriedChartConfig.mockReturnValue({ data: { data: [] }, isLoading: false, @@ -268,9 +268,8 @@ describe('DBNumberChart', () => { }); renderWithMantine(); - expect(screen.getByText('No data found within time range.')).toHaveClass( - 'effect-pulse', - ); + expect(screen.getByText('Loading Chart Data...')).toBeInTheDocument(); + expect(screen.queryByText('No data found within time range.')).toBeNull(); }); }); diff --git a/packages/app/src/components/__tests__/DBPieChart.test.tsx b/packages/app/src/components/__tests__/DBPieChart.test.tsx index 06a23f9bf..1c51d3872 100644 --- a/packages/app/src/components/__tests__/DBPieChart.test.tsx +++ b/packages/app/src/components/__tests__/DBPieChart.test.tsx @@ -118,7 +118,7 @@ describe('DBPieChart', () => { ); }); - it('pulses the empty state while a refetch is running', () => { + it('shows loading instead of the stale empty result during a refetch', () => { mockUseQueriedChartConfig.mockReturnValue({ data: { data: [], meta: groupedData.meta }, isLoading: false, @@ -127,9 +127,8 @@ describe('DBPieChart', () => { }); renderWithMantine(); - expect(screen.getByText('No data found within time range.')).toHaveClass( - 'effect-pulse', - ); + expect(screen.getByText('Loading Chart Data...')).toBeInTheDocument(); + expect(screen.queryByText('No data found within time range.')).toBeNull(); }); }); diff --git a/packages/app/src/components/__tests__/DBTableChart.test.tsx b/packages/app/src/components/__tests__/DBTableChart.test.tsx index 75151f58e..06ebdba06 100644 --- a/packages/app/src/components/__tests__/DBTableChart.test.tsx +++ b/packages/app/src/components/__tests__/DBTableChart.test.tsx @@ -208,7 +208,7 @@ describe('DBTableChart', () => { ).toBeUndefined(); }); - it('pulses the empty state while a refetch is running', () => { + it('shows loading instead of the stale empty result during a refetch', () => { const current = jest.mocked(useOffsetPaginatedQuery)(baseTestConfig); jest.mocked(useOffsetPaginatedQuery).mockReturnValue({ ...current, @@ -216,12 +216,11 @@ describe('DBTableChart', () => { isPlaceholderData: true, }); - const { getByText } = renderWithMantine( + const { getByText, queryByText } = renderWithMantine( , ); - expect(getByText('No data found within time range.')).toHaveClass( - 'effect-pulse', - ); + expect(getByText('Loading Chart Data...')).toBeInTheDocument(); + expect(queryByText('No data found within time range.')).toBeNull(); }); describe('row links', () => { diff --git a/packages/app/src/components/__tests__/DBTimeChart.test.tsx b/packages/app/src/components/__tests__/DBTimeChart.test.tsx index bf460c349..629333a9b 100644 --- a/packages/app/src/components/__tests__/DBTimeChart.test.tsx +++ b/packages/app/src/components/__tests__/DBTimeChart.test.tsx @@ -161,6 +161,68 @@ describe('DBTimeChart', () => { expect(secondCallOptions.enabled).toBe(false); }); + describe('empty state', () => { + const emptyResult = { + data: { + data: [], + meta: [ + { name: 'timestamp', type: 'DateTime' }, + { name: 'value', type: 'UInt64' }, + ], + rows: 0, + isComplete: true, + }, + isLoading: false, + isFetching: false, + isError: false, + isSuccess: true, + isPlaceholderData: false, + }; + // Zero-filled buckets would otherwise render an empty line, not the empty state. + const emptyConfig = { ...baseTestConfig, fillNulls: false as const }; + const isPreviousPeriod = (config: { dateRange: [Date, Date] }) => + config.dateRange[0] < baseTestConfig.dateRange[0]; + + it('shows loading while the query is still fetching', () => { + mockUseQueriedChartConfig.mockReturnValue({ + ...emptyResult, + data: { ...emptyResult.data, isComplete: false }, + isFetching: true, + }); + + renderWithMantine(); + + expect(screen.getByText('Loading Chart Data...')).toBeInTheDocument(); + }); + + it('shows no data when the previous-period query fails', () => { + mockUseQueriedChartConfig.mockImplementation(config => + isPreviousPeriod(config) + ? { + data: undefined, + isLoading: false, + isFetching: false, + isError: true, + error: new Error('previous period failed'), + isSuccess: false, + isPlaceholderData: false, + } + : emptyResult, + ); + + renderWithMantine( + , + ); + + expect( + screen.getByText('No data found within time range.'), + ).toBeInTheDocument(); + expect(screen.queryByText('Loading Chart Data...')).toBeNull(); + }); + }); + it('passes the same config to useMVOptimizationExplanation, useQueriedChartConfig, and MVOptimizationIndicator', () => { // Mock useSource to return a source so MVOptimizationIndicator is rendered jest.mocked(useSource).mockReturnValue({ diff --git a/packages/app/src/components/charts/ChartEmptyState.tsx b/packages/app/src/components/charts/ChartEmptyState.tsx new file mode 100644 index 000000000..83916b759 --- /dev/null +++ b/packages/app/src/components/charts/ChartEmptyState.tsx @@ -0,0 +1,14 @@ +import { Loader } from '@mantine/core'; + +export default function ChartEmptyState({ + isLoading = false, +}: { + isLoading?: boolean; +}) { + return ( +
+ {isLoading && } + {isLoading ? 'Loading Chart Data...' : 'No data found within time range.'} +
+ ); +} diff --git a/packages/app/src/theme/mantineTheme.ts b/packages/app/src/theme/mantineTheme.ts index c6a520ea8..f4c2b68e1 100644 --- a/packages/app/src/theme/mantineTheme.ts +++ b/packages/app/src/theme/mantineTheme.ts @@ -4,6 +4,7 @@ import { Button, Checkbox, Code, + Loader, MantineTheme, MantineThemeOverride, Radio, @@ -87,6 +88,11 @@ const makeTheme = ({ }, }), }), + Loader: Loader.extend({ + defaultProps: { + color: 'var(--color-text-muted)', + }, + }), Tooltip: Tooltip.extend({ styles: () => ({ tooltip: {