Skip to content

Commit 27c00d4

Browse files
fix(app): give Slowest cards the ChartCard header divider
Route the "Slowest Queries" (ClickHouse page) and Slowest Events tile titles through a titled ChartContainer instead of a bare Text, so they get the same full-bleed card header divider as sibling chart cards and their tables are properly bounded by the reactive container. Remove the headerless ChartCard prop (padding-only patch) in favor of this approach, keep the ChartCardProps interface, narrowed style type, and enforced paddingInline divider invariant, and add ChartCard tests. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent c0cc4b5 commit 27c00d4

6 files changed

Lines changed: 176 additions & 92 deletions

File tree

‎agent_docs/code_style.md‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -166,16 +166,22 @@ It is a bordered surface with the same header treatment as a custom dashboard
166166
tile (a full-bleed divider under the title). It replaces the older `ChartBox`;
167167
don't hand-roll a bordered `<div>`/`<Paper>` around a chart.
168168

169-
`ChartCard` renders the card **chrome only**. It draws the header divider by
170-
providing `ChartContainerCardHeaderProvider`, so put a chart that renders a
171-
`ChartContainer` inside it (`DBTimeChart`, `DBHeatmapChart`, `DBListBarChart`,
172-
…). The tile-level controls (fullscreen, line/bar display switcher, kebab menu)
173-
belong to dashboard tiles and are intentionally **not** part of `ChartCard`.
169+
`ChartCard` renders the card **chrome only**. The header divider is drawn only
170+
when a descendant renders a `ChartContainer` with a `title` (or `toolbarItems`) —
171+
`ChartCard` supplies the `ChartContainerCardHeaderProvider` that switches that
172+
header into card mode — so put a chart that renders a `ChartContainer` inside it
173+
(`DBTimeChart`, `DBTableChart`, `DBHeatmapChart`, `DBListBarChart`, …). Content
174+
with its own heading (e.g. a bespoke table card) should still route that heading
175+
through a titled `ChartContainer` rather than a bare `Text`, so it gets the same
176+
card header — divider and top padding included — instead of sitting flush against
177+
the top border. The tile-level controls (fullscreen, line/bar display switcher,
178+
kebab menu) belong to dashboard tiles and are intentionally **not** part of
179+
`ChartCard`.
174180

175181
| Prop | Type | Description |
176182
|------|------|-------------|
177-
| `children` | `ReactNode` | The chart, usually a `DB*Chart` that renders a `ChartContainer` |
178-
| `style` | `CSSProperties` | Sizing/overflow override — pass a fixed `height`, or `flex: 1; height: 100%` to fill a flex row |
183+
| `children` | `ReactNode` | The chart, usually a `DB*Chart` (or a titled `ChartContainer`) |
184+
| `style` | `CSSProperties` | Sizing/overflow override — pass a fixed `height`, or `flex: 1; height: 100%` to fill a flex row (`paddingInline` is pinned to keep the divider aligned) |
179185
| `data-testid` | `string` | Test hook |
180186

