From 34696af55e390c37911099c9554ba7a873c007f6 Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 20 Nov 2025 13:06:38 -0600 Subject: [PATCH 01/38] Add ChartSettingsPanel.tsx --- .../components/chart/ChartSettingsPanel.tsx | 228 ++++++++++++++++++ 1 file changed, 228 insertions(+) create mode 100644 packages/components/src/internal/components/chart/ChartSettingsPanel.tsx diff --git a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx new file mode 100644 index 0000000000..c141565d2c --- /dev/null +++ b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx @@ -0,0 +1,228 @@ +import React, { ChangeEvent, FC, memo, useCallback, useMemo } from 'react'; +import { BaseChartModel, BaseChartModelSetter, ChartConfig, ChartConfigSetter, ChartTypeInfo } from './models'; +import { LABKEY_VIS } from '../../constants'; +import { HIDDEN_CHART_TYPES, ICONS } from './constants'; +import { SelectInput, SelectInputOption } from '../forms/input/SelectInput'; +import { SVGIcon } from '../base/SVGIcon'; +import { CheckboxLK } from '../../Checkbox'; +import { ChartFieldOption } from './ChartFieldOption'; +import { QueryModel } from '../../../public/QueryModel/QueryModel'; +import { TrendlineOption } from './TrendlineOption'; +import { deepCopyChartConfig, hasTrendline } from './utils'; + +type LabelKey = keyof ChartConfig['labels']; + +interface LabelInputProps { + label: string; + name: LabelKey; + onChange: (name: LabelKey, value: string) => void; + value: string; +} + +const LabelInput: FC = memo(({ label, name, onChange, value }) => { + const onChange_ = useCallback( + (event: ChangeEvent) => onChange(name, event.target.value), + [name, onChange] + ); + return ( +
+ + +
+ ); +}); +LabelInput.displayName = 'LabelInput'; + +interface BoolSettingInputProps { + label: string; + name: string; + onChange: (name: LabelKey, value: boolean) => void; + value: boolean; +} +const BoolSettingInput: FC = memo(({ label, name, onChange, value }) => { + const onChange_ = useCallback( + (event: ChangeEvent) => { + onChange(name, event.target.checked); + }, + [name, onChange] + ); + return ( + + {label} + + ); +}); +BoolSettingInput.displayName = 'BoolSettingInput'; + +// TODO: use the same icons as the ChartMenu +// - Not the most straightforward because those icons are set server-side +function chartTypeOptionRenderer(option: SelectInputOption) { + const data = option.data.data; + return ( +
+ + {data.title} +
+ ); +} + +interface ChartTypeDropdownProps { + onChange: (chartTypeInfo: ChartTypeInfo) => void; + selectedType: ChartTypeInfo; +} + +const ChartTypeDropdown: FC = memo(({ onChange, selectedType }) => { + const chartTypes = useMemo(() => { + const allTypes = LABKEY_VIS?.GenericChartHelper.getRenderTypes(); + return allTypes + .filter(type => !type.hidden && !HIDDEN_CHART_TYPES.includes(type.name)) + .map(type => ({ + data: type, + label: type.title, + value: type.name, + })); + }, []); + const onChange_ = useCallback( + (_, __, opt) => { + onChange(opt.data); + }, + [onChange] + ); + + return ( +
+ +
+ +
+
+ ); +}); +ChartTypeDropdown.displayName = 'ChartTypeDropdown'; + +interface Props { + allowInherit: boolean; + canShare: boolean; + chartConfig: ChartConfig; + chartModel: BaseChartModel; + chartType: ChartTypeInfo; + model: QueryModel; + setChartConfig: ChartConfigSetter; + setChartModel: BaseChartModelSetter; +} + +export const ChartSettingsPanel: FC = memo(props => { + const { allowInherit, canShare, chartConfig, chartType, chartModel, model, setChartConfig, setChartModel } = props; + const showTrendline = hasTrendline(chartType); + const fields = chartType.fields.filter(f => f.name !== 'trendline'); + + const onChartModelChange = useCallback( + (key: keyof BaseChartModel, value: boolean | string) => { + setChartModel(current => ({ ...current, [key]: value })); + }, + [setChartModel] + ); + + const onNameChange = useCallback( + (event: ChangeEvent) => { + onChartModelChange('name', event.target.value); + }, + [onChartModelChange] + ); + + const onTypeChange = useCallback( + (type: ChartTypeInfo) => { + setChartConfig(current => ({ + ...deepCopyChartConfig(undefined), + labels: { ...current.labels }, + renderType: type.name, + })); + }, + [setChartConfig] + ); + + const onLabelChange = useCallback( + (key: LabelKey, value: string) => { + setChartConfig(current => ({ ...current, labels: { ...current.labels, [key]: value } })); + }, + [setChartConfig] + ); + + return ( +
+

Settings

+
+ + +
+ + {canShare && ( + + )} + + {allowInherit && ( + + )} + + {!chartModel && } + + {fields.map(field => ( + + ))} + + {showTrendline && ( + + )} + +

Customize

+ + + {/* TODO: width/height */} +
+ ); +}); +ChartSettingsPanel.displayName = 'ChartSettingsPanel'; From 67915e187f6c107ce54336af55923819bec48e82 Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 20 Nov 2025 13:08:23 -0600 Subject: [PATCH 02/38] Update ChartFieldAggregateOptions, ChartFieldOption to use setChartConfig --- .../chart/ChartFieldAggregateOptions.tsx | 60 ++++++------ .../components/chart/ChartFieldOption.tsx | 95 ++++++++++++------- 2 files changed, 88 insertions(+), 67 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.tsx b/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.tsx index bc61847f5c..462d2db0e6 100644 --- a/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.tsx @@ -4,11 +4,12 @@ import { Popover } from '../../Popover'; import { RadioGroupInput } from '../forms/input/RadioGroupInput'; import { BAR_CHART_AGGREGATE_NAME, BAR_CHART_ERROR_BAR_NAME } from './constants'; -import { ChartFieldInfo, ChartTypeInfo } from './models'; +import { ChartConfig, ChartConfigSetter, ChartFieldInfo, ChartTypeInfo } from './models'; import { LabelOverlay } from '../forms/LabelOverlay'; -import { SelectInput, SelectInputOption } from '../forms/input/SelectInput'; +import { SelectInput } from '../forms/input/SelectInput'; +import { Utils } from '@labkey/api'; -const BAR_CHART_AGGREGATE_METHODS = [ +const AGGREGATE_METHODS = [ { label: 'None', value: '' }, { label: 'Count (non-blank)', value: 'COUNT' }, { label: 'Sum', value: 'SUM' }, @@ -29,29 +30,27 @@ const ERROR_BAR_TYPES = [ ]; interface OwnProps { - asOverlay?: boolean; + asOverlay?: boolean; // TODO: defaults to true, but is always passed as false. Can be removed? + chartConfig: ChartConfig; field: ChartFieldInfo; - fieldValues: Record; - onErrorBarChange: (name: string, value: string) => void; - onSelectFieldChange: (name: string, value: string, selectedOption: SelectInputOption) => void; selectedType: ChartTypeInfo; + setChartConfig: ChartConfigSetter; } export const ChartFieldAggregateOptions: FC = memo(props => { - const { field, fieldValues, onSelectFieldChange, onErrorBarChange, asOverlay = true, selectedType } = props; - const fieldValue = fieldValues?.[field.name]; - const aggregateValue = fieldValues?.[BAR_CHART_AGGREGATE_NAME]?.value; - const errorBarValue = fieldValues?.[BAR_CHART_ERROR_BAR_NAME]?.value; + const { asOverlay = true, chartConfig, field, selectedType, setChartConfig } = props; + const yMeasure = Array.isArray(chartConfig.measures.y) ? chartConfig.measures.y[0] : chartConfig.measures.y; + // Some older charts stored aggregate as an object that looked like: { label: 'Mean', value: 'MEAN' } + const aggregateValue = Utils.isObject(yMeasure.aggregate) ? yMeasure.aggregate.value : yMeasure.aggregate; + const errorBarValue = yMeasure.errorBars; const includeNone = selectedType.name === 'line_plot'; const includeCount = selectedType.name === 'bar_chart'; - const defaultAggregateValue = useMemo(() => (includeNone ? '' : 'SUM'), [includeNone]); - const errorBarRadioEnabled = useMemo(() => aggregateValue === 'MEAN', [aggregateValue]); + const defaultAggregateValue = includeNone ? '' : 'SUM'; + const errorBarRadioEnabled = aggregateValue === 'MEAN'; const aggregateOptions = useMemo(() => { - const options = BAR_CHART_AGGREGATE_METHODS.filter(option => { - if (option.value === 'COUNT' && !includeCount) { - return false; - } + const options = AGGREGATE_METHODS.filter(option => { + if (option.value === 'COUNT' && !includeCount) return false; return !(option.value === '' && !includeNone); }); @@ -66,24 +65,21 @@ export const ChartFieldAggregateOptions: FC = memo(props => { })); }, [errorBarRadioEnabled, errorBarValue]); - const onAggregateChange = useCallback( - (name: string, value: string, selectedOption: SelectInputOption) => { - onSelectFieldChange(name, value, selectedOption); + const onChange = useCallback( + (propName: string, value: string) => { + setChartConfig(current => ({ + ...current, + measures: { + ...current.measures, + [field.name]: { ...current.measures[field.name], [propName]: value }, + }, + })); }, - [onSelectFieldChange] + [field.name, setChartConfig] ); - const onErrorBarValueChange = useCallback( - (value: string) => { - onErrorBarChange(BAR_CHART_ERROR_BAR_NAME, value); - }, - [onErrorBarChange] - ); - - // Only show the aggregate options if there is a field selected - if (!fieldValue?.value) { - return null; - } + const onAggregateChange = useCallback((_: never, value: string) => onChange('aggregate', value), [onChange]); + const onErrorBarValueChange = useCallback((value: string) => onChange('errorBars', value), [onChange]); const inputs = ( <> diff --git a/packages/components/src/internal/components/chart/ChartFieldOption.tsx b/packages/components/src/internal/components/chart/ChartFieldOption.tsx index 26e01eda9b..7cfe2ea2bb 100644 --- a/packages/components/src/internal/components/chart/ChartFieldOption.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldOption.tsx @@ -1,61 +1,84 @@ import React, { FC, memo, useCallback, useMemo, useState } from 'react'; -import { SelectInput, SelectInputOption } from '../forms/input/SelectInput'; +import { SelectInput } from '../forms/input/SelectInput'; import { QueryModel } from '../../../public/QueryModel/QueryModel'; import { LABKEY_VIS } from '../../constants'; +import { QueryColumn } from '../../../public/QueryColumn'; import { ChartFieldRangeScaleOptions } from './ChartFieldRangeScaleOptions'; -import { ChartFieldInfo, ChartTypeInfo, ScaleType } from './models'; -import { getFieldDataType, getSelectOptions, shouldShowAggregateOptions, shouldShowRangeScaleOptions } from './utils'; +import { ChartConfig, ChartConfigSetter, ChartFieldInfo, ChartTypeInfo, ScaleType } from './models'; +import { getFieldDataType, getSelectOptions, hasTrendline, shouldShowAggregateOptions, shouldShowRangeScaleOptions } from './utils'; + import { ChartFieldAggregateOptions } from './ChartFieldAggregateOptions'; const DEFAULT_SCALE_VALUES = { type: 'automatic', trans: 'linear' }; interface OwnProps { + chartConfig: ChartConfig; field: ChartFieldInfo; - fieldValues: Record; model: QueryModel; - onErrorBarChange: (name: string, value: string) => void; - onScaleChange: (field: string, key: string, value: number | string, reset?: boolean) => void; - onSelectFieldChange: (name: string, value: string, selectedOption: SelectInputOption) => void; - scaleValues: ScaleType; selectedType: ChartTypeInfo; + setChartConfig: ChartConfigSetter; } export const ChartFieldOption: FC = memo(props => { - const { - field, - model, - selectedType, - onSelectFieldChange, - scaleValues, - fieldValues, - onScaleChange, - onErrorBarChange, - } = props; - const fieldValue = fieldValues?.[field.name]; + const { chartConfig, field, model, selectedType, setChartConfig } = props; + const { measures, scales } = chartConfig; + const scaleValues = scales[field.name] ?? {}; + const fieldValue = measures?.[field.name]; const [scale, setScale] = useState(scaleValues?.type ? scaleValues : DEFAULT_SCALE_VALUES); - const options = useMemo(() => getSelectOptions(model, selectedType, field), [model, selectedType, field]); const isNumericType = useMemo( - () => LABKEY_VIS.GenericChartHelper.isNumericType(getFieldDataType(fieldValue?.data)), - [fieldValue?.data] + () => LABKEY_VIS.GenericChartHelper.isNumericType(getFieldDataType(fieldValue)), + [fieldValue] ); const showRangeScaleOptions = isNumericType && shouldShowRangeScaleOptions(field, selectedType); const showAggregateOptions = isNumericType && shouldShowAggregateOptions(field, selectedType); - // Issue 52050: use fieldKey for special characters - const selectInputValue = useMemo(() => fieldValue?.data.fieldKey ?? fieldValue?.value, [fieldValue]); + const onScaleChange = useCallback( + (field: string, key: string, value: number | string, reset = false) => { + setChartConfig(current => { + const scales = current.scales ? { ...current.scales } : {}; + if (!scales[field] || reset) scales[field] = DEFAULT_SCALE_VALUES; + if (key) scales[field][key] = value; + return { ...current, scales }; + }); + }, + [setChartConfig] + ); - const onSelectFieldChange_ = useCallback( - (name: string, value: string, selectedOption: SelectInputOption) => { - onScaleChange(field.name, undefined, undefined, true); + const onSelectChange = useCallback( + (name: string, _: never, col: QueryColumn) => { setScale(DEFAULT_SCALE_VALUES); - onSelectFieldChange(name, value, selectedOption); + setChartConfig(current => { + let geomOptions = current.geomOptions; + const measures = { ...current.measures }; + const scales = { ...current.scales }; + + if (!col) { + delete measures[name]; + delete scales[name]; + } else { + measures[name] = { + fieldKey: col.fieldKey, + label: col.caption, + name: col.name, + type: col.jsonType, + }; + scales[name] = DEFAULT_SCALE_VALUES; + } + + if (name === 'x' && hasTrendline(selectedType)) { + const trendlineType = LABKEY_VIS.GenericChartHelper.TRENDLINE_OPTIONS['']; + geomOptions = { ...geomOptions, trendlineType }; + } + + return { ...current, geomOptions, measures }; + }); }, - [field.name, onScaleChange, onSelectFieldChange] + [selectedType, setChartConfig] ); return ( @@ -68,12 +91,15 @@ export const ChartFieldOption: FC = memo(props => { {showRangeScaleOptions && ( = memo(props => { setScale={setScale} showScaleTrans={selectedType.name !== 'bar_chart'} > - {showAggregateOptions && ( + {fieldValue && showAggregateOptions && ( )} From 14cff78cea3c593a23b6f6db3782f887546bf260 Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 20 Nov 2025 13:09:03 -0600 Subject: [PATCH 03/38] Add new types for updating ChartConfig and Report models --- .../src/internal/components/chart/models.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/components/src/internal/components/chart/models.ts b/packages/components/src/internal/components/chart/models.ts index 999f8dfcb0..f895a1e934 100644 --- a/packages/components/src/internal/components/chart/models.ts +++ b/packages/components/src/internal/components/chart/models.ts @@ -12,6 +12,9 @@ export interface ChartConfig { width: number; } +export type ChartConfigMutator = (currentConfig: ChartConfig) => ChartConfig; +export type ChartConfigSetter = (mutator: ChartConfigMutator) => void; + export interface ChartQueryConfig { columns: string[]; containerFilter: Query.ContainerFilter; @@ -38,6 +41,15 @@ export interface GenericChartModel extends Visualization.VisualizationGetRespons visualizationConfig: VisualizationConfigModel; } +export interface BaseChartModel { + inheritable: boolean; + name: string; + shared: boolean; +} + +export type BaseChartModelMutator = (currentModel: BaseChartModel) => BaseChartModel; +export type BaseChartModelSetter = (mutator: BaseChartModelMutator) => void; + export interface TrendlineType { equation?: string; label: string; From f602e94d3d3da0504659a7ef1a863b1e3c1e3dfd Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 20 Nov 2025 13:09:26 -0600 Subject: [PATCH 04/38] TrendlineOption: use setChartConfig --- .../components/chart/ChartSettingsPanel.tsx | 3 +- .../components/chart/TrendlineOption.tsx | 146 ++++++++++-------- .../src/internal/components/chart/utils.ts | 10 +- 3 files changed, 90 insertions(+), 69 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx index c141565d2c..70edb837fb 100644 --- a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx +++ b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx @@ -208,7 +208,8 @@ export const ChartSettingsPanel: FC = memo(props => { {showTrendline && ( )} diff --git a/packages/components/src/internal/components/chart/TrendlineOption.tsx b/packages/components/src/internal/components/chart/TrendlineOption.tsx index aa2f3c9cce..848e3bf9ac 100644 --- a/packages/components/src/internal/components/chart/TrendlineOption.tsx +++ b/packages/components/src/internal/components/chart/TrendlineOption.tsx @@ -1,7 +1,6 @@ import React, { ChangeEvent, FC, memo, useCallback, useEffect, useMemo, useState } from 'react'; -import { SelectInput, SelectInputOption } from '../forms/input/SelectInput'; -import { SchemaQuery } from '../../../public/SchemaQuery'; +import { SelectInput } from '../forms/input/SelectInput'; import { LABKEY_VIS } from '../../constants'; import { LabelOverlay } from '../forms/LabelOverlay'; @@ -10,9 +9,10 @@ import { OverlayTrigger } from '../../OverlayTrigger'; import { Popover } from '../../Popover'; import { RadioGroupInput, RadioGroupOption } from '../forms/input/RadioGroupInput'; -import { ChartFieldInfo, ChartTypeInfo, TrendlineType } from './models'; +import { ChartConfig, ChartConfigSetter, ChartFieldInfo, ChartTypeInfo, TrendlineType } from './models'; import { getFieldDataType, getSelectOptions } from './utils'; import { QueryModel } from '../../../public/QueryModel/QueryModel'; +import { QueryColumn } from '../../../public/QueryColumn'; const ASYMPTOTE_TYPES = [ { value: 'automatic', label: 'Automatic' }, @@ -20,37 +20,46 @@ const ASYMPTOTE_TYPES = [ ]; interface TrendlineOptionProps { - fieldValues: Record; + chartConfig: ChartConfig; model: QueryModel; - onFieldChange: (key: string, value: SelectInputOption) => void; - schemaQuery: SchemaQuery; selectedType: ChartTypeInfo; + setChartConfig: ChartConfigSetter; } export const TrendlineOption: FC = memo(props => { const TRENDLINE_OPTIONS: TrendlineType[] = Object.values(LABKEY_VIS.GenericChartHelper.TRENDLINE_OPTIONS); - const { fieldValues, onFieldChange, schemaQuery, model, selectedType } = props; - const showFieldOptions = fieldValues.trendlineType && fieldValues.trendlineType.value !== ''; + const { chartConfig, model, selectedType, setChartConfig } = props; + const schemaQuery = model.schemaQuery; + const geomOptions = chartConfig?.geomOptions; + const measures = chartConfig?.measures; + const selectedTrendlineType = useMemo(() => { + return TRENDLINE_OPTIONS.find(option => option.value === geomOptions?.trendlineType); + }, [TRENDLINE_OPTIONS, geomOptions?.trendlineType]); + const showFieldOptions = selectedTrendlineType?.showMin || selectedTrendlineType?.showMax; // hide the trendline option if no x-axis value selected and for date field selection on x-axis const hidden = useMemo(() => { - const jsonType = getFieldDataType(fieldValues.x?.data); - return !fieldValues.x?.value || jsonType === 'date' || jsonType === 'time'; - }, [fieldValues.x]); + const jsonType = getFieldDataType(measures?.x); + return !measures?.x || jsonType === 'date' || jsonType === 'time'; + }, [measures?.x]); const [loadingTrendlineOptions, setLoadingTrendlineOptions] = useState(true); const [asymptoteType, setAsymptoteType] = useState('automatic'); const [asymptoteMin, setAsymptoteMin] = useState(''); const [asymptoteMax, setAsymptoteMax] = useState(''); + const invalidRange = useMemo( + () => !!asymptoteMin && !!asymptoteMax && asymptoteMax <= asymptoteMin, + [asymptoteMin, asymptoteMax] + ); useEffect(() => { - if (loadingTrendlineOptions && (!!fieldValues.trendlineAsymptoteMin || !!fieldValues.trendlineAsymptoteMax)) { + if (loadingTrendlineOptions && (!!geomOptions?.trendlineAsymptoteMin || !!geomOptions?.trendlineAsymptoteMax)) { setAsymptoteType('manual'); - setAsymptoteMin(fieldValues.trendlineAsymptoteMin?.value); - setAsymptoteMax(fieldValues.trendlineAsymptoteMax?.value); + setAsymptoteMin(geomOptions?.trendlineAsymptoteMin?.value); + setAsymptoteMax(geomOptions?.trendlineAsymptoteMax?.value); setLoadingTrendlineOptions(false); } - }, [fieldValues, loadingTrendlineOptions]); + }, [geomOptions, loadingTrendlineOptions]); const onTrendlineAsymptoteMin = useCallback((event: ChangeEvent) => { setAsymptoteMin(event.target.value); @@ -60,20 +69,43 @@ export const TrendlineOption: FC = memo(props => { setAsymptoteMax(event.target.value); }, []); - const clearTrendlineAsymptote = useCallback(() => { - setAsymptoteMin(''); - onFieldChange('trendlineAsymptoteMin', undefined); - setAsymptoteMax(''); - onFieldChange('trendlineAsymptoteMax', undefined); - }, [onFieldChange]); + const setGeomOptions = useCallback( + options => { + setChartConfig(current => ({ + ...current, + geomOptions: { ...current.geomOptions, ...options }, + })); + }, + [setChartConfig] + ); + + const applyTrendlineAsymptote = useCallback(() => { + if (invalidRange) return; + setGeomOptions({ trendlineAsymptoteMin: asymptoteMin, trendlineAsymptoteMax: asymptoteMax }); + }, [asymptoteMin, asymptoteMax, invalidRange, setGeomOptions]); + + const clearTrendlineAsymptote = useCallback( + (updateChartConfig: boolean) => { + setAsymptoteMin(''); + setAsymptoteMax(''); + if (updateChartConfig) { + setGeomOptions({ trendlineAsymptoteMin: undefined, trendlineAsymptoteMax: undefined }); + } + }, + [setGeomOptions] + ); const onTrendlineFieldChange = useCallback( - (key: string, _, selectedOption: SelectInputOption) => { + (_: never, value: string) => { setAsymptoteType('automatic'); - clearTrendlineAsymptote(); - onFieldChange(key, selectedOption); + clearTrendlineAsymptote(false); + setGeomOptions({ + trendlineType: value, + trendlineAsymptoteMin: undefined, + trendlineAsymptoteMax: undefined, + }); }, - [clearTrendlineAsymptote, onFieldChange] + [clearTrendlineAsymptote, setGeomOptions] ); const trendlineOptions = useMemo(() => { @@ -85,7 +117,7 @@ export const TrendlineOption: FC = memo(props => { const onAsymptoteTypeChange = useCallback( (selected: string) => { if (selected === 'automatic') { - clearTrendlineAsymptote(); + clearTrendlineAsymptote(true); } setAsymptoteType(selected); }, @@ -121,7 +153,7 @@ export const TrendlineOption: FC = memo(props => { options={trendlineOptions} placeholder="Select trendline option" showLabel={false} - value={fieldValues.trendlineType?.value ?? ''} + value={selectedTrendlineType?.value ?? ''} /> {showFieldOptions && (
@@ -129,16 +161,17 @@ export const TrendlineOption: FC = memo(props => { overlay={ } @@ -155,32 +188,39 @@ export const TrendlineOption: FC = memo(props => { TrendlineOption.displayName = 'TrendlineOption'; interface TrendlineOptionPopoverProps { + applyTrendlineAsymptote: () => void; asymptoteMax: string; asymptoteMin: string; asymptoteType: string; - fieldValues: Record; + chartConfig: ChartConfig; model: QueryModel; onAsymptoteTypeChange: (selected: string) => void; - onFieldChange: (key: string, value: SelectInputOption) => void; onTrendlineAsymptoteMax: (event: ChangeEvent) => void; onTrendlineAsymptoteMin: (event: ChangeEvent) => void; selectedType: ChartTypeInfo; + setGeomOptions: (options: Record) => void; } const TrendlineOptionPopover: FC = props => { const { - fieldValues, + applyTrendlineAsymptote, model, selectedType, asymptoteType, + chartConfig, onAsymptoteTypeChange, - onFieldChange, asymptoteMin, asymptoteMax, onTrendlineAsymptoteMin, onTrendlineAsymptoteMax, + setGeomOptions, } = props; - const showAsymptoteOptions = fieldValues.trendlineType?.showMin || fieldValues.trendlineType?.showMax; + const geomOptions = chartConfig.geomOptions; + const TRENDLINE_OPTIONS: TrendlineType[] = Object.values(LABKEY_VIS.GenericChartHelper.TRENDLINE_OPTIONS); + const selectedTrendlineType = useMemo(() => { + return TRENDLINE_OPTIONS.find(option => option.value === geomOptions?.trendlineType); + }, [TRENDLINE_OPTIONS, geomOptions?.trendlineType]); + const showAsymptoteOptions = selectedTrendlineType?.showMin || selectedTrendlineType?.showMax; const invalidRange = useMemo( () => !!asymptoteMin && !!asymptoteMax && asymptoteMax <= asymptoteMin, [asymptoteMin, asymptoteMax] @@ -195,35 +235,19 @@ const TrendlineOptionPopover: FC = props => { } as ChartFieldInfo; return getSelectOptions(model, selectedType, field); }, [model, selectedType]); - // Issue 52050: use fieldKey for special characters - const parameterInputValue = useMemo(() => { - if (fieldValues?.trendlineParameters) { - return fieldValues.trendlineParameters.data?.fieldKey ?? fieldValues.trendlineParameters.value; - } - return undefined; - }, [fieldValues]); const asymptoteTypeOptions = useMemo(() => { return ASYMPTOTE_TYPES.map( - option => - ({ - ...option, - selected: asymptoteType === option.value, - }) as RadioGroupOption + option => ({ ...option, selected: asymptoteType === option.value }) as RadioGroupOption ); }, [asymptoteType]); - const applyTrendlineAsymptote = useCallback(() => { - if (invalidRange) return; - onFieldChange('trendlineAsymptoteMin', { value: asymptoteMin }); - onFieldChange('trendlineAsymptoteMax', { value: asymptoteMax }); - }, [onFieldChange, asymptoteMin, asymptoteMax, invalidRange]); - + // chartConfig.geomOptions.trendlineParameters, const onParameterFieldChange = useCallback( - (name: string, _: string, selectedOption: SelectInputOption) => { - onFieldChange(name, selectedOption); + (_: never, __: never, col: QueryColumn) => { + setGeomOptions({ trendlineParameters: col.fieldKey }); }, - [onFieldChange] + [setGeomOptions] ); return ( @@ -241,7 +265,7 @@ const TrendlineOptionPopover: FC = props => {
{asymptoteType === 'manual' && (
- {fieldValues.trendlineType?.showMin && ( + {selectedTrendlineType?.showMin && ( = props => { value={asymptoteMin} /> )} - {fieldValues.trendlineType?.showMin && fieldValues.trendlineType?.showMax && ( - - - )} - {fieldValues.trendlineType?.showMax && ( + {selectedTrendlineType?.showMin && selectedTrendlineType?.showMax && -} + {selectedTrendlineType?.showMax && ( = props => {
diff --git a/packages/components/src/internal/components/chart/utils.ts b/packages/components/src/internal/components/chart/utils.ts index 0da1bc3662..bd132017ba 100644 --- a/packages/components/src/internal/components/chart/utils.ts +++ b/packages/components/src/internal/components/chart/utils.ts @@ -4,6 +4,7 @@ import { QueryModel } from '../../../public/QueryModel/QueryModel'; import { SelectInputOption } from '../forms/input/SelectInput'; import { naturalSortByProperty } from '../../../public/sort'; import { LABKEY_VIS } from '../../constants'; +import { QueryColumn } from '../../../public/QueryColumn'; export interface HorizontalBarData { backgroundColor?: string; @@ -96,11 +97,7 @@ export const shouldShowAggregateOptions = (field: ChartFieldInfo, selectedType: return field.name === 'y' && (isBar || isLine); }; -export const getSelectOptions = ( - model: QueryModel, - chartType: ChartTypeInfo, - field: ChartFieldInfo -): SelectInputOption[] => { +export const getSelectOptions = (model: QueryModel, chartType: ChartTypeInfo, field: ChartFieldInfo): QueryColumn[] => { const allowableTypes = LABKEY_VIS.GenericChartHelper.getAllowableTypes(field); return model.queryInfo @@ -116,6 +113,5 @@ export const getSelectOptions = ( ); return hasMatchingType || isMeasureDimensionMatch; }) - .sort(naturalSortByProperty('caption')) - .map(col => ({ label: col.caption, value: col.fieldKey, data: col })); + .sort(naturalSortByProperty('caption')); }; From 3c0e98ad47c935e5e4adc237498f95e56613761d Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 20 Nov 2025 13:10:19 -0600 Subject: [PATCH 05/38] utils.ts: Add deepCopyChartConfig and hasTrendline --- .../src/internal/components/chart/utils.ts | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/components/src/internal/components/chart/utils.ts b/packages/components/src/internal/components/chart/utils.ts index bd132017ba..da5669aec0 100644 --- a/packages/components/src/internal/components/chart/utils.ts +++ b/packages/components/src/internal/components/chart/utils.ts @@ -1,5 +1,5 @@ import { Map } from 'immutable'; -import { ChartFieldInfo, ChartTypeInfo } from './models'; +import { ChartConfig, ChartFieldInfo, ChartTypeInfo } from './models'; import { QueryModel } from '../../../public/QueryModel/QueryModel'; import { SelectInputOption } from '../forms/input/SelectInput'; import { naturalSortByProperty } from '../../../public/sort'; @@ -97,6 +97,36 @@ export const shouldShowAggregateOptions = (field: ChartFieldInfo, selectedType: return field.name === 'y' && (isBar || isLine); }; +/** + * Deep copies an existing ChartConfig or creates an empty one. Use before manipulating an existing chart config. + */ +export function deepCopyChartConfig(chartConfig: ChartConfig): ChartConfig { + if (!chartConfig) { + return { + geomOptions: {}, + gridLinesVisible: 'both', + height: undefined, + labels: {}, + measures: {}, + pointType: 'outliers', + renderType: 'bar_chart', + scales: {}, + width: undefined, + }; + } + return { + ...chartConfig, + geomOptions: { ...chartConfig.geomOptions }, + labels: { ...chartConfig.labels }, + measures: { ...chartConfig.measures }, + scales: { ...chartConfig.scales }, + }; +} + +export function hasTrendline(chartType: ChartTypeInfo) { + return chartType.fields.find(f => f.name === 'trendline') !== undefined; +} + export const getSelectOptions = (model: QueryModel, chartType: ChartTypeInfo, field: ChartFieldInfo): QueryColumn[] => { const allowableTypes = LABKEY_VIS.GenericChartHelper.getAllowableTypes(field); From 57719be98d38e7aca867fd167e67e3a54433d37a Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 20 Nov 2025 13:51:56 -0600 Subject: [PATCH 06/38] ChartFieldRangeScaleOptions: Simplify state management --- .../components/chart/ChartFieldOption.tsx | 21 ++++----- .../chart/ChartFieldRangeScaleOptions.tsx | 44 ++++++++----------- 2 files changed, 30 insertions(+), 35 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartFieldOption.tsx b/packages/components/src/internal/components/chart/ChartFieldOption.tsx index 7cfe2ea2bb..31c1fc73c4 100644 --- a/packages/components/src/internal/components/chart/ChartFieldOption.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldOption.tsx @@ -38,15 +38,18 @@ export const ChartFieldOption: FC = memo(props => { const showAggregateOptions = isNumericType && shouldShowAggregateOptions(field, selectedType); const onScaleChange = useCallback( - (field: string, key: string, value: number | string, reset = false) => { - setChartConfig(current => { - const scales = current.scales ? { ...current.scales } : {}; - if (!scales[field] || reset) scales[field] = DEFAULT_SCALE_VALUES; - if (key) scales[field][key] = value; - return { ...current, scales }; - }); + (scale: ScaleType, localOnly = false) => { + setScale(current => ({ ...current, ...scale })); + + if (!localOnly) { + setChartConfig(current => { + let updatedScale = current.scales?.[field.name] ?? DEFAULT_SCALE_VALUES; + updatedScale = { ...updatedScale, scale }; + return { ...current, scales: { ...current.scales, [field.name]: updatedScale } }; + }); + } }, - [setChartConfig] + [field.name, setChartConfig] ); const onSelectChange = useCallback( @@ -103,10 +106,8 @@ export const ChartFieldOption: FC = memo(props => { /> {showRangeScaleOptions && ( {fieldValue && showAggregateOptions && ( diff --git a/packages/components/src/internal/components/chart/ChartFieldRangeScaleOptions.tsx b/packages/components/src/internal/components/chart/ChartFieldRangeScaleOptions.tsx index b3d63663fd..f8824c0b3b 100644 --- a/packages/components/src/internal/components/chart/ChartFieldRangeScaleOptions.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldRangeScaleOptions.tsx @@ -1,9 +1,9 @@ import React, { ChangeEvent, FC, memo, PropsWithChildren, useCallback, useMemo } from 'react'; import { OverlayTrigger } from '../../OverlayTrigger'; import { Popover } from '../../Popover'; -import { RadioGroupInput, RadioGroupOption } from '../forms/input/RadioGroupInput'; +import { RadioGroupInput } from '../forms/input/RadioGroupInput'; -import { ChartFieldInfo, ScaleType } from './models'; +import { ScaleType } from './models'; const SCALE_TRANS_TYPES = [ { value: 'linear', label: 'Linear' }, @@ -16,15 +16,13 @@ const SCALE_RANGE_TYPES = [ ]; interface OwnProps extends PropsWithChildren { - field: ChartFieldInfo; - onScaleChange: (field: string, key: string, value: number | string, reset?: boolean) => void; + onScaleChange: (scale: Partial, localOnly?: boolean) => void; scale: ScaleType; - setScale: (scale: ScaleType) => void; showScaleTrans: boolean; } export const ChartFieldRangeScaleOptions: FC = memo(props => { - const { field, scale, setScale, onScaleChange, showScaleTrans, children } = props; + const { scale, onScaleChange, showScaleTrans, children } = props; const placement = useMemo(() => (!showScaleTrans && children ? 'left' : 'bottom'), [showScaleTrans, children]); const scaleTransOptions = useMemo(() => { @@ -47,45 +45,41 @@ export const ChartFieldRangeScaleOptions: FC = memo(props => { const onScaleTransChange = useCallback( (selected: string) => { - onScaleChange(field.name, 'trans', selected); - setScale({ ...scale, trans: selected }); + onScaleChange({ trans: selected }); }, - [field.name, onScaleChange, setScale, scale] + [onScaleChange] ); const onScaleTypeChange = useCallback( (selected: string) => { - let scale_ = { ...scale, type: selected }; - onScaleChange(field.name, 'type', selected); - if (selected === 'automatic') { - onScaleChange(field.name, 'min', undefined); - onScaleChange(field.name, 'max', undefined); - scale_ = { ...scale_, min: undefined, max: undefined }; - } - setScale(scale_); + let scale_: Partial = { type: selected }; + if (selected === 'automatic') scale_ = { ...scale_, min: undefined, max: undefined }; + onScaleChange(scale_); }, - [field.name, onScaleChange, scale, setScale] + [onScaleChange] ); const onScaleMinChange = useCallback( (event: ChangeEvent) => { - setScale({ ...scale, min: event.target.value }); + onScaleChange({ min: event.target.value }, true); }, - [setScale, scale] + [onScaleChange] ); const onScaleMaxChange = useCallback( (event: ChangeEvent) => { - setScale({ ...scale, max: event.target.value }); + onScaleChange({ max: event.target.value }, true); }, - [setScale, scale] + [onScaleChange] ); const onScaleRangeBlur = useCallback(() => { if (invalidRange) return; - onScaleChange(field.name, 'min', parseFloat(scale.min?.toString())); - onScaleChange(field.name, 'max', parseFloat(scale.max?.toString())); - }, [field.name, onScaleChange, scale.max, scale.min, invalidRange]); + onScaleChange({ + min: parseFloat(scale.min?.toString()), + max: parseFloat(scale.max?.toString()), + }); + }, [invalidRange, onScaleChange, scale]); return (
From 8f7b358ed8950aa01b29fbe8cf8b7c39e88998f8 Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 20 Nov 2025 13:52:36 -0600 Subject: [PATCH 07/38] charts.scss: Add styles for new chart builder layout --- packages/components/src/theme/charts.scss | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/components/src/theme/charts.scss b/packages/components/src/theme/charts.scss index c8985585ae..8b835c21d1 100644 --- a/packages/components/src/theme/charts.scss +++ b/packages/components/src/theme/charts.scss @@ -195,11 +195,27 @@ width: calc(100% - 100px); min-width: 850px; } -.chart-builder-modal .col-left { +.chart-builder-modal__body { + display: flex; +} + +.chart-builder-modal__settings-panel { height: 560px; - width: 116px; + width: 300px; border-right: solid 1px $gray-border-light; + overflow-y: auto; + padding-right: 15px; +} + +.chart-builder-modal__settings-panel h4 { + margin: 0 0 8px; } + +.chart-builder-modal__chart-preview { + flex: 1; + padding: 8px; +} + .chart-builder-modal .col-right { height: 560px; width: calc(100% - 116px) From 1e9b56ef929725bc045a6ee112c01ca7a7c8810b Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 20 Nov 2025 15:22:19 -0600 Subject: [PATCH 08/38] ChartFieldOption: Update labels when changing measures --- .../src/internal/components/chart/ChartFieldOption.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/components/src/internal/components/chart/ChartFieldOption.tsx b/packages/components/src/internal/components/chart/ChartFieldOption.tsx index 31c1fc73c4..4c3ac6e364 100644 --- a/packages/components/src/internal/components/chart/ChartFieldOption.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldOption.tsx @@ -59,10 +59,12 @@ export const ChartFieldOption: FC = memo(props => { let geomOptions = current.geomOptions; const measures = { ...current.measures }; const scales = { ...current.scales }; + const labels = { ...current.labels }; if (!col) { delete measures[name]; delete scales[name]; + delete labels[name]; } else { measures[name] = { fieldKey: col.fieldKey, @@ -71,6 +73,7 @@ export const ChartFieldOption: FC = memo(props => { type: col.jsonType, }; scales[name] = DEFAULT_SCALE_VALUES; + labels[name] = col.caption; } if (name === 'x' && hasTrendline(selectedType)) { From 9edd5babe3d1a1dc94f8cb4cd3537d5662bb4544 Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 20 Nov 2025 15:29:59 -0600 Subject: [PATCH 09/38] ChartSettingsPanel: Better handle chart type changes --- .../components/chart/ChartSettingsPanel.tsx | 11 ++--- .../src/internal/components/chart/utils.ts | 45 +++++++++++++++++-- 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx index 70edb837fb..a7d80d9fea 100644 --- a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx +++ b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx @@ -143,11 +143,12 @@ export const ChartSettingsPanel: FC = memo(props => { const onTypeChange = useCallback( (type: ChartTypeInfo) => { - setChartConfig(current => ({ - ...deepCopyChartConfig(undefined), - labels: { ...current.labels }, - renderType: type.name, - })); + setChartConfig(current => { + return { + ...deepCopyChartConfig(undefined, type.name), + labels: { ...current.labels }, + }; + }); }, [setChartConfig] ); diff --git a/packages/components/src/internal/components/chart/utils.ts b/packages/components/src/internal/components/chart/utils.ts index da5669aec0..cc348dd88c 100644 --- a/packages/components/src/internal/components/chart/utils.ts +++ b/packages/components/src/internal/components/chart/utils.ts @@ -1,7 +1,7 @@ import { Map } from 'immutable'; import { ChartConfig, ChartFieldInfo, ChartTypeInfo } from './models'; +import { BLUE_HEX_COLOR, MAX_POINT_DISPLAY } from './constants'; import { QueryModel } from '../../../public/QueryModel/QueryModel'; -import { SelectInputOption } from '../forms/input/SelectInput'; import { naturalSortByProperty } from '../../../public/sort'; import { LABKEY_VIS } from '../../constants'; import { QueryColumn } from '../../../public/QueryColumn'; @@ -97,19 +97,56 @@ export const shouldShowAggregateOptions = (field: ChartFieldInfo, selectedType: return field.name === 'y' && (isBar || isLine); }; +const makeGeomOptions = (chartType: string) => ({ + binShape: 'hex', + binSingleColor: '000000', + binThreshold: MAX_POINT_DISPLAY, + boxFillColor: chartType === 'box_plot' ? 'none' : BLUE_HEX_COLOR, + chartLayout: 'single', + chartSubjectSelection: 'subjects', + colorPaletteScale: 'ColorDiscrete', + colorRange: 'BlueWhite', + displayIndividual: true, + displayAggregate: false, + errorBars: 'None', + gradientColor: 'FFFFFF', + gradientPercentage: 95, + hideDataPoints: false, + hideTrendLine: false, + lineColor: '000000', + lineWidth: chartType === 'line_plot' ? 3 : 1, + marginBottom: null, + marginLeft: null, + marginRight: null, + marginTop: 20, // this will be saved with the chartConfig, but we will override it for the preview in the modal + opacity: chartType === 'bar_chart' || chartType === 'line_plot' ? 1.0 : 0.5, + pieHideWhenLessThanPercentage: 5, + pieInnerRadius: 0, + pieOuterRadius: 80, + piePercentagesColor: '333333', + pointFillColor: BLUE_HEX_COLOR, + pointSize: chartType === 'box_plot' ? 3 : 5, + position: chartType === 'box_plot' ? 'jitter' : null, + showOutliers: true, + showPiePercentages: true, + trendlineType: undefined, + trendlineAsymptoteMin: undefined, + trendlineAsymptoteMax: undefined, +}); + /** * Deep copies an existing ChartConfig or creates an empty one. Use before manipulating an existing chart config. */ -export function deepCopyChartConfig(chartConfig: ChartConfig): ChartConfig { +export function deepCopyChartConfig(chartConfig: ChartConfig, chartType = 'bar_chart'): ChartConfig { if (!chartConfig) { return { - geomOptions: {}, + geomOptions: makeGeomOptions(chartType), gridLinesVisible: 'both', height: undefined, labels: {}, measures: {}, pointType: 'outliers', - renderType: 'bar_chart', + renderType: chartType, scales: {}, width: undefined, }; From 879f70379f70bc70af7d08db4a2031d74521c76b Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 20 Nov 2025 16:29:57 -0600 Subject: [PATCH 10/38] Improve ChartConfig type Which of course found several bugs! --- .../components/chart/ChartFieldOption.tsx | 17 +++++++++-------- .../components/chart/TrendlineOption.tsx | 4 ++-- .../src/internal/components/chart/models.ts | 12 ++++++------ 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartFieldOption.tsx b/packages/components/src/internal/components/chart/ChartFieldOption.tsx index 4c3ac6e364..c42390dfa4 100644 --- a/packages/components/src/internal/components/chart/ChartFieldOption.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldOption.tsx @@ -26,13 +26,14 @@ interface OwnProps { export const ChartFieldOption: FC = memo(props => { const { chartConfig, field, model, selectedType, setChartConfig } = props; const { measures, scales } = chartConfig; - const scaleValues = scales[field.name] ?? {}; - const fieldValue = measures?.[field.name]; - const [scale, setScale] = useState(scaleValues?.type ? scaleValues : DEFAULT_SCALE_VALUES); + const measure = measures?.[field.name]; + const [scale, setScale] = useState(() => { + return scales[field.name] ?? DEFAULT_SCALE_VALUES; + }); const options = useMemo(() => getSelectOptions(model, selectedType, field), [model, selectedType, field]); const isNumericType = useMemo( - () => LABKEY_VIS.GenericChartHelper.isNumericType(getFieldDataType(fieldValue)), - [fieldValue] + () => LABKEY_VIS.GenericChartHelper.isNumericType(getFieldDataType(measure)), + [measure] ); const showRangeScaleOptions = isNumericType && shouldShowRangeScaleOptions(field, selectedType); const showAggregateOptions = isNumericType && shouldShowAggregateOptions(field, selectedType); @@ -44,7 +45,7 @@ export const ChartFieldOption: FC = memo(props => { if (!localOnly) { setChartConfig(current => { let updatedScale = current.scales?.[field.name] ?? DEFAULT_SCALE_VALUES; - updatedScale = { ...updatedScale, scale }; + updatedScale = { ...updatedScale, ...scale }; return { ...current, scales: { ...current.scales, [field.name]: updatedScale } }; }); } @@ -104,7 +105,7 @@ export const ChartFieldOption: FC = memo(props => { placeholder="Select a field" showLabel={false} // Issue 52050: use fieldKey for special characters - value={fieldValue?.fieldKey} + value={measure?.fieldKey} valueKey="fieldKey" /> {showRangeScaleOptions && ( @@ -113,7 +114,7 @@ export const ChartFieldOption: FC = memo(props => { scale={scale} showScaleTrans={selectedType.name !== 'bar_chart'} > - {fieldValue && showAggregateOptions && ( + {measure && showAggregateOptions && ( = memo(props => { useEffect(() => { if (loadingTrendlineOptions && (!!geomOptions?.trendlineAsymptoteMin || !!geomOptions?.trendlineAsymptoteMax)) { setAsymptoteType('manual'); - setAsymptoteMin(geomOptions?.trendlineAsymptoteMin?.value); - setAsymptoteMax(geomOptions?.trendlineAsymptoteMax?.value); + setAsymptoteMin(geomOptions?.trendlineAsymptoteMin?.toString()); + setAsymptoteMax(geomOptions?.trendlineAsymptoteMax?.toString()); setLoadingTrendlineOptions(false); } }, [geomOptions, loadingTrendlineOptions]); diff --git a/packages/components/src/internal/components/chart/models.ts b/packages/components/src/internal/components/chart/models.ts index f895a1e934..ef25d3a620 100644 --- a/packages/components/src/internal/components/chart/models.ts +++ b/packages/components/src/internal/components/chart/models.ts @@ -1,15 +1,15 @@ import { Filter, Query, Visualization } from '@labkey/api'; export interface ChartConfig { - geomOptions: any; + geomOptions: Record; gridLinesVisible: string; - height: number; - labels: any; - measures: any; + height?: number; + labels: Record; + measures: Record>; // TODO: we can probably do better than any pointType: string; renderType: string; - scales: any; - width: number; + scales: Record; + width?: number; } export type ChartConfigMutator = (currentConfig: ChartConfig) => ChartConfig; From eaad74c908c51d97b4cccd762d4e8552d988b081 Mon Sep 17 00:00:00 2001 From: alanv Date: Fri, 21 Nov 2025 01:33:04 -0600 Subject: [PATCH 11/38] Fix issue with bar chart y axis labels --- .../chart/ChartFieldAggregateOptions.tsx | 43 ++++++++++--------- .../components/chart/ChartFieldOption.tsx | 17 +++++++- .../chart/ChartFieldRangeScaleOptions.tsx | 2 +- .../components/chart/ChartSettingsPanel.tsx | 6 ++- .../internal/components/chart/constants.ts | 11 ++++- .../src/internal/components/chart/utils.ts | 22 +++++++++- 6 files changed, 73 insertions(+), 28 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.tsx b/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.tsx index 462d2db0e6..0ab9bbf1a2 100644 --- a/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.tsx @@ -3,21 +3,13 @@ import { OverlayTrigger } from '../../OverlayTrigger'; import { Popover } from '../../Popover'; import { RadioGroupInput } from '../forms/input/RadioGroupInput'; -import { BAR_CHART_AGGREGATE_NAME, BAR_CHART_ERROR_BAR_NAME } from './constants'; import { ChartConfig, ChartConfigSetter, ChartFieldInfo, ChartTypeInfo } from './models'; import { LabelOverlay } from '../forms/LabelOverlay'; import { SelectInput } from '../forms/input/SelectInput'; import { Utils } from '@labkey/api'; +import { getBarChartAxisLabel } from './utils'; +import { AGGREGATE_METHODS } from './constants'; -const AGGREGATE_METHODS = [ - { label: 'None', value: '' }, - { label: 'Count (non-blank)', value: 'COUNT' }, - { label: 'Sum', value: 'SUM' }, - { label: 'Min', value: 'MIN' }, - { label: 'Max', value: 'MAX' }, - { label: 'Mean', value: 'MEAN' }, - { label: 'Median', value: 'MEDIAN' }, -]; const BAR_CHART_AGGREGATE_METHOD_TIP = 'The aggregate method that will be used to determine the bar height for a given x-axis category / dimension. Field values that are blank are not included in calculated aggregate values.'; const BAR_CHART_ERROR_BAR_TIP = @@ -67,15 +59,26 @@ export const ChartFieldAggregateOptions: FC = memo(props => { const onChange = useCallback( (propName: string, value: string) => { - setChartConfig(current => ({ - ...current, - measures: { - ...current.measures, - [field.name]: { ...current.measures[field.name], [propName]: value }, - }, - })); + setChartConfig(current => { + const updatedConfig = { + ...current, + measures: { + ...current.measures, + [field.name]: { ...current.measures[field.name], [propName]: value }, + }, + }; + + if (selectedType.name === 'bar_chart') { + updatedConfig.labels = { + ...updatedConfig.labels, + y: getBarChartAxisLabel(updatedConfig, current), + }; + } + + return updatedConfig; + }); }, - [field.name, setChartConfig] + [field.name, selectedType.name, setChartConfig] ); const onAggregateChange = useCallback((_: never, value: string) => onChange('aggregate', value), [onChange]); @@ -90,7 +93,7 @@ export const ChartFieldAggregateOptions: FC = memo(props => { = memo(props => { diff --git a/packages/components/src/internal/components/chart/ChartFieldOption.tsx b/packages/components/src/internal/components/chart/ChartFieldOption.tsx index c42390dfa4..f415bb3c41 100644 --- a/packages/components/src/internal/components/chart/ChartFieldOption.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldOption.tsx @@ -9,7 +9,14 @@ import { QueryColumn } from '../../../public/QueryColumn'; import { ChartFieldRangeScaleOptions } from './ChartFieldRangeScaleOptions'; import { ChartConfig, ChartConfigSetter, ChartFieldInfo, ChartTypeInfo, ScaleType } from './models'; -import { getFieldDataType, getSelectOptions, hasTrendline, shouldShowAggregateOptions, shouldShowRangeScaleOptions } from './utils'; +import { + getBarChartAxisLabel, + getFieldDataType, + getSelectOptions, + hasTrendline, + shouldShowAggregateOptions, + shouldShowRangeScaleOptions, +} from './utils'; import { ChartFieldAggregateOptions } from './ChartFieldAggregateOptions'; @@ -82,7 +89,13 @@ export const ChartFieldOption: FC = memo(props => { geomOptions = { ...geomOptions, trendlineType }; } - return { ...current, geomOptions, measures }; + const updatedConfig = { ...current, geomOptions, measures, labels }; + + if (selectedType.name === 'bar_chart') { + updatedConfig.labels.y = getBarChartAxisLabel(updatedConfig, current); + } + + return updatedConfig; }); }, [selectedType, setChartConfig] diff --git a/packages/components/src/internal/components/chart/ChartFieldRangeScaleOptions.tsx b/packages/components/src/internal/components/chart/ChartFieldRangeScaleOptions.tsx index f8824c0b3b..199891fe1b 100644 --- a/packages/components/src/internal/components/chart/ChartFieldRangeScaleOptions.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldRangeScaleOptions.tsx @@ -85,7 +85,7 @@ export const ChartFieldRangeScaleOptions: FC = memo(props => {
+ {children} {showScaleTrans && (
diff --git a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx index a7d80d9fea..6fa11b4f19 100644 --- a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx +++ b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx @@ -117,13 +117,15 @@ interface Props { chartConfig: ChartConfig; chartModel: BaseChartModel; chartType: ChartTypeInfo; + isNew: boolean; model: QueryModel; setChartConfig: ChartConfigSetter; setChartModel: BaseChartModelSetter; } export const ChartSettingsPanel: FC = memo(props => { - const { allowInherit, canShare, chartConfig, chartType, chartModel, model, setChartConfig, setChartModel } = props; + const { allowInherit, canShare, chartConfig, chartType, chartModel, isNew, model, setChartConfig, setChartModel } = + props; const showTrendline = hasTrendline(chartType); const fields = chartType.fields.filter(f => f.name !== 'trendline'); @@ -193,7 +195,7 @@ export const ChartSettingsPanel: FC = memo(props => { /> )} - {!chartModel && } + {!isNew && } {fields.map(field => ( f.name === 'trendline') !== undefined; } +export const getDefaultBarChartAxisLabel = (config: ChartConfig): string => { + const aggregate = config.measures.y?.aggregate; + const label = AGGREGATE_METHODS.find(m => m.value === aggregate)?.label; + const prefix = (label ?? aggregate ?? 'Sum') + ' of '; + return config.measures.y ? prefix + config.measures.y.label : 'Count'; +}; + +export const getBarChartAxisLabel = (updated: ChartConfig, prev: ChartConfig) => { + const emptyLabel = !updated.labels.y?.trim(); + const isPrevUsingDefault = prev.labels.y === getDefaultBarChartAxisLabel(prev); + + if (emptyLabel || isPrevUsingDefault) { + return getDefaultBarChartAxisLabel(updated); + } + + return updated.labels.y; +}; + export const getSelectOptions = (model: QueryModel, chartType: ChartTypeInfo, field: ChartFieldInfo): QueryColumn[] => { const allowableTypes = LABKEY_VIS.GenericChartHelper.getAllowableTypes(field); From 377a3704331c25c03fe697bffaa82a69a84ca5a8 Mon Sep 17 00:00:00 2001 From: alanv Date: Fri, 21 Nov 2025 01:34:04 -0600 Subject: [PATCH 12/38] ChartBuilderModal: Update layout, don't convert ChartConfig to FieldValues --- .../chart/ChartBuilderModal.test.tsx | 163 ------ .../components/chart/ChartBuilderModal.tsx | 551 ++---------------- 2 files changed, 57 insertions(+), 657 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartBuilderModal.test.tsx b/packages/components/src/internal/components/chart/ChartBuilderModal.test.tsx index 24d3b8e15a..8f56347296 100644 --- a/packages/components/src/internal/components/chart/ChartBuilderModal.test.tsx +++ b/packages/components/src/internal/components/chart/ChartBuilderModal.test.tsx @@ -19,7 +19,6 @@ import { import { ChartBuilderModal, - getChartBuilderChartConfig, getChartBuilderQueryConfig, getChartRenderMsg, getDefaultBarChartAxisLabel, @@ -623,168 +622,6 @@ describe('getChartBuilderQueryConfig', () => { }); }); -describe('getChartBuilderChartConfig', () => { - const fieldValues = { - x: { value: 'field/1', label: 'Field 1', data: { fieldKey: 'field$S1', name: 'field/1' } }, - y: { value: 'field2', label: 'Field 2', data: { fieldKey: 'field2', name: 'field2' } }, - }; - - test('based on fieldValues', () => { - const config = getChartBuilderChartConfig(BAR_CHART_TYPE, fieldValues, undefined); - expect(config.scales).toStrictEqual({}); - expect(Object.keys(config.labels)).toStrictEqual(['main', 'subtitle', 'x', 'y']); - expect(config.labels.main).toBe(''); - expect(config.labels.subtitle).toBe(''); - expect(config.pointType).toBe('all'); - expect(config.measures.x.name).toBe('field/1'); - expect(config.measures.x.fieldKey).toBe('field$S1'); - expect(config.measures.y.name).toBe('field2'); - expect(config.measures.y.fieldKey).toBe('field2'); - expect(config.labels.x).toBe('Field 1'); - expect(config.labels.y).toBe('Sum of Field 2'); - }); - - test('based on savedConfig, without y-label', () => { - const savedConfig = { - pointType: 'outliers', - scales: { x: 'linear', y: 'log' }, - labels: { main: 'Main', subtitle: 'Subtitle', color: 'Something', x: 'X Label' }, - measures: { x: { name: 'saved1' }, y: { name: 'saved2' } }, - height: 1, - width: 2, - } as ChartConfig; - - const config = getChartBuilderChartConfig(BAR_CHART_TYPE, fieldValues, savedConfig); - expect(config.scales).toStrictEqual(savedConfig.scales); - expect(Object.keys(config.labels)).toStrictEqual(['main', 'subtitle', 'color', 'x', 'y']); - expect(config.labels.main).toBe('Main'); - expect(config.labels.subtitle).toBe('Subtitle'); - expect(config.pointType).toBe('outliers'); - expect(config.measures.x.name).toBe('field/1'); - expect(config.measures.y.name).toBe('field2'); - expect(config.labels.x).toBe('X Label'); - expect(config.labels.y).toBe('Sum of Field 2'); - expect(config.geomOptions.trendlineType).toBe(undefined); - expect(config.geomOptions.trendlineAsymptoteMin).toBe(undefined); - expect(config.geomOptions.trendlineAsymptoteMax).toBe(undefined); - }); - - test('based on savedConfig, with y-label', () => { - const savedConfig = { - pointType: 'outliers', - scales: { x: 'linear', y: 'log' }, - labels: { main: 'Main', subtitle: 'Subtitle', color: 'Something', x: 'X Label', y: 'Y Label' }, - measures: { x: { name: 'saved1' }, y: { name: 'saved2' } }, - height: 1, - width: 2, - } as ChartConfig; - - const config = getChartBuilderChartConfig(BAR_CHART_TYPE, fieldValues, savedConfig); - expect(config.scales).toStrictEqual(savedConfig.scales); - expect(Object.keys(config.labels)).toStrictEqual(['main', 'subtitle', 'color', 'x', 'y']); - expect(config.labels.main).toBe('Main'); - expect(config.labels.subtitle).toBe('Subtitle'); - expect(config.pointType).toBe('outliers'); - expect(config.measures.x.name).toBe('field/1'); - expect(config.measures.y.name).toBe('field2'); - expect(config.labels.x).toBe('X Label'); - expect(config.labels.y).toBe('Y Label'); - }); - - test('based on savedConfig, with trendline options', () => { - const savedConfig = { - pointType: 'outliers', - scales: { x: 'linear', y: 'log' }, - labels: { main: 'Main', subtitle: 'Subtitle', color: 'Something', x: 'X Label', y: 'Y Label' }, - measures: { x: { name: 'saved1' }, y: { name: 'saved2' } }, - height: 1, - width: 2, - geomOptions: { - trendlineType: 'Linear', - trendlineAsymptoteMin: '0.1', - trendlineAsymptoteMax: '1.0', - }, - } as ChartConfig; - - const config = getChartBuilderChartConfig(BAR_CHART_TYPE, fieldValues, savedConfig); - expect(config.geomOptions.trendlineType).toBe('Linear'); - expect(config.geomOptions.trendlineAsymptoteMin).toBe('0.1'); - expect(config.geomOptions.trendlineAsymptoteMax).toBe('1.0'); - }); - - test('box plot specifics', () => { - const boxType = { - name: 'box_plot', - fields: [{ name: 'x' }, { name: 'y' }], - } as ChartTypeInfo; - - const config = getChartBuilderChartConfig(boxType, fieldValues, undefined); - expect(config.geomOptions.boxFillColor).toBe('none'); - expect(config.geomOptions.lineWidth).toBe(1); - expect(config.geomOptions.opacity).toBe(0.5); - expect(config.geomOptions.pointSize).toBe(3); - expect(config.geomOptions.position).toBe('jitter'); - }); - - test('line plot specifics', () => { - const boxType = { - name: 'line_plot', - fields: [{ name: 'x' }, { name: 'y' }], - } as ChartTypeInfo; - - const config = getChartBuilderChartConfig(boxType, fieldValues, undefined); - expect(config.geomOptions.boxFillColor).not.toBe('none'); - expect(config.geomOptions.lineWidth).toBe(3); - expect(config.geomOptions.opacity).toBe(1.0); - expect(config.geomOptions.pointSize).toBe(5); - expect(config.geomOptions.position).toBe(null); - expect(config.geomOptions.trendlineType).toBe(undefined); - expect(config.geomOptions.trendlineAsymptoteMin).toBe(undefined); - expect(config.geomOptions.trendlineAsymptoteMax).toBe(undefined); - }); - - test('line plot with trendline options', () => { - const boxType = { - name: 'line_plot', - fields: [{ name: 'x' }, { name: 'y' }], - } as ChartTypeInfo; - - const trendlineFieldValues = { - x: { value: 'field1', label: 'Field 1', data: { fieldKey: 'field1' } }, - y: { value: 'field2', label: 'Field 2', data: { fieldKey: 'field2' } }, - trendlineType: { value: 'Linear' }, - trendlineAsymptoteMin: { value: '0.1' }, - trendlineAsymptoteMax: { value: '1.0' }, - }; - - const config = getChartBuilderChartConfig(boxType, trendlineFieldValues, undefined); - expect(config.geomOptions.trendlineType).toBe('Linear'); - expect(config.geomOptions.trendlineAsymptoteMin).toBe('0.1'); - expect(config.geomOptions.trendlineAsymptoteMax).toBe('1.0'); - }); - - test('bar chart specifics', () => { - const boxType = { - name: 'bar_chart', - fields: [{ name: 'x' }, { name: 'y' }], - } as ChartTypeInfo; - - const fieldValues2 = { - x: { value: 'field1', label: 'Field 1', data: { fieldKey: 'field1' } }, - y: { value: 'field2', label: 'Field 2', data: { fieldKey: 'field2' } }, - 'aggregate-method': { value: 'MEAN', name: 'Mean' }, - }; - - const config = getChartBuilderChartConfig(boxType, fieldValues2, undefined); - expect(config.geomOptions.boxFillColor).not.toBe('none'); - expect(config.geomOptions.lineWidth).toBe(1); - expect(config.geomOptions.opacity).toBe(1.0); - expect(config.geomOptions.pointSize).toBe(5); - expect(config.geomOptions.position).toBe(null); - expect(config.labels.y).toBe('Mean of Field 2'); - }); -}); - describe('getDefaultBarChartAxisLabel', () => { test('no aggregate', () => { expect(getDefaultBarChartAxisLabel({ measures: {} } as ChartConfig)).toBe('Count'); diff --git a/packages/components/src/internal/components/chart/ChartBuilderModal.tsx b/packages/components/src/internal/components/chart/ChartBuilderModal.tsx index 5445e6378d..953d40fa97 100644 --- a/packages/components/src/internal/components/chart/ChartBuilderModal.tsx +++ b/packages/components/src/internal/components/chart/ChartBuilderModal.tsx @@ -1,14 +1,11 @@ -import React, { FC, Fragment, memo, useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import classNames from 'classnames'; +import React, { FC, memo, useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import { PermissionTypes, Utils } from '@labkey/api'; +import { PermissionTypes } from '@labkey/api'; import { generateId } from '../../util/utils'; import { LABKEY_VIS } from '../../constants'; import { Modal } from '../../Modal'; -import { SelectInputOption } from '../forms/input/SelectInput'; - import { LoadingSpinner } from '../base/LoadingSpinner'; import { QueryModel } from '../../../public/QueryModel/QueryModel'; @@ -22,32 +19,20 @@ import { FormButtons } from '../../FormButtons'; import { getContainerFilterForFolder } from '../../query/api'; -import { SVGIcon } from '../base/SVGIcon'; - import { isAppHomeFolder } from '../../app/utils'; import { deleteChart, saveChart, SaveReportConfig } from './actions'; -import { - BAR_CHART_AGGREGATE_NAME, - BAR_CHART_ERROR_BAR_NAME, - BLUE_HEX_COLOR, - HIDDEN_CHART_TYPES, - ICONS, - MAX_POINT_DISPLAY, - MAX_ROWS_PREVIEW, - RIGHT_COL_FIELDS, -} from './constants'; - -import { ChartConfig, ChartQueryConfig, ChartTypeInfo, GenericChartModel, TrendlineType } from './models'; -import { TrendlineOption } from './TrendlineOption'; -import { ChartFieldOption } from './ChartFieldOption'; -import { getFieldDataType } from './utils'; +import { BLUE_HEX_COLOR, HIDDEN_CHART_TYPES, MAX_POINT_DISPLAY, MAX_ROWS_PREVIEW } from './constants'; + +import { BaseChartModel, ChartConfig, ChartQueryConfig, ChartTypeInfo, GenericChartModel } from './models'; +import { deepCopyChartConfig } from './utils'; +import { ChartSettingsPanel } from './ChartSettingsPanel'; export const getChartRenderMsg = (chartConfig: ChartConfig, rowCount: number, isPreview: boolean): string => { const msg = []; if (isPreview && rowCount === MAX_ROWS_PREVIEW) { msg.push(`The preview is being limited to ${MAX_ROWS_PREVIEW.toLocaleString()} rows.`); } - if (chartConfig.renderType === 'line_plot' && rowCount > chartConfig.geomOptions.binThreshold) { + if (chartConfig.renderType === 'line_plot' && rowCount > (chartConfig.geomOptions.binThreshold as number)) { msg.push(`The number of individual points exceeds ${MAX_POINT_DISPLAY.toLocaleString()}.`); msg.push('Data points will not be shown on this line plot.'); } else if (chartConfig.renderType === 'scatter_plot' && rowCount > MAX_POINT_DISPLAY) { @@ -59,342 +44,31 @@ export const getChartRenderMsg = (chartConfig: ChartConfig, rowCount: number, is export const getChartBuilderQueryConfig = ( model: QueryModel, - fieldValues: Record, chartConfig: ChartConfig, savedConfig: ChartQueryConfig ): ChartQueryConfig => { const { schemaQuery, containerPath } = model; const { schemaName, queryName, viewName } = schemaQuery; + const columns = Object.values(chartConfig.measures) + .map(measure => measure?.fieldKey) + .filter(fk => fk !== undefined); + return { maxRows: -1, // this will be saved with the queryConfig, but we will override it for the preview in the modal requiredVersion: '17.1', // Issue 47898: include formattedValue in response row objects schemaName: savedConfig?.schemaName || schemaName, queryName: savedConfig?.queryName || queryName, viewName: savedConfig?.viewName || viewName, - columns: Object.values(fieldValues) - .filter(field => field?.value && typeof field.value === 'string') // just those fields with values - .filter(field => !field.equation) // exclude the trendlineType field (which has an equation value) - .map(field => field.data?.fieldKey ?? field.value), // Issue 52050: use fieldKey for special characters + columns, sort: LABKEY_VIS.GenericChartHelper.getQueryConfigSortKey(chartConfig.measures), filterArray: savedConfig?.filterArray ?? [], containerPath: savedConfig?.containerPath || containerPath, } as ChartQueryConfig; }; -export const getDefaultBarChartAxisLabel = (config: ChartConfig): string => { - const aggregate = config.measures.y?.aggregate; - const prefix = (aggregate?.name ?? aggregate?.label ?? 'Sum') + ' of '; - return config.measures.y ? prefix + config.measures.y.label : 'Count'; -}; - -export const getChartBuilderChartConfig = ( - chartType: ChartTypeInfo, - fieldValues: Record, - savedConfig: ChartConfig -): ChartConfig => { - const config = { - renderType: chartType.name, - measures: {}, - scales: { - ...savedConfig?.scales, - }, - labels: { - main: '', - subtitle: '', - ...savedConfig?.labels, - }, - pointType: savedConfig?.pointType ?? 'all', - gridLinesVisible: chartType.name === 'bar_chart' || chartType.name === 'box_plot' ? 'x' : 'both', - geomOptions: { - binShape: 'hex', - binSingleColor: '000000', - binThreshold: MAX_POINT_DISPLAY, - boxFillColor: chartType.name === 'box_plot' ? 'none' : BLUE_HEX_COLOR, - chartLayout: 'single', - chartSubjectSelection: 'subjects', - colorPaletteScale: 'ColorDiscrete', - colorRange: 'BlueWhite', - displayIndividual: true, - displayAggregate: false, - errorBars: 'None', - gradientColor: 'FFFFFF', - gradientPercentage: 95, - hideDataPoints: false, - hideTrendLine: false, - lineColor: '000000', - lineWidth: chartType.name === 'line_plot' ? 3 : 1, - marginBottom: null, - marginLeft: null, - marginRight: null, - marginTop: 20, // this will be saved with the chartConfig, but we will override it for the preview in the modal - opacity: chartType.name === 'bar_chart' || chartType.name === 'line_plot' ? 1.0 : 0.5, - pieHideWhenLessThanPercentage: 5, - pieInnerRadius: 0, - pieOuterRadius: 80, - piePercentagesColor: '333333', - pointFillColor: BLUE_HEX_COLOR, - pointSize: chartType.name === 'box_plot' ? 3 : 5, - position: chartType.name === 'box_plot' ? 'jitter' : null, - showOutliers: true, - showPiePercentages: true, - trendlineType: undefined, - trendlineAsymptoteMin: undefined, - trendlineAsymptoteMax: undefined, - trendlineParameters: undefined, - ...savedConfig?.geomOptions, - }, - } as ChartConfig; - - chartType.fields.forEach(field => { - const fieldConfig = fieldValues[field.name]; - if (fieldConfig?.value) { - config.measures[field.name] = { - fieldKey: fieldConfig.data.fieldKey, - name: fieldConfig.data.name, - label: fieldConfig.label, - queryName: fieldConfig.data.queryName, - schemaName: fieldConfig.data.schemaName, - type: getFieldDataType(fieldConfig.data), - }; - - // check if the field has an aggregate method and error bar method (bar chart y-axis only) - if (fieldValues[BAR_CHART_AGGREGATE_NAME] && field.name === 'y') { - config.measures[field.name].aggregate = { ...fieldValues[BAR_CHART_AGGREGATE_NAME] }; - if (fieldValues[BAR_CHART_ERROR_BAR_NAME]) { - config.measures[field.name].errorBars = fieldValues[BAR_CHART_ERROR_BAR_NAME]?.value; - } - } - - // update axis label if it is a new report or if the saved report that didn't have this measure or was using the default field label for the axis label - if ( - !savedConfig || - !savedConfig.measures[field.name] || - savedConfig.labels[field.name] === savedConfig.measures[field.name].label - ) { - config.labels[field.name] = fieldValues[field.name].label; - } - } - }); - - if (fieldValues.scales?.value) { - Object.keys(fieldValues.scales.value).forEach(key => { - config.scales[key] = { ...fieldValues.scales.value[key] }; - }); - } - - if (chartType.name === 'line_plot' && fieldValues.trendlineType) { - const type = fieldValues.trendlineType?.value ?? ''; - config.geomOptions.trendlineType = type === '' ? undefined : type; - config.geomOptions.trendlineAsymptoteMin = fieldValues.trendlineAsymptoteMin?.value; - config.geomOptions.trendlineAsymptoteMax = fieldValues.trendlineAsymptoteMax?.value; - config.geomOptions.trendlineParameters = fieldValues.trendlineParameters?.value; - } - - if ( - chartType.name === 'bar_chart' && - (!savedConfig || - !savedConfig.labels?.['y'] || - savedConfig.labels?.['y'] === getDefaultBarChartAxisLabel(savedConfig)) - ) { - config.labels['y'] = getDefaultBarChartAxisLabel(config); - } - - return config; -}; - -interface ChartTypeSideBarProps { - chartTypes: ChartTypeInfo[]; - onChange: (e: React.MouseEvent) => void; - savedChartModel: GenericChartModel; - selectedType: ChartTypeInfo; -} - -const ChartTypeSideBar: FC = memo(props => { - const { chartTypes, savedChartModel, selectedType, onChange } = props; - - return ( - <> - {chartTypes.map(type => { - const selected = selectedType.name === type.name; - const selectable = !savedChartModel && selectedType.name !== type.name; - - return ( -
- -
{type.title}
-
- ); - })} - - ); -}); -ChartTypeSideBar.displayName = 'ChartTypeSideBar'; - -interface ChartTypeQueryFormProps { - allowInherit: boolean; - canShare: boolean; - fieldValues: Record; - inheritable: boolean; - model: QueryModel; - name: string; - onFieldChange: (key: string, value: SelectInputOption) => void; - onNameChange: (event: React.ChangeEvent) => void; - onToggleInheritable: () => void; - onToggleShared: () => void; - savedChartModel: GenericChartModel; - selectedType: ChartTypeInfo; - shared: boolean; -} - -const ChartTypeQueryForm: FC = memo(props => { - const { - canShare, - onNameChange, - name, - shared, - onToggleShared, - allowInherit, - inheritable, - onToggleInheritable, - selectedType, - fieldValues, - model, - onFieldChange, - } = props; - - const leftColFields = useMemo(() => { - return selectedType.fields.filter( - field => !RIGHT_COL_FIELDS.includes(field.name) && (selectedType.name !== 'bar_chart' || field.name !== 'y') - ); - }, [selectedType]); - const rightColFields = useMemo(() => { - return selectedType.fields.filter( - field => - (RIGHT_COL_FIELDS.includes(field.name) && !field.altSelectionOnly) || - (selectedType.name === 'bar_chart' && field.name === 'y') - ); - }, [selectedType]); - - const hasTrendlineOption = useMemo( - () => selectedType.fields.filter(field => field.name === 'trendline').length > 0, - [selectedType] - ); - - const onErrorBarChange = useCallback( - (name: string, value: string) => { - onFieldChange(name, { value }); - }, - [onFieldChange] - ); - - const onSelectFieldChange = useCallback( - (key: string, _: never, selectedOption: SelectInputOption) => { - // clear / reset trendline option here if x change - if (hasTrendlineOption && key === 'x') { - onFieldChange('trendlineType', LABKEY_VIS.GenericChartHelper.TRENDLINE_OPTIONS['']); - } - - onFieldChange(key, selectedOption); - }, - [onFieldChange, hasTrendlineOption] - ); - - const onFieldScaleChange = useCallback( - (field: string, key: string, value: number | string, reset = false) => { - const scales = fieldValues.scales?.value ?? {}; - if (!scales[field] || reset) scales[field] = { type: 'automatic', trans: 'linear' }; - if (key) scales[field][key] = value; - onFieldChange('scales', { value: scales }); - }, - [fieldValues.scales?.value, onFieldChange] - ); - - return ( -
-
-
- - - {canShare && ( -
- - Make this chart available to all users -
- )} - {allowInherit && ( -
- - Make this chart available in child folders -
- )} -
-
- {leftColFields.map(field => ( - - ))} -
-
- {rightColFields.map(field => ( - - - - ))} - {hasTrendlineOption && ( - - )} -
-
-
- ); -}); -ChartTypeQueryForm.displayName = 'ChartTypeQueryForm'; - interface ChartPreviewProps { - fieldValues: Record; + chartConfig: ChartConfig; hasRequiredValues: boolean; model: QueryModel; savedChartModel: GenericChartModel; @@ -403,7 +77,7 @@ interface ChartPreviewProps { } const ChartPreview: FC = memo(props => { - const { hasRequiredValues, model, selectedType, fieldValues, savedChartModel, setReportConfig } = props; + const { chartConfig, hasRequiredValues, model, selectedType, savedChartModel, setReportConfig } = props; const divId = useMemo(() => generateId('chart-'), []); const ref = useRef(undefined); const containerFilter = useMemo(() => getContainerFilterForFolder(model.containerPath), [model.containerPath]); @@ -416,14 +90,8 @@ const ChartPreview: FC = memo(props => { if (!hasRequiredValues) return; - const chartConfig = getChartBuilderChartConfig( - selectedType, - fieldValues, - savedChartModel?.visualizationConfig?.chartConfig - ); const queryConfig = getChartBuilderQueryConfig( model, - fieldValues, chartConfig, savedChartModel?.visualizationConfig?.queryConfig ); @@ -475,7 +143,7 @@ const ChartPreview: FC = memo(props => { } // adjust height, width, and marginTop for the chart config for the preview, but not to save with the chart - const width = ref?.current.getBoundingClientRect().width || 750; + const width = ref?.current?.getBoundingClientRect().width || 750; const chartConfig_ = { ...chartConfig, height: 350, @@ -492,10 +160,10 @@ const ChartPreview: FC = memo(props => { setLoadingData(false); } ); - }, [divId, model, hasRequiredValues, selectedType, fieldValues, savedChartModel, containerFilter, setReportConfig]); + }, [divId, model, hasRequiredValues, selectedType, savedChartModel, containerFilter, setReportConfig, chartConfig]); return ( - <> +
{previewMsg && {previewMsg}} {!hasRequiredValues &&
Select required fields to preview the chart.
} @@ -510,7 +178,7 @@ const ChartPreview: FC = memo(props => {
)} - +
); }); ChartPreview.displayName = 'ChartPreview'; @@ -604,11 +272,6 @@ interface ChartBuilderModalProps extends RequiresModelAndActions { export const ChartBuilderModal: FC = memo(({ actions, model, onHide, savedChartModel }) => { const CHART_TYPES = useMemo(() => LABKEY_VIS?.GenericChartHelper.getRenderTypes(), []); - const TRENDLINE_OPTIONS: TrendlineType[] = useMemo( - () => Object.values(LABKEY_VIS.GenericChartHelper.TRENDLINE_OPTIONS), - [] - ); - const { user, container, moduleContext } = useServerContext(); const canShare = useMemo( () => savedChartModel?.canShare ?? hasPermissions(user, [PermissionTypes.ShareReportPermission]), @@ -623,116 +286,34 @@ export const ChartBuilderModal: FC = memo(({ actions, mo () => CHART_TYPES.filter(type => !type.hidden && !HIDDEN_CHART_TYPES.includes(type.name)), [CHART_TYPES] ); - const chartConfig = useMemo(() => savedChartModel?.visualizationConfig?.chartConfig, [savedChartModel]); + const [chartModel, setChartModel] = useState(() => ({ + inheritable: savedChartModel?.inheritable ?? false, + name: savedChartModel?.name ?? '', + shared: savedChartModel?.shared ?? true, + })); + const [chartConfig, setChartConfig] = useState(() => + deepCopyChartConfig(savedChartModel?.visualizationConfig?.chartConfig) + ); const [saving, setSaving] = useState(false); const [error, setError] = useState(); const [reportConfig, setReportConfig] = useState(); - const [selectedType, setSelectedChartType] = useState( - chartTypes.find(c => chartConfig?.renderType === c.name) ?? chartTypes[0] + const selectedType = useMemo( + () => chartTypes.find(c => chartConfig.renderType === c.name), + [chartConfig.renderType, chartTypes] ); - const [name, setName] = useState(savedChartModel?.name ?? ''); - const [shared, setShared] = useState(savedChartModel?.shared ?? canShare); - const [inheritable, setInheritable] = useState(savedChartModel?.inheritable ?? false); - - const initFieldValues = useMemo(() => { - if (savedChartModel) { - const measures = chartConfig?.measures || {}; - const fieldValues_ = Object.keys(measures).reduce((result, key) => { - let measure = measures[key]; - if (measure) { - // Currently only supporting a single measure per axis (i.e. not supporting y-axis left/right) - if (Utils.isArray(measure)) measure = measure[0]; - result[key] = { label: measure.label, value: measure.name, data: measure }; - } - return result; - }, {}); - - // handle scales - if (chartConfig?.scales) { - fieldValues_['scales'] = { value: { ...chartConfig.scales } }; - } - // handle bar chart aggregate method and error bars - const y = Utils.isArray(measures.y) ? measures.y[0] : measures.y; - if (y?.aggregate) { - fieldValues_[BAR_CHART_AGGREGATE_NAME] = Utils.isObject(y.aggregate) - ? { ...y.aggregate } - : { value: y.aggregate }; - if (y.errorBars) { - fieldValues_[BAR_CHART_ERROR_BAR_NAME] = { value: y.errorBars }; - } - } - - // handle trendline options - if (chartConfig?.geomOptions?.trendlineType) { - fieldValues_['trendlineType'] = TRENDLINE_OPTIONS.find( - option => option.value === chartConfig.geomOptions.trendlineType - ); - if (chartConfig.geomOptions.trendlineAsymptoteMin) { - fieldValues_['trendlineAsymptoteMin'] = { - value: chartConfig.geomOptions.trendlineAsymptoteMin, - }; - } - if (chartConfig.geomOptions.trendlineAsymptoteMax) { - fieldValues_['trendlineAsymptoteMax'] = { - value: chartConfig.geomOptions.trendlineAsymptoteMax, - }; - } - if (chartConfig.geomOptions.trendlineParameters) { - fieldValues_['trendlineParameters'] = { - value: chartConfig.geomOptions.trendlineParameters, - }; - } - } - - return fieldValues_; - } - - return {}; - }, [savedChartModel, chartConfig, TRENDLINE_OPTIONS]); - const [fieldValues, setFieldValues] = useState>(initFieldValues); - - const hasName = useMemo(() => name?.trim().length > 0, [name]); + const hasName = useMemo(() => chartModel.name?.trim().length > 0, [chartModel.name]); const hasRequiredValues = useMemo(() => { - return selectedType.fields.find(field => field.required && !fieldValues[field.name]) === undefined; - }, [selectedType, fieldValues]); - - const onChartTypeChange = useCallback( - e => { - // don't allow changing chart type for a saved report - if (savedChartModel) return; - - const selectedName = e.target.getAttribute('data-name') ?? e.target.parentElement.getAttribute('data-name'); - setSelectedChartType(chartTypes.find(type => type.name === selectedName) || chartTypes[0]); - setFieldValues({}); - }, - [chartTypes, savedChartModel] - ); - - const onNameChange = useCallback((event: React.ChangeEvent) => { - setName(event.target.value); - }, []); - - const onToggleShared = useCallback(() => { - setShared(prev => !prev); - }, []); - - const onToggleInheritable = useCallback(() => { - setInheritable(prev => !prev); - }, []); - - const onFieldChange = useCallback((key: string, value: SelectInputOption) => { - setReportConfig(undefined); // clear report config state, it will be reset after the preview loads - setFieldValues(prev => ({ ...prev, [key]: value })); - }, []); + return selectedType.fields.find(field => field.required && !chartConfig.measures[field.name]) === undefined; + }, [selectedType.fields, chartConfig.measures]); const onSaveChart = useCallback(async () => { const _reportConfig = { ...reportConfig, reportId: savedChartModel?.reportId, - name: name?.trim(), - public: shared, - inheritable, + name: chartModel.name.trim(), + public: chartModel.shared, + inheritable: chartModel.inheritable, } as SaveReportConfig; setSaving(true); @@ -747,7 +328,7 @@ export const ChartBuilderModal: FC = memo(({ actions, mo setError(e.exception ?? e); setSaving(false); } - }, [savedChartModel, reportConfig, name, shared, inheritable, actions, model.id, onHide]); + }, [actions, chartModel, model.id, onHide, reportConfig, savedChartModel]); const afterDelete = useCallback(async () => { onHide('Successfully deleted chart: ' + savedChartModel.name + '.'); @@ -779,44 +360,26 @@ export const ChartBuilderModal: FC = memo(({ actions, mo title={savedChartModel ? 'Edit Chart' : 'Create Chart'} > {error && {error}} -
-
- -
-
- -
-
- -
-
-
+
+ +
); From ec06980f80542d82704f7431bda4dbd7b77d6748 Mon Sep 17 00:00:00 2001 From: alanv Date: Fri, 21 Nov 2025 16:26:40 -0600 Subject: [PATCH 13/38] ChartFieldOption: Fix issue with default trendline type --- .../src/internal/components/chart/ChartFieldOption.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/internal/components/chart/ChartFieldOption.tsx b/packages/components/src/internal/components/chart/ChartFieldOption.tsx index f415bb3c41..7535366d8e 100644 --- a/packages/components/src/internal/components/chart/ChartFieldOption.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldOption.tsx @@ -85,7 +85,7 @@ export const ChartFieldOption: FC = memo(props => { } if (name === 'x' && hasTrendline(selectedType)) { - const trendlineType = LABKEY_VIS.GenericChartHelper.TRENDLINE_OPTIONS['']; + const trendlineType = LABKEY_VIS.GenericChartHelper.TRENDLINE_OPTIONS[''].value; geomOptions = { ...geomOptions, trendlineType }; } From 4d2c23d0b9906306c31414244fc7acacd533480e Mon Sep 17 00:00:00 2001 From: alanv Date: Fri, 21 Nov 2025 16:55:08 -0600 Subject: [PATCH 14/38] ChartBuilderModal: Fix margins for title/subtitle --- .../components/chart/ChartBuilderModal.tsx | 3 --- .../components/chart/ChartSettingsPanel.tsx | 26 +++++++++++++++++-- .../src/internal/components/chart/utils.ts | 2 +- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartBuilderModal.tsx b/packages/components/src/internal/components/chart/ChartBuilderModal.tsx index 953d40fa97..f425d693e1 100644 --- a/packages/components/src/internal/components/chart/ChartBuilderModal.tsx +++ b/packages/components/src/internal/components/chart/ChartBuilderModal.tsx @@ -149,9 +149,6 @@ const ChartPreview: FC = memo(props => { height: 350, width, }; - if (!savedChartModel || savedChartModel.visualizationConfig.chartConfig.geomOptions.marginTop === 20) { - chartConfig_.geomOptions.marginTop = 15; - } if (ref.current) ref.current.innerHTML = ''; // clear again, right before render LABKEY_VIS.GenericChartHelper.generateChartSVG(divId, chartConfig_, measureStore, trendlineData); diff --git a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx index 6fa11b4f19..bf562878ac 100644 --- a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx +++ b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx @@ -27,7 +27,13 @@ const LabelInput: FC = memo(({ label, name, onChange, value }) return (
- +
); }); @@ -157,7 +163,23 @@ export const ChartSettingsPanel: FC = memo(props => { const onLabelChange = useCallback( (key: LabelKey, value: string) => { - setChartConfig(current => ({ ...current, labels: { ...current.labels, [key]: value } })); + setChartConfig(current => { + const labels = { ...current.labels, [key]: value }; + let geomOptions = current.geomOptions; + let marginTop = 15; + const hasTitle = !!labels.main?.trim(); + const hasSubtitle = !!labels.subtitle?.trim(); + + if (hasTitle && hasSubtitle) marginTop += 50; + else if (hasTitle) marginTop += 25; + // Yes, really, subtitle only gets the most padding. Our charting library is probably setting some + // default amount if main is present + else if (hasSubtitle) marginTop += 55; + + if (marginTop != geomOptions.marginTop) geomOptions = { ...geomOptions, marginTop }; + + return { ...current, labels, geomOptions }; + }); }, [setChartConfig] ); diff --git a/packages/components/src/internal/components/chart/utils.ts b/packages/components/src/internal/components/chart/utils.ts index 3487b28ee7..5a7650794b 100644 --- a/packages/components/src/internal/components/chart/utils.ts +++ b/packages/components/src/internal/components/chart/utils.ts @@ -118,7 +118,7 @@ const makeGeomOptions = (chartType: string) => ({ marginBottom: null, marginLeft: null, marginRight: null, - marginTop: 20, // this will be saved with the chartConfig, but we will override it for the preview in the modal + marginTop: 15, opacity: chartType === 'bar_chart' || chartType === 'line_plot' ? 1.0 : 0.5, pieHideWhenLessThanPercentage: 5, pieInnerRadius: 0, From 3e0fe5da51d9eb1c9130a9190c25a148c45d2b75 Mon Sep 17 00:00:00 2001 From: alanv Date: Fri, 21 Nov 2025 17:41:06 -0600 Subject: [PATCH 15/38] ChartSettingsPanel: Improve chart type options rendering --- .../components/chart/ChartSettingsPanel.tsx | 50 ++++++++++++------- .../internal/components/chart/constants.ts | 12 ++--- packages/components/src/theme/charts.scss | 9 ++++ 3 files changed, 46 insertions(+), 25 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx index bf562878ac..ebf6b158e3 100644 --- a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx +++ b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx @@ -9,6 +9,7 @@ import { ChartFieldOption } from './ChartFieldOption'; import { QueryModel } from '../../../public/QueryModel/QueryModel'; import { TrendlineOption } from './TrendlineOption'; import { deepCopyChartConfig, hasTrendline } from './utils'; +import classNames from 'classnames'; type LabelKey = keyof ChartConfig['labels']; @@ -60,16 +61,34 @@ const BoolSettingInput: FC = memo(({ label, name, onChang }); BoolSettingInput.displayName = 'BoolSettingInput'; -// TODO: use the same icons as the ChartMenu -// - Not the most straightforward because those icons are set server-side -function chartTypeOptionRenderer(option: SelectInputOption) { - const data = option.data.data; +interface ChartTypeOptionRendererProps { + chartType: ChartTypeInfo; + isValueRenderer: boolean; +} +const ChartTypeOptionRenderer: FC = memo(({ chartType, isValueRenderer }) => { + const icon = ICONS[chartType.name]; + const isSvg = icon.endsWith('.svg'); + const className = classNames('chart-builder-type-option', { 'chart-builder-type-option--value': isValueRenderer }); return ( -
- - {data.title} -
+ + {isSvg && ( + + )} + {!isSvg && } + {chartType.title} + ); +}); +ChartTypeOptionRenderer.displayName = 'ChartTypeOptionRenderer'; + +function chartTypeOptionRenderer(option) { + const chartType: ChartTypeInfo = option.data as ChartTypeInfo; + return ; +} + +function chartTypeValueRenderer(option) { + const chartType: ChartTypeInfo = option.data as ChartTypeInfo; + return ; } interface ChartTypeDropdownProps { @@ -80,17 +99,11 @@ interface ChartTypeDropdownProps { const ChartTypeDropdown: FC = memo(({ onChange, selectedType }) => { const chartTypes = useMemo(() => { const allTypes = LABKEY_VIS?.GenericChartHelper.getRenderTypes(); - return allTypes - .filter(type => !type.hidden && !HIDDEN_CHART_TYPES.includes(type.name)) - .map(type => ({ - data: type, - label: type.title, - value: type.name, - })); + return allTypes.filter(type => !type.hidden && !HIDDEN_CHART_TYPES.includes(type.name)); }, []); const onChange_ = useCallback( (_, __, opt) => { - onChange(opt.data); + onChange(opt); }, [onChange] ); @@ -103,13 +116,14 @@ const ChartTypeDropdown: FC = memo(({ onChange, selected clearable={false} containerClass="" inputClass="col-xs-12" + labelKey="title" name="chartType" onChange={onChange_} optionRenderer={chartTypeOptionRenderer} options={chartTypes} value={selectedType.name} - // TODO: using chartTypeOptionRenderer as valueRenderer makes the dropdown too tall - // valueRenderer={chartTypeOptionRenderer} + valueKey="name" + valueRenderer={chartTypeValueRenderer} />
diff --git a/packages/components/src/internal/components/chart/constants.ts b/packages/components/src/internal/components/chart/constants.ts index f3fc809977..a92d60e7cb 100644 --- a/packages/components/src/internal/components/chart/constants.ts +++ b/packages/components/src/internal/components/chart/constants.ts @@ -2,14 +2,12 @@ export const HIDDEN_CHART_TYPES = ['time_chart']; export const MAX_ROWS_PREVIEW = 10000; export const MAX_POINT_DISPLAY = 10000; export const BLUE_HEX_COLOR = '3366FF'; -export const BAR_CHART_AGGREGATE_NAME = 'aggregate-method'; -export const BAR_CHART_ERROR_BAR_NAME = 'error-bar-method'; export const ICONS = { - bar_chart: 'bar_chart', - box_plot: 'box_plot', - pie_chart: 'pie_chart', - scatter_plot: 'xy_scatter', - line_plot: 'xy_line', + bar_chart: 'fa-bar-chart', + box_plot: 'box_plot_icon.svg', + line_plot: 'fa-line-chart', + pie_chart: 'fa-pie-chart', + scatter_plot: 'scatter_plot_icon.svg', }; export const AGGREGATE_METHODS = [ diff --git a/packages/components/src/theme/charts.scss b/packages/components/src/theme/charts.scss index 8b835c21d1..1450ac6bc5 100644 --- a/packages/components/src/theme/charts.scss +++ b/packages/components/src/theme/charts.scss @@ -191,6 +191,15 @@ text-decoration: none; } +.chart-builder-type-option > img, +.chart-builder-type-option > .fa { + margin-right: 5px; +} +.chart-builder-type-option--value { + margin-top: -27px; + margin-left: 5px; +} + .chart-builder-modal { width: calc(100% - 100px); min-width: 850px; From aac25b454808bf5fbd18702870eea2ff94b2b642 Mon Sep 17 00:00:00 2001 From: cnathe Date: Tue, 25 Nov 2025 15:03:16 -0600 Subject: [PATCH 16/38] First round of jest test updates to use chartConfig instead of fieldValues --- .../chart/ChartBuilderModal.test.tsx | 92 ++++--------- .../chart/ChartFieldAggregateOptions.test.tsx | 88 ++++++------ .../chart/ChartFieldOption.test.tsx | 70 +++++----- .../components/chart/TrendlineOption.test.tsx | 130 +++++++++++------- .../internal/components/chart/utils.test.ts | 27 +++- 5 files changed, 218 insertions(+), 189 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartBuilderModal.test.tsx b/packages/components/src/internal/components/chart/ChartBuilderModal.test.tsx index 8f56347296..6f7943e5ad 100644 --- a/packages/components/src/internal/components/chart/ChartBuilderModal.test.tsx +++ b/packages/components/src/internal/components/chart/ChartBuilderModal.test.tsx @@ -21,7 +21,6 @@ import { ChartBuilderModal, getChartBuilderQueryConfig, getChartRenderMsg, - getDefaultBarChartAxisLabel, } from './ChartBuilderModal'; import { MAX_POINT_DISPLAY, MAX_ROWS_PREVIEW } from './constants'; import { ChartConfig, ChartQueryConfig, ChartTypeInfo, GenericChartModel } from './models'; @@ -117,19 +116,19 @@ const SERVER_CONTEXT = { describe('ChartBuilderModal', () => { function validate(isNew: boolean, canShare = true, canDelete = false, allowInherit = false): void { expect(document.querySelectorAll('.chart-builder-modal')).toHaveLength(1); + expect(document.querySelectorAll('.chart-builder-modal__settings-panel')).toHaveLength(1); + expect(document.querySelectorAll('.chart-builder-modal__chart-preview')).toHaveLength(1); expect(document.querySelector('.modal-title').textContent).toBe(isNew ? 'Create Chart' : 'Edit Chart'); expect(document.querySelectorAll('.btn')).toHaveLength(canDelete ? 3 : 2); - expect(document.querySelectorAll('.alert')).toHaveLength(0); - expect(document.querySelectorAll('.col-left')).toHaveLength(1); - expect(document.querySelectorAll('.col-right')).toHaveLength(1); + // TODO update this part of jest test // hidden chart types are filtered out - const chartTypeItems = document.querySelectorAll('.chart-builder-type'); - expect(chartTypeItems).toHaveLength(3); - expect(chartTypeItems[0].textContent).toBe('Bar'); - expect(chartTypeItems[1].textContent).toBe('Scatter'); - expect(chartTypeItems[2].textContent).toBe('Line'); + // const chartTypeItems = document.querySelectorAll('.chart-builder-type'); + // expect(chartTypeItems).toHaveLength(3); + // expect(chartTypeItems[0].textContent).toBe('Bar'); + // expect(chartTypeItems[1].textContent).toBe('Scatter'); + // expect(chartTypeItems[2].textContent).toBe('Line'); expect(document.querySelectorAll('input[name="name"]')).toHaveLength(1); expect(document.querySelectorAll('input[name="shared"]')).toHaveLength(canShare ? 1 : 0); @@ -153,13 +152,8 @@ describe('ChartBuilderModal', () => { validate(true); // default to selecting the first chart type - expect(document.querySelector('.selected').textContent).toBe('Bar'); - expect(document.querySelector('.selectable').textContent).toBe('Scatter'); - // selected should use blue icon and selectable should use gray icon - expect(document.querySelector('.selected').querySelector('img').getAttribute('alt')).toBe('bar_chart-icon'); - expect(document.querySelector('.selectable').querySelector('img').getAttribute('alt')).toBe( - 'xy_scatter_gray-icon' - ); + expect(document.querySelector('.chart-builder-type-option--value').textContent).toBe('Bar'); + expect(document.querySelector('input[name=chartType]').getAttribute('value')).toBe('bar_chart'); // default to shared expect(document.querySelector('input[name="shared"]').getAttribute('checked')).toBe(''); @@ -176,7 +170,7 @@ describe('ChartBuilderModal', () => { ); validate(true, false); - expect(document.querySelectorAll('input')).toHaveLength(5); + expect(document.querySelectorAll('input')).toHaveLength(9); }); test('allowInherit false, user perm', () => { @@ -192,7 +186,7 @@ describe('ChartBuilderModal', () => { ); validate(true); - expect(document.querySelectorAll('input')).toHaveLength(6); + expect(document.querySelectorAll('input')).toHaveLength(10); }); test('allowInherit false, non-project', () => { @@ -208,7 +202,7 @@ describe('ChartBuilderModal', () => { ); validate(true); - expect(document.querySelectorAll('input')).toHaveLength(6); + expect(document.querySelectorAll('input')).toHaveLength(10); }); test('allowInherit true', () => { @@ -224,7 +218,7 @@ describe('ChartBuilderModal', () => { ); validate(true, true, false, true); - expect(document.querySelectorAll('input')).toHaveLength(7); + expect(document.querySelectorAll('input')).toHaveLength(11); }); test('field inputs displayed for selected chart type', async () => { @@ -238,7 +232,7 @@ describe('ChartBuilderModal', () => { validate(true); // verify field inputs displayed for default / first chart type - expect(document.querySelectorAll('input')).toHaveLength(6); + expect(document.querySelectorAll('input')).toHaveLength(10); BAR_CHART_TYPE.fields.forEach(field => { expect(document.querySelectorAll(`input[name="${field.name}"]`)).toHaveLength(1); }); @@ -293,13 +287,11 @@ describe('ChartBuilderModal', () => { ); validate(false, true, true); - expect(document.querySelectorAll('input')).toHaveLength(8); + expect(document.querySelectorAll('input')).toHaveLength(10); // default to selecting the chart type based on saved config - expect(document.querySelector('.selected').textContent).toBe('Scatter'); - expect(document.querySelectorAll('.selectable')).toHaveLength(0); - // selected should use blue icon - expect(document.querySelector('.selected').querySelector('img').getAttribute('alt')).toBe('xy_scatter-icon'); + expect(document.querySelector('.chart-builder-type-option--value').textContent).toBe('Scatter'); + expect(document.querySelector('input[name=chartType]').getAttribute('value')).toBe('scatter_plot'); // click delete button and verify confirm text / buttons await userEvent.click(document.querySelector('.btn-danger')); @@ -345,7 +337,7 @@ describe('ChartBuilderModal', () => { ); validate(false, true, true); - expect(document.querySelectorAll('input')).toHaveLength(6); + expect(document.querySelectorAll('input')).toHaveLength(8); expect(document.querySelector('input[name=y]').getAttribute('value')).toBe('field2'); expect(document.querySelectorAll('.fa-gear')).toHaveLength(1); // gear icon for y-axis await userEvent.click(document.querySelector('.fa-gear')); @@ -390,7 +382,7 @@ describe('ChartBuilderModal', () => { ); validate(false, true, true); - expect(document.querySelectorAll('input')).toHaveLength(6); + expect(document.querySelectorAll('input')).toHaveLength(8); expect(document.querySelector('input[name=y]').getAttribute('value')).toBe('field2'); expect(document.querySelectorAll('.fa-gear')).toHaveLength(1); // gear icon for y-axis await userEvent.click(document.querySelector('.fa-gear')); @@ -438,7 +430,7 @@ describe('ChartBuilderModal', () => { ); validate(false, true, true); - expect(document.querySelectorAll('input')).toHaveLength(10); + expect(document.querySelectorAll('input')).toHaveLength(12); expect(document.querySelector('input[name=x]').getAttribute('value')).toBe('field1'); expect(document.querySelector('input[name=y]').getAttribute('value')).toBe('field2'); expect(document.querySelectorAll('input[name=aggregate-method]')).toHaveLength(0); @@ -484,7 +476,7 @@ describe('ChartBuilderModal', () => { ); validate(false, true, true); - expect(document.querySelectorAll('input')).toHaveLength(10); + expect(document.querySelectorAll('input')).toHaveLength(12); expect(document.querySelector('input[name=x]').getAttribute('value')).toBe('field1'); expect(document.querySelector('input[name=y]').getAttribute('value')).toBe('field2'); expect(document.querySelectorAll('input[name=aggregate-method]')).toHaveLength(0); @@ -542,7 +534,7 @@ describe('ChartBuilderModal', () => { ); validate(false, false, false); - expect(document.querySelectorAll('input')).toHaveLength(7); + expect(document.querySelectorAll('input')).toHaveLength(9); expect(document.querySelector('input[name="shared"]')).toBeNull(); }); }); @@ -583,15 +575,13 @@ describe('getChartRenderMsg', () => { }); describe('getChartBuilderQueryConfig', () => { - const chartConfig = { measures: {} } as ChartConfig; - const fieldValues = { - x: { value: 'field1', label: 'Field 1', data: { fieldKey: 'field1' } }, - y: { value: undefined }, - scales: { value: { x: { type: 'automatic', trans: 'linear' }, y: { type: 'automatic', trans: 'linear' } } }, - }; + const chartConfig = { measures: { + x: { name: 'field1', label: 'Field 1', fieldKey: 'field1' }, + y: { name: undefined }, + } } as ChartConfig; test('based on model', () => { - const config = getChartBuilderQueryConfig(model, fieldValues, chartConfig, undefined); + const config = getChartBuilderQueryConfig(model, chartConfig, undefined); expect(config.maxRows).toBe(-1); expect(config.requiredVersion).toBe('17.1'); expect(config.schemaName).toBe('schema'); @@ -610,7 +600,7 @@ describe('getChartBuilderQueryConfig', () => { filterArray: [{ name: 'savedFilter' }], } as ChartQueryConfig; - const config = getChartBuilderQueryConfig(model, fieldValues, chartConfig, savedConfig); + const config = getChartBuilderQueryConfig(model, chartConfig, savedConfig); expect(config.maxRows).toBe(-1); expect(config.requiredVersion).toBe('17.1'); expect(config.schemaName).toBe('savedSchema'); @@ -621,27 +611,3 @@ describe('getChartBuilderQueryConfig', () => { expect(config.filterArray.length).toBe(1); }); }); - -describe('getDefaultBarChartAxisLabel', () => { - test('no aggregate', () => { - expect(getDefaultBarChartAxisLabel({ measures: {} } as ChartConfig)).toBe('Count'); - expect(getDefaultBarChartAxisLabel({ measures: { x: { label: 'Test' } } } as ChartConfig)).toBe('Count'); - }); - - test('with aggregate', () => { - expect(getDefaultBarChartAxisLabel({ measures: { y: { label: 'Test' } } } as ChartConfig)).toBe('Sum of Test'); - expect(getDefaultBarChartAxisLabel({ measures: { y: { label: 'Test', aggregate: {} } } } as ChartConfig)).toBe( - 'Sum of Test' - ); - expect( - getDefaultBarChartAxisLabel({ - measures: { y: { label: 'Test', aggregate: { name: 'Min' } } }, - } as ChartConfig) - ).toBe('Min of Test'); - expect( - getDefaultBarChartAxisLabel({ - measures: { y: { label: 'Test', aggregate: { label: 'Max' } } }, - } as ChartConfig) - ).toBe('Max of Test'); - }); -}); diff --git a/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.test.tsx b/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.test.tsx index 6b64cfdfe2..a09dcb5905 100644 --- a/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.test.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.test.tsx @@ -2,24 +2,31 @@ import React from 'react'; import { render } from '@testing-library/react'; import { userEvent } from '@testing-library/user-event'; import { ChartFieldAggregateOptions } from './ChartFieldAggregateOptions'; -import { BAR_CHART_AGGREGATE_NAME, BAR_CHART_ERROR_BAR_NAME } from './constants'; -import { ChartTypeInfo } from './models'; +import { ChartConfig, ChartTypeInfo } from './models'; const field = { name: 'testField', label: 'Test Label', required: false }; -const fieldValues = { - testField: { value: 'ABC' }, - [BAR_CHART_AGGREGATE_NAME]: { value: 'SUM' }, - [BAR_CHART_ERROR_BAR_NAME]: undefined, -}; +const chartConfig = { + geomOptions: {}, + gridLinesVisible: undefined, + labels: {}, + measures: { + y: { + aggregate: { value: 'SUM' }, + errorBars: undefined, + }, + }, + pointType: undefined, + renderType: 'bar_chart', + scales: {}, +} as ChartConfig; function renderComponent(props = {}) { return render( ); @@ -57,11 +64,16 @@ describe('ChartFieldAggregateOptions', () => { }); test('error bar radios are enabled for aggregate MEAN', async () => { - const fieldValuesMean = { - ...fieldValues, - [BAR_CHART_AGGREGATE_NAME]: { value: 'MEAN' }, - }; - renderComponent({ fieldValues: fieldValuesMean }); + const meanChartConfig = { + ...chartConfig, + measures: { + y: { + aggregate: { value: 'MEAN' }, + errorBars: undefined, + }, + }, + } as ChartConfig; + renderComponent({ chartConfig: meanChartConfig }); await userEvent.click(document.querySelector('.fa-gear')); expect(document.querySelector('input[name="error-bar-method"][value=""]').hasAttribute('disabled')).toBeFalsy(); expect( @@ -73,13 +85,17 @@ describe('ChartFieldAggregateOptions', () => { expect(document.querySelectorAll('.radioinput-label.selected')[0].textContent).toBe('None'); }); - test('error bar radio value selected when fieldValues set', async () => { - const fieldValuesSEM = { - ...fieldValues, - [BAR_CHART_AGGREGATE_NAME]: { value: 'MEAN' }, - [BAR_CHART_ERROR_BAR_NAME]: { value: 'SEM' }, - }; - renderComponent({ fieldValues: fieldValuesSEM }); + test('error bar radio value selected when values set', async () => { + const semChartConfig = { + ...chartConfig, + measures: { + y: { + aggregate: { value: 'MEAN' }, + errorBars: 'SEM', + }, + }, + } as ChartConfig; + renderComponent({ chartConfig: semChartConfig }); await userEvent.click(document.querySelector('.fa-gear')); expect(document.querySelector('input[name="error-bar-method"][value=""]').hasAttribute('disabled')).toBeFalsy(); expect( @@ -93,25 +109,17 @@ describe('ChartFieldAggregateOptions', () => { ); }); - test('does not render if no field is selected', async () => { - const emptyFieldValues = { - testField: undefined, - [BAR_CHART_AGGREGATE_NAME]: { value: 'MEAN' }, - [BAR_CHART_ERROR_BAR_NAME]: { value: 'SEM' }, - }; - renderComponent({ fieldValues: emptyFieldValues }); - await userEvent.click(document.querySelector('.fa-gear')); - expect(document.querySelectorAll('label')).toHaveLength(0); - expect(document.querySelectorAll('input[type="radio"]')).toHaveLength(0); - }); - test('renders inline inputs when asOverlay is false', () => { - const fieldValuesSEM = { - ...fieldValues, - [BAR_CHART_AGGREGATE_NAME]: { value: 'MEAN' }, - [BAR_CHART_ERROR_BAR_NAME]: { value: 'SEM' }, - }; - renderComponent({ fieldValues: fieldValuesSEM, asOverlay: false }); + const semChartConfig = { + ...chartConfig, + measures: { + y: { + aggregate: { value: 'MEAN' }, + errorBars: 'SEM', + }, + }, + } as ChartConfig; + renderComponent({ chartConfig: semChartConfig, asOverlay: false }); expect(document.querySelectorAll('.field-option-icon')).toHaveLength(0); expect(document.querySelectorAll('.fa-gear')).toHaveLength(0); expect(document.querySelectorAll('.lk-popover')).toHaveLength(0); diff --git a/packages/components/src/internal/components/chart/ChartFieldOption.test.tsx b/packages/components/src/internal/components/chart/ChartFieldOption.test.tsx index c67e2e8455..2af13d38fb 100644 --- a/packages/components/src/internal/components/chart/ChartFieldOption.test.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldOption.test.tsx @@ -65,14 +65,14 @@ describe('ChartFieldOption', () => { test('line chart for x, showFieldOptions for int', async () => { render( ); @@ -86,14 +86,14 @@ describe('ChartFieldOption', () => { test('line chart for x, not showFieldOptions for date', async () => { render( ); @@ -107,14 +107,14 @@ describe('ChartFieldOption', () => { test('bar chart for x, not showFieldOptions', async () => { render( ); @@ -128,14 +128,14 @@ describe('ChartFieldOption', () => { test('label for not required', async () => { render( ); @@ -147,14 +147,14 @@ describe('ChartFieldOption', () => { test('default values set for scale', async () => { render( ); @@ -180,14 +180,14 @@ describe('ChartFieldOption', () => { test('initial values set from scaleValues', async () => { render( ); @@ -218,14 +218,14 @@ describe('ChartFieldOption', () => { test('invalid scale range, max < min', async () => { render( ); diff --git a/packages/components/src/internal/components/chart/TrendlineOption.test.tsx b/packages/components/src/internal/components/chart/TrendlineOption.test.tsx index f80075bd5a..eebc9a300b 100644 --- a/packages/components/src/internal/components/chart/TrendlineOption.test.tsx +++ b/packages/components/src/internal/components/chart/TrendlineOption.test.tsx @@ -10,21 +10,31 @@ import { TrendlineOption } from './TrendlineOption'; import { makeTestQueryModel } from '../../../public/QueryModel/testUtils'; import { QueryInfo } from '../../../public/QueryInfo'; import { ViewInfo } from '../../ViewInfo'; -import { ChartTypeInfo } from './models'; +import { ChartConfig, ChartTypeInfo } from './models'; LABKEY_VIS = { GenericChartHelper: { getAllowableTypes: () => ['text'], isMeasureDimensionMatch: () => true, TRENDLINE_OPTIONS: [ - { value: 'option1', label: 'Option 1', schemaPrefix: undefined }, - { value: 'option2', label: 'Option 2', schemaPrefix: null }, - { value: 'option3', label: 'Option 3', schemaPrefix: 'other' }, - { value: 'option4', label: 'Option 4', schemaPrefix: 'assay' }, + { value: 'option1', label: 'Option 1', schemaPrefix: undefined, showMin: false, showMax: false }, + { value: 'option2', label: 'Option 2', schemaPrefix: null, showMin: false, showMax: false }, + { value: 'option3', label: 'Option 3', schemaPrefix: 'other', showMin: true, showMax: true }, + { value: 'option4', label: 'Option 4', schemaPrefix: 'assay', showMin: true, showMax: false }, ], }, }; +const baseChartConfig = { + geomOptions: {}, + gridLinesVisible: undefined, + labels: {}, + measures: {}, + pointType: undefined, + renderType: 'bar_chart', + scales: {}, +} as ChartConfig; + const LINE_PLOT_TYPE = { name: 'line_plot', } as ChartTypeInfo; @@ -57,10 +67,10 @@ describe('TrendlineOption', () => { test('hidden without x-axis value selected', async () => { render( ); @@ -76,18 +86,23 @@ describe('TrendlineOption', () => { }); test('shown with x-axis value selected, non-date', async () => { - const fieldValues = { - x: { data: { jsonType: 'int' }, value: 'field1' }, - trendlineAsymptoteMin: { value: undefined }, - trendlineAsymptoteMax: { value: undefined }, + const chartConfig = { + ...baseChartConfig, + geomOptions: { + trendlineAsymptoteMin: undefined, + trendlineAsymptoteMax: undefined, trendlineParameters: { value: undefined }, - }; + }, + measures: { + x: { name: 'field1', jsonType: 'int' }, + }, + } as ChartConfig; render( ); @@ -109,18 +124,22 @@ describe('TrendlineOption', () => { }); test('hidden with x-axis value selected, date', async () => { - const fieldValues = { - x: { data: { jsonType: 'date' }, value: 'field1' }, - trendlineAsymptoteMin: { value: undefined }, - trendlineAsymptoteMax: { value: undefined }, + const chartConfig = { + ...baseChartConfig, + geomOptions: { + trendlineAsymptoteMin: undefined, + trendlineAsymptoteMax: undefined, trendlineParameters: { value: undefined }, - }; + }, + measures: { + x: { name: 'field1', jsonType: 'date' }, + }, + } as ChartConfig; render( ); @@ -133,18 +152,22 @@ describe('TrendlineOption', () => { }); test('hidden with x-axis value selected, time', async () => { - const fieldValues = { - x: { data: { type: 'time' }, value: 'field1' }, - trendlineAsymptoteMin: { value: undefined }, - trendlineAsymptoteMax: { value: undefined }, + const chartConfig = { + ...baseChartConfig, + geomOptions: { + trendlineAsymptoteMin: undefined, + trendlineAsymptoteMax: undefined, trendlineParameters: { value: undefined }, - }; + }, + measures: { + x: { name: 'field1', jsonType: 'time' }, + }, + } as ChartConfig; render( ); @@ -157,19 +180,23 @@ describe('TrendlineOption', () => { }); test('show asymptote min and max', async () => { - const fieldValues = { - x: { data: { jsonType: 'int' }, value: 'field1' }, - trendlineType: { value: 'option1', showMin: true, showMax: true }, - trendlineAsymptoteMin: { value: '0.1' }, - trendlineAsymptoteMax: { value: '1.0' }, + const chartConfig = { + ...baseChartConfig, + geomOptions: { + trendlineType: 'option3', + trendlineAsymptoteMin: 0.1, + trendlineAsymptoteMax: 1.0, trendlineParameters: { value: undefined }, - }; + }, + measures: { + x: { name: 'field1', jsonType: 'int' }, + }, + } as ChartConfig; render( ); @@ -186,7 +213,7 @@ describe('TrendlineOption', () => { await userEvent.click(document.querySelector('input[value="manual"]')); expect(document.querySelectorAll('input[type="number"]')).toHaveLength(2); expect(document.querySelector('input[name="trendlineAsymptoteMin"]').getAttribute('value')).toBe('0.1'); - expect(document.querySelector('input[name="trendlineAsymptoteMax"]').getAttribute('value')).toBe('1.0'); + expect(document.querySelector('input[name="trendlineAsymptoteMax"]').getAttribute('value')).toBe('1'); // clicking automatic should hide the inputs and clear values await userEvent.click(document.querySelector('input[value="automatic"]')); @@ -198,20 +225,23 @@ describe('TrendlineOption', () => { }); test('show asymptote min but not max', async () => { - const fieldValues = { - x: { data: { jsonType: 'int' }, value: 'field1' }, - trendlineType: { value: 'option1', showMin: true, showMax: false }, - trendlineAsymptoteMin: { value: '0.1' }, - trendlineAsymptoteMax: { value: undefined }, + const chartConfig = { + ...baseChartConfig, + geomOptions: { + trendlineType: 'option4', + trendlineAsymptoteMin: 0.1, + trendlineAsymptoteMax: undefined, trendlineParameters: { value: undefined }, - }; + }, + measures: { + x: { name: 'field1', jsonType: 'int' }, + }, + } as ChartConfig; render( ); await waitFor(() => { diff --git a/packages/components/src/internal/components/chart/utils.test.ts b/packages/components/src/internal/components/chart/utils.test.ts index 364a65dfe5..2ebc0224fe 100644 --- a/packages/components/src/internal/components/chart/utils.test.ts +++ b/packages/components/src/internal/components/chart/utils.test.ts @@ -3,12 +3,13 @@ import { Map } from 'immutable'; import { createHorizontalBarCountLegendData, createHorizontalBarLegendData, + getDefaultBarChartAxisLabel, getFieldDataType, getSelectOptions, shouldShowAggregateOptions, shouldShowRangeScaleOptions, } from './utils'; -import { ChartFieldInfo, ChartTypeInfo } from './models'; +import { ChartConfig, ChartFieldInfo, ChartTypeInfo } from './models'; import { LABKEY_VIS } from '../../constants'; import { makeTestQueryModel } from '../../../public/QueryModel/testUtils'; import { SchemaQuery } from '../../../public/SchemaQuery'; @@ -371,6 +372,30 @@ describe('shouldShowAggregateOptions', () => { }); }); +describe('getDefaultBarChartAxisLabel', () => { + test('no aggregate', () => { + expect(getDefaultBarChartAxisLabel({ measures: {} } as ChartConfig)).toBe('Count'); + expect(getDefaultBarChartAxisLabel({ measures: { x: { label: 'Test' } } } as ChartConfig)).toBe('Count'); + }); + + test('with aggregate', () => { + expect(getDefaultBarChartAxisLabel({ measures: { y: { label: 'Test' } } } as ChartConfig)).toBe('Sum of Test'); + expect(getDefaultBarChartAxisLabel({ measures: { y: { label: 'Test', aggregate: undefined } } } as ChartConfig)).toBe( + 'Sum of Test' + ); + expect( + getDefaultBarChartAxisLabel({ + measures: { y: { label: 'Test', aggregate: 'Min' } }, + } as ChartConfig) + ).toBe('Min of Test'); + expect( + getDefaultBarChartAxisLabel({ + measures: { y: { label: 'Test', aggregate: 'Max' } }, + } as ChartConfig) + ).toBe('Max of Test'); + }); +}); + describe('getSelectOptions', () => { const columns = [ { fieldKey: 'intCol', jsonType: 'int' }, From d2b05b54f807b701a0c093ac5956b52be6f37cd7 Mon Sep 17 00:00:00 2001 From: alanv Date: Tue, 25 Nov 2025 13:19:20 -0600 Subject: [PATCH 17/38] Checkbox.tsx - minor cleanup --- packages/components/src/internal/Checkbox.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/components/src/internal/Checkbox.tsx b/packages/components/src/internal/Checkbox.tsx index 1aaa6285c2..6901ecbc2d 100644 --- a/packages/components/src/internal/Checkbox.tsx +++ b/packages/components/src/internal/Checkbox.tsx @@ -19,7 +19,6 @@ export const CheckboxLK: FC = memo(props => {
- ) + ); }); CheckboxLK.displayName = 'Checkbox'; From 3d0e54ce4ff372471fb11b99aaff99b5a250ce86 Mon Sep 17 00:00:00 2001 From: alanv Date: Tue, 25 Nov 2025 15:17:20 -0600 Subject: [PATCH 18/38] ChartSettingsPanel: Add width/height settings --- .../components/chart/ChartBuilderModal.tsx | 4 +- .../components/chart/ChartSettingsPanel.tsx | 161 +++++++++++++++--- .../src/internal/components/chart/utils.ts | 2 +- 3 files changed, 143 insertions(+), 24 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartBuilderModal.tsx b/packages/components/src/internal/components/chart/ChartBuilderModal.tsx index f425d693e1..d806a855e9 100644 --- a/packages/components/src/internal/components/chart/ChartBuilderModal.tsx +++ b/packages/components/src/internal/components/chart/ChartBuilderModal.tsx @@ -142,11 +142,9 @@ const ChartPreview: FC = memo(props => { } } - // adjust height, width, and marginTop for the chart config for the preview, but not to save with the chart - const width = ref?.current?.getBoundingClientRect().width || 750; + const width = chartConfig.width ?? (ref?.current?.getBoundingClientRect().width || 750); const chartConfig_ = { ...chartConfig, - height: 350, width, }; diff --git a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx index ebf6b158e3..afee000c09 100644 --- a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx +++ b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx @@ -1,8 +1,8 @@ -import React, { ChangeEvent, FC, memo, useCallback, useMemo } from 'react'; +import React, { ChangeEvent, FC, memo, useCallback, useMemo, useState } from 'react'; import { BaseChartModel, BaseChartModelSetter, ChartConfig, ChartConfigSetter, ChartTypeInfo } from './models'; import { LABKEY_VIS } from '../../constants'; import { HIDDEN_CHART_TYPES, ICONS } from './constants'; -import { SelectInput, SelectInputOption } from '../forms/input/SelectInput'; +import { SelectInput } from '../forms/input/SelectInput'; import { SVGIcon } from '../base/SVGIcon'; import { CheckboxLK } from '../../Checkbox'; import { ChartFieldOption } from './ChartFieldOption'; @@ -10,14 +10,19 @@ import { QueryModel } from '../../../public/QueryModel/QueryModel'; import { TrendlineOption } from './TrendlineOption'; import { deepCopyChartConfig, hasTrendline } from './utils'; import classNames from 'classnames'; +import { useEnterEscape } from '../../../public/useEnterEscape'; + +interface InputProps { + label: string; + name: string; + value: string; +} type LabelKey = keyof ChartConfig['labels']; -interface LabelInputProps { - label: string; +interface LabelInputProps extends InputProps { name: LabelKey; onChange: (name: LabelKey, value: string) => void; - value: string; } const LabelInput: FC = memo(({ label, name, onChange, value }) => { @@ -26,26 +31,27 @@ const LabelInput: FC = memo(({ label, name, onChange, value }) [name, onChange] ); return ( -
- - +
+
+ + +
); }); LabelInput.displayName = 'LabelInput'; -interface BoolSettingInputProps { - label: string; - name: string; +interface BoolSettingInputProps extends Omit { onChange: (name: LabelKey, value: boolean) => void; value: boolean; } + const BoolSettingInput: FC = memo(({ label, name, onChange, value }) => { const onChange_ = useCallback( (event: ChangeEvent) => { @@ -61,6 +67,121 @@ const BoolSettingInput: FC = memo(({ label, name, onChang }); BoolSettingInput.displayName = 'BoolSettingInput'; +interface NumberInputProps extends Omit { + disabled: boolean; + name: 'height' | 'width'; + onBlur: () => void; + onChange: (name: 'height' | 'width', value: string) => void; + value: string; +} + +const NumberInput: FC = memo(({ disabled, label, name, onBlur, onChange, value }) => { + const onInputChange = useCallback( + (event: ChangeEvent) => onChange(name, event.target.value), + [name, onChange] + ); + const onKeyDown = useEnterEscape(onBlur); + + return ( +
+ + +
+ ); +}); +NumberInput.displayName = 'NumberInput'; + +interface SizeInputsProps { + height: number; + setChartConfig: ChartConfigSetter; + width: number; +} +const SizeInputs: FC = memo(({ height, setChartConfig, width }) => { + // We store sizes in a separate useState so the user can edit the values without us rerendering on every keystroke, + // and so we can easily handle invalid values. + const [sizes, setSizes] = useState>(() => ({ + height: height?.toString() ?? '', + width: width?.toString() ?? '', + })); + const [useFullWidth, setUseFullWidth] = useState(width === undefined); + const onCheckboxChange = useCallback( + (e: ChangeEvent) => { + const checked = e.target.checked; + + if (checked) { + console.log('Setting full width, clearing entered width value'); + setChartConfig(current => ({ ...current, width: undefined })); + setSizes(current => ({ ...current, width: '' })); + } + + setUseFullWidth(checked); + }, + [setChartConfig] + ); + const onNumberChange = useCallback((name, value) => setSizes(current => ({ ...current, [name]: value })), []); + const onBlur = useCallback(() => { + setChartConfig(current => { + const updatedHeight = parseInt(sizes.height, 10); + const updatedWidth = parseInt(sizes.width, 10); + const heightChanged = !isNaN(updatedHeight) && updatedHeight !== current.height; + const widthChanged = !isNaN(updatedWidth) && updatedWidth !== current.width; + + if (!heightChanged && !widthChanged) return current; + + return { + ...current, + height: heightChanged ? updatedHeight : current.height, + width: widthChanged ? updatedWidth : current.width, + }; + }); + }, [setChartConfig, sizes]); + + return ( + <> + {/* intentionally not setting form-group class on the div below */} +
+
+ +
+
+ +
+
+
+
+ + Full Width + +
+
+ + ); +}); +SizeInputs.displayName = 'SizeInputs'; + interface ChartTypeOptionRendererProps { chartType: ChartTypeInfo; isValueRenderer: boolean; @@ -199,7 +320,7 @@ export const ChartSettingsPanel: FC = memo(props => { ); return ( -
+

Settings

@@ -261,8 +382,8 @@ export const ChartSettingsPanel: FC = memo(props => { onChange={onLabelChange} value={chartConfig?.labels?.subtitle} /> - {/* TODO: width/height */} -
+ + ); }); ChartSettingsPanel.displayName = 'ChartSettingsPanel'; diff --git a/packages/components/src/internal/components/chart/utils.ts b/packages/components/src/internal/components/chart/utils.ts index 5a7650794b..53a6f42418 100644 --- a/packages/components/src/internal/components/chart/utils.ts +++ b/packages/components/src/internal/components/chart/utils.ts @@ -142,7 +142,7 @@ export function deepCopyChartConfig(chartConfig: ChartConfig, chartType = 'bar_c return { geomOptions: makeGeomOptions(chartType), gridLinesVisible: 'both', - height: undefined, + height: 500, labels: {}, measures: {}, pointType: 'all', From 5d1274dd202671640cc4eb4269a2837d87dc1ea0 Mon Sep 17 00:00:00 2001 From: alanv Date: Tue, 25 Nov 2025 15:28:53 -0600 Subject: [PATCH 19/38] ChartFieldAggregateOptions: Remove asOverlay option --- .../chart/ChartFieldAggregateOptions.tsx | 26 ++----------------- .../components/chart/ChartFieldOption.tsx | 1 - .../chart/ChartFieldRangeScaleOptions.tsx | 2 -- 3 files changed, 2 insertions(+), 27 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.tsx b/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.tsx index 0ab9bbf1a2..fe40354e32 100644 --- a/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.tsx @@ -1,6 +1,4 @@ import React, { FC, memo, useCallback, useMemo } from 'react'; -import { OverlayTrigger } from '../../OverlayTrigger'; -import { Popover } from '../../Popover'; import { RadioGroupInput } from '../forms/input/RadioGroupInput'; import { ChartConfig, ChartConfigSetter, ChartFieldInfo, ChartTypeInfo } from './models'; @@ -22,7 +20,6 @@ const ERROR_BAR_TYPES = [ ]; interface OwnProps { - asOverlay?: boolean; // TODO: defaults to true, but is always passed as false. Can be removed? chartConfig: ChartConfig; field: ChartFieldInfo; selectedType: ChartTypeInfo; @@ -30,7 +27,7 @@ interface OwnProps { } export const ChartFieldAggregateOptions: FC = memo(props => { - const { asOverlay = true, chartConfig, field, selectedType, setChartConfig } = props; + const { chartConfig, field, selectedType, setChartConfig } = props; const yMeasure = Array.isArray(chartConfig.measures.y) ? chartConfig.measures.y[0] : chartConfig.measures.y; // Some older charts stored aggregate as an object that looked like: { label: 'Mean', value: 'MEAN' } const aggregateValue = Utils.isObject(yMeasure.aggregate) ? yMeasure.aggregate.value : yMeasure.aggregate; @@ -84,7 +81,7 @@ export const ChartFieldAggregateOptions: FC = memo(props => { const onAggregateChange = useCallback((_: never, value: string) => onChange('aggregate', value), [onChange]); const onErrorBarValueChange = useCallback((value: string) => onChange('errorBars', value), [onChange]); - const inputs = ( + return ( <>
); - - if (!asOverlay) { - return inputs; - } - - return ( -
- - {inputs} - - } - triggerType="click" - > - - -
- ); }); ChartFieldAggregateOptions.displayName = 'ChartFieldAggregateOptions'; diff --git a/packages/components/src/internal/components/chart/ChartFieldOption.tsx b/packages/components/src/internal/components/chart/ChartFieldOption.tsx index 7535366d8e..9a3c1989b8 100644 --- a/packages/components/src/internal/components/chart/ChartFieldOption.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldOption.tsx @@ -129,7 +129,6 @@ export const ChartFieldOption: FC = memo(props => { > {measure && showAggregateOptions && ( = memo(props => { const { scale, onScaleChange, showScaleTrans, children } = props; - const placement = useMemo(() => (!showScaleTrans && children ? 'left' : 'bottom'), [showScaleTrans, children]); - const scaleTransOptions = useMemo(() => { return SCALE_TRANS_TYPES.map(option => ({ ...option, selected: scale.trans === option.value })); }, [scale.trans]); From 4f73c890767b3135eac9bcc9ecac7838a48580d3 Mon Sep 17 00:00:00 2001 From: alanv Date: Wed, 26 Nov 2025 09:52:26 -0600 Subject: [PATCH 20/38] ChartBuilderModal: Add trendlineParameters to chart preview query config --- .../src/internal/components/chart/ChartBuilderModal.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/components/src/internal/components/chart/ChartBuilderModal.tsx b/packages/components/src/internal/components/chart/ChartBuilderModal.tsx index d806a855e9..e52bab7330 100644 --- a/packages/components/src/internal/components/chart/ChartBuilderModal.tsx +++ b/packages/components/src/internal/components/chart/ChartBuilderModal.tsx @@ -54,6 +54,8 @@ export const getChartBuilderQueryConfig = ( .map(measure => measure?.fieldKey) .filter(fk => fk !== undefined); + if (chartConfig.geomOptions.trendlineParameters) columns.push(chartConfig.geomOptions.trendlineParameters); + return { maxRows: -1, // this will be saved with the queryConfig, but we will override it for the preview in the modal requiredVersion: '17.1', // Issue 47898: include formattedValue in response row objects From b88a22e487a94091ebe9db1cc176605a382e5765 Mon Sep 17 00:00:00 2001 From: alanv Date: Wed, 26 Nov 2025 09:53:03 -0600 Subject: [PATCH 21/38] TrendlineOption: Handle clearing parameter field --- .../src/internal/components/chart/TrendlineOption.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/internal/components/chart/TrendlineOption.tsx b/packages/components/src/internal/components/chart/TrendlineOption.tsx index 34973e8d42..140d7164f9 100644 --- a/packages/components/src/internal/components/chart/TrendlineOption.tsx +++ b/packages/components/src/internal/components/chart/TrendlineOption.tsx @@ -245,7 +245,7 @@ const TrendlineOptionPopover: FC = props => { // chartConfig.geomOptions.trendlineParameters, const onParameterFieldChange = useCallback( (_: never, __: never, col: QueryColumn) => { - setGeomOptions({ trendlineParameters: col.fieldKey }); + setGeomOptions({ trendlineParameters: col?.fieldKey }); }, [setGeomOptions] ); From 39b29f9d65c06f89f449942df9a7615bfc8ceae8 Mon Sep 17 00:00:00 2001 From: alanv Date: Wed, 26 Nov 2025 10:00:13 -0600 Subject: [PATCH 22/38] ChartBuilderModal: set default height in preview --- .../src/internal/components/chart/ChartBuilderModal.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartBuilderModal.tsx b/packages/components/src/internal/components/chart/ChartBuilderModal.tsx index e52bab7330..64993fbf52 100644 --- a/packages/components/src/internal/components/chart/ChartBuilderModal.tsx +++ b/packages/components/src/internal/components/chart/ChartBuilderModal.tsx @@ -145,10 +145,8 @@ const ChartPreview: FC = memo(props => { } const width = chartConfig.width ?? (ref?.current?.getBoundingClientRect().width || 750); - const chartConfig_ = { - ...chartConfig, - width, - }; + const height = chartConfig.height ?? 500; + const chartConfig_ = { ...chartConfig, height, width }; if (ref.current) ref.current.innerHTML = ''; // clear again, right before render LABKEY_VIS.GenericChartHelper.generateChartSVG(divId, chartConfig_, measureStore, trendlineData); From b1a4f6db2ace7b57eb829574cda51cc2a7f570b8 Mon Sep 17 00:00:00 2001 From: alanv Date: Wed, 26 Nov 2025 10:29:47 -0600 Subject: [PATCH 23/38] Add ChartFieldAdditionalOptions - renders label input, as well as scale and aggregate options --- .../chart/ChartFieldAdditionalOptions.tsx | 86 ++++++++++++++ .../components/chart/ChartFieldOption.tsx | 41 ++----- .../chart/ChartFieldRangeScaleOptions.tsx | 108 ++++++++---------- .../components/chart/ChartSettingsPanel.tsx | 3 +- packages/components/src/theme/charts.scss | 5 + 5 files changed, 152 insertions(+), 91 deletions(-) create mode 100644 packages/components/src/internal/components/chart/ChartFieldAdditionalOptions.tsx diff --git a/packages/components/src/internal/components/chart/ChartFieldAdditionalOptions.tsx b/packages/components/src/internal/components/chart/ChartFieldAdditionalOptions.tsx new file mode 100644 index 0000000000..c489332207 --- /dev/null +++ b/packages/components/src/internal/components/chart/ChartFieldAdditionalOptions.tsx @@ -0,0 +1,86 @@ +import React, { ChangeEvent, FC, memo, useCallback, useMemo } from 'react'; +import { OverlayTrigger } from '../../OverlayTrigger'; +import { Popover } from '../../Popover'; +import { ChartConfig, ChartConfigSetter, ChartFieldInfo, ChartTypeInfo, ScaleType } from './models'; +import { getFieldDataType, shouldShowAggregateOptions, shouldShowRangeScaleOptions } from './utils'; +import { LABKEY_VIS } from '../../constants'; +import { ChartFieldAggregateOptions } from './ChartFieldAggregateOptions'; +import { ChartFieldRangeScaleOptions } from './ChartFieldRangeScaleOptions'; + +interface FieldLabelInputProps { + label: string; + name: string; + setChartConfig: ChartConfigSetter; + value: string; +} + +const FieldLabelInput: FC = memo(({ label, name, setChartConfig, value }) => { + const onChange = useCallback( + (e: ChangeEvent) => { + setChartConfig(current => ({ ...current, labels: { ...current.labels, [name]: e.target.value } })); + }, + [name, setChartConfig] + ); + + return ( +
+ + +
+ ); +}); +FieldLabelInput.displayName = 'FieldLabelInput'; + +interface Props { + chartConfig: ChartConfig; + field: ChartFieldInfo; + onScaleChange: (scale: Partial, localOnly?: boolean) => void; + scale: ScaleType; + selectedType: ChartTypeInfo; + setChartConfig: ChartConfigSetter; +} + +export const ChartFieldAdditionalOptions: FC = memo(props => { + const { chartConfig, field, onScaleChange, scale, selectedType, setChartConfig } = props; + const { measures } = chartConfig; + const measure = measures?.[field.name]; + const isNumericType = useMemo( + () => LABKEY_VIS.GenericChartHelper.isNumericType(getFieldDataType(measure)), + [measure] + ); + const showRangeScaleOptions = isNumericType && shouldShowRangeScaleOptions(field, selectedType); + const showAggregateOptions = isNumericType && shouldShowAggregateOptions(field, selectedType); + const overlay = ( + + + {showAggregateOptions && ( + + )} + {showRangeScaleOptions && ( + + )} + + ); + return ( +
+ + + +
+ ); +}); +ChartFieldAdditionalOptions.displayName = 'ChartFieldAdditionalOptions'; diff --git a/packages/components/src/internal/components/chart/ChartFieldOption.tsx b/packages/components/src/internal/components/chart/ChartFieldOption.tsx index 9a3c1989b8..8a9d0133ea 100644 --- a/packages/components/src/internal/components/chart/ChartFieldOption.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldOption.tsx @@ -7,18 +7,10 @@ import { QueryModel } from '../../../public/QueryModel/QueryModel'; import { LABKEY_VIS } from '../../constants'; import { QueryColumn } from '../../../public/QueryColumn'; -import { ChartFieldRangeScaleOptions } from './ChartFieldRangeScaleOptions'; import { ChartConfig, ChartConfigSetter, ChartFieldInfo, ChartTypeInfo, ScaleType } from './models'; -import { - getBarChartAxisLabel, - getFieldDataType, - getSelectOptions, - hasTrendline, - shouldShowAggregateOptions, - shouldShowRangeScaleOptions, -} from './utils'; +import { getBarChartAxisLabel, getSelectOptions, hasTrendline } from './utils'; -import { ChartFieldAggregateOptions } from './ChartFieldAggregateOptions'; +import { ChartFieldAdditionalOptions } from './ChartFieldAdditionalOptions'; const DEFAULT_SCALE_VALUES = { type: 'automatic', trans: 'linear' }; @@ -38,12 +30,7 @@ export const ChartFieldOption: FC = memo(props => { return scales[field.name] ?? DEFAULT_SCALE_VALUES; }); const options = useMemo(() => getSelectOptions(model, selectedType, field), [model, selectedType, field]); - const isNumericType = useMemo( - () => LABKEY_VIS.GenericChartHelper.isNumericType(getFieldDataType(measure)), - [measure] - ); - const showRangeScaleOptions = isNumericType && shouldShowRangeScaleOptions(field, selectedType); - const showAggregateOptions = isNumericType && shouldShowAggregateOptions(field, selectedType); + const showAdditionalOptions = measure && (field.name === 'x' || field.name === 'y'); const onScaleChange = useCallback( (scale: ScaleType, localOnly = false) => { @@ -110,7 +97,7 @@ export const ChartFieldOption: FC = memo(props => {
= memo(props => { value={measure?.fieldKey} valueKey="fieldKey" /> - {showRangeScaleOptions && ( - - {measure && showAggregateOptions && ( - - )} - + selectedType={selectedType} + setChartConfig={setChartConfig} + /> )}
diff --git a/packages/components/src/internal/components/chart/ChartFieldRangeScaleOptions.tsx b/packages/components/src/internal/components/chart/ChartFieldRangeScaleOptions.tsx index 847b5da256..278917e966 100644 --- a/packages/components/src/internal/components/chart/ChartFieldRangeScaleOptions.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldRangeScaleOptions.tsx @@ -1,6 +1,4 @@ -import React, { ChangeEvent, FC, memo, PropsWithChildren, useCallback, useMemo } from 'react'; -import { OverlayTrigger } from '../../OverlayTrigger'; -import { Popover } from '../../Popover'; +import React, { ChangeEvent, FC, memo, useCallback, useMemo } from 'react'; import { RadioGroupInput } from '../forms/input/RadioGroupInput'; import { ScaleType } from './models'; @@ -15,14 +13,14 @@ const SCALE_RANGE_TYPES = [ { value: 'manual', label: 'Manual' }, ]; -interface OwnProps extends PropsWithChildren { +interface OwnProps { onScaleChange: (scale: Partial, localOnly?: boolean) => void; scale: ScaleType; showScaleTrans: boolean; } export const ChartFieldRangeScaleOptions: FC = memo(props => { - const { scale, onScaleChange, showScaleTrans, children } = props; + const { scale, onScaleChange, showScaleTrans } = props; const scaleTransOptions = useMemo(() => { return SCALE_TRANS_TYPES.map(option => ({ ...option, selected: scale.trans === option.value })); }, [scale.trans]); @@ -80,61 +78,51 @@ export const ChartFieldRangeScaleOptions: FC = memo(props => { }, [invalidRange, onScaleChange, scale]); return ( -
- - {children} - {showScaleTrans && ( -
- - -
- )} -
- - -
- {scale.type === 'manual' && ( -
- - - - - {invalidRange &&
Invalid range (Max <= Min)
} -
- )} - - } - triggerType="click" - > - -
+
+ {showScaleTrans && ( +
+ + +
+ )} +
+ + +
+ {scale.type === 'manual' && ( +
+ + - + + {invalidRange &&
Invalid range (Max <= Min)
} +
+ )}
); }); diff --git a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx index afee000c09..d5b356c4ea 100644 --- a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx +++ b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx @@ -287,9 +287,10 @@ export const ChartSettingsPanel: FC = memo(props => { const onTypeChange = useCallback( (type: ChartTypeInfo) => { setChartConfig(current => { + const { main, subtitle } = current.labels; return { ...deepCopyChartConfig(undefined, type.name), - labels: { ...current.labels }, + labels: { main, subtitle }, }; }); }, diff --git a/packages/components/src/theme/charts.scss b/packages/components/src/theme/charts.scss index 1450ac6bc5..50d0f7f689 100644 --- a/packages/components/src/theme/charts.scss +++ b/packages/components/src/theme/charts.scss @@ -292,9 +292,14 @@ margin: 5px 0 0 5px; } +.chart-field-additional-options { + width: 350px; +} + .chart-builder-scale-range-inputs { padding-left: 50px; } + .chart-builder-asymptote-inputs { padding-left: 80px; } From a1da26690d5ea60b55bee32d7b09eb11a66bc68a Mon Sep 17 00:00:00 2001 From: alanv Date: Wed, 26 Nov 2025 16:26:03 -0600 Subject: [PATCH 24/38] TrendlineOption: fix when options appear --- .../components/chart/ChartSettingsPanel.tsx | 6 ++-- .../components/chart/TrendlineOption.tsx | 34 +++++++++++-------- packages/components/src/theme/charts.scss | 3 +- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx index d5b356c4ea..6693266552 100644 --- a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx +++ b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx @@ -191,7 +191,7 @@ const ChartTypeOptionRenderer: FC = memo(({ chartT const isSvg = icon.endsWith('.svg'); const className = classNames('chart-builder-type-option', { 'chart-builder-type-option--value': isValueRenderer }); return ( - + {isSvg && ( )} @@ -321,7 +321,7 @@ export const ChartSettingsPanel: FC = memo(props => { ); return ( -
+

Settings

@@ -384,7 +384,7 @@ export const ChartSettingsPanel: FC = memo(props => { value={chartConfig?.labels?.subtitle} /> - +
); }); ChartSettingsPanel.displayName = 'ChartSettingsPanel'; diff --git a/packages/components/src/internal/components/chart/TrendlineOption.tsx b/packages/components/src/internal/components/chart/TrendlineOption.tsx index 140d7164f9..da646877bb 100644 --- a/packages/components/src/internal/components/chart/TrendlineOption.tsx +++ b/packages/components/src/internal/components/chart/TrendlineOption.tsx @@ -30,18 +30,18 @@ export const TrendlineOption: FC = memo(props => { const TRENDLINE_OPTIONS: TrendlineType[] = Object.values(LABKEY_VIS.GenericChartHelper.TRENDLINE_OPTIONS); const { chartConfig, model, selectedType, setChartConfig } = props; const schemaQuery = model.schemaQuery; - const geomOptions = chartConfig?.geomOptions; - const measures = chartConfig?.measures; - const selectedTrendlineType = useMemo(() => { - return TRENDLINE_OPTIONS.find(option => option.value === geomOptions?.trendlineType); - }, [TRENDLINE_OPTIONS, geomOptions?.trendlineType]); - const showFieldOptions = selectedTrendlineType?.showMin || selectedTrendlineType?.showMax; + const { geomOptions, measures } = chartConfig; + const selectedTrendlineType = useMemo( + () => TRENDLINE_OPTIONS.find(option => option.value === geomOptions.trendlineType), + [TRENDLINE_OPTIONS, geomOptions.trendlineType] + ); + const showFieldOptions = !!geomOptions.trendlineType; // hide the trendline option if no x-axis value selected and for date field selection on x-axis const hidden = useMemo(() => { - const jsonType = getFieldDataType(measures?.x); - return !measures?.x || jsonType === 'date' || jsonType === 'time'; - }, [measures?.x]); + const jsonType = getFieldDataType(measures.x); + return !measures.x || jsonType === 'date' || jsonType === 'time'; + }, [measures.x]); const [loadingTrendlineOptions, setLoadingTrendlineOptions] = useState(true); const [asymptoteType, setAsymptoteType] = useState('automatic'); @@ -53,10 +53,10 @@ export const TrendlineOption: FC = memo(props => { ); useEffect(() => { - if (loadingTrendlineOptions && (!!geomOptions?.trendlineAsymptoteMin || !!geomOptions?.trendlineAsymptoteMax)) { + if (loadingTrendlineOptions && (!!geomOptions.trendlineAsymptoteMin || !!geomOptions.trendlineAsymptoteMax)) { setAsymptoteType('manual'); - setAsymptoteMin(geomOptions?.trendlineAsymptoteMin?.toString()); - setAsymptoteMax(geomOptions?.trendlineAsymptoteMax?.toString()); + setAsymptoteMin(geomOptions.trendlineAsymptoteMin?.toString()); + setAsymptoteMax(geomOptions.trendlineAsymptoteMax?.toString()); setLoadingTrendlineOptions(false); } }, [geomOptions, loadingTrendlineOptions]); @@ -159,7 +159,11 @@ export const TrendlineOption: FC = memo(props => {
+ = props => { const geomOptions = chartConfig.geomOptions; const TRENDLINE_OPTIONS: TrendlineType[] = Object.values(LABKEY_VIS.GenericChartHelper.TRENDLINE_OPTIONS); const selectedTrendlineType = useMemo(() => { - return TRENDLINE_OPTIONS.find(option => option.value === geomOptions?.trendlineType); - }, [TRENDLINE_OPTIONS, geomOptions?.trendlineType]); + return TRENDLINE_OPTIONS.find(option => option.value === geomOptions.trendlineType); + }, [TRENDLINE_OPTIONS, geomOptions.trendlineType]); const showAsymptoteOptions = selectedTrendlineType?.showMin || selectedTrendlineType?.showMax; const invalidRange = useMemo( () => !!asymptoteMin && !!asymptoteMax && asymptoteMax <= asymptoteMin, diff --git a/packages/components/src/theme/charts.scss b/packages/components/src/theme/charts.scss index 50d0f7f689..e7c00d3f56 100644 --- a/packages/components/src/theme/charts.scss +++ b/packages/components/src/theme/charts.scss @@ -292,7 +292,8 @@ margin: 5px 0 0 5px; } -.chart-field-additional-options { +.chart-field-additional-options, +.chart-field-trendline-options { width: 350px; } From a012447b7b0db344cbf1b0c06bd56304886368df Mon Sep 17 00:00:00 2001 From: alanv Date: Mon, 1 Dec 2025 12:55:50 -0600 Subject: [PATCH 25/38] ChartBuilderModal: Fix layout issues --- .../components/chart/ChartBuilderModal.tsx | 44 ++++++++--------- packages/components/src/theme/charts.scss | 49 ++++++------------- 2 files changed, 37 insertions(+), 56 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartBuilderModal.tsx b/packages/components/src/internal/components/chart/ChartBuilderModal.tsx index 64993fbf52..e7fd150a21 100644 --- a/packages/components/src/internal/components/chart/ChartBuilderModal.tsx +++ b/packages/components/src/internal/components/chart/ChartBuilderModal.tsx @@ -144,7 +144,7 @@ const ChartPreview: FC = memo(props => { } } - const width = chartConfig.width ?? (ref?.current?.getBoundingClientRect().width || 750); + const width = chartConfig.width ?? (ref?.current?.getBoundingClientRect().width - 15 || 750); const height = chartConfig.height ?? 500; const chartConfig_ = { ...chartConfig, height, width }; @@ -159,7 +159,7 @@ const ChartPreview: FC = memo(props => { return (
- +

Preview

{previewMsg && {previewMsg}} {!hasRequiredValues &&
Select required fields to preview the chart.
} {hasRequiredValues && ( @@ -355,27 +355,25 @@ export const ChartBuilderModal: FC = memo(({ actions, mo title={savedChartModel ? 'Edit Chart' : 'Create Chart'} > {error && {error}} -
- - -
+ + ); }); diff --git a/packages/components/src/theme/charts.scss b/packages/components/src/theme/charts.scss index e7c00d3f56..22f6ccdfb4 100644 --- a/packages/components/src/theme/charts.scss +++ b/packages/components/src/theme/charts.scss @@ -204,58 +204,41 @@ width: calc(100% - 100px); min-width: 850px; } -.chart-builder-modal__body { + +.lk-modal .chart-builder-modal .modal-body { display: flex; + flex: 1; + gap: 8px; + padding: 0; + overflow: hidden; } .chart-builder-modal__settings-panel { - height: 560px; width: 300px; border-right: solid 1px $gray-border-light; + padding: 15px; overflow-y: auto; - padding-right: 15px; } +.chart-builder-modal__chart-preview h4, .chart-builder-modal__settings-panel h4 { margin: 0 0 8px; } .chart-builder-modal__chart-preview { flex: 1; - padding: 8px; -} - -.chart-builder-modal .col-right { - height: 560px; - width: calc(100% - 116px) + overflow: auto; + padding-top: 15px; + display: flex; + flex-direction: column; } -.chart-builder-type { - border: solid 1px transparent; - border-radius: 3px; - margin-bottom: 10px; - padding: 5px; - - .title { - text-align: center; - color: $text-color; - } - - &.selected { - background-color: $blue-icon-background; - border-color: $blue-border-solid; - } - - &.selectable:hover { - cursor: pointer; - background-color: $gray-lighter; - border-color: $gray-border; - } +.chart-builder-preview-body { + flex: 1; } -.chart-builder-type-inputs { - margin: 0; - border-bottom: solid 1px $gray-border-light; +.chart-builder-preview-body .svg-chart__chart { + height: 100%; } .chart-builder-modal .fields-col { From 18bfb0f61c24c3afec1482e555df177109dc7760 Mon Sep 17 00:00:00 2001 From: alanv Date: Mon, 1 Dec 2025 13:35:41 -0600 Subject: [PATCH 26/38] ChartSettingsPanel: let users reset height/width to empty value --- .../components/chart/ChartSettingsPanel.tsx | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx index 6693266552..2cf8ddff29 100644 --- a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx +++ b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx @@ -12,6 +12,15 @@ import { deepCopyChartConfig, hasTrendline } from './utils'; import classNames from 'classnames'; import { useEnterEscape } from '../../../public/useEnterEscape'; +function changedValue(strVal: string, currentVal: number): [value: number, changed: boolean] { + strVal = strVal.trim(); + const value = strVal === '' ? undefined : parseInt(strVal, 10); + // If the number is NaN then we don't want to trigger change + const changed = (value === undefined || !isNaN(value)) && value !== currentVal; + + return [value, changed]; +} + interface InputProps { label: string; name: string; @@ -130,17 +139,15 @@ const SizeInputs: FC = memo(({ height, setChartConfig, width }) const onNumberChange = useCallback((name, value) => setSizes(current => ({ ...current, [name]: value })), []); const onBlur = useCallback(() => { setChartConfig(current => { - const updatedHeight = parseInt(sizes.height, 10); - const updatedWidth = parseInt(sizes.width, 10); - const heightChanged = !isNaN(updatedHeight) && updatedHeight !== current.height; - const widthChanged = !isNaN(updatedWidth) && updatedWidth !== current.width; + const [height, heightChanged] = changedValue(sizes.height, current.height); + const [width, widthChanged] = changedValue(sizes.width, current.width); if (!heightChanged && !widthChanged) return current; return { ...current, - height: heightChanged ? updatedHeight : current.height, - width: widthChanged ? updatedWidth : current.width, + height: heightChanged ? height : current.height, + width: widthChanged ? width : current.width, }; }); }, [setChartConfig, sizes]); From dd7cb8dcb6e375023818883c7be7348cc83d946f Mon Sep 17 00:00:00 2001 From: alanv Date: Mon, 1 Dec 2025 13:38:40 -0600 Subject: [PATCH 27/38] ChartFieldOption: hide additional options for pie chart categories --- .../src/internal/components/chart/ChartFieldOption.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/components/src/internal/components/chart/ChartFieldOption.tsx b/packages/components/src/internal/components/chart/ChartFieldOption.tsx index 8a9d0133ea..08c5f5922f 100644 --- a/packages/components/src/internal/components/chart/ChartFieldOption.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldOption.tsx @@ -30,7 +30,8 @@ export const ChartFieldOption: FC = memo(props => { return scales[field.name] ?? DEFAULT_SCALE_VALUES; }); const options = useMemo(() => getSelectOptions(model, selectedType, field), [model, selectedType, field]); - const showAdditionalOptions = measure && (field.name === 'x' || field.name === 'y'); + const isPieChart = selectedType.name === 'pie_chart'; + const showAdditionalOptions = !isPieChart && measure && (field.name === 'x' || field.name === 'y'); const onScaleChange = useCallback( (scale: ScaleType, localOnly = false) => { From 2136b905c746081a589f7aa2d0c0dc132c84a8b7 Mon Sep 17 00:00:00 2001 From: alanv Date: Mon, 1 Dec 2025 13:44:24 -0600 Subject: [PATCH 28/38] ChartFieldAdditionalOptions: emit label changes on blur/enter key --- .../chart/ChartFieldAdditionalOptions.tsx | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartFieldAdditionalOptions.tsx b/packages/components/src/internal/components/chart/ChartFieldAdditionalOptions.tsx index c489332207..9f7f5d9ef6 100644 --- a/packages/components/src/internal/components/chart/ChartFieldAdditionalOptions.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldAdditionalOptions.tsx @@ -1,4 +1,4 @@ -import React, { ChangeEvent, FC, memo, useCallback, useMemo } from 'react'; +import React, { ChangeEvent, FC, memo, useCallback, useMemo, useState } from 'react'; import { OverlayTrigger } from '../../OverlayTrigger'; import { Popover } from '../../Popover'; import { ChartConfig, ChartConfigSetter, ChartFieldInfo, ChartTypeInfo, ScaleType } from './models'; @@ -6,6 +6,7 @@ import { getFieldDataType, shouldShowAggregateOptions, shouldShowRangeScaleOptio import { LABKEY_VIS } from '../../constants'; import { ChartFieldAggregateOptions } from './ChartFieldAggregateOptions'; import { ChartFieldRangeScaleOptions } from './ChartFieldRangeScaleOptions'; +import { useEnterEscape } from '../../../public/useEnterEscape'; interface FieldLabelInputProps { label: string; @@ -15,17 +16,25 @@ interface FieldLabelInputProps { } const FieldLabelInput: FC = memo(({ label, name, setChartConfig, value }) => { - const onChange = useCallback( - (e: ChangeEvent) => { - setChartConfig(current => ({ ...current, labels: { ...current.labels, [name]: e.target.value } })); - }, - [name, setChartConfig] - ); + const [inputValue, setInputValue] = useState(value ?? ''); + const onChange = useCallback((e: ChangeEvent) => setInputValue(e.target.value), []); + const onBlur = useCallback(() => { + setChartConfig(current => ({ ...current, labels: { ...current.labels, [name]: inputValue.trim() } })); + }, [inputValue, name, setChartConfig]); + const onKeyDown = useEnterEscape(onBlur); return (
- +
); }); From d6102f0d7484d867f04451f133a6aad0561c0430 Mon Sep 17 00:00:00 2001 From: alanv Date: Mon, 1 Dec 2025 13:52:10 -0600 Subject: [PATCH 29/38] SVGChart: honor user configured height/width --- .../src/internal/components/chart/Chart.tsx | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/packages/components/src/internal/components/chart/Chart.tsx b/packages/components/src/internal/components/chart/Chart.tsx index cde101b31a..25e7361639 100644 --- a/packages/components/src/internal/components/chart/Chart.tsx +++ b/packages/components/src/internal/components/chart/Chart.tsx @@ -59,23 +59,28 @@ interface Dimensions { width: number; } -const MAX_HEIGHT = 500; +const MAX_DEFAULT_HEIGHT = 500; function computeDimensions(chartConfig: ChartConfig, measureStore, defaultWidth: number): Dimensions { - // Issue 49754: use getChartTypeBasedWidth() to determine width - const width = LABKEY_VIS.GenericChartHelper.getChartTypeBasedWidth( - chartConfig.renderType, - chartConfig.measures, - measureStore, - defaultWidth - ); - const dimensions = { - width, - height: (width * 9) / 16, // 16:9 aspect ratio - }; - if (dimensions.height > MAX_HEIGHT) dimensions.height = MAX_HEIGHT; + let width = chartConfig.width; + let height = chartConfig.height; + + if (width === undefined) { + // Issue 49754: use getChartTypeBasedWidth() to determine width + width = LABKEY_VIS.GenericChartHelper.getChartTypeBasedWidth( + chartConfig.renderType, + chartConfig.measures, + measureStore, + defaultWidth + ); + } + + if (height === undefined) { + height = (width * 9) / 16; // 16:9 aspect ratio + if (height > MAX_DEFAULT_HEIGHT) height = MAX_DEFAULT_HEIGHT; + } - return dimensions; + return { height, width }; } interface Props { From 0a8993f833782803f07a8d897fae72f93399457a Mon Sep 17 00:00:00 2001 From: alanv Date: Mon, 1 Dec 2025 15:10:38 -0600 Subject: [PATCH 30/38] ChartSettingsPanel: keep title margins when changing chart type --- .../components/chart/ChartSettingsPanel.tsx | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx index 2cf8ddff29..dd15de81a2 100644 --- a/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx +++ b/packages/components/src/internal/components/chart/ChartSettingsPanel.tsx @@ -21,6 +21,20 @@ function changedValue(strVal: string, currentVal: number): [value: number, chang return [value, changed]; } +function computeMarginTop(main: string, subtitle: string): number { + let marginTop = 15; + const hasTitle = !!main?.trim(); + const hasSubtitle = !!subtitle?.trim(); + + if (hasTitle && hasSubtitle) marginTop += 50; + else if (hasTitle) marginTop += 25; + // Yes, really, subtitle only gets the most padding. Our charting library is probably setting some + // default amount if main is present + else if (hasSubtitle) marginTop += 55; + + return marginTop; +} + interface InputProps { label: string; name: string; @@ -295,9 +309,12 @@ export const ChartSettingsPanel: FC = memo(props => { (type: ChartTypeInfo) => { setChartConfig(current => { const { main, subtitle } = current.labels; + const newConfig = deepCopyChartConfig(undefined, type.name); return { - ...deepCopyChartConfig(undefined, type.name), + ...newConfig, labels: { main, subtitle }, + // Keep marginTop to account for main / subtitle labels + geomOptions: { ...newConfig.geomOptions, marginTop: computeMarginTop(main, subtitle) }, }; }); }, @@ -309,16 +326,7 @@ export const ChartSettingsPanel: FC = memo(props => { setChartConfig(current => { const labels = { ...current.labels, [key]: value }; let geomOptions = current.geomOptions; - let marginTop = 15; - const hasTitle = !!labels.main?.trim(); - const hasSubtitle = !!labels.subtitle?.trim(); - - if (hasTitle && hasSubtitle) marginTop += 50; - else if (hasTitle) marginTop += 25; - // Yes, really, subtitle only gets the most padding. Our charting library is probably setting some - // default amount if main is present - else if (hasSubtitle) marginTop += 55; - + const marginTop = computeMarginTop(labels.main, labels.subtitle); if (marginTop != geomOptions.marginTop) geomOptions = { ...geomOptions, marginTop }; return { ...current, labels, geomOptions }; From a1fb0799ddf03c50681f1ea2421a913b0bbfa007 Mon Sep 17 00:00:00 2001 From: alanv Date: Mon, 1 Dec 2025 15:15:20 -0600 Subject: [PATCH 31/38] justfile: add comments --- packages/components/justfile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/components/justfile b/packages/components/justfile index 10bfabdb5e..a1379954a7 100644 --- a/packages/components/justfile +++ b/packages/components/justfile @@ -4,7 +4,7 @@ [private] default: - just --list + @just --list currentBranch := `git branch --show-current` branchAsTag := replace(currentBranch, '_', '-') @@ -129,5 +129,8 @@ patch: publish: npm publish +# Runs publish sleep bump pb: publish sleep bump + +# Runs publish sleep bumpApps pba: publish sleep bumpApps From b4c71fbef519ecd1994d335b056da530ba394475 Mon Sep 17 00:00:00 2001 From: alanv Date: Mon, 1 Dec 2025 16:13:56 -0600 Subject: [PATCH 32/38] Drop .chart-body min-height --- packages/components/src/theme/charts.scss | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/components/src/theme/charts.scss b/packages/components/src/theme/charts.scss index 22f6ccdfb4..0cbf3148fd 100644 --- a/packages/components/src/theme/charts.scss +++ b/packages/components/src/theme/charts.scss @@ -113,7 +113,6 @@ } .chart-body { position: relative; - min-height: 500px; } .report-list__chart-preview .chart-body { min-height: 300px; From 7723a3417c690014de44eb7d9d202482a9bde1ff Mon Sep 17 00:00:00 2001 From: alanv Date: Mon, 1 Dec 2025 16:19:35 -0600 Subject: [PATCH 33/38] Use set placement to right for LabelOverlays --- .../internal/components/chart/ChartFieldAggregateOptions.tsx | 4 ++-- .../src/internal/components/chart/TrendlineOption.tsx | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.tsx b/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.tsx index fe40354e32..73a3620c20 100644 --- a/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.tsx +++ b/packages/components/src/internal/components/chart/ChartFieldAggregateOptions.tsx @@ -85,7 +85,7 @@ export const ChartFieldAggregateOptions: FC = memo(props => { <>
= memo(props => {
= memo(props => {