villebro commented on code in PR #19918:
URL: https://github.com/apache/superset/pull/19918#discussion_r864523160
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -318,57 +322,68 @@ export default function transformProps(
.map(entry => entry.name || '')
.concat(extractAnnotationLabels(annotationLayers, annotationData));
+ let xAxis: any = {
+ type: xAxisType,
+ name: xAxisTitle,
+ nameGap: convertInteger(xAxisTitleMargin),
+ nameLocation: 'middle',
+ axisLabel: {
+ hideOverlap: true,
+ formatter: xAxisFormatter,
+ rotate: xAxisLabelRotation,
+ },
+ minInterval:
+ xAxisType === 'time' && timeGrainSqla
+ ? TimeGrainToTimestamp[timeGrainSqla]
+ : 0,
+ };
+ let yAxis: any = {
+ ...defaultYAxis,
+ type: logAxis ? 'log' : 'value',
+ min,
+ max,
+ minorTick: { show: true },
+ minorSplitLine: { show: minorSplitLine },
+ axisLabel: { formatter },
+ scale: truncateYAxis,
+ name: yAxisTitle,
+ nameGap: convertInteger(yAxisTitleMargin),
+ nameLocation: yAxisTitlePosition === 'Left' ? 'middle' : 'end',
+ };
+
+ if (isHorizontal) {
+ const temp = xAxis;
+ xAxis = yAxis;
+ yAxis = temp;
+ }
+
const echartOptions: EChartsCoreOption = {
useUTC: true,
grid: {
...defaultGrid,
...padding,
},
- xAxis: {
- type: xAxisType,
- name: xAxisTitle,
- nameGap: convertInteger(xAxisTitleMargin),
- nameLocation: 'middle',
- axisLabel: {
- hideOverlap: true,
- formatter: xAxisFormatter,
- rotate: xAxisLabelRotation,
- },
- minInterval:
- xAxisType === 'time' && timeGrainSqla
- ? TimeGrainToTimestamp[timeGrainSqla]
- : 0,
- },
- yAxis: {
- ...defaultYAxis,
- type: logAxis ? 'log' : 'value',
- min,
- max,
- minorTick: { show: true },
- minorSplitLine: { show: minorSplitLine },
- axisLabel: { formatter },
- scale: truncateYAxis,
- name: yAxisTitle,
- nameGap: convertInteger(yAxisTitleMargin),
- nameLocation: yAxisTitlePosition === 'Left' ? 'middle' : 'end',
- },
+ xAxis,
+ yAxis,
tooltip: {
...defaultTooltip,
appendToBody: true,
trigger: richTooltip ? 'axis' : 'item',
formatter: (params: any) => {
+ const xIndex = isHorizontal ? 1 : 0;
+ const yIndex = isHorizontal ? 0 : 1;
Review Comment:
Slight simplification:
```suggestion
const [xIndex, yIndex] = isHorizontal ? [1, 0] : [0, 1];
```
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Bar/controlPanel.tsx:
##########
@@ -87,6 +89,28 @@ const config: ControlPanelConfig = {
sections.advancedAnalyticsControls,
sections.annotationsAndLayersControls,
sections.forecastIntervalControls,
+ {
+ label: t('Chart Orientation'),
+ expanded: true,
+ controlSetRows: [
+ [
+ {
+ name: 'bar_orient',
Review Comment:
could we call this `orientation`? I'm sure transposing will be relevant for
other chart types as well, so I'm not sure we need to restrict this to the bar
chart. I would also move this control to the `src/controls.tsx` file to make it
readily available to other charts in this plugin.
```suggestion
name: 'orientation',
```
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -318,57 +322,68 @@ export default function transformProps(
.map(entry => entry.name || '')
.concat(extractAnnotationLabels(annotationLayers, annotationData));
+ let xAxis: any = {
+ type: xAxisType,
+ name: xAxisTitle,
+ nameGap: convertInteger(xAxisTitleMargin),
+ nameLocation: 'middle',
+ axisLabel: {
+ hideOverlap: true,
+ formatter: xAxisFormatter,
+ rotate: xAxisLabelRotation,
+ },
+ minInterval:
+ xAxisType === 'time' && timeGrainSqla
+ ? TimeGrainToTimestamp[timeGrainSqla]
+ : 0,
+ };
+ let yAxis: any = {
+ ...defaultYAxis,
+ type: logAxis ? 'log' : 'value',
+ min,
+ max,
+ minorTick: { show: true },
+ minorSplitLine: { show: minorSplitLine },
+ axisLabel: { formatter },
+ scale: truncateYAxis,
+ name: yAxisTitle,
+ nameGap: convertInteger(yAxisTitleMargin),
+ nameLocation: yAxisTitlePosition === 'Left' ? 'middle' : 'end',
+ };
+
+ if (isHorizontal) {
+ const temp = xAxis;
+ xAxis = yAxis;
+ yAxis = temp;
Review Comment:
nit: I believe you could also do this:
```suggestion
[xAxis, yAxis] = [yAxis, xAxis]
```
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts:
##########
@@ -38,6 +38,11 @@ export enum EchartsTimeseriesContributionType {
Column = 'column',
}
+export enum EchartsOrientType {
Review Comment:
nit: I think `OrientationType` could be a simpler name for this (no need to
prefix with `Echarts` as that's implied)
--
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]
