Skip to content

Commit c4c2287

Browse files
committed
fix: Persist timeseries sub display type selection from editor
1 parent e87660a commit c4c2287

11 files changed

Lines changed: 117 additions & 18 deletions

File tree

‎packages/app/src/HDXMultiSeriesTimeChart.tsx‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import { AxisDomain } from 'recharts/types/util/types';
3232
import { convertGranularityToSeconds } from '@hyperdx/common-utils/dist/core/utils';
3333
import {
3434
DisplayType,
35+
getDefaultLineInterpolation,
3536
LineInterpolation,
3637
} from '@hyperdx/common-utils/dist/types';
3738
import { Popover } from '@mantine/core';
@@ -1158,16 +1159,13 @@ const CURVE_TYPE_BY_INTERPOLATION: Record<LineInterpolation, CurveType> = {
11581159
step: 'stepAfter',
11591160
};
11601161

1161-
// Monotone curves fit each stacked edge independently, so adjacent edges can
1162-
// cross between points and the bands overlap; linear segments stay ordered
1163-
// wherever the points are. Hence stacked lines default to linear.
11641162
export function getLineCurveType(
11651163
displayType: DisplayType,
11661164
lineInterpolation: LineInterpolation | undefined,
11671165
): CurveType {
1168-
const fallback =
1169-
displayType === DisplayType.StackedLine ? 'linear' : 'smooth';
1170-
return CURVE_TYPE_BY_INTERPOLATION[lineInterpolation ?? fallback];
1166+
return CURVE_TYPE_BY_INTERPOLATION[
1167+
lineInterpolation ?? getDefaultLineInterpolation(displayType)
1168+
];
11711169
}
11721170

