rusackas commented on code in PR #40967:
URL: https://github.com/apache/superset/pull/40967#discussion_r3625655166
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -321,7 +326,7 @@ export default function transformProps(
seriesType,
);
- const [rawSeries, sortedTotalValues, minPositiveValue] = extractSeries(
+ const [allRawSeries, sortedTotalValues, minPositiveValue] = extractSeries(
Review Comment:
Confirmed: `minPositiveValue` was still computed from `allRawSeries` before
the size-metric series get filtered out, so log-axis bounds could be pulled
from a hidden series with a different value range. Recomputed it from the
rendered `rawSeries` instead. `sortedTotalValues` is fine as-is, it only feeds
`shouldCalculateDataBounds`, which is Bar-only.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Scatter/controlPanel.tsx:
##########
@@ -51,18 +56,342 @@ const {
logAxis,
markerEnabled,
markerSize,
+ maxMarkerSize,
+ minMarkerSize,
minorSplitLine,
+ orientation,
rowLimit,
truncateYAxis,
yAxisBounds,
} = DEFAULT_FORM_DATA;
+
+const isHorizontal = (controls: ControlStateMapping) =>
+ controls?.orientation?.value === OrientationType.Horizontal;
+const isVertical = (controls: ControlStateMapping) => !isHorizontal(controls);
+
+function createAxisTitleControl(axis: 'x' | 'y'): ControlSetRow[] {
+ const isXAxis = axis === 'x';
+ return [
+ [
+ {
+ name: 'x_axis_title',
+ config: {
+ type: 'TextControl',
+ label: t('Axis Title'),
+ renderTrigger: true,
+ default: '',
+ visibility: ({ controls }: ControlPanelsContainerProps) =>
+ isXAxis ? isVertical(controls) : isHorizontal(controls),
+ disableStash: true,
+ resetOnHide: false,
+ },
+ },
+ ],
+ [
+ {
+ name: 'x_axis_title_margin',
+ config: {
+ type: 'SelectControl',
+ freeForm: true,
+ clearable: true,
+ label: t('Axis title margin'),
+ renderTrigger: true,
+ default: sections.TITLE_MARGIN_OPTIONS[3],
+ choices: formatSelectOptions(sections.TITLE_MARGIN_OPTIONS),
+ visibility: ({ controls }: ControlPanelsContainerProps) =>
+ isXAxis ? isVertical(controls) : isHorizontal(controls),
+ disableStash: true,
+ resetOnHide: false,
+ },
+ },
+ ],
+ [
+ {
+ name: 'y_axis_title',
+ config: {
+ type: 'TextControl',
+ label: t('Axis Title'),
+ renderTrigger: true,
+ default: '',
+ visibility: ({ controls }: ControlPanelsContainerProps) =>
+ isXAxis ? isHorizontal(controls) : isVertical(controls),
+ disableStash: true,
+ resetOnHide: false,
+ },
+ },
+ ],
+ [
+ {
+ name: 'y_axis_title_margin',
+ config: {
+ type: 'SelectControl',
+ freeForm: true,
+ clearable: true,
+ label: t('Axis title margin'),
+ renderTrigger: true,
+ default: sections.TITLE_MARGIN_OPTIONS[4],
+ choices: formatSelectOptions(sections.TITLE_MARGIN_OPTIONS),
+ visibility: ({ controls }: ControlPanelsContainerProps) =>
+ isXAxis ? isHorizontal(controls) : isVertical(controls),
+ disableStash: true,
+ resetOnHide: false,
+ },
+ },
+ ],
+ [
+ {
+ name: 'y_axis_title_position',
+ config: {
+ type: 'SelectControl',
+ freeForm: true,
+ clearable: false,
+ label: t('Axis title position'),
+ renderTrigger: true,
+ default: sections.TITLE_POSITION_OPTIONS[0][0],
+ choices: sections.TITLE_POSITION_OPTIONS,
+ visibility: ({ controls }: ControlPanelsContainerProps) =>
+ isXAxis ? isHorizontal(controls) : isVertical(controls),
+ disableStash: true,
+ resetOnHide: false,
+ },
+ },
+ ],
+ ];
+}
+
+function createAxisControl(axis: 'x' | 'y'): ControlSetRow[] {
+ const isXAxis = axis === 'x';
+ // The dimension (x_axis) controls follow the dimension axis: they show
+ // under "X Axis" when vertical and under "Y Axis" when horizontal. The
+ // metric controls follow the opposite axis.
+ const showsDimensionAxis = (controls: ControlStateMapping) =>
+ isXAxis ? isVertical(controls) : isHorizontal(controls);
+ const showsMetricAxis = (controls: ControlStateMapping) =>
+ isXAxis ? isHorizontal(controls) : isVertical(controls);
+ return [
+ [
+ {
+ 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) =>
+ showsDimensionAxis(controls) &&
+ checkColumnType(
+ getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
+ controls?.datasource?.datasource,
+ [GenericDataType.Temporal],
+ ),
+ disableStash: true,
+ resetOnHide: false,
+ },
+ },
+ ],
+ [
+ {
+ name: 'x_axis_number_format',
+ config: {
+ ...sharedControls.x_axis_number_format,
+ default: '~g',
+ mapStateToProps: undefined,
+ visibility: ({ controls }: ControlPanelsContainerProps) =>
+ showsDimensionAxis(controls) &&
+ checkColumnType(
+ getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
+ controls?.datasource?.datasource,
+ [GenericDataType.Numeric],
+ ),
+ disableStash: true,
+ resetOnHide: false,
+ },
+ },
+ ],
+ [
+ {
+ name: xAxisLabelRotation.name,
+ config: {
+ ...xAxisLabelRotation.config,
+ visibility: ({ controls }: ControlPanelsContainerProps) =>
+ showsDimensionAxis(controls),
+ disableStash: true,
+ resetOnHide: false,
+ },
+ },
+ ],
+ [
+ {
+ name: xAxisLabelInterval.name,
+ config: {
+ ...xAxisLabelInterval.config,
+ visibility: ({ controls }: ControlPanelsContainerProps) =>
+ showsDimensionAxis(controls),
+ disableStash: true,
+ resetOnHide: false,
+ },
+ },
+ ],
+ [
+ {
+ name: 'y_axis_format',
+ config: {
+ ...sharedControls.y_axis_format,
+ label: t('Axis Format'),
+ visibility: ({ controls }: ControlPanelsContainerProps) =>
+ showsMetricAxis(controls),
+ disableStash: true,
+ resetOnHide: false,
+ },
+ },
+ ],
+ ['currency_format'],
Review Comment:
Good catch, fixed. `currency_format` now has the same `showsMetricAxis`
visibility gate as `y_axis_format`/`logAxis` instead of rendering
unconditionally under both sections.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Scatter/controlPanel.tsx:
##########
@@ -99,114 +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,
- 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),
},
},
],
+ ['zoomable'],
+ [minorTicks],
+ ...legendSection,
+ [<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
+ ...createAxisControl('x'),
+ [truncateXAxis],
+ [xAxisBounds],
+ [forceMaxInterval],
Review Comment:
Those are keyed off / bounds specifically, they're about the underlying
dimension column regardless of which physical axis it renders on, not the
visual X-position. Bar's control panel pins them under the X Axis header the
same way (Timeseries/Regular/Bar/controlPanel.tsx), so this matches the
existing convention rather than a scatter-specific bug.
--
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]
