Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add show axis to timeseries #26409

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
4 changes: 4 additions & 0 deletions desktop.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[ViewState]
Mode=
Vid=
FolderType=Generic
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ export default function transformProps(
legendOrientation,
addYAxisTitleOffset,
false,
true,
null,
addXAxisTitleOffset,
yAxisTitlePosition,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ export default function transformProps(chartProps: EchartsBubbleChartProps) {
legendOrientation,
true,
false,
true,
legendMargin,
true,
'Left',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,7 @@ export default function transformProps(
legendOrientation,
addYAxisTitleOffset,
zoomable,
true,
null,
addXAxisTitleOffset,
yAxisTitlePosition,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const {
yAxisBounds,
zoomable,
orientation,
showAxis,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be here twice

} = DEFAULT_FORM_DATA;

function createAxisTitleControl(axis: 'x' | 'y'): ControlSetRow[] {
Expand Down Expand Up @@ -174,6 +175,20 @@ function createAxisControl(axis: 'x' | 'y'): ControlSetRow[] {
},
},
],
[
{
name: 'showAxis',
config: {
type: 'CheckboxControl',
label: t('Show Axis'),
renderTrigger: true,
default: showAxis,
description: t('Changing this control takes effect instantly'),
visibility: ({ controls }: ControlPanelsContainerProps) =>
isXAxis ? isHorizontal(controls) : isVertical(controls),
},
},
],
[
{
name: 'y_axis_format',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export const DEFAULT_FORM_DATA: EchartsTimeseriesFormData = {
orientation: OrientationType.vertical,
sort_series_type: 'sum',
sort_series_ascending: false,
showAxis: true,
};

export const TIME_SERIES_DESCRIPTION_TEXT: string = t(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ export default function transformProps(
sortSeriesAscending,
timeGrainSqla,
timeCompare,
showAxis,
stack,
tooltipTimeFormat,
tooltipSortByMetric,
Expand Down Expand Up @@ -432,6 +433,7 @@ export default function transformProps(
legendOrientation,
addYAxisLabelOffset,
zoomable,
showAxis,
legendMargin,
addXAxisLabelOffset,
yAxisTitlePosition,
Expand Down Expand Up @@ -487,6 +489,7 @@ export default function transformProps(
defaultFormatter,
),
},
show: showAxis,
scale: truncateYAxis,
name: yAxisTitle,
nameGap: convertInteger(yAxisTitleMargin),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,7 @@
legendOrientation: LegendOrientation,
addYAxisTitleOffset: boolean,
zoomable: boolean,
showAxis: boolean,
margin?: string | number | null,
addXAxisTitleOffset?: boolean,
yAxisTitlePosition?: string,
Expand All @@ -559,6 +560,25 @@
const yAxisOffset = addYAxisTitleOffset
? TIMESERIES_CONSTANTS.yAxisLabelTopOffset
: 0;
// When the Y-axis is hidden, move the graph to the left to try to keep the graph centered
const yAxisLeftOffset = showAxis
? 0
: TIMESERIES_CONSTANTS.yAxisLabelLeftOffset;
let leftOffset: number;
if (yAxisTitlePosition === 'Left' && showAxis) {
leftOffset =
TIMESERIES_CONSTANTS.gridOffsetLeft + (Number(yAxisTitleMargin) || 0);
} else if (yAxisTitlePosition === 'Left' && !showAxis) {
leftOffset =

Check warning on line 572 in superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts

View check run for this annotation

Codecov / codecov/patch

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts#L572

Added line #L572 was not covered by tests
TIMESERIES_CONSTANTS.gridOffsetLeft +
(Number(yAxisTitleMargin) || 0) -
(Number(yAxisLeftOffset) || 0);
} else if (yAxisTitlePosition !== 'Left' && !showAxis) {
leftOffset =

Check warning on line 577 in superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts

View check run for this annotation

Codecov / codecov/patch

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts#L577

Added line #L577 was not covered by tests
TIMESERIES_CONSTANTS.gridOffsetLeft - (Number(yAxisLeftOffset) || 0);
} else {
leftOffset = TIMESERIES_CONSTANTS.gridOffsetLeft;
}
Comment on lines +564 to +581
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is maybe slightly heavy on the eyes. Would it be possible to add a few comments here explaining what's going on? Or better yet, maybe add a few self explanatory tests?

const xAxisOffset = addXAxisTitleOffset ? Number(xAxisTitleMargin) || 0 : 0;
return getChartPadding(showLegend, legendOrientation, margin, {
top:
Expand All @@ -568,10 +588,7 @@
bottom: zoomable
? TIMESERIES_CONSTANTS.gridOffsetBottomZoomable + xAxisOffset
: TIMESERIES_CONSTANTS.gridOffsetBottom + xAxisOffset,
left:
yAxisTitlePosition === 'Left'
? TIMESERIES_CONSTANTS.gridOffsetLeft + (Number(yAxisTitleMargin) || 0)
: TIMESERIES_CONSTANTS.gridOffsetLeft,
left: leftOffset,
right:
showLegend && legendOrientation === LegendOrientation.Right
? 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ export type EchartsTimeseriesFormData = QueryFormData & {
showExtraControls: boolean;
percentageThreshold: number;
orientation?: OrientationType;
showAxis: boolean;
} & LegendFormData &
TitleFormData;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export const TIMESERIES_CONSTANTS = {
dataZoomStart: 0,
dataZoomEnd: 100,
yAxisLabelTopOffset: 20,
yAxisLabelLeftOffset: 40,
extraControlsOffset: 22,
};

Expand Down
Loading