diff --git a/.changeset/number-tile-format-survives-apply.md b/.changeset/number-tile-format-survives-apply.md new file mode 100644 index 0000000000..709f98becd --- /dev/null +++ b/.changeset/number-tile-format-survives-apply.md @@ -0,0 +1,12 @@ +--- +"@hyperdx/app": patch +--- + +fix(dashboards): keep the auto-detected number format when applying display settings + +Opening Display Settings on a number tile that auto-detects its format from the +datasource (for example p95 of a trace Duration column) and clicking Apply no +longer rewrites the format to Number. The drawer now reflects the +datasource-derived format, and Apply persists `numberFormat` only when the user +explicitly changes it; otherwise it stays unset so render-time auto-detection +keeps driving the format. diff --git a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx index 1e7b2bbc31..6c6cab067c 100644 --- a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx +++ b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx @@ -117,7 +117,13 @@ export default function ChartDisplaySettingsDrawer({ [settings, defaultNumberFormat], ); - const { control, handleSubmit, reset, setValue } = useForm({ + const { + control, + handleSubmit, + reset, + setValue, + formState: { dirtyFields }, + } = useForm({ defaultValues: appliedDefaults, }); @@ -137,13 +143,23 @@ export default function ChartDisplaySettingsDrawer({ handleSubmit(formValues => { // Strip client-side localIds before passing rules to the config. const { colorRules, ...rest } = formValues; + // Persist numberFormat only when the user actually chose one: either the + // tile already had an explicit override (settings.numberFormat) or the + // user changed the format control in this session (dirtyFields). Otherwise + // emit undefined so the datasource-derived format keeps driving render + // instead of freezing the drawer's inferred fallback into the config. + const numberFormatExplicit = + settings.numberFormat != null || dirtyFields.numberFormat != null; onChange({ ...rest, + numberFormat: numberFormatExplicit + ? formValues.numberFormat + : undefined, colorRules: colorRules ? stripLocalIds(colorRules) : undefined, }); })(); onClose(); - }, [onChange, handleSubmit, onClose]); + }, [onChange, handleSubmit, onClose, settings.numberFormat, dirtyFields]); const resetToDefaults = useCallback(() => { reset( diff --git a/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx b/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx index 8662972fcd..e31ae52c3b 100644 --- a/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx @@ -196,6 +196,7 @@ export default function EditTimeChartForm({ }, [isDirty, onDirtyChange]); const select = useWatch({ control, name: 'select' }); + const series = useWatch({ control, name: 'series' }); const sourceId = useWatch({ control, name: 'source' }); const alert = useWatch({ control, name: 'alert' }); const seriesReturnType = useWatch({ control, name: 'seriesReturnType' }); @@ -263,13 +264,18 @@ export default function EditTimeChartForm({ ], }); + // Format auto-detected purely from the datasource (e.g. duration for a trace + // Duration column), used as the drawer's fallback when no explicit + // numberFormat is set. Reads the live `series`, the field the builder edits; + // `select` is only synced from `series` on submit and on display-type / source + // resets, so it goes stale after an aggregation edit and resolves undefined. + // The drawer prioritizes an explicit `numberFormat` over this fallback. const autoDetectedNumberFormat = useMemo( () => - numberFormat ?? - (Array.isArray(select) - ? getFirstSeriesNumberFormat(select, tableSource) - : undefined), - [numberFormat, select, tableSource], + Array.isArray(series) + ? getFirstSeriesNumberFormat(series, tableSource) + : undefined, + [series, tableSource], ); const displaySettings: ChartConfigDisplaySettings = useMemo( @@ -592,7 +598,12 @@ export default function EditTimeChartForm({ colorRules, backgroundChart, }: ChartConfigDisplaySettings) => { - setValue('numberFormat', numberFormat); + // Only persist an explicit numberFormat. When the drawer emits undefined + // (the user never chose a format), leave it unset so render-time + // auto-detection keeps driving the format from the datasource. + if (numberFormat !== undefined) { + setValue('numberFormat', numberFormat); + } setValue('alignDateRangeToGranularity', alignDateRangeToGranularity); setValue('fillNulls', fillNulls); setValue('compareToPreviousPeriod', compareToPreviousPeriod); diff --git a/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx b/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx index fa388de29f..365cc5cba7 100644 --- a/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx +++ b/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx @@ -263,4 +263,89 @@ describe('ChartDisplaySettingsDrawer', () => { expect(onChange.mock.calls[0][0].seriesLimit).toBeNull(); }); }); + + describe('number format persistence', () => { + // A duration number tile (e.g. p95 Duration from a trace source) auto-detects + // a duration format from the datasource; the drawer receives it as + // `defaultNumberFormat` and shows it as the fallback when no explicit + // numberFormat is set. + const durationFormat = { output: 'duration' as const, factor: 1e-9 }; + const numberBuilderProps = { + ...baseProps, + configType: 'builder' as const, + displayType: DisplayType.Number, + }; + + it('does not persist the auto-detected format when Apply is clicked without changing it', async () => { + const onChange = jest.fn(); + const user = userEvent.setup(); + + renderWithMantine( + , + ); + + await user.click(screen.getByRole('button', { name: /apply/i })); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange.mock.calls[0][0].numberFormat).toBeUndefined(); + }); + + it('persists the format when the user changes the output format', async () => { + const onChange = jest.fn(); + const user = userEvent.setup(); + + renderWithMantine( + , + ); + + await user.selectOptions( + screen.getByRole('combobox', { name: /output format/i }), + 'number', + ); + await user.click(screen.getByRole('button', { name: /apply/i })); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange.mock.calls[0][0].numberFormat).toMatchObject({ + output: 'number', + }); + }); + + it('preserves an existing explicit format when only another setting changes', async () => { + const onChange = jest.fn(); + const user = userEvent.setup(); + + renderWithMantine( + , + ); + + // Change the tile color, not the format. + await user.click(screen.getByTestId('color-swatch-input-trigger')); + await user.click( + await screen.findByTestId('color-swatch-option-chart-blue'), + ); + await user.click(screen.getByRole('button', { name: /apply/i })); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange.mock.calls[0][0]).toMatchObject({ + color: 'chart-blue', + numberFormat: { output: 'currency' }, + }); + }); + }); });