diff --git a/static/app/types/echarts.tsx b/static/app/types/echarts.tsx index d6746fe62813a7..9fd7c71af83deb 100644 --- a/static/app/types/echarts.tsx +++ b/static/app/types/echarts.tsx @@ -162,7 +162,7 @@ export type EChartFinishedHandler = EChartEventHandler>; export type EChartRenderedHandler = EChartEventHandler>; -export type EchartBrushAreas = Array<{ +type EchartBrushAreas = Array<{ coordRange: number[] | number[][]; panelId: string; range: number[] | number[][]; diff --git a/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx b/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx index dfd1983d93b200..826429956b5dc3 100644 --- a/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx +++ b/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx @@ -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, @@ -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, @@ -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, - Partial { + extends Partial { /** * An array of `Plottable` objects. This can be any object that implements the `Plottable` interface. */ @@ -106,6 +80,11 @@ export interface TimeSeriesWidgetVisualizationProps * Reference to the chart instance */ chartRef?: React.Ref; + /** + * The props for the chart x range selection on drag. + */ + chartXRangeSelection?: Partial; + /** * A mapping of time series field name to boolean. If the value is `false`, the series is hidden from view */ @@ -159,7 +138,7 @@ export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizati // the backend zerofills the data const chartRef = useRef(null); - const {register: registerWithWidgetSyncContext} = useWidgetSyncContext(); + const {register: registerWithWidgetSyncContext, groupName} = useWidgetSyncContext(); const pageFilters = usePageFilters(); const {start, end, period, utc} = @@ -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, + }); + const plottablesByType = groupBy(props.plottables, plottable => plottable.dataType); // Count up the field types of all the plottables @@ -609,70 +596,73 @@ export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizati }; return ( - { - 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} - /> + + {ActionMenu} + { + 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} + /> + ); } diff --git a/static/app/views/explore/components/attributeBreakdowns/chartSelectionContext.tsx b/static/app/views/explore/components/attributeBreakdowns/chartSelectionContext.tsx index 1a2892a37d3434..8443b9efb96963 100644 --- a/static/app/views/explore/components/attributeBreakdowns/chartSelectionContext.tsx +++ b/static/app/views/explore/components/attributeBreakdowns/chartSelectionContext.tsx @@ -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 = { diff --git a/static/app/views/explore/components/attributeBreakdowns/cohortComparisonContent.tsx b/static/app/views/explore/components/attributeBreakdowns/cohortComparisonContent.tsx index 6c130f260e9240..c5bf31c8e32833 100644 --- a/static/app/views/explore/components/attributeBreakdowns/cohortComparisonContent.tsx +++ b/static/app/views/explore/components/attributeBreakdowns/cohortComparisonContent.tsx @@ -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'; @@ -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'; @@ -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(''); @@ -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; @@ -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 ( - + {isLoading ? ( diff --git a/static/app/views/explore/components/attributeBreakdowns/content.tsx b/static/app/views/explore/components/attributeBreakdowns/content.tsx index f461879402c8d6..c0d8018dee76c7 100644 --- a/static/app/views/explore/components/attributeBreakdowns/content.tsx +++ b/static/app/views/explore/components/attributeBreakdowns/content.tsx @@ -8,7 +8,7 @@ export function AttributeBreakdownsContent() { if (chartSelection) { return ( ); diff --git a/static/app/views/explore/components/attributeBreakdowns/floatingTrigger.tsx b/static/app/views/explore/components/attributeBreakdowns/floatingTrigger.tsx index 5ca60441ab1942..b28b4faa511d14 100644 --- a/static/app/views/explore/components/attributeBreakdowns/floatingTrigger.tsx +++ b/static/app/views/explore/components/attributeBreakdowns/floatingTrigger.tsx @@ -1,40 +1,33 @@ import {useCallback} from 'react'; -import {createPortal} from 'react-dom'; import styled from '@emotion/styled'; import {updateDateTime} from 'sentry/actionCreators/pageFilters'; +import type {Selection} from 'sentry/components/charts/useChartXRangeSelection'; import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; import {getUtcDateString} from 'sentry/utils/dates'; import useRouter from 'sentry/utils/useRouter'; import type {ChartInfo} from 'sentry/views/explore/components/chart/types'; -import type {BoxSelectOptions} from 'sentry/views/explore/hooks/useChartBoxSelect'; import {Tab} from 'sentry/views/explore/hooks/useTab'; import type {Mode} from 'sentry/views/explore/queryParams/mode'; import {useChartSelection} from './chartSelectionContext'; type Props = { - boxSelectOptions: BoxSelectOptions; chartInfo: ChartInfo; + clearSelection: () => void; + selection: Selection; setTab: (tab: Mode | Tab) => void; - triggerWrapperRef: React.RefObject; }; -export function FloatingTrigger({ - boxSelectOptions, - chartInfo, - triggerWrapperRef, - setTab, -}: Props) { +export function FloatingTrigger({chartInfo, selection, clearSelection, setTab}: Props) { const router = useRouter(); - const triggerPosition = boxSelectOptions.floatingTriggerPosition; const {setChartSelection} = useChartSelection(); const handleZoomIn = useCallback(() => { - const coordRange = boxSelectOptions.xRange; - let startTimestamp = coordRange?.[0]; - let endTimestamp = coordRange?.[1]; + const coordRange = selection.range; + let startTimestamp = coordRange[0]; + let endTimestamp = coordRange[1]; if (!startTimestamp || !endTimestamp) { return; @@ -55,37 +48,24 @@ export function FloatingTrigger({ router, {save: true} ); - boxSelectOptions.clearSelection(); - }, [boxSelectOptions, router]); + clearSelection(); + }, [clearSelection, selection, router]); const handleFindAttributeBreakdowns = useCallback(() => { setChartSelection({ - boxSelectOptions, + selection, chartInfo, }); setTab(Tab.ATTRIBUTE_BREAKDOWNS); - }, [boxSelectOptions, chartInfo, setChartSelection, setTab]); + }, [selection, chartInfo, setChartSelection, setTab]); - if (!triggerPosition) return null; - - return createPortal( -
- - {t('Zoom in')} - - {t('Compare Attribute Breakdowns')} - - -
, - document.body + return ( + + {t('Zoom in')} + + {t('Compare Attribute Breakdowns')} + + ); } diff --git a/static/app/views/explore/components/chart/chartVisualization.tsx b/static/app/views/explore/components/chart/chartVisualization.tsx index a37345b7e0871f..bd540e1dda1495 100644 --- a/static/app/views/explore/components/chart/chartVisualization.tsx +++ b/static/app/views/explore/components/chart/chartVisualization.tsx @@ -4,6 +4,7 @@ import {useTheme} from '@emotion/react'; import styled from '@emotion/styled'; import TransparentLoadingMask from 'sentry/components/charts/transparentLoadingMask'; +import type {ChartXRangeSelectionProps} from 'sentry/components/charts/useChartXRangeSelection'; import {t} from 'sentry/locale'; import type {ReactEchartsRef} from 'sentry/types/echarts'; import {markDelayedData} from 'sentry/utils/timeSeries/markDelayedData'; @@ -11,7 +12,6 @@ import usePrevious from 'sentry/utils/usePrevious'; import {Area} from 'sentry/views/dashboards/widgets/timeSeriesWidget/plottables/area'; import {Bars} from 'sentry/views/dashboards/widgets/timeSeriesWidget/plottables/bars'; import {Line} from 'sentry/views/dashboards/widgets/timeSeriesWidget/plottables/line'; -import type {BoxSelectProps} from 'sentry/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization'; import {TimeSeriesWidgetVisualization} from 'sentry/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization'; import {Widget} from 'sentry/views/dashboards/widgets/widget/widget'; import type {ChartInfo} from 'sentry/views/explore/components/chart/types'; @@ -19,17 +19,15 @@ import {SAMPLING_MODE} from 'sentry/views/explore/hooks/useProgressiveQuery'; import {ChartType} from 'sentry/views/insights/common/components/chart'; import {INGESTION_DELAY} from 'sentry/views/insights/settings'; -interface ChartVisualizationProps extends Partial { +interface ChartVisualizationProps { chartInfo: ChartInfo; chartRef?: Ref; + chartXRangeSelection?: Partial; hidden?: boolean; } export function ChartVisualization({ - brush, - onBrushEnd, - onBrushStart, - toolBox, + chartXRangeSelection, chartInfo, chartRef, }: ChartVisualizationProps) { @@ -88,11 +86,8 @@ export function ChartVisualization({ ); @@ -112,11 +107,8 @@ export function ChartVisualization({ return ( ); } diff --git a/static/app/views/explore/hooks/useAttributeBreakdownComparison.tsx b/static/app/views/explore/hooks/useAttributeBreakdownComparison.tsx index 4a721522dab798..5b7f0b2f0a8942 100644 --- a/static/app/views/explore/hooks/useAttributeBreakdownComparison.tsx +++ b/static/app/views/explore/hooks/useAttributeBreakdownComparison.tsx @@ -1,5 +1,6 @@ import {useMemo} from 'react'; +import type {Selection} from 'sentry/components/charts/useChartXRangeSelection'; import {pageFiltersToQueryParams} from 'sentry/components/organizations/pageFilters/parse'; import {getUtcDateString} from 'sentry/utils/dates'; import {FieldKey} from 'sentry/utils/fields'; @@ -9,7 +10,6 @@ import {useLocation} from 'sentry/utils/useLocation'; import useOrganization from 'sentry/utils/useOrganization'; import usePageFilters from 'sentry/utils/usePageFilters'; import type {ChartInfo} from 'sentry/views/explore/components/chart/types'; -import type {BoxSelectOptions} from 'sentry/views/explore/hooks/useChartBoxSelect'; import {SAMPLING_MODE} from 'sentry/views/explore/hooks/useProgressiveQuery'; import {useSpansDataset} from 'sentry/views/explore/spans/spansQueryParams'; @@ -34,11 +34,11 @@ export type AttributeBreakdownsComparison = { }; function useAttributeBreakdownComparison({ - boxSelectOptions, + selection, chartInfo, }: { - boxSelectOptions: BoxSelectOptions; chartInfo: ChartInfo; + selection: Selection; }) { const location = useLocation(); const organization = useOrganization(); @@ -46,8 +46,8 @@ function useAttributeBreakdownComparison({ const {selection: pageFilters} = usePageFilters(); const aggregateExtrapolation = location.query.extrapolate ?? '1'; - const enableQuery = boxSelectOptions.xRange !== null; - const [x1, x2] = boxSelectOptions.xRange!; + const enableQuery = selection !== null; + const [x1, x2] = selection.range; // Ensure that we pass the existing queries in the search bar to the attribute breakdowns queries const currentQuery = location.query.query?.toString() ?? ''; diff --git a/static/app/views/explore/hooks/useChartBoxSelect.spec.tsx b/static/app/views/explore/hooks/useChartBoxSelect.spec.tsx deleted file mode 100644 index 31e668871caae9..00000000000000 --- a/static/app/views/explore/hooks/useChartBoxSelect.spec.tsx +++ /dev/null @@ -1,306 +0,0 @@ -import type EChartsReact from 'echarts-for-react'; -import {OrganizationFixture} from 'sentry-fixture/organization'; - -import {act, renderHookWithProviders, waitFor} from 'sentry-test/reactTestingLibrary'; - -import {EXPLORE_CHART_BRUSH_OPTION, useChartBoxSelect} from './useChartBoxSelect'; - -describe('useChartBoxSelect', () => { - const organization = OrganizationFixture({ - features: ['performance-spans-suspect-attributes'], - }); - const mockChartRef = { - current: null as EChartsReact | null, - }; - - const mockChartWrapperRef = { - current: null as HTMLDivElement | null, - }; - - const mockTriggerWrapperRef = { - current: null as HTMLDivElement | null, - }; - - const mockChartInstance = { - getModel: jest.fn(), - dispatchAction: jest.fn(), - setOption: jest.fn(), - convertToPixel: jest.fn().mockReturnValue([100, 200]), - getDom: jest.fn().mockReturnValue({ - getBoundingClientRect: jest.fn().mockReturnValue({ - left: 50, - top: 100, - }), - }), - }; - - const mockAxis = { - axis: { - scale: { - getExtent: jest.fn(), - }, - }, - }; - - beforeEach(() => { - jest.clearAllMocks(); - mockChartRef.current = null; - }); - - describe('initial state', () => { - it('should initialize with null brushArea', () => { - const {result} = renderHookWithProviders( - () => - useChartBoxSelect({ - chartRef: mockChartRef, - chartWrapperRef: mockChartWrapperRef, - triggerWrapperRef: mockTriggerWrapperRef, - }), - {organization} - ); - expect(result.current.xRange).toBeNull(); - }); - }); - - describe('brush configuration', () => { - it('should return correct brush configuration when enabled', () => { - const {result} = renderHookWithProviders( - () => - useChartBoxSelect({ - chartRef: mockChartRef, - chartWrapperRef: mockChartWrapperRef, - triggerWrapperRef: mockTriggerWrapperRef, - }), - {organization} - ); - - expect(result.current.brush).toEqual(EXPLORE_CHART_BRUSH_OPTION); - }); - }); - - describe('onBrushEnd handler', () => { - it('should handle brush end event correctly', async () => { - // Mock chart instance and methods - const mockEchartsInstance = { - ...mockChartInstance, - getModel: jest.fn().mockReturnValue({ - getComponent: jest.fn((type: string) => { - if (type === 'xAxis') { - return { - axis: { - scale: { - getExtent: () => [0, 100], - }, - }, - }; - } - if (type === 'yAxis') { - return { - axis: { - scale: { - getExtent: () => [0, 50], - }, - }, - }; - } - return mockAxis; - }), - }), - } as any; - - mockChartRef.current = { - getEchartsInstance: () => mockEchartsInstance, - } as unknown as EChartsReact; - - const {result} = renderHookWithProviders( - () => - useChartBoxSelect({ - chartRef: mockChartRef, - chartWrapperRef: mockChartWrapperRef, - triggerWrapperRef: mockTriggerWrapperRef, - }), - {organization} - ); - - const mockEvent = { - areas: [ - { - coordRange: [10, 90], // flat x range array - range: [10, 90], - }, - ], - brushId: 'test-brush-id', - type: 'brushend' as const, - }; - - act(() => { - result.current.onBrushEnd(mockEvent, mockEchartsInstance); - }); - - await waitFor(() => { - expect(result.current.xRange).toEqual([10, 90]); - }); - }); - - it('should constrain brush area to axis bounds', async () => { - const mockEchartsInstance = { - ...mockChartInstance, - getModel: jest.fn().mockReturnValue({ - getComponent: jest.fn((type: string) => { - if (type === 'xAxis') { - return { - axis: { - scale: { - getExtent: () => [0, 100], - }, - }, - }; - } - if (type === 'yAxis') { - return { - axis: { - scale: { - getExtent: () => [0, 50], - }, - }, - }; - } - return mockAxis; - }), - }), - } as any; - - mockChartRef.current = { - getEchartsInstance: () => mockEchartsInstance, - } as unknown as EChartsReact; - - const {result} = renderHookWithProviders( - () => - useChartBoxSelect({ - chartRef: mockChartRef, - chartWrapperRef: mockChartWrapperRef, - triggerWrapperRef: mockTriggerWrapperRef, - }), - {organization} - ); - - const mockEvent = { - areas: [ - { - coordRange: [-10, 150], // flat x range array exceeding bounds - range: [-10, 150], - }, - ], - brushId: 'test-brush-id', - type: 'brushend' as const, - }; - - act(() => { - result.current.onBrushEnd(mockEvent, mockEchartsInstance); - }); - - await waitFor(() => { - expect(result.current.xRange).toEqual([0, 100]); // constrained to x bounds - }); - }); - - it('should not set brush area if chartRef is null', () => { - const {result} = renderHookWithProviders( - () => - useChartBoxSelect({ - chartRef: mockChartRef, - chartWrapperRef: mockChartWrapperRef, - triggerWrapperRef: mockTriggerWrapperRef, - }), - {organization} - ); - - const mockEvent = { - areas: [ - { - coordRange: [10, 90], // flat x range array - range: [10, 90], - }, - ], - brushId: 'test-brush-id', - type: 'brushend' as const, - }; - - act(() => { - result.current.onBrushEnd(mockEvent, mockChartInstance as any); - }); - - expect(result.current.xRange).toBeNull(); - }); - }); - - describe('chart redraw effect', () => { - it('should dispatch brush action when brushArea changes', () => { - const mockEchartsInstance = { - ...mockChartInstance, - getModel: jest.fn().mockReturnValue({ - getComponent: jest.fn((type: string) => { - if (type === 'xAxis') { - return { - axis: { - scale: { - getExtent: () => [0, 100], - }, - }, - }; - } - if (type === 'yAxis') { - return { - axis: { - scale: { - getExtent: () => [0, 50], - }, - }, - }; - } - return mockAxis; - }), - }), - }; - - mockChartRef.current = { - getEchartsInstance: () => mockEchartsInstance, - } as unknown as EChartsReact; - - const {result} = renderHookWithProviders( - () => - useChartBoxSelect({ - chartRef: mockChartRef, - chartWrapperRef: mockChartWrapperRef, - triggerWrapperRef: mockTriggerWrapperRef, - }), - {organization} - ); - - const mockEvent = { - areas: [ - { - coordRange: [10, 90], // flat x range array - range: [10, 90], - }, - ], - brushId: 'test-brush-id', - type: 'brushend' as const, - }; - - act(() => { - result.current.onBrushEnd(mockEvent, mockEchartsInstance as any); - }); - - expect(mockEchartsInstance.dispatchAction).toHaveBeenCalledWith({ - type: 'brush', - areas: [ - { - ...mockEvent.areas[0], - coordRange: [10, 90], // flat x range array - }, - ], - }); - }); - }); -}); diff --git a/static/app/views/explore/hooks/useChartBoxSelect.tsx b/static/app/views/explore/hooks/useChartBoxSelect.tsx deleted file mode 100644 index 02e6bc384b8522..00000000000000 --- a/static/app/views/explore/hooks/useChartBoxSelect.tsx +++ /dev/null @@ -1,269 +0,0 @@ -import {useCallback, useEffect, useMemo, useReducer, useState} from 'react'; -import type {BrushComponentOption, EChartsOption, ToolboxComponentOption} from 'echarts'; -import * as echarts from 'echarts'; -import type EChartsReact from 'echarts-for-react'; - -import ToolBox from 'sentry/components/charts/components/toolBox'; -import type { - EchartBrushAreas, - EChartBrushEndHandler, - EChartBrushStartHandler, -} from 'sentry/types/echarts'; -import useOrganization from 'sentry/utils/useOrganization'; -import {useWidgetSyncContext} from 'sentry/views/dashboards/contexts/widgetSyncContext'; - -type Props = { - chartRef: React.RefObject; - chartWrapperRef: React.RefObject; - triggerWrapperRef: React.RefObject; -}; - -export type BoxSelectOptions = { - brush: EChartsOption['brush']; - clearSelection: () => void; - floatingTriggerPosition: {left: number; top: number} | null; - onBrushEnd: EChartBrushEndHandler; - onBrushStart: EChartBrushStartHandler; - reActivateSelection: () => void; - toolBox: ToolboxComponentOption | undefined; - xRange: [number, number] | null; -}; - -export const EXPLORE_CHART_BRUSH_OPTION: BrushComponentOption = { - mainType: 'brush', - toolbox: ['rect', 'clear'], - brushMode: 'single', - brushType: 'lineX', - throttleType: 'debounce', - throttleDelay: 100, - xAxisIndex: 0, - brushStyle: {}, - removeOnClick: false, - transformable: true, -}; - -export function useChartBoxSelect({ - chartRef, - chartWrapperRef, - triggerWrapperRef, -}: Props): BoxSelectOptions { - const organization = useOrganization(); - - const {groupName} = useWidgetSyncContext(); - - const boxSelectIsEnabled = organization.features.includes( - 'performance-spans-suspect-attributes' - ); - - // This contains the coordinate range of the selected area, that we expose to the parent component. - // It is clamped to extremes of the chart's x and y axes. - const [brushArea, setBrushArea] = useState(null); - - // This exposes the page coordinates when the user finishes drawing the box. This is used - // to render floating CTAs on top of the chart. - const [floatingTriggerPosition, setFloatingTriggerPosition] = useState<{ - left: number; - top: number; - } | null>(null); - - // This increments a counter to force a re-activation of the brush mode. We expose the - // re-activation function in the return value, so that the parent component can call it - // for example when the chart data changes. - const [forceReActivateSelection, reActivateSelection] = useReducer( - x => (x + 1) % Number.MAX_SAFE_INTEGER, - 0 - ); - - const onBrushStart = useCallback( - (_evt: any, chart: any) => { - // Echarts either lets us connect all interactivity of the charts in a group or none of them. - // We need connectivity for cursor syncing, but having that enabled while drawing, leads to a - // box drawn for all of the charts in the group. We are going for chart specific box selections, - // so we disconnect the group while drawing. - echarts?.disconnect(groupName); - - chart.dispatchAction({type: 'hideTip'}); - chart.setOption( - { - tooltip: { - show: false, - axisPointer: {show: false}, - }, - }, - {silent: true} - ); - }, - [groupName] - ); - - const onBrushEnd = useCallback( - (evt: any, chart: any) => { - if (!chartRef.current) return; - - const xAxis = chart.getModel().getComponent('xAxis', 0); - const yAxis = chart.getModel().getComponent('yAxis', 0); - - const xMin = xAxis.axis.scale.getExtent()[0]; - const xMax = xAxis.axis.scale.getExtent()[1]; - const yMin = yAxis.axis.scale.getExtent()[0]; - - const area = evt.areas[0]; - - const [x0, x1] = area.coordRange; - - const clampedCoordRange: [number, number] = [ - Math.max(xMin, x0), - Math.min(xMax, x1), - ]; - - const newBrushArea: EchartBrushAreas = [ - { - ...area, - coordRange: clampedCoordRange, - }, - ]; - - setBrushArea(newBrushArea); - - // Convert *bottom-right* of lineX brush to pixels - const [xPixel, yPixel] = chart.convertToPixel({xAxisIndex: 0, yAxisIndex: 0}, [ - clampedCoordRange[1], - yMin, - ]); - - const chartRect = chart.getDom().getBoundingClientRect(); - - if (chartRect) { - setFloatingTriggerPosition({ - left: chartRect.left + xPixel, - top: chartRect.top + yPixel + window.scrollY, - }); - } - }, - [chartRef] - ); - - const clearSelection = useCallback(() => { - const chartInstance = chartRef.current?.getEchartsInstance(); - chartInstance?.dispatchAction({type: 'brush', areas: []}); - setBrushArea(null); - setFloatingTriggerPosition(null); - }, [chartRef]); - - const handleOutsideClick = useCallback( - (event: MouseEvent) => { - if ( - chartWrapperRef.current && - !chartWrapperRef.current.contains(event.target as Node) && - triggerWrapperRef.current && - !triggerWrapperRef.current.contains(event.target as Node) - ) { - clearSelection(); - } - }, - [chartWrapperRef, triggerWrapperRef, clearSelection] - ); - - const enableBrushMode = useCallback(() => { - const chartInstance = chartRef.current?.getEchartsInstance(); - chartInstance?.dispatchAction({ - type: 'takeGlobalCursor', - key: 'brush', - brushOption: EXPLORE_CHART_BRUSH_OPTION, - }); - }, [chartRef]); - - useEffect(() => { - if (!boxSelectIsEnabled) { - return; - } - - const chartInstance = chartRef.current?.getEchartsInstance(); - - // Re-draw the box in the chart when a new brush area is set - if (brushArea) { - chartInstance?.dispatchAction({ - type: 'brush', - areas: brushArea, - }); - - chartInstance?.setOption( - { - tooltip: {show: true}, - }, - {silent: true} - ); - - // We re-connect the group after drawing the box, so that the cursor is synced across all charts again. - // Check the onBrushStart handler for more details. - echarts?.connect(groupName); - } - - // Activate brush mode on load and when we re-draw the box/clear the selection - const frame = requestAnimationFrame(() => { - enableBrushMode(); - }); - - window.addEventListener('click', handleOutsideClick, {capture: true}); - - // eslint-disable-next-line consistent-return - return () => { - window.removeEventListener('click', handleOutsideClick, {capture: true}); - cancelAnimationFrame(frame); - }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [ - brushArea, - chartRef.current, - enableBrushMode, - handleOutsideClick, - floatingTriggerPosition, - forceReActivateSelection, - ]); - - const brush: BrushComponentOption | undefined = useMemo(() => { - return boxSelectIsEnabled ? EXPLORE_CHART_BRUSH_OPTION : undefined; - }, [boxSelectIsEnabled]); - - const toolBox = useMemo(() => { - if (!boxSelectIsEnabled) { - return undefined; - } - - return ToolBox( - { - show: false, // Prevent the toolbox from being shown, we enable selection on load - }, - { - brush: { - type: ['lineX'], - }, - } - ); - }, [boxSelectIsEnabled]); - - const config: BoxSelectOptions = useMemo(() => { - const coordRange = brushArea?.[0]?.coordRange ?? null; - return { - brush, - xRange: coordRange ? (coordRange as [number, number]) : null, - onBrushEnd, - onBrushStart, - toolBox, - floatingTriggerPosition, - reActivateSelection, - clearSelection, - }; - }, [ - brushArea, - onBrushEnd, - brush, - toolBox, - onBrushStart, - floatingTriggerPosition, - reActivateSelection, - clearSelection, - ]); - - return config; -} diff --git a/static/app/views/explore/spans/charts/index.tsx b/static/app/views/explore/spans/charts/index.tsx index 8733c3cf9ca367..cff8be105372f1 100644 --- a/static/app/views/explore/spans/charts/index.tsx +++ b/static/app/views/explore/spans/charts/index.tsx @@ -1,4 +1,4 @@ -import {Fragment, useEffect, useMemo, useRef} from 'react'; +import {Fragment, useMemo, useRef} from 'react'; import styled from '@emotion/styled'; import {CompactSelect} from 'sentry/components/core/compactSelect'; @@ -9,17 +9,18 @@ import {space} from 'sentry/styles/space'; import type {ReactEchartsRef} from 'sentry/types/echarts'; import type {Confidence} from 'sentry/types/organization'; import {defined} from 'sentry/utils'; +import useOrganization from 'sentry/utils/useOrganization'; import {determineSeriesSampleCountAndIsSampled} from 'sentry/views/alerts/rules/metric/utils/determineSeriesSampleCount'; import {WidgetSyncContextProvider} from 'sentry/views/dashboards/contexts/widgetSyncContext'; import {Widget} from 'sentry/views/dashboards/widgets/widget/widget'; import {AttributeComparisonCTA} from 'sentry/views/explore/components/attributeBreakdowns/attributeComparisonCTA'; +import {useChartSelection} from 'sentry/views/explore/components/attributeBreakdowns/chartSelectionContext'; import {FloatingTrigger} from 'sentry/views/explore/components/attributeBreakdowns/floatingTrigger'; import {ChartVisualization} from 'sentry/views/explore/components/chart/chartVisualization'; import type {ChartInfo} from 'sentry/views/explore/components/chart/types'; import ChartContextMenu from 'sentry/views/explore/components/chartContextMenu'; import type {BaseVisualize} from 'sentry/views/explore/contexts/pageParamsContext/visualizes'; import {DEFAULT_VISUALIZATION} from 'sentry/views/explore/contexts/pageParamsContext/visualizes'; -import {useChartBoxSelect} from 'sentry/views/explore/hooks/useChartBoxSelect'; import {useChartInterval} from 'sentry/views/explore/hooks/useChartInterval'; import {type SamplingMode} from 'sentry/views/explore/hooks/useProgressiveQuery'; import type {Tab} from 'sentry/views/explore/hooks/useTab'; @@ -161,26 +162,15 @@ function Chart({ topEvents, setTab, }: ChartProps) { + const organization = useOrganization(); + const {setChartSelection} = useChartSelection(); const [interval, setInterval, intervalOptions] = useChartInterval(); const chartHeight = visualize.visible ? CHART_HEIGHT : 50; const chartRef = useRef(null); - const triggerWrapperRef = useRef(null); const chartWrapperRef = useRef(null); - const boxSelectOptions = useChartBoxSelect({ - chartRef, - chartWrapperRef, - triggerWrapperRef, - }); - - // Re-activate box selection when the series data changes - useEffect(() => { - boxSelectOptions.reActivateSelection(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [timeseriesResult]); - const chartType = visualize.chartType; const chartIcon = chartType === ChartType.LINE ? 'line' : chartType === ChartType.AREA ? 'area' : 'bar'; @@ -273,10 +263,24 @@ function Chart({ { + setChartSelection(null); + }, + disabled: !organization.features.includes( + 'performance-spans-suspect-attributes' + ), + actionMenuRenderer: (selection, clearSelection) => { + return ( + + ); + }, + }} /> ) } @@ -304,17 +308,7 @@ function Chart({ return ( - {index === 0 ? ( // Only show the CTA on the first chart - {widget} - ) : ( - widget - )} - + {index === 0 ? {widget} : widget} ); }