Skip to content

Commit 93e0631

Browse files
committed
Better preserve the chart config when the query gets changed
1 parent a501bcf commit 93e0631

File tree

2 files changed

+33
-9
lines changed

2 files changed

+33
-9
lines changed

apps/webapp/app/components/code/ChartConfigPanel.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,11 @@ export function ChartConfigPanel({ columns, config, onChange, className }: Chart
147147
if (needsUpdate) {
148148
onChangeRef.current({ ...currentConfig, ...updates });
149149
}
150-
// Only re-run when the actual column structure changes, not on every config change
151-
}, [columnsKey, columns, dateTimeColumns, categoricalColumns, numericColumns]);
150+
// Only re-run when the actual column structure changes, not on every config change.
151+
// columnsKey (a string) is stable when columns match, so this won't re-fire
152+
// unnecessarily when the same query is re-run with identical columns.
153+
// eslint-disable-next-line react-hooks/exhaustive-deps
154+
}, [columnsKey]);
152155

153156
const updateConfig = useCallback(
154157
(updates: Partial<ChartConfiguration>) => {

apps/webapp/app/components/query/QueryEditor.tsx

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -506,18 +506,39 @@ export function QueryEditor({
506506

507507
const isLoading = fetcher.state === "submitting" || fetcher.state === "loading";
508508

509-
// Create a stable key from columns to detect schema changes
510-
const columnsKey = results?.columns
511-
? results.columns.map((c) => `${c.name}:${c.type}`).join(",")
509+
// Stable string key of result column names (types excluded — they can vary between runs)
510+
const columnNamesKey = results?.columns
511+
? results.columns.map((c) => c.name).join(",")
512512
: "";
513513

514-
// Reset chart config only when column schema actually changes
515-
// This allows re-running queries with different WHERE clauses without losing config
514+
// Use a ref so the effect can read chartConfig without re-firing on every config tweak
515+
const chartConfigRef = useRef(chartConfig);
516+
chartConfigRef.current = chartConfig;
517+
518+
// Reset chart config only when a column referenced by the current config is no
519+
// longer present in the results. This means:
520+
// - Re-running the same query preserves all settings
521+
// - Adding new columns preserves settings (existing config columns still exist)
522+
// - Removing/renaming a column used in the config triggers a reset
516523
useEffect(() => {
517-
if (columnsKey && !initialChartConfig) {
524+
if (!columnNamesKey || initialChartConfig) return;
525+
526+
const config = chartConfigRef.current;
527+
const configColumns = [
528+
config.xAxisColumn,
529+
...config.yAxisColumns,
530+
config.groupByColumn,
531+
config.sortByColumn,
532+
].filter((col): col is string => col != null);
533+
534+
// Nothing configured yet — ChartConfigPanel will auto-select defaults
535+
if (configColumns.length === 0) return;
536+
537+
const availableColumns = new Set(columnNamesKey.split(","));
538+
if (configColumns.some((col) => !availableColumns.has(col))) {
518539
setChartConfig(defaultChartConfig);
519540
}
520-
}, [columnsKey, initialChartConfig]);
541+
}, [columnNamesKey, initialChartConfig]);
521542

522543
const handleChartConfigChange = useCallback((config: ChartConfiguration) => {
523544
setChartConfig(config);

0 commit comments

Comments
 (0)