Skip to content

Commit dd364a9

Browse files
committed
ChartSettingsPanel: Improve chart type options rendering
1 parent dc89379 commit dd364a9

3 files changed

Lines changed: 46 additions & 25 deletions

File tree

packages/components/src/internal/components/chart/ChartSettingsPanel.tsx

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { ChartFieldOption } from './ChartFieldOption';
99
import { QueryModel } from '../../../public/QueryModel/QueryModel';
1010
import { TrendlineOption } from './TrendlineOption';
1111
import { deepCopyChartConfig, hasTrendline } from './utils';
12+
import classNames from 'classnames';
1213

1314
type LabelKey = keyof ChartConfig['labels'];
1415

@@ -60,16 +61,34 @@ const BoolSettingInput: FC<BoolSettingInputProps> = memo(({ label, name, onChang
6061
});
6162
BoolSettingInput.displayName = 'BoolSettingInput';
6263

63-
// TODO: use the same icons as the ChartMenu
64-
// - Not the most straightforward because those icons are set server-side
65-
function chartTypeOptionRenderer(option: SelectInputOption) {
66-
const data = option.data.data;
64+
interface ChartTypeOptionRendererProps {
65+
chartType: ChartTypeInfo;
66+
isValueRenderer: boolean;
67+
}
68+
const ChartTypeOptionRenderer: FC<ChartTypeOptionRendererProps> = memo(({ chartType, isValueRenderer }) => {
69+
const icon = ICONS[chartType.name];
70+
const isSvg = icon.endsWith('.svg');
71+
const className = classNames('chart-builder-type-option', { 'chart-builder-type-option--value': isValueRenderer });
6772
return (
68-
<div className="chart-builder-type-option">
69-
<SVGIcon height={null} iconSrc={ICONS[data.name] + '_gray'} width={32} />
70-
<span>{data.title}</span>
71-
</div>
73+
<span className={className}>
74+
{isSvg && (
75+
<SVGIcon height={null} iconDir="visualization/report" iconSrc={icon.replace('.svg', '')} width={16} />
76+
)}
77+
{!isSvg && <span className={`fa ${icon}`} />}
78+
<span>{chartType.title}</span>
79+
</span>
7280
);
81+
});
82+
ChartTypeOptionRenderer.displayName = 'ChartTypeOptionRenderer';
83+
84+
function chartTypeOptionRenderer(option) {
85+
const chartType: ChartTypeInfo = option.data as ChartTypeInfo;
86+
return <ChartTypeOptionRenderer chartType={chartType} isValueRenderer={false} />;
87+
}
88+
89+
function chartTypeValueRenderer(option) {
90+
const chartType: ChartTypeInfo = option.data as ChartTypeInfo;
91+
return <ChartTypeOptionRenderer chartType={chartType} isValueRenderer />;
7392
}
7493

7594
interface ChartTypeDropdownProps {
@@ -80,17 +99,11 @@ interface ChartTypeDropdownProps {
8099
const ChartTypeDropdown: FC<ChartTypeDropdownProps> = memo(({ onChange, selectedType }) => {
81100
const chartTypes = useMemo(() => {
82101
const allTypes = LABKEY_VIS?.GenericChartHelper.getRenderTypes();
83-
return allTypes
84-
.filter(type => !type.hidden && !HIDDEN_CHART_TYPES.includes(type.name))
85-
.map(type => ({
86-
data: type,
87-
label: type.title,
88-
value: type.name,
89-
}));
102+
return allTypes.filter(type => !type.hidden && !HIDDEN_CHART_TYPES.includes(type.name));
90103
}, []);
91104
const onChange_ = useCallback(
92105
(_, __, opt) => {
93-
onChange(opt.data);
106+
onChange(opt);
94107
},
95108
[onChange]
96109
);
@@ -103,13 +116,14 @@ const ChartTypeDropdown: FC<ChartTypeDropdownProps> = memo(({ onChange, selected
103116
clearable={false}
104117
containerClass=""
105118
inputClass="col-xs-12"
119+
labelKey="title"
106120
name="chartType"
107121
onChange={onChange_}
108122
optionRenderer={chartTypeOptionRenderer}
109123
options={chartTypes}
110124
value={selectedType.name}
111-
// TODO: using chartTypeOptionRenderer as valueRenderer makes the dropdown too tall
112-
// valueRenderer={chartTypeOptionRenderer}
125+
valueKey="name"
126+
valueRenderer={chartTypeValueRenderer}
113127
/>
114128
</div>
115129
</div>

packages/components/src/internal/components/chart/constants.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,12 @@ export const HIDDEN_CHART_TYPES = ['time_chart'];
22
export const MAX_ROWS_PREVIEW = 10000;
33
export const MAX_POINT_DISPLAY = 10000;
44
export const BLUE_HEX_COLOR = '3366FF';
5-
export const BAR_CHART_AGGREGATE_NAME = 'aggregate-method';
6-
export const BAR_CHART_ERROR_BAR_NAME = 'error-bar-method';
75
export const ICONS = {
8-
bar_chart: 'bar_chart',
9-
box_plot: 'box_plot',
10-
pie_chart: 'pie_chart',
11-
scatter_plot: 'xy_scatter',
12-
line_plot: 'xy_line',
6+
bar_chart: 'fa-bar-chart',
7+
box_plot: 'box_plot_icon.svg',
8+
line_plot: 'fa-line-chart',
9+
pie_chart: 'fa-pie-chart',
10+
scatter_plot: 'scatter_plot_icon.svg',
1311
};
1412

1513
export const AGGREGATE_METHODS = [

packages/components/src/theme/charts.scss

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,15 @@
191191
text-decoration: none;
192192
}
193193

194+
.chart-builder-type-option > img,
195+
.chart-builder-type-option > .fa {
196+
margin-right: 5px;
197+
}
198+
.chart-builder-type-option--value {
199+
margin-top: -27px;
200+
margin-left: 5px;
201+
}
202+
194203
.chart-builder-modal {
195204
width: calc(100% - 100px);
196205
min-width: 850px;

0 commit comments

Comments
 (0)