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.

Sign up for GitHub

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

ref(dashboards): Simplify WidgetCardChart #83535

Merged
merged 5 commits into from
Jan 20, 2025
Merged

ref(dashboards): Simplify WidgetCardChart #83535

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
553b2de
Move error handling higher up
gggritso Jan 15, 2025
9d666b3
Move series colorization higher in the component
gggritso Jan 15, 2025
10d95c8
Resolve duplication of JSX
gggritso Jan 15, 2025
2b23a6f
Unroll bad change
gggritso Jan 15, 2025
e4c11bd
Merge branch 'master' into ref/dashboards/simplify-widget-card-chart
gggritso Jan 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
158 changes: 50 additions & 108 deletions static/app/views/dashboards/widgetCard/chart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,20 +131,8 @@ class WidgetCardChart extends Component<WidgetCardChartProps> {
return !isEqual(currentProps, nextProps);
}

tableResultComponent({
loading,
errorMessage,
tableResults,
}: TableResultProps): React.ReactNode {
tableResultComponent({loading, tableResults}: TableResultProps): React.ReactNode {
const {location, widget, selection} = this.props;
if (errorMessage) {
return (
<StyledErrorPanel>
<IconWarning color="gray500" size="lg" />
</StyledErrorPanel>
);
}

if (typeof tableResults === 'undefined') {
// Align height to other charts.
return <LoadingPlaceholder />;
Expand Down Expand Up @@ -178,19 +166,7 @@ class WidgetCardChart extends Component<WidgetCardChartProps> {
});
}

bigNumberComponent({
loading,
errorMessage,
tableResults,
}: TableResultProps): React.ReactNode {
if (errorMessage) {
return (
<StyledErrorPanel>
<IconWarning color="gray500" size="lg" />
</StyledErrorPanel>
);
}

bigNumberComponent({loading, tableResults}: TableResultProps): React.ReactNode {
if (typeof tableResults === 'undefined' || loading) {
return <BigNumber>{'\u2014'}</BigNumber>;
}
Expand Down Expand Up @@ -291,12 +267,20 @@ class WidgetCardChart extends Component<WidgetCardChartProps> {
showConfidenceWarning,
} = this.props;

if (errorMessage) {
return (
<StyledErrorPanel>
<IconWarning color="gray500" size="lg" />
</StyledErrorPanel>
);
}

if (widget.displayType === 'table') {
return getDynamicText({
value: (
<TransitionChart loading={loading} reloading={loading}>
<LoadingScreen loading={loading} />
{this.tableResultComponent({tableResults, loading, errorMessage})}
{this.tableResultComponent({tableResults, loading})}
</TransitionChart>
),
fixed: <Placeholder height="200px" testId="skeleton-ui" />,
Expand All @@ -308,24 +292,49 @@ class WidgetCardChart extends Component<WidgetCardChartProps> {
<TransitionChart loading={loading} reloading={loading}>
<LoadingScreen loading={loading} />
<BigNumberResizeWrapper>
{this.bigNumberComponent({tableResults, loading, errorMessage})}
{this.bigNumberComponent({tableResults, loading})}
</BigNumberResizeWrapper>
</TransitionChart>
);
}

if (errorMessage) {
return (
<StyledErrorPanel>
<IconWarning color="gray500" size="lg" />
</StyledErrorPanel>
);
}

const {location, selection, onLegendSelectChanged, widgetLegendState} = this.props;
const {start, end, period, utc} = selection.datetime;
const {projects, environments} = selection;

const otherRegex = new RegExp(`(?:.* : ${OTHER}$)|^${OTHER}$`);
const shouldColorOther = timeseriesResults?.some(({seriesName}) =>
seriesName?.match(otherRegex)
);
const colors = timeseriesResults
? (theme.charts
.getColorPalette(timeseriesResults.length - (shouldColorOther ? 3 : 2))
?.slice() as string[])
: [];
// TODO(wmak): Need to change this when updating dashboards to support variable topEvents
if (shouldColorOther) {
colors[colors.length] = theme.chartOther;
}

// Create a list of series based on the order of the fields,
const series = timeseriesResults
? timeseriesResults
.map((values, i: number) => {
let seriesName = '';
if (values.seriesName !== undefined) {
seriesName = isEquation(values.seriesName)
? getEquation(values.seriesName)
: values.seriesName;
}
return {
...values,
seriesName,
fieldName: seriesName,
color: colors[i],
};
})
.filter(Boolean) // NOTE: `timeseriesResults` is a sparse array! We have to filter out the empty slots after the colors are assigned, since the colors are assigned based on sparse array index
: [];

const legend = {
left: 0,
top: 0,
Expand Down Expand Up @@ -461,60 +470,20 @@ class WidgetCardChart extends Component<WidgetCardChartProps> {
},
};

const forwardedRef = this.props.chartGroup ? this.handleRef : undefined;

return (
<ChartZoom period={period} start={start} end={end} utc={utc}>
{zoomRenderProps => {
if (errorMessage) {
return (
<StyledErrorPanel>
<IconWarning color="gray500" size="lg" />
</StyledErrorPanel>
);
}

const otherRegex = new RegExp(`(?:.* : ${OTHER}$)|^${OTHER}$`);
const shouldColorOther = timeseriesResults?.some(({seriesName}) =>
seriesName?.match(otherRegex)
);
const colors = timeseriesResults
? (theme.charts
.getColorPalette(timeseriesResults.length - (shouldColorOther ? 3 : 2))
?.slice() as string[])
: [];
// TODO(wmak): Need to change this when updating dashboards to support variable topEvents
if (shouldColorOther) {
colors[colors.length] = theme.chartOther;
}

// Create a list of series based on the order of the fields,
const series = timeseriesResults
? timeseriesResults
.map((values, i: number) => {
let seriesName = '';
if (values.seriesName !== undefined) {
seriesName = isEquation(values.seriesName)
? getEquation(values.seriesName)
: values.seriesName;
}
return {
...values,
seriesName,
color: colors[i],
};
})
.filter(Boolean) // NOTE: `timeseriesResults` is a sparse array! We have to filter out the empty slots after the colors are assigned, since the colors are assigned based on sparse array index
: [];

const forwardedRef = this.props.chartGroup ? this.handleRef : undefined;

return widgetLegendState.widgetRequiresLegendUnselection(widget) ? (
return (
<ReleaseSeries
end={end}
start={start}
period={period}
environments={environments}
projects={projects}
memoized
enabled={widgetLegendState.widgetRequiresLegendUnselection(widget)}
>
{({releaseSeries}) => {
// make series name into seriesName:widgetId form for individual widget legend control
Expand Down Expand Up @@ -559,33 +528,6 @@ class WidgetCardChart extends Component<WidgetCardChartProps> {
);
}}
</ReleaseSeries>
) : (
<TransitionChart loading={loading} reloading={loading}>
<LoadingScreen loading={loading} />
<ChartWrapper autoHeightResize={shouldResize ?? true} noPadding={noPadding}>
<RenderedChartContainer>
{getDynamicText({
value: this.chartComponent({
...zoomRenderProps,
...chartOptions,
// Override default datazoom behaviour for updating Global Selection Header
...(onZoom ? {onDataZoom: onZoom} : {}),
legend,
series,
onLegendSelectChanged,
forwardedRef,
}),
fixed: <Placeholder height="200px" testId="skeleton-ui" />,
})}
</RenderedChartContainer>
{showConfidenceWarning && confidence && (
<ConfidenceWarning
query={widget.queries[0]?.conditions ?? ''}
confidence={confidence}
/>
)}
</ChartWrapper>
</TransitionChart>
);
}}
</ChartZoom>
Expand Down
Loading