Skip to content
2 changes: 1 addition & 1 deletion static/app/types/echarts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export type EChartFinishedHandler = EChartEventHandler<Record<string, unknown>>;

export type EChartRenderedHandler = EChartEventHandler<Record<string, unknown>>;

export type EchartBrushAreas = Array<{
type EchartBrushAreas = Array<{
coordRange: number[] | number[][];
panelId: string;
range: number[] | number[][];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
import {useCallback, useRef} from 'react';
import {Fragment, useCallback, useRef} from 'react';
import {useTheme} from '@emotion/react';
import styled from '@emotion/styled';
import {mergeRefs} from '@react-aria/utils';
import * as Sentry from '@sentry/react';
import type {
EChartsOption,
SeriesOption,
ToolboxComponentOption,
YAXisComponentOption,
} from 'echarts';
import type {SeriesOption, YAXisComponentOption} from 'echarts';
import type {
TooltipFormatterCallback,
TopLevelFormatterParams,
Expand All @@ -20,13 +15,15 @@ import sum from 'lodash/sum';
import BaseChart from 'sentry/components/charts/baseChart';
import {getFormatter} from 'sentry/components/charts/components/tooltip';
import TransparentLoadingMask from 'sentry/components/charts/transparentLoadingMask';
import {
useChartXRangeSelection,
type ChartXRangeSelectionProps,
} from 'sentry/components/charts/useChartXRangeSelection';
import {useChartZoom} from 'sentry/components/charts/useChartZoom';
import {isChartHovered, truncationFormatter} from 'sentry/components/charts/utils';
import LoadingIndicator from 'sentry/components/loadingIndicator';
import {space} from 'sentry/styles/space';
import type {
EChartBrushEndHandler,
EChartBrushStartHandler,
EChartClickHandler,
EChartDataZoomHandler,
EChartDownplayHandler,
Expand Down Expand Up @@ -64,31 +61,8 @@ import {TimeSeriesWidgetYAxis} from './timeSeriesWidgetYAxis';

const {error, warn} = Sentry.logger;

export interface BoxSelectProps {
/**
* The brush options for the chart.
*/
brush: EChartsOption['brush'];

/**
* Callback that returns an updated ECharts brush selection when the user finishes a brush operation.
*/
onBrushEnd: EChartBrushEndHandler;

/**
* Callback that returns an updated ECharts brush selection when the user starts a brush operation.
*/
onBrushStart: EChartBrushStartHandler;

/**
* The toolBox options for the chart.
*/
toolBox?: ToolboxComponentOption;
}

export interface TimeSeriesWidgetVisualizationProps
extends Partial<LoadableChartWidgetProps>,
Partial<BoxSelectProps> {
extends Partial<LoadableChartWidgetProps> {
/**
* An array of `Plottable` objects. This can be any object that implements the `Plottable` interface.
*/
Expand All @@ -106,6 +80,11 @@ export interface TimeSeriesWidgetVisualizationProps
* Reference to the chart instance
*/
chartRef?: React.Ref<ReactEchartsRef>;
/**
* The props for the chart x range selection on drag.
*/
chartXRangeSelection?: Partial<ChartXRangeSelectionProps>;

/**
* A mapping of time series field name to boolean. If the value is `false`, the series is hidden from view
*/
Expand Down Expand Up @@ -159,7 +138,7 @@ export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizati
// the backend zerofills the data

const chartRef = useRef<ReactEchartsRef | null>(null);
const {register: registerWithWidgetSyncContext} = useWidgetSyncContext();
const {register: registerWithWidgetSyncContext, groupName} = useWidgetSyncContext();

const pageFilters = usePageFilters();
const {start, end, period, utc} =
Expand All @@ -175,6 +154,14 @@ export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizati
saveOnZoom: true,
});

const {brush, onBrushEnd, onBrushStart, toolBox, ActionMenu} = useChartXRangeSelection({
chartRef,
deps: [props.plottables],
disabled: true,
chartsGroupName: groupName,
...props.chartXRangeSelection,
});
Comment on lines +157 to +163
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I missed this in the original PR, but it looks like useChartXRangeSelection accepts a actionMenuRenderer, and then returns the rendered component (ActionMenu) using some states within the hook.

This feels unconventional to me because the hook now handles rendering components outside of echarts as well as brush logic in echarts. I don't think this is a blocker, but would it make more sense for the hook to just return the states necessary for rendering instead? And then the parent can handle the rendering using those states? Unless there is some limitation that forces us to do it this way. Feel free to address in a follow up if necessary!


const plottablesByType = groupBy(props.plottables, plottable => plottable.dataType);

// Count up the field types of all the plottables
Expand Down Expand Up @@ -609,70 +596,73 @@ export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizati
};

return (
<BaseChart
ref={mergeRefs(props.ref, props.chartRef, chartRef, handleChartRef)}
autoHeightResize
series={allSeries}
grid={{
// NOTE: Adding a few pixels of left padding prevents ECharts from
// incorrectly truncating long labels. See
// https://github.com/apache/echarts/issues/15562
left: 2,
top: showLegend ? 25 : 10,
right: 8,
bottom: 0,
containLabel: true,
...releaseBubbleGrid,
...xAxisGrid,
}}
legend={
showLegend
? {
top: 0,
left: 0,
formatter(seriesName: string) {
return truncationFormatter(
aliases[seriesName] ?? seriesName,
true,
// Escaping the legend string will cause some special
// characters to render as their HTML equivalents.
// So disable it here.
false
);
},
selected: props.legendSelection,
}
: undefined
}
onLegendSelectChanged={event => {
props?.onLegendSelectionChange?.(event.selected);
}}
tooltip={{
appendToBody: true,
trigger: 'axis',
axisPointer: {
type: 'cross',
},
formatter: formatTooltip,
}}
xAxis={xAxis}
yAxes={yAxes}
{...chartZoomProps}
onDataZoom={props.onZoom ?? onDataZoom}
toolBox={props.toolBox ?? chartZoomProps.toolBox}
brush={props.brush}
onBrushStart={props.onBrushStart}
onBrushEnd={props.onBrushEnd}
isGroupedByDate
useMultilineDate
start={start ? new Date(start) : undefined}
end={end ? new Date(end) : undefined}
period={period}
utc={utc ?? undefined}
onHighlight={handleHighlight}
onDownplay={handleDownplay}
onClick={handleClick}
/>
<Fragment>
{ActionMenu}
<BaseChart
ref={mergeRefs(props.ref, props.chartRef, chartRef, handleChartRef)}
autoHeightResize
series={allSeries}
grid={{
// NOTE: Adding a few pixels of left padding prevents ECharts from
// incorrectly truncating long labels. See
// https://github.com/apache/echarts/issues/15562
left: 2,
top: showLegend ? 25 : 10,
right: 8,
bottom: 0,
containLabel: true,
...releaseBubbleGrid,
...xAxisGrid,
}}
legend={
showLegend
? {
top: 0,
left: 0,
formatter(seriesName: string) {
return truncationFormatter(
aliases[seriesName] ?? seriesName,
true,
// Escaping the legend string will cause some special
// characters to render as their HTML equivalents.
// So disable it here.
false
);
},
selected: props.legendSelection,
}
: undefined
}
onLegendSelectChanged={event => {
props?.onLegendSelectionChange?.(event.selected);
}}
tooltip={{
appendToBody: true,
trigger: 'axis',
axisPointer: {
type: 'cross',
},
formatter: formatTooltip,
}}
xAxis={xAxis}
yAxes={yAxes}
{...chartZoomProps}
onDataZoom={props.onZoom ?? onDataZoom}
toolBox={toolBox ?? chartZoomProps.toolBox}
brush={brush}
onBrushStart={onBrushStart}
onBrushEnd={onBrushEnd}
isGroupedByDate
useMultilineDate
start={start ? new Date(start) : undefined}
end={end ? new Date(end) : undefined}
period={period}
utc={utc ?? undefined}
onHighlight={handleHighlight}
onDownplay={handleDownplay}
onClick={handleClick}
/>
</Fragment>
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import {createContext, useContext, useMemo, useState} from 'react';

import type {Selection} from 'sentry/components/charts/useChartXRangeSelection';
import type {ChartInfo} from 'sentry/views/explore/components/chart/types';
import type {BoxSelectOptions} from 'sentry/views/explore/hooks/useChartBoxSelect';

type ChartSelectionState = {
boxSelectOptions: BoxSelectOptions;
chartInfo: ChartInfo;
selection: Selection;
} | null;

type ChartSelectionContextValue = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {Button} from '@sentry/scraps/button/button';
import {ButtonBar} from '@sentry/scraps/button/buttonBar';
import {Flex} from '@sentry/scraps/layout';

import type {Selection} from 'sentry/components/charts/useChartXRangeSelection';
import {Text} from 'sentry/components/core/text';
import LoadingError from 'sentry/components/loadingError';
import LoadingIndicator from 'sentry/components/loadingIndicator';
Expand All @@ -19,7 +20,6 @@ import {getUserTimezone} from 'sentry/utils/dates';
import {useDebouncedValue} from 'sentry/utils/useDebouncedValue';
import type {ChartInfo} from 'sentry/views/explore/components/chart/types';
import useAttributeBreakdownComparison from 'sentry/views/explore/hooks/useAttributeBreakdownComparison';
import type {BoxSelectOptions} from 'sentry/views/explore/hooks/useChartBoxSelect';
import {prettifyAggregation} from 'sentry/views/explore/utils';

import {Chart} from './cohortComparisonChart';
Expand All @@ -32,14 +32,14 @@ const CHARTS_PER_PAGE = CHARTS_COLUMN_COUNT * 4;
const PERCENTILE_FUNCTION_PREFIXES = ['p50', 'p75', 'p90', 'p95', 'p99', 'avg'];

export function CohortComparison({
boxSelectOptions,
selection,
chartInfo,
}: {
boxSelectOptions: BoxSelectOptions;
chartInfo: ChartInfo;
selection: Selection;
}) {
const {data, isLoading, isError} = useAttributeBreakdownComparison({
boxSelectOptions,
selection,
chartInfo,
});
const [searchQuery, setSearchQuery] = useState('');
Expand Down Expand Up @@ -85,11 +85,11 @@ export function CohortComparison({
}, [filteredRankedAttributes]);

const selectionHint = useMemo(() => {
if (!boxSelectOptions.xRange) {
if (!selection) {
return null;
}

const [x1, x2] = boxSelectOptions.xRange;
const [x1, x2] = selection.range;

let startTimestamp = Math.floor(x1 / 60_000) * 60_000;
const endTimestamp = Math.ceil(x2 / 60_000) * 60_000;
Expand Down Expand Up @@ -120,10 +120,10 @@ export function CohortComparison({
: t(`Selection is data between %s - %s`, startDate, endDate),
baseline: t('Baseline is all other spans from your query'),
};
}, [boxSelectOptions.xRange, chartInfo.yAxis]);
}, [selection, chartInfo.yAxis]);

return (
<Panel>
<Panel data-explore-chart-selection-region>
<Flex direction="column" gap="xl" padding="xl">
{isLoading ? (
<LoadingIndicator />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export function AttributeBreakdownsContent() {
if (chartSelection) {
return (
<CohortComparison
boxSelectOptions={chartSelection.boxSelectOptions}
selection={chartSelection.selection}
chartInfo={chartSelection.chartInfo}
/>
);
Expand Down
Loading
Loading