181187
```tsx

‎packages/app/src/ClickhousePage.tsx‎

Lines changed: 42 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import { TimePicker } from '@/components/TimePicker';
3939
import { withAppNav } from '@/layout';
4040

4141
import { ChartCard } from './components/charts/ChartCard';
42+
import ChartContainer from './components/charts/ChartContainer';
4243
import DBHeatmapChart from './components/DBHeatmapChart';
4344
import { DBSqlRowTable } from './components/DBRowTable';
4445
import DBTableChart from './components/DBTableChart';
@@ -796,59 +797,53 @@ function ClickhousePage() {
796797
</ChartCard>
797798
</Grid.Col>
798799
<Grid.Col span={12}>
799-
<ChartCard
800-
style={{
801-
height: 400,
802-
overflow: 'hidden',
803-
}}
804-
>
805-
<Text size="sm" mb="md">
806-
Slowest Queries
807-
</Text>
808-
<DBSqlRowTable
809-
renderRowDetails={row => {
810-
return (
811-
<ReactCodeMirror
812-
extensions={[clickhouseSql()]}
813-
editable={false}
814-
value={formatSql(String(row.query ?? ''))}
815-
theme={colorScheme === 'dark' ? 'dark' : 'light'}
816-
lang="sql"
817-
maxHeight="200px"
818-
/>
819-
);
820-
}}
821-
config={{
822-
select: `event_time, query_kind,
800+
<ChartCard style={{ height: 400 }}>
801+
<ChartContainer title="Slowest Queries">
802+
<DBSqlRowTable
803+
renderRowDetails={row => {
804+
return (
805+
<ReactCodeMirror
806+
extensions={[clickhouseSql()]}
807+
editable={false}
808+
value={formatSql(String(row.query ?? ''))}
809+
theme={colorScheme === 'dark' ? 'dark' : 'light'}
810+
lang="sql"
811+
maxHeight="200px"
812+
/>
813+
);
814+
}}
815+
config={{
816+
select: `event_time, query_kind,
823817
read_rows,
824818
formatReadableSize(memory_usage) as memory_usage,
825819
query_duration_ms,
826820
query`,
827-
dateRange: searchedTimeRange,
828-
from,
829-
where: `(
821+
dateRange: searchedTimeRange,
822+
from,
823+
where: `(
830824
type='ExceptionWhileProcessing' OR type='QueryFinish'
831825
)`,
832-
timestampValueExpression: 'event_time',
833-
connection,
834-
orderBy: [
835-
{
836-
valueExpression: 'query_duration_ms',
837-
ordering: 'DESC',
838-
},
839-
],
840-
filters: [
841-
...filters,
842-
{
843-
type: 'sql_ast',
844-
operator: '=',
845-
left: 'query_kind',
846-
right: `'Select'`,
847-
},
848-
],
849-
limit: { limit: 100 },
850-
}}
851-
/>
826+
timestampValueExpression: 'event_time',
827+
connection,
828+
orderBy: [
829+
{
830+
valueExpression: 'query_duration_ms',
831+
ordering: 'DESC',
832+
},
833+
],
834+
filters: [
835+
...filters,
836+
{
837+
type: 'sql_ast',
838+
operator: '=',
839+
left: 'query_kind',
840+
right: `'Select'`,
841+
},
842+
],
843+
limit: { limit: 100 },
844+
}}
845+
/>
846+
</ChartContainer>
852847
</ChartCard>
853848
</Grid.Col>
854849
</Grid>

‎packages/app/src/components/ServiceDashboardSlowestEventsTile.tsx‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@ import {
44
pickSampleWeightExpressionProps,
55
type TTraceSource,
66
} from '@hyperdx/common-utils/dist/types';
7-
import { Group, Text } from '@mantine/core';
7+
import { Text } from '@mantine/core';
88

99
import { ChartCard } from '@/components/charts/ChartCard';
1010
import { useQueriedChartConfig } from '@/hooks/useChartConfig';
1111
import { useServiceDashboardExpressions } from '@/serviceDashboard';
1212

13+
import ChartContainer from './charts/ChartContainer';
1314
import ChartErrorState from './charts/ChartErrorState';
1415
import DBSqlRowTableWithSideBar from './DBSqlRowTableWithSidebar';
1516

@@ -67,24 +68,27 @@ export default function SlowestEventsTile({
6768

6869
return (
6970
<ChartCard style={{ height }}>
70-
<Group justify="space-between" align="center" mb="sm">
71-
<Text size="sm">{title}</Text>
72-
<Text size="xs">(Slower than {roundedP95}ms)</Text>
73-
</Group>
74-
{isLoading && !data ? (
75-
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
76-
Loading Chart Data...
77-
</div>
78-
) : isError ? (
79-
<ChartErrorState error={error} />
80-
) : data?.data.length === 0 ? (
81-
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
82-
No data found within time range.
83-
</div>
84-
) : (
85-
source &&
86-
expressions && (
87-
<>
71+
<ChartContainer
72+
title={<Text size="sm">{title}</Text>}
73+
toolbarItems={[
74+
<Text key="slower-than" size="xs">
75+
(Slower than {roundedP95}ms)
76+
</Text>,
77+
]}
78+
>
79+
{isLoading && !data ? (
80+
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
81+
Loading Chart Data...
82+
</div>
83+
) : isError ? (
84+
<ChartErrorState error={error} />
85+
) : data?.data.length === 0 ? (
86+
<div className="d-flex h-100 w-100 align-items-center justify-content-center text-muted">
87+
No data found within time range.
88+
</div>
89+
) : (
90+
source &&
91+
expressions && (
8892
<DBSqlRowTableWithSideBar
8993
sourceId={source.id}
9094
config={{
@@ -134,9 +138,9 @@ export default function SlowestEventsTile({
134138
isLive={false}
135139
queryKeyPrefix="service-dashboard-slowest-transactions"
136140
/>
137-
</>
138-
)
139-
)}
141+
)
142+
)}
143+
</ChartContainer>
140144
</ChartCard>
141145
);
142146
}

‎packages/app/src/components/charts/ChartCard.stories.tsx‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React from 'react';
2-
import { Box, Flex, SegmentedControl } from '@mantine/core';
2+
import { Box, Flex, SegmentedControl, Text } from '@mantine/core';
33
import type { Meta } from '@storybook/nextjs';
44

55
import { ChartCard } from './ChartCard';
@@ -119,14 +119,18 @@ export const RedRow = () => {
119119
};
120120

121121
/**
122-
* With no title and no toolbar the header row is omitted entirely, so the card
123-
* degrades to a plain bordered surface (no divider).
122+
* Non-chart content (e.g. a table card) still routes its heading through a
123+
* titled `ChartContainer` so it gets the same card header — divider included —
124+
* as the chart cards above, instead of a bare `Text` that would sit flush
125+
* against the top border with no divider.
124126
*/
125-
export const NoHeader = () => (
127+
export const TableCard = () => (
126128
<Box style={{ width: 480, height: 260 }}>
127129
<ChartCard>
128-
<ChartContainer>
129-
<FakeChart color="var(--color-chart-green)" />
130+
<ChartContainer title="Slowest Queries">
131+
<Text size="sm" c="dimmed" p="sm">
132+
table rows…
133+
</Text>
130134
</ChartContainer>
131135
</ChartCard>
132136
</Box>

‎packages/app/src/components/charts/ChartCard.tsx‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,52 @@
1-
import { Box, BoxComponentProps } from '@mantine/core';
1+
import { Box } from '@mantine/core';
22

33
import {
44
ChartContainerCardHeaderProvider,
55
DASHBOARD_TILE_PADDING_INLINE,
66
} from './ChartContainer';
77

8+
export interface ChartCardProps {
9+
children: React.ReactNode;
10+
/**
11+
* Sizing/overflow override merged over the card defaults. Kept as plain
12+
* `CSSProperties` (not Mantine's `MantineStyleProp`) because the defaults are
13+
* object-spread — a resolver function or style array would be silently
14+
* dropped. `paddingInline` is intentionally not honored here (see below).
15+
*/
16+
style?: React.CSSProperties;
17+
'data-testid'?: string;
18+
}
19+
820
/**
921
* Card wrapper that gives a chart the same look as a tile on a custom
1022
* dashboard: a bordered, rounded box whose chart header renders in "card mode"
1123
* (centered title row + a full-bleed bottom divider under the title).
1224
*
1325
* Not dashboard-specific — use it anywhere a chart should read as a card (search
14-
* page, service dashboards, the ClickHouse page, etc.). It replaces the older
15-
* plain `ChartBox` so these surfaces share one consistent card treatment.
26+
* page, service dashboards, the ClickHouse page, etc.).
27+
*
28+
* The header divider is only drawn when a descendant renders a `ChartContainer`
29+
* with a `title` or `toolbarItems` (e.g. `DBTimeChart`, `DBTableChart`); this
30+
* component just provides the context that switches that header into card mode.
31+
* Content with its own heading should render it through a titled
32+
* `ChartContainer` (rather than a bare `Text`) so it gets the same card header —
33+
* divider included — and the top padding the header supplies.
1634
*
1735
* This is the *visual* chrome only. The dashboard tile's toolbar controls
1836
* (fullscreen, line/bar display switcher, kebab menu) live on the tile itself
1937
* and are intentionally not included here.
2038
*
2139
* Keep the horizontal padding equal to DASHBOARD_TILE_PADDING_INLINE: the card
2240
* header's separator is drawn full-bleed by cancelling exactly this inset, so
23-
* any other value would leave the divider misaligned with the card edges.
41+
* any other value would leave the divider misaligned with the card edges. It is
42+
* re-applied after the caller `style` so an override can't silently break the
43+
* divider alignment.
2444
*/
2545
export function ChartCard({
2646
children,
2747
style,
2848
'data-testid': dataTestId,
29-
}: {
30-
children: React.ReactNode;
31-
style?: BoxComponentProps['style'];
32-
'data-testid'?: string;
33-
}) {
49+
}: ChartCardProps) {
3450
return (
3551
<Box
3652
data-testid={dataTestId}
@@ -47,8 +63,9 @@ export function ChartCard({
4763
// inset so the header divider bleeds to the card edges.
4864
paddingTop: 0,
4965
paddingBottom: 'var(--mantine-spacing-xs)',
50-
paddingInline: DASHBOARD_TILE_PADDING_INLINE,
5166
...style,
67+
// Enforce the full-bleed divider invariant regardless of caller style.
68+
paddingInline: DASHBOARD_TILE_PADDING_INLINE,
5269
}}
5370
>
5471
<ChartContainerCardHeaderProvider>
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import { screen } from '@testing-library/react';
2+
3+
import { ChartCard } from '@/components/charts/ChartCard';
4+
import ChartContainer from '@/components/charts/ChartContainer';
5+
6+
// The header row (the Group wrapping the title) gets a bottom border only in
7+
// "card mode", which ChartCard turns on via its context provider.
8+
function headerRowFor(title: string) {
9+
return screen.getByText(title).parentElement as HTMLElement;
10+
}
11+
12+
describe('ChartCard', () => {
13+
it('puts a nested titled ChartContainer into card mode so it draws the divider', () => {
14+
renderWithMantine(
15+
<ChartCard>
16+
<ChartContainer title="Throughput">
17+
<div>chart</div>
18+
</ChartContainer>
19+
</ChartCard>,
20+
);
21+
22+
expect(headerRowFor('Throughput').style.borderBottom).toBe(
23+
'1px solid var(--color-border)',
24+
);
25+
});
26+
27+
it('leaves a standalone ChartContainer header plain (no divider)', () => {
28+
renderWithMantine(
29+
<ChartContainer title="Throughput">
30+
<div>chart</div>
31+
</ChartContainer>,
32+
);
33+
34+
expect(headerRowFor('Throughput').style.borderBottom).toBe('');
35+
});
36+
37+
it('merges a caller style over the card defaults', () => {
38+
renderWithMantine(
39+
<ChartCard style={{ height: 321 }} data-testid="card">
40+
<div>chart</div>
41+
</ChartCard>,
42+
);
43+
44+
expect(screen.getByTestId('card').style.height).toBe('321px');
45+
});
46+
47+
it('re-applies the divider padding invariant even if a caller overrides it', () => {
48+
renderWithMantine(
49+
<ChartCard style={{ paddingInline: 999 }} data-testid="card">
50+
<div>chart</div>
51+
</ChartCard>,
52+
);
53+
54+
// paddingInline is pinned to the tile inset regardless of caller style so
55+
// the full-bleed header divider stays aligned with the card edges.
56+
expect(screen.getByTestId('card').style.paddingInline).not.toBe('999px');
57+
});
58+
});

0 commit comments

Comments
 (0)