11731171
// A stacked bar's rendered height sums its series at each timestamp - leave

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { isTimeSeriesDisplayType } from '@hyperdx/common-utils/dist/core/utils';
88
import {
99
ChartConfigWithDateRange,
1010
DisplayType,
11+
getDefaultLineInterpolation,
1112
LineInterpolation,
1213
LineInterpolationSchema,
1314
MAX_LEGEND_TEMPLATE_LENGTH,
@@ -232,8 +233,7 @@ export default function ChartDisplaySettingsDrawer({
232233

233234
const showLineInterpolation =
234235
displayType === DisplayType.Line || displayType === DisplayType.StackedLine;
235-
const defaultLineInterpolation =
236-
displayType === DisplayType.StackedLine ? 'linear' : 'smooth';
236+
const defaultLineInterpolation = getDefaultLineInterpolation(displayType);
237237

238238
// Series Limit applies to every time chart. On builder group-by charts a
239239
// positive value drives the __hdx_series_limit SQL CTE (trimming what's

‎packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ type ChartPreviewPanelProps = {
179179
/** Required dashboard filters with nothing selected that block this tile. */
180180
missingRequiredFilterNames?: string[];
181181
setValue: (name: 'orderBy', value: string) => void;
182+
setDisplayType: (displayType: DisplayType) => void;
182183
onSubmit: () => void;
183184
};
184185

@@ -197,6 +198,7 @@ export function ChartPreviewPanel({
197198
dbTimeChartConfig,
198199
missingRequiredFilterNames,
199200
setValue,
201+
setDisplayType,
200202
onSubmit,
201203
}: ChartPreviewPanelProps) {
202204
const [isSampleEventsOpen, setIsSampleEventsOpen] = useState(false);
@@ -305,6 +307,7 @@ export function ChartPreviewPanel({
305307
<DBTimeChart
306308
sourceId={sourceId}
307309
config={dbTimeChartConfig}
310+
setDisplayType={setDisplayType}
308311
onTimeRangeSelect={onTimeRangeSelect}
309312
referenceLines={
310313
alert &&

‎packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { isReducibleRangeQuery } from '@hyperdx/common-utils/dist/core/promql';
1515
import {
1616
displayTypeSupportsBuilderAlerts,
1717
displayTypeSupportsRawSqlAlerts,
18+
isTimeSeriesDisplayType,
1819
} from '@hyperdx/common-utils/dist/core/utils';
1920
import {
2021
displayTypeRequiresSource,
@@ -1168,8 +1169,16 @@ export default function EditTimeChartForm({
11681169
name="displayType"
11691170
render={({ field: { onChange, value } }) => (
11701171
<Tabs
1171-
value={value}
1172-
onChange={onChange}
1172+
// Stacked variants are picked from the preview's display
1173+
// switcher and live under the Time Series tab.
1174+
value={
1175+
isTimeSeriesDisplayType(value) ? DisplayType.Line : value
1176+
}
1177+
onChange={v => {
1178+
if (v === DisplayType.Line && isTimeSeriesDisplayType(value))
1179+
return;
1180+
onChange(v);
1181+
}}
11731182
radius={'xs'}
11741183
mb="md"
11751184
data-testid="chart-type-input"
@@ -1352,6 +1361,9 @@ export default function EditTimeChartForm({
13521361
previewDashboardFilters?.missingRequiredFilterNames
13531362
}
13541363
setValue={(name, value) => setValue(name, value)}
1364+
setDisplayType={type =>
1365+
setValue('displayType', type, { shouldDirty: true })
1366+
}
13551367
onSubmit={onSubmit}
13561368
/>
13571369
<SaveToDashboardModal

‎packages/app/src/components/DBEditTimeChartForm/__tests__/ChartPreviewPanel.test.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ const renderPanel = (
114114
showSampleEvents={false}
115115
showGeneratedPromql={false}
116116
setValue={jest.fn()}
117+
setDisplayType={jest.fn()}
117118
onSubmit={jest.fn()}
118119
{...overrides}
119120
/>,

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -646,12 +646,20 @@ function DBTimeChartComponent({
646646
const [displayTypeLocal, setDisplayTypeLocal] = useState(displayTypeProp);
647647

648648
const displayType = useMemo(() => {
649-
if (setDisplayType) {
649+
// Stacking would sum the current and previous periods together.
650+
if (config.compareToPreviousPeriod) {
651+
return DisplayType.Line;
652+
} else if (setDisplayType) {
650653
return displayTypeProp;
651654
} else {
652655
return displayTypeLocal;
653656
}
654-
}, [displayTypeLocal, displayTypeProp, setDisplayType]);
657+
}, [
658+
config.compareToPreviousPeriod,
659+
displayTypeLocal,
660+
displayTypeProp,
661+
setDisplayType,
662+
]);
655663

656664
const handleSetDisplayType = useCallback(
657665
(type: DisplayType) => {
@@ -664,12 +672,6 @@ function DBTimeChartComponent({
664672
[setDisplayType],
665673
);
666674

667-
useEffect(() => {
668-
if (config.compareToPreviousPeriod) {
669-
setDisplayTypeLocal(DisplayType.Line);
670-
}
671-
}, [config.compareToPreviousPeriod]);
672-
673675
const [activeClickPayload, setActiveClickPayload] = useState<
674676
ActiveClickPayload | undefined
675677
>(undefined);

‎packages/app/src/components/__tests__/DBTimeChart.test.tsx‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import React from 'react';
2+
import { DisplayType } from '@hyperdx/common-utils/dist/types';
23
import { screen, waitFor } from '@testing-library/react';
34
import userEvent from '@testing-library/user-event';
45

@@ -219,6 +220,29 @@ describe('DBTimeChart', () => {
219220
expect(options?.enabled).toBe(true);
220221
});
221222

223+
it.each([
224+
['controlled', jest.fn()],
225+
['uncontrolled', undefined],
226+
])(
227+
'renders a stacked chart as lines when comparing to the previous period (%s)',
228+
(_, setDisplayType) => {
229+
renderWithMantine(
230+
<DBTimeChart
231+
config={{
232+
...baseTestConfig,
233+
displayType: DisplayType.StackedBar,
234+
compareToPreviousPeriod: true,
235+
}}
236+
setDisplayType={setDisplayType}
237+
/>,
238+
);
239+
240+
expect(
241+
screen.getByTestId(`display-switcher-${DisplayType.Line}`),
242+
).toHaveAttribute('data-active', 'true');
243+
},
244+
);
245+
222246
describe('load-all series escape hatch', () => {
223247
// A high-cardinality group-by response: MAX_RENDERED_TIME_CHART_SERIES (250)
224248
// default cap + 50 extra groups, so 50 series are hidden and the

‎packages/app/tests/e2e/components/ChartEditorComponent.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1255,6 +1255,11 @@ export class ChartEditorComponent {
12551255
await this.applyDisplaySettings();
12561256
}
12571257

1258+
/** Switch line / stacked bar / stacked line from the preview's toolbar. */
1259+
async setPreviewDisplayType(type: 'line' | 'stacked_bar' | 'stacked_line') {
1260+
await this.editorForm().getByTestId(`display-switcher-${type}`).click();
1261+
}
1262+
12581263
/** A "Line interpolation" option's (visually hidden) radio in the open drawer. */
12591264
lineInterpolationOption(mode: 'Smooth' | 'Linear' | 'Step'): Locator {
12601265
return this.page

‎packages/app/tests/e2e/features/dashboard.spec.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,17 @@ test.describe('Dashboard', { tag: ['@dashboard'] }, () => {
334334
});
335335
});
336336

337+
test('should warn when closing tile editor after changing only the chart type', async () => {
338+
await dashboardPage.openNewTileEditor();
339+
await dashboardPage.chartEditor.waitForDataToLoad();
340+
await dashboardPage.chartEditor.setPreviewDisplayType('stacked_bar');
341+
342+
await dashboardPage.page.keyboard.press('Escape');
343+
await expect(dashboardPage.unsavedChangesConfirmModal).toBeAttached({
344+
timeout: 5000,
345+
});
346+
});
347+
337348
test('should warn when closing tile editor with unsaved display settings changes', async () => {
338349
await dashboardPage.openNewTileEditor();
339350

‎packages/app/tests/e2e/features/line-interpolation.spec.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,38 @@ test.describe('Line interpolation', { tag: ['@dashboard'] }, () => {
4343
await expect(editor.lineInterpolationOption('Step')).toBeChecked();
4444
});
4545
});
46+
47+
test('stacked line picked in the editor saves and defaults to linear', async ({
48+
page,
49+
}) => {
50+
const dashboardPage = new DashboardPage(page);
51+
const editor = dashboardPage.chartEditor;
52+
53+
await dashboardPage.goto();
54+
await dashboardPage.createNewDashboard();
55+
await dashboardPage.addTile();
56+
await expect(editor.nameInput).toBeVisible();
57+
await editor.waitForDataToLoad();
58+
await editor.setChartName('Stacked line tile');
59+
await editor.setPreviewDisplayType('stacked_line');
60+
61+
await editor.openDisplaySettings();
62+
await expect(editor.lineInterpolationOption('Linear')).toBeChecked();
63+
await editor.applyDisplaySettings();
64+
65+
await editor.save();
66+
await expect(dashboardPage.getTiles()).toHaveCount(1, { timeout: 10000 });
67+
await expect(
68+
dashboardPage
69+
.getTiles()
70+
.first()
71+
.getByTestId('display-switcher-stacked_line'),
72+
).toHaveAttribute('data-active', 'true');
73+
74+
await dashboardPage.editTile(0);
75+
await expect(editor.nameInput).toBeVisible();
76+
await editor.setPreviewDisplayType('stacked_bar');
77+
await editor.openDisplaySettings();
78+
await expect(page.getByTestId('line-interpolation-control')).toBeHidden();
79+
});
4680
});

0 commit comments

Comments
 (0)