rusackas commented on code in PR #40967:
URL: https://github.com/apache/superset/pull/40967#discussion_r3394643797
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Scatter/controlPanel.tsx:
##########
@@ -99,115 +428,56 @@ const config: ControlPanelConfig = {
'Size of marker. Also applies to forecast observations.',
),
visibility: ({ controls }: ControlPanelsContainerProps) =>
- Boolean(controls?.markerEnabled?.value),
+ Boolean(controls?.markerEnabled?.value) &&
+ !controls?.size?.value,
Review Comment:
That's intentional — once a size metric is driving the dots, the fixed
marker slider doesn't apply, so hiding it keeps the panel honest. Points with
no size value fall to the min, which is the sensible floor here rather than a
separate tunable.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Scatter/controlPanel.tsx:
##########
@@ -99,115 +428,56 @@ const config: ControlPanelConfig = {
'Size of marker. Also applies to forecast observations.',
),
visibility: ({ controls }: ControlPanelsContainerProps) =>
- Boolean(controls?.markerEnabled?.value),
+ Boolean(controls?.markerEnabled?.value) &&
+ !controls?.size?.value,
},
},
],
- ['zoomable'],
- [minorTicks],
- ...legendSection,
- [<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
-
[
{
- name: 'x_axis_time_format',
- config: {
- ...sharedControls.x_axis_time_format,
- default: 'smart_date',
- description: `${D3_TIME_FORMAT_DOCS}.
${TIME_SERIES_DESCRIPTION_TEXT}`,
- visibility: ({ controls }: ControlPanelsContainerProps) =>
- checkColumnType(
- getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
- controls?.datasource?.datasource,
- [GenericDataType.Temporal],
- ),
- disableStash: true,
- resetOnHide: false,
- },
- },
- {
- name: 'x_axis_number_format',
+ name: 'minMarkerSize',
config: {
- ...sharedControls.x_axis_number_format,
- default: '~g',
- mapStateToProps: undefined,
- visibility: ({ controls }: ControlPanelsContainerProps) =>
- checkColumnType(
- getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
- controls?.datasource?.datasource,
- [GenericDataType.Numeric],
- ),
- },
- },
- ],
- [xAxisLabelRotation],
- [xAxisLabelInterval],
- [forceMaxInterval],
- // eslint-disable-next-line react/jsx-key
- ...richTooltipSection,
- // eslint-disable-next-line react/jsx-key
- [<ControlSubSectionHeader>{t('Y Axis')}</ControlSubSectionHeader>],
- ['y_axis_format'],
- ['currency_format'],
- [
- {
- name: 'logAxis',
- config: {
- type: 'CheckboxControl',
- label: t('Logarithmic y-axis'),
- renderTrigger: true,
- default: logAxis,
- description: t('Logarithmic y-axis'),
- },
- },
- ],
- [
- {
- name: 'minorSplitLine',
- config: {
- type: 'CheckboxControl',
- label: t('Minor Split Line'),
- renderTrigger: true,
- default: minorSplitLine,
- description: t('Draw split lines for minor y-axis ticks'),
- },
- },
- ],
- [truncateXAxis],
- [xAxisBounds],
- [
- {
- name: 'truncateYAxis',
- config: {
- type: 'CheckboxControl',
- label: t('Truncate Y Axis'),
- default: truncateYAxis,
+ type: 'SliderControl',
+ label: t('Minimum dot size'),
renderTrigger: true,
+ min: 1,
+ max: 100,
+ default: minMarkerSize,
description: t(
- 'Truncate Y Axis. Can be overridden by specifying a min or max
bound.',
+ 'Size of the dot representing the smallest value of the dot
size metric.',
),
+ visibility: ({ controls }: ControlPanelsContainerProps) =>
+ Boolean(controls?.size?.value),
},
},
- ],
- [
{
- name: 'y_axis_bounds',
+ name: 'maxMarkerSize',
config: {
- type: 'BoundsControl',
- label: t('Y Axis Bounds'),
+ type: 'SliderControl',
+ label: t('Maximum dot size'),
renderTrigger: true,
- default: yAxisBounds,
+ min: 1,
+ max: 100,
+ default: maxMarkerSize,
description: t(
- 'Bounds for the Y-axis. When left empty, the bounds are ' +
- 'dynamically defined based on the min/max of the data. Note
that ' +
- "this feature will only expand the axis range. It won't " +
- "narrow the data's extent.",
+ 'Size of the dot representing the largest value of the dot
size metric.',
),
visibility: ({ controls }: ControlPanelsContainerProps) =>
- Boolean(controls?.truncateYAxis?.value),
+ Boolean(controls?.size?.value),
Review Comment:
Fair point, though both sliders clamp to 1–100 and inverting them just flips
the scale rather than breaking anything. I'd sooner leave it than wire
cross-field validation into the slider controls for an unusual input.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
