Skip to content

Commit

Permalink
fix: Time-series Line Chart Display unnecessary total (apache#31181)
Browse files Browse the repository at this point in the history
  • Loading branch information
michael-s-molina authored Nov 27, 2024
1 parent f0811c8 commit dbcb473
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ export default function transformProps(
stack,
tooltipTimeFormat,
tooltipSortByMetric,
showTooltipTotal,
showTooltipPercentage,
truncateXAxis,
truncateYAxis,
xAxis: xAxisOrig,
Expand All @@ -192,7 +194,9 @@ export default function transformProps(
}: EchartsTimeseriesFormData = { ...DEFAULT_FORM_DATA, ...formData };
const refs: Refs = {};
const groupBy = ensureIsArray(groupby);
const labelMap = Object.entries(label_map).reduce((acc, entry) => {
const labelMap: { [key: string]: string[] } = Object.entries(
label_map,
).reduce((acc, entry) => {
if (
entry[1].length > groupBy.length &&
Array.isArray(timeCompare) &&
Expand Down Expand Up @@ -487,7 +491,9 @@ export default function transformProps(
minorTick: { show: minorTicks },
minInterval:
xAxisType === AxisType.Time && timeGrainSqla
? TIMEGRAIN_TO_TIMESTAMP[timeGrainSqla]
? TIMEGRAIN_TO_TIMESTAMP[
timeGrainSqla as keyof typeof TIMEGRAIN_TO_TIMESTAMP
]
: 0,
...getMinAndMaxFromBounds(
xAxisType,
Expand Down Expand Up @@ -567,8 +573,9 @@ export default function transformProps(
value.observation !== undefined ? acc + value.observation : acc,
0,
);
const showTotal = Boolean(isMultiSeries) && richTooltip && !isForecast;
const showPercentage = showTotal && !forcePercentFormatter;
const allowTotal = Boolean(isMultiSeries) && richTooltip && !isForecast;
const showPercentage =
allowTotal && !forcePercentFormatter && showTooltipPercentage;
const keys = Object.keys(forecastValues);
let focusedRow;
sortedKeys
Expand Down Expand Up @@ -599,7 +606,7 @@ export default function transformProps(
focusedRow = rows.length - focusedRow - 1;
}
}
if (showTotal) {
if (allowTotal && showTooltipTotal) {
const totalRow = ['Total', formatter.format(total)];
if (showPercentage) {
totalRow.push(percentFormatter.format(1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ export type EchartsTimeseriesFormData = QueryFormData & {
stack: StackType;
timeCompare?: string[];
tooltipTimeFormat?: string;
showTooltipTotal?: boolean;
showTooltipPercentage?: boolean;
truncateXAxis: boolean;
truncateYAxis: boolean;
yAxisFormat?: string;
Expand Down
31 changes: 31 additions & 0 deletions superset-frontend/plugins/plugin-chart-echarts/src/controls.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,40 @@ const tooltipSortByMetricControl: ControlSetItem = {
},
};

const tooltipTotalControl: ControlSetItem = {
name: 'showTooltipTotal',
config: {
type: 'CheckboxControl',
label: t('Show total'),
renderTrigger: true,
default: true,
description: t('Whether to display the total value in the tooltip'),
visibility: ({ controls, form_data }: ControlPanelsContainerProps) =>
Boolean(controls?.rich_tooltip?.value) &&
form_data.viz_type !== 'mixed_timeseries',
},
};

const tooltipPercentageControl: ControlSetItem = {
name: 'showTooltipPercentage',
config: {
type: 'CheckboxControl',
label: t('Show percentage'),
renderTrigger: true,
default: true,
description: t('Whether to display the percentage value in the tooltip'),
visibility: ({ controls, form_data }: ControlPanelsContainerProps) =>
Boolean(controls?.rich_tooltip?.value) &&
!controls?.contributionMode?.value &&
form_data.viz_type !== 'mixed_timeseries',
},
};

export const richTooltipSection: ControlSetRow[] = [
[<ControlSubSectionHeader>{t('Tooltip')}</ControlSubSectionHeader>],
[richTooltipControl],
[tooltipTotalControl],
[tooltipPercentageControl],
[tooltipSortByMetricControl],
[tooltipTimeFormatControl],
];
Expand Down

0 comments on commit dbcb473

Please sign in to comment.