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

fix: Drill to detail blocked by tooltip #22082

Merged
merged 11 commits into from
Nov 23, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,13 @@ import {
extractTimegrain,
QueryFormData,
} from '@superset-ui/core';
import { BigNumberTotalChartProps } from '../types';
import { BigNumberTotalChartProps, BigNumberVizProps } from '../types';
import { getDateFormatter, parseMetricValue } from '../utils';
import { Refs } from '../../types';

export default function transformProps(chartProps: BigNumberTotalChartProps) {
export default function transformProps(
chartProps: BigNumberTotalChartProps,
): BigNumberVizProps {
const { width, height, queriesData, formData, rawFormData, hooks } =
chartProps;
const {
Expand All @@ -38,6 +41,7 @@ export default function transformProps(chartProps: BigNumberTotalChartProps) {
timeFormat,
yAxisFormat,
} = formData;
const refs: Refs = {};
Copy link
Member

@villebro villebro Nov 23, 2022

Choose a reason for hiding this comment

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

the refs object is needed here so the downstream code (transformProps -> Chart plugin component -> Echart component) can add the reference to the chart div for the tooltip positioning callback to calculate the current location of the pointer.

const { data = [], coltypes = [] } = queriesData[0];
const granularity = extractTimegrain(rawFormData as QueryFormData);
const metricName = getMetricLabel(metric);
Expand Down Expand Up @@ -76,5 +80,6 @@ export default function transformProps(chartProps: BigNumberTotalChartProps) {
subheaderFontSize,
subheader: formattedSubheader,
onContextMenu,
refs,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,14 @@ import React, { MouseEvent } from 'react';
import {
t,
getNumberFormatter,
NumberFormatter,
smartDateVerboseFormatter,
TimeFormatter,
computeMaxFontSize,
BRAND_COLOR,
styled,
BinaryQueryObjectFilterClause,
} from '@superset-ui/core';
import { EChartsCoreOption } from 'echarts';
import Echart from '../components/Echart';
import { BigNumberWithTrendlineFormData, TimeSeriesDatum } from './types';
import { BigNumberVizProps } from './types';
import { EventHandlers } from '../types';

const defaultNumberFormatter = getNumberFormatter();
Expand All @@ -44,36 +41,7 @@ const PROPORTION = {
TRENDLINE: 0.3,
};

type BigNumberVisProps = {
className?: string;
width: number;
height: number;
bigNumber?: number | null;
bigNumberFallback?: TimeSeriesDatum;
headerFormatter: NumberFormatter | TimeFormatter;
formatTime: TimeFormatter;
headerFontSize: number;
kickerFontSize: number;
subheader: string;
subheaderFontSize: number;
showTimestamp?: boolean;
showTrendLine?: boolean;
startYAxisAtZero?: boolean;
timeRangeFixed?: boolean;
timestamp?: number;
trendLineData?: TimeSeriesDatum[];
mainColor: string;
echartOptions: EChartsCoreOption;
onContextMenu?: (
clientX: number,
clientY: number,
filters?: BinaryQueryObjectFilterClause[],
) => void;
xValueFormatter?: TimeFormatter;
formData?: BigNumberWithTrendlineFormData;
};

class BigNumberVis extends React.PureComponent<BigNumberVisProps> {
class BigNumberVis extends React.PureComponent<BigNumberVizProps> {
static defaultProps = {
className: '',
headerFormatter: defaultNumberFormatter,
Expand Down Expand Up @@ -108,7 +76,7 @@ class BigNumberVis extends React.PureComponent<BigNumberVisProps> {

renderFallbackWarning() {
const { bigNumberFallback, formatTime, showTimestamp } = this.props;
if (!bigNumberFallback || showTimestamp) return null;
if (!formatTime || !bigNumberFallback || showTimestamp) return null;
return (
<span
className="alert alert-warning"
Expand All @@ -125,7 +93,13 @@ class BigNumberVis extends React.PureComponent<BigNumberVisProps> {

renderKicker(maxHeight: number) {
const { timestamp, showTimestamp, formatTime, width } = this.props;
if (!showTimestamp) return null;
if (
!formatTime ||
!showTimestamp ||
typeof timestamp === 'string' ||
typeof timestamp === 'boolean'
)
return null;

const text = timestamp === null ? '' : formatTime(timestamp);

Expand Down Expand Up @@ -155,6 +129,7 @@ class BigNumberVis extends React.PureComponent<BigNumberVisProps> {

renderHeader(maxHeight: number) {
const { bigNumber, headerFormatter, width } = this.props;
// @ts-ignore
const text = bigNumber === null ? t('No data') : headerFormatter(bigNumber);

const container = this.createTemporaryContainer();
Expand Down Expand Up @@ -231,7 +206,7 @@ class BigNumberVis extends React.PureComponent<BigNumberVisProps> {
}

renderTrendline(maxHeight: number) {
const { width, trendLineData, echartOptions } = this.props;
const { width, trendLineData, echartOptions, refs } = this.props;

// if can't find any non-null values, no point rendering the trendline
if (!trendLineData?.some(d => d[1] !== null)) {
Expand Down Expand Up @@ -264,12 +239,15 @@ class BigNumberVis extends React.PureComponent<BigNumberVisProps> {
};

return (
<Echart
width={Math.floor(width)}
height={maxHeight}
echartOptions={echartOptions}
eventHandlers={eventHandlers}
/>
echartOptions && (
<Echart
refs={refs}
width={Math.floor(width)}
height={maxHeight}
echartOptions={echartOptions}
eventHandlers={eventHandlers}
/>
)
);
}

Expand All @@ -292,7 +270,9 @@ class BigNumberVis extends React.PureComponent<BigNumberVisProps> {
<div className="text-container" style={{ height: allTextHeight }}>
{this.renderFallbackWarning()}
{this.renderKicker(
Math.ceil(kickerFontSize * (1 - PROPORTION.TRENDLINE) * height),
Math.ceil(
(kickerFontSize || 0) * (1 - PROPORTION.TRENDLINE) * height,
),
)}
{this.renderHeader(
Math.ceil(headerFontSize * (1 - PROPORTION.TRENDLINE) * height),
Expand All @@ -311,7 +291,7 @@ class BigNumberVis extends React.PureComponent<BigNumberVisProps> {
return (
<div className={className} style={{ height }}>
{this.renderFallbackWarning()}
{this.renderKicker(kickerFontSize * height)}
{this.renderKicker((kickerFontSize || 0) * height)}
Copy link
Member

Choose a reason for hiding this comment

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

lots of minor checks that needed to be added when the interfaces/types where updated

{this.renderHeader(Math.ceil(headerFontSize * height))}
{this.renderSubheader(Math.ceil(subheaderFontSize * height))}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,14 @@ import {
} from '@superset-ui/core';
import { EChartsCoreOption, graphic } from 'echarts';
import {
BigNumberVizProps,
BigNumberDatum,
BigNumberWithTrendlineChartProps,
TimeSeriesDatum,
} from '../types';
import { getDateFormatter, parseMetricValue } from '../utils';
import { getDefaultPosition } from '../../utils/tooltip';
import { Refs } from '../../types';

const defaultNumberFormatter = getNumberFormatter();
export function renderTooltipFactory(
Expand All @@ -60,7 +63,7 @@ const formatPercentChange = getNumberFormatter(

export default function transformProps(
chartProps: BigNumberWithTrendlineChartProps,
) {
): BigNumberVizProps {
const {
width,
height,
Expand Down Expand Up @@ -95,6 +98,7 @@ export default function transformProps(
from_dttm: fromDatetime,
to_dttm: toDatetime,
} = queriesData[0];
const refs: Refs = {};
const metricName = getMetricLabel(metric);
const compareLag = Number(compareLag_) || 0;
let formattedSubheader = subheader;
Expand All @@ -103,7 +107,7 @@ export default function transformProps(
const mainColor = `rgb(${r}, ${g}, ${b})`;

const xAxisLabel = getXAxisLabel(rawFormData) as string;
let trendLineData;
let trendLineData: TimeSeriesDatum[] | undefined;
let percentChange = 0;
let bigNumber = data.length === 0 ? null : data[0][metricName];
let timestamp = data.length === 0 ? null : data[0][xAxisLabel];
Expand Down Expand Up @@ -144,6 +148,7 @@ export default function transformProps(
}
}
sortedData.reverse();
// @ts-ignore
trendLineData = showTrendLine ? sortedData : undefined;
}

Expand Down Expand Up @@ -229,10 +234,10 @@ export default function transformProps(
bottom: 0,
},
tooltip: {
position: getDefaultPosition(refs),
appendToBody: true,
show: !inContextMenu,
trigger: 'axis',
confine: true,
formatter: renderTooltipFactory(formatTime, headerFormatter),
},
aria: {
Expand All @@ -250,6 +255,7 @@ export default function transformProps(
width,
height,
bigNumber,
// @ts-ignore
bigNumberFallback,
className,
headerFormatter,
Expand All @@ -267,5 +273,6 @@ export default function transformProps(
echartOptions,
onContextMenu,
xValueFormatter: formatTime,
refs,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,17 @@
* under the License.
*/

import { EChartsCoreOption } from 'echarts';
import {
BinaryQueryObjectFilterClause,
ChartDataResponseResult,
ChartProps,
DataRecordValue,
NumberFormatter,
QueryFormData,
QueryFormMetric,
TimeFormatter,
} from '@superset-ui/core';
import { BaseChartProps, Refs } from '../types';

export interface BigNumberDatum {
[key: string]: number | null;
Expand All @@ -43,15 +48,50 @@ export type BigNumberWithTrendlineFormData = BigNumberTotalFormData & {
compareLag?: string | number;
};

export type BigNumberTotalChartProps = ChartProps<QueryFormData> & {
formData: BigNumberTotalFormData;
queriesData: (ChartDataResponseResult & {
data?: BigNumberDatum[];
})[];
};
export interface BigNumberTotalChartDataResponseResult
extends ChartDataResponseResult {
data: BigNumberDatum[];
}

export type BigNumberWithTrendlineChartProps = BigNumberTotalChartProps & {
formData: BigNumberWithTrendlineFormData;
};
export type BigNumberTotalChartProps =
BaseChartProps<BigNumberTotalFormData> & {
formData: BigNumberTotalFormData;
queriesData: BigNumberTotalChartDataResponseResult[];
};

export type BigNumberWithTrendlineChartProps =
BaseChartProps<BigNumberWithTrendlineFormData> & {
formData: BigNumberWithTrendlineFormData;
};

export type TimeSeriesDatum = [number, number | null];

export type BigNumberVizProps = {
Copy link
Member

Choose a reason for hiding this comment

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

note: This whole chart should be refactored to be a functional component

className?: string;
width: number;
height: number;
bigNumber?: DataRecordValue;
bigNumberFallback?: TimeSeriesDatum;
headerFormatter: NumberFormatter | TimeFormatter;
formatTime?: TimeFormatter;
headerFontSize: number;
kickerFontSize?: number;
subheader: string;
subheaderFontSize: number;
showTimestamp?: boolean;
showTrendLine?: boolean;
startYAxisAtZero?: boolean;
timeRangeFixed?: boolean;
timestamp?: DataRecordValue;
trendLineData?: TimeSeriesDatum[];
mainColor?: string;
echartOptions?: EChartsCoreOption;
onContextMenu?: (
clientX: number,
clientY: number,
filters?: BinaryQueryObjectFilterClause[],
) => void;
xValueFormatter?: TimeFormatter;
formData?: BigNumberWithTrendlineFormData;
refs: Refs;
};
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export default function EchartsBoxPlot(props: BoxPlotChartTransformedProps) {
groupby,
selectedValues,
formData,
refs,
} = props;
const handleChange = useCallback(
(values: string[]) => {
Expand Down Expand Up @@ -72,6 +73,7 @@ export default function EchartsBoxPlot(props: BoxPlotChartTransformedProps) {

return (
<Echart
refs={refs}
height={height}
width={width}
echartOptions={echartOptions}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@ import {
sanitizeHtml,
} from '../utils/series';
import { convertInteger } from '../utils/convertInteger';
import { defaultGrid, defaultTooltip, defaultYAxis } from '../defaults';
import { defaultGrid, defaultYAxis } from '../defaults';
import { getPadding } from '../Timeseries/transformers';
import { OpacityEnum } from '../constants';
import { getDefaultPosition } from '../utils/tooltip';
import { Refs } from '../types';

export default function transformProps(
chartProps: EchartsBoxPlotChartProps,
Expand Down Expand Up @@ -71,6 +73,7 @@ export default function transformProps(
yAxisTitlePosition,
sliceId,
} = formData as BoxPlotQueryFormData;
const refs: Refs = {};
const colorFn = CategoricalColorNamespace.getScale(colorScheme as string);
const numberFormatter = getNumberFormatter(numberFormat);
const metricLabels = metrics.map(getMetricLabel);
Expand Down Expand Up @@ -270,7 +273,7 @@ export default function transformProps(
nameLocation: yAxisTitlePosition === 'Left' ? 'middle' : 'end',
},
tooltip: {
...defaultTooltip,
position: getDefaultPosition(refs),
show: !inContextMenu,
trigger: 'item',
axisPointer: {
Expand All @@ -291,5 +294,6 @@ export default function transformProps(
groupby,
selectedValues,
onContextMenu,
refs,
};
}
Loading