-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
Changes from all commits
bf5cd42
a09ec93
c8ce2ab
2b78498
45c7df7
060812f
f8a1e8d
93ce8da
26ee293
58ccf03
7ad716d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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(); | ||
|
@@ -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, | ||
|
@@ -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" | ||
|
@@ -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); | ||
|
||
|
@@ -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(); | ||
|
@@ -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)) { | ||
|
@@ -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} | ||
/> | ||
) | ||
); | ||
} | ||
|
||
|
@@ -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), | ||
|
@@ -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)} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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 = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
}; |
There was a problem hiding this comment.
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.