Skip to content

fix: loading states unified across the app #3608

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
knudtty wants to merge 1 commit into
base: main
Choose a base branch
from
+88 −82
Open

fix: loading states unified across the app #3608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/chart-loading-spinner.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
16 changes: 7 additions & 9 deletions packages/app/src/components/DBBarChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { memo } from 'react';
import cx from 'classnames';
import {
Bar,
BarChart,
Expand All @@ -9,7 +8,7 @@ import {
XAxis,
YAxis,
} from 'recharts';
import { Flex } from '@mantine/core';
import { Flex, Loader } from '@mantine/core';

import type { NumberFormat } from '@/types';
import { useContentFontSize } from '@/useUserPreferences';
Expand Down Expand Up @@ -86,6 +85,7 @@ export const DBBarChart = (props: CategoricalChartProps) => {
<ChartContainer title={props.title} toolbarItems={toolbarItems}>
{isLoading && !data ? (
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
<Loader size="xs" mr="xs" />
Loading Chart Data...
</div>
) : isError && error ? (
Expand All @@ -96,13 +96,11 @@ export const DBBarChart = (props: CategoricalChartProps) => {
variant={props.errorVariant}
/>
) : data?.data.length === 0 ? (
<div
className={cx(
'd-flex h-100 w-100 align-items-center justify-content-center text-muted',
{ 'effect-pulse': isPlaceholderData },
)}
>
No data found within time range.
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
{isPlaceholderData && <Loader size="xs" mr="xs" />}

@github-actions github-actions Bot Nov 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 minor — Same loading/empty markup copied into 8 chart components

The same <div className="d-flex h-100 ... text-muted"><Loader size="xs" mr="xs" />Loading Chart Data...</div> block and the matching isPlaceholderData ternary are now repeated in DBBarChart, DBHistogramChart, DBListBarChart, DBNumberChart, DBPieChart, DBTableChart, DBTimeChart and ServiceDashboardSlowestEventsTile (one copy has already drifted, see that finding). Move it into one shared ChartLoadingState/ChartEmptyState({ isLoading }) component next to ChartErrorState, and have every chart use it.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

{isPlaceholderData
? 'Loading Chart Data...'

@github-actions github-actions Bot Nov 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 minor — New loading copy uses Title Case

The new strings 'Loading Chart Data...' break the REQUIRED rule "UI text: use sentence case". The PR description itself writes "Loading chart data...". Use 'Loading chart data...' in all the new branches. Also update the existing initial-loading branches and the test assertions that match on the old text.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

: 'No data found within time range.'}
</div>
) : (
<Flex
Expand Down
16 changes: 7 additions & 9 deletions packages/app/src/components/DBHistogramChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { memo, useEffect, useMemo, useState } from 'react';
import cx from 'classnames';
import { omit } from 'lodash';
import { useHotkeys } from 'react-hotkeys-hook';
import {
Expand All @@ -11,7 +10,7 @@ import {
YAxis,
} from 'recharts';
import { BuilderChartConfigWithDateRange } from '@hyperdx/common-utils/dist/types';
import { Text } from '@mantine/core';
import { Loader, Text } from '@mantine/core';

import { buildMVDateRangeIndicator, INTEGER_NUMBER_FORMAT } from '@/ChartUtils';
import { useQueriedChartConfig } from '@/hooks/useChartConfig';
Expand Down Expand Up @@ -298,18 +297,17 @@ export default function DBHistogramChart({
<ChartContainer title={title} toolbarItems={toolbarItemsMemo}>
{isLoading && !data ? (
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
<Loader size="xs" mr="xs" />
Loading Chart Data...
</div>
) : isError ? (
<ChartErrorState error={error} variant={errorVariant} />
) : data?.data.length === 0 ? (
<div
className={cx(
'd-flex h-100 w-100 align-items-center justify-content-center text-muted',
{ 'effect-pulse': isPlaceholderData },
)}
>
No data found within time range.
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
{isPlaceholderData && <Loader size="xs" mr="xs" />}
{isPlaceholderData
? 'Loading Chart Data...'
: 'No data found within time range.'}
</div>
) : (
<HistogramChart
Expand Down
16 changes: 7 additions & 9 deletions packages/app/src/components/DBListBarChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { useMemo } from 'react';
import Link from 'next/link';
import cx from 'classnames';
import { omit } from 'lodash';
import { BuilderChartConfigWithDateRange } from '@hyperdx/common-utils/dist/types';
import type { FloatingPosition } from '@mantine/core';
import { Box, Flex, HoverCard, Text } from '@mantine/core';
import { Box, Flex, HoverCard, Loader, Text } from '@mantine/core';

import { buildMVDateRangeIndicator } from '@/ChartUtils';
import { useQueriedChartConfig } from '@/hooks/useChartConfig';
Expand Down Expand Up @@ -279,18 +278,17 @@ export default function DBListBarChart({
>
{isLoading && !data ? (
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
<Loader size="xs" mr="xs" />
Loading Chart Data...
</div>
) : isError ? (
<ChartErrorState error={error} variant={errorVariant} />
) : data?.data.length === 0 ? (
<div
className={cx(
'd-flex h-100 w-100 align-items-center justify-content-center text-muted',
{ 'effect-pulse': isPlaceholderData },
)}
>
No data found within time range.
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
{isPlaceholderData && <Loader size="xs" mr="xs" />}
{isPlaceholderData
? 'Loading Chart Data...'
: 'No data found within time range.'}
</div>
) : (
<div className={isPlaceholderData ? 'effect-pulse' : undefined}>
Expand Down
16 changes: 7 additions & 9 deletions packages/app/src/components/DBNumberChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { useLayoutEffect, useMemo, useRef, useState } from 'react';
import cx from 'classnames';
import { ErrorBoundary } from 'react-error-boundary';
import {
filterColumnMetaByType,
Expand All @@ -14,7 +13,7 @@ import {
ChartConfigWithDateRange,
resolveChartPaletteToken,
} from '@hyperdx/common-utils/dist/types';
import { Flex, Text } from '@mantine/core';
import { Flex, Loader, Text } from '@mantine/core';

import {
buildMVDateRangeIndicator,
Expand Down Expand Up @@ -383,20 +382,19 @@ export default function DBNumberChart({
<ChartContainer title={title} toolbarItems={toolbarItemsMemo}>
{isLoading && !data ? (
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
<Loader size="xs" mr="xs" />
Loading Chart Data...
</div>
) : isError ? (
<ChartErrorState error={error} variant={errorVariant} />
) : resultError ? (
<ChartErrorState error={resultError} variant={errorVariant} />
) : data?.data.length === 0 ? (
<div
className={cx(
'd-flex h-100 w-100 align-items-center justify-content-center text-muted',
{ 'effect-pulse': isPlaceholderData },
)}
>
No data found within time range.
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
{isPlaceholderData && <Loader size="xs" mr="xs" />}
{isPlaceholderData
? 'Loading Chart Data...'
: 'No data found within time range.'}
</div>
) : (
<div
Expand Down
16 changes: 7 additions & 9 deletions packages/app/src/components/DBPieChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { memo } from 'react';
import cx from 'classnames';
import { Cell, Pie, PieChart, ResponsiveContainer, Tooltip } from 'recharts';
import { Box, Flex, ScrollArea, Text } from '@mantine/core';
import { Box, Flex, Loader, ScrollArea, Text } from '@mantine/core';

import type { NumberFormat } from '@/types';
import { formatNumber, truncateMiddle } from '@/utils';
Expand Down Expand Up @@ -106,20 +105,19 @@ export const DBPieChart = (props: CategoricalChartProps) => {
<ChartContainer title={title} toolbarItems={toolbarItems}>
{isLoading && !data ? (
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
<Loader size="xs" mr="xs" />
Loading Chart Data...
</div>
) : isError && error ? (
<ChartErrorState error={error} variant={errorVariant} />
) : responseFormatError ? (
<ChartErrorState error={responseFormatError} variant={errorVariant} />
) : data?.data.length === 0 ? (
<div
className={cx(
'd-flex h-100 w-100 align-items-center justify-content-center text-muted',
{ 'effect-pulse': isPlaceholderData },
)}
>
No data found within time range.
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
{isPlaceholderData && <Loader size="xs" mr="xs" />}
{isPlaceholderData
? 'Loading Chart Data...'
: 'No data found within time range.'}
</div>
) : (
<Flex
Expand Down
16 changes: 7 additions & 9 deletions packages/app/src/components/DBTableChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { useCallback, useMemo, useState } from 'react';
import cx from 'classnames';
import { inferNumericColumn } from '@hyperdx/common-utils/dist/clickhouse';
import { isRatioChartConfig } from '@hyperdx/common-utils/dist/core/renderChartConfig';
import {
Expand All @@ -12,7 +11,7 @@ import {
ChartPaletteToken,
ColorCondition,
} from '@hyperdx/common-utils/dist/types';
import { Text } from '@mantine/core';
import { Loader, Text } from '@mantine/core';
import { SortingState } from '@tanstack/react-table';

import {
Expand Down Expand Up @@ -336,18 +335,17 @@ export default function DBTableChart({
<ChartContainer title={title} toolbarItems={toolbarItemsMemo}>
{isLoading && !data ? (
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
<Loader size="xs" mr="xs" />
Loading Chart Data...
</div>
) : isError && error ? (
<ChartErrorState error={error} variant={errorVariant} />
) : data?.data.length === 0 ? (
<div
className={cx(
'd-flex h-100 w-100 align-items-center justify-content-center text-muted',
{ 'effect-pulse': isPlaceholderData },
)}
>
No data found within time range.
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
{isPlaceholderData && <Loader size="xs" mr="xs" />}
{isPlaceholderData
? 'Loading Chart Data...'
: 'No data found within time range.'}
</div>
) : (
<Table
Expand Down
8 changes: 6 additions & 2 deletions packages/app/src/components/DBTimeChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
ChartConfigWithDateRange,
DisplayType,
} from '@hyperdx/common-utils/dist/types';
import { Popover, Portal } from '@mantine/core';
import { Loader, Popover, Portal } from '@mantine/core';
import {
IconChartAreaLine,
IconChartBar,
Expand Down Expand Up @@ -1088,6 +1088,7 @@ function DBTimeChartComponent({
<ChartContainer title={title} toolbarItems={toolbarItemsMemo}>
{isLoading && !data ? (
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
<Loader size="xs" mr="xs" />
Loading Chart Data...
</div>
) : isError ? (
Expand All @@ -1103,7 +1104,10 @@ function DBTimeChartComponent({
/>
) : graphResults.length === 0 ? (
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
No data found within time range.
{isLoadingOrPlaceholder && <Loader size="xs" mr="xs" />}

@github-actions github-actions Bot Nov 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 major — Time chart can get stuck on "Loading Chart Data..." if the previous-period query fails

The empty branch now keys off isLoadingOrPlaceholder (line 610), and that flag includes !data?.isComplete || (config.compareToPreviousPeriod && !previousPeriodData?.isComplete). If compare-to-previous is on, the current period comes back empty, and the previous-period query errors, previousPeriodData stays undefined. The tile then shows a spinner forever, where it used to show "No data found". Base this branch on whether a fetch is actually in flight (isLoading || isPlaceholderData || isPreviousPeriodLoading), or treat a previous-period error as finished. Also add a DBTimeChart test for this branch: none of the PR's tests cover it.

Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond

{isLoadingOrPlaceholder
? 'Loading Chart Data...'
: 'No data found within time range.'}
</div>
) : (
<>
Expand Down
3 changes: 2 additions & 1 deletion packages/app/src/components/ServiceDashboardSlowestEventsTile.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
pickSampleWeightExpressionProps,
type TTraceSource,
} from '@hyperdx/common-utils/dist/types';
import { Text } from '@mantine/core';
import { Loader, Text } from '@mantine/core';

import { ChartCard } from '@/components/charts/ChartCard';
import { useQueriedChartConfig } from '@/hooks/useChartConfig';
Expand Down Expand Up @@ -78,6 +78,7 @@ export default function SlowestEventsTile({
>
{isLoading && !data ? (
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
<Loader size="xs" mr="xs" />
Loading Chart Data...
</div>
) : isError ? (
Expand Down
10 changes: 6 additions & 4 deletions packages/app/src/components/__tests__/DBBarChart.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -123,9 +123,11 @@ describe('DBBarChart', () => {
});

renderWithMantine(<DBBarChart config={baseTestConfig} />);
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();
});
});

Expand Down
7 changes: 3 additions & 4 deletions packages/app/src/components/__tests__/DBHistogramChart.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -202,9 +202,8 @@ describe('DBHistogramChart', () => {

renderWithMantine(<DBHistogramChart config={baseTestConfig} />);

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', () => {
Expand Down
7 changes: 3 additions & 4 deletions packages/app/src/components/__tests__/DBListBarChart.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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', () => {
Expand Down
7 changes: 3 additions & 4 deletions packages/app/src/components/__tests__/DBNumberChart.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -268,9 +268,8 @@ describe('DBNumberChart', () => {
});

renderWithMantine(<DBNumberChart config={baseTestConfig} />);
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();
});
});

Expand Down
Loading
Loading