From 6d5ff8e38138ba914eaab8020e040ceea059263e Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Thu, 20 Nov 2025 13:34:42 -0500 Subject: [PATCH 01/19] feat(charts): Adding new chart range selection hook --- .../charts/useChartXRangeSelection.spec.tsx | 576 ++++++++++++++++++ .../charts/useChartXRangeSelection.tsx | 410 +++++++++++++ 2 files changed, 986 insertions(+) create mode 100644 static/app/components/charts/useChartXRangeSelection.spec.tsx create mode 100644 static/app/components/charts/useChartXRangeSelection.tsx diff --git a/static/app/components/charts/useChartXRangeSelection.spec.tsx b/static/app/components/charts/useChartXRangeSelection.spec.tsx new file mode 100644 index 00000000000000..e641099ba4ed55 --- /dev/null +++ b/static/app/components/charts/useChartXRangeSelection.spec.tsx @@ -0,0 +1,576 @@ +import type EChartsReact from 'echarts-for-react'; +import type {EChartsInstance} from 'echarts-for-react'; + +import {act, renderHook, waitFor} from 'sentry-test/reactTestingLibrary'; + +import {useChartXRangeSelection} from './useChartXRangeSelection'; + +describe('useChartXRangeSelection', () => { + const mockChartRef = { + current: null as EChartsReact | null, + }; + + const mockChartInstance: EChartsInstance = { + getModel: jest.fn(), + dispatchAction: jest.fn(), + setOption: jest.fn(), + convertToPixel: jest.fn().mockReturnValue(100), + getDom: jest.fn().mockReturnValue({ + getBoundingClientRect: jest.fn().mockReturnValue({ + left: 50, + top: 100, + }), + }), + } as unknown as EChartsInstance; + + const mockAxis = { + axis: { + scale: { + getExtent: jest.fn(), + }, + }, + }; + + beforeEach(() => { + jest.clearAllMocks(); + mockChartRef.current = null; + }); + + describe('initial state', () => { + it('should return brush configuration when not disabled', () => { + const {result} = renderHook(() => + useChartXRangeSelection({ + chartRef: mockChartRef, + }) + ); + + expect(result.current.brush).toBeDefined(); + expect(result.current.toolBox).toBeDefined(); + }); + + it('should return undefined brush when disabled', () => { + const {result} = renderHook(() => + useChartXRangeSelection({ + chartRef: mockChartRef, + disabled: true, + }) + ); + + expect(result.current.brush).toBeUndefined(); + expect(result.current.toolBox).toBeUndefined(); + }); + }); + + describe('onBrushStart handler', () => { + it('should hide tooltip when brush starts', () => { + const onSelectionStart = jest.fn(); + + const {result} = renderHook(() => + useChartXRangeSelection({ + chartRef: mockChartRef, + onSelectionStart, + }) + ); + + act(() => { + result.current.onBrushStart({} as any, mockChartInstance); + }); + + expect(mockChartInstance.dispatchAction).toHaveBeenCalledWith({ + type: 'hideTip', + }); + expect(onSelectionStart).toHaveBeenCalled(); + }); + + it('should disconnect chart group when chartsGroupName is provided', () => { + const disconnectSpy = jest.fn(); + jest.spyOn(require('echarts'), 'disconnect').mockImplementation(disconnectSpy); + + const {result} = renderHook(() => + useChartXRangeSelection({ + chartRef: mockChartRef, + chartsGroupName: 'test-group', + }) + ); + + act(() => { + result.current.onBrushStart({} as any, mockChartInstance); + }); + + expect(disconnectSpy).toHaveBeenCalledWith('test-group'); + }); + }); + + describe('onBrushEnd handler', () => { + it('should set selection with clamped coordinates', async () => { + const onSelectionEnd = jest.fn(); + + 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; + }), + }), + convertToPixel: jest.fn((_config, value) => { + if (value === 100) return 500; + if (value === 90) return 450; + if (value === 10) return 50; + return 0; + }), + } as any; + + mockChartRef.current = { + getEchartsInstance: () => mockEchartsInstance, + } as unknown as EChartsReact; + + const {result} = renderHook(() => + useChartXRangeSelection({ + chartRef: mockChartRef, + onSelectionEnd, + }) + ); + + const mockEvent = { + areas: [ + { + coordRange: [10, 90], + panelId: 'test-panel-id', + }, + ], + }; + + act(() => { + result.current.onBrushEnd(mockEvent as any, mockEchartsInstance); + }); + + expect(onSelectionEnd).toHaveBeenCalledWith( + { + range: [10, 90], + panelId: 'test-panel-id', + }, + expect.any(Function) + ); + }); + + it('should clamp coordinates that exceed axis bounds', async () => { + const onSelectionEnd = jest.fn(); + + 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; + }), + }), + convertToPixel: jest.fn().mockReturnValue(100), + } as any; + + mockChartRef.current = { + getEchartsInstance: () => mockEchartsInstance, + } as unknown as EChartsReact; + + const {result} = renderHook(() => + useChartXRangeSelection({ + chartRef: mockChartRef, + onSelectionEnd, + }) + ); + + const mockEvent = { + areas: [ + { + coordRange: [-10, 150], // Exceeds bounds + panelId: 'test-panel-id', + }, + ], + }; + + act(() => { + result.current.onBrushEnd(mockEvent as any, mockEchartsInstance); + }); + + expect(onSelectionEnd).toHaveBeenCalledWith( + { + range: [0, 100], // Clamped to bounds + panelId: 'test-panel-id', + }, + expect.any(Function) + ); + }); + + it('should reconnect chart group after brush ends', async () => { + const connectSpy = jest.fn(); + jest.spyOn(require('echarts'), 'connect').mockImplementation(connectSpy); + + 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; + }), + }), + convertToPixel: jest.fn().mockReturnValue(100), + dispatchAction: jest.fn(), + } as any; + + mockChartRef.current = { + getEchartsInstance: () => mockEchartsInstance, + } as unknown as EChartsReact; + + const {result} = renderHook(() => + useChartXRangeSelection({ + chartRef: mockChartRef, + chartsGroupName: 'test-group', + }) + ); + + const mockEvent = { + areas: [{coordRange: [10, 90], panelId: 'test-panel-id'}], + }; + + act(() => { + result.current.onBrushEnd(mockEvent as any, mockEchartsInstance as any); + }); + + // Wait for effect to run + await waitFor(() => { + expect(connectSpy).toHaveBeenCalledWith('test-group'); + }); + }); + }); + + describe('actionMenuRenderer', () => { + it('should render action menu when selection is made', 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; + }), + }), + convertToPixel: jest.fn().mockReturnValue(100), + } as any; + + mockChartRef.current = { + getEchartsInstance: () => mockEchartsInstance, + } as unknown as EChartsReact; + + const actionMenuRenderer = jest.fn((_selection, _clearSelection) => ( +
Action Menu
+ )); + + const {result} = renderHook(() => + useChartXRangeSelection({ + chartRef: mockChartRef, + actionMenuRenderer, + }) + ); + + act(() => { + result.current.onBrushEnd( + {areas: [{coordRange: [10, 90], panelId: 'test-panel-id'}]} as any, + mockEchartsInstance + ); + }); + + await waitFor(() => { + expect(result.current.ActionMenu).not.toBeNull(); + }); + + expect(actionMenuRenderer).toHaveBeenCalledWith( + {range: [10, 90], panelId: 'test-panel-id'}, + expect.any(Function) + ); + }); + + it('should position action menu to the left when selection is on the right side', 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; + }), + }), + convertToPixel: jest.fn((_config, value) => { + if (value === 100) return 500; // xMax + if (value === 90) return 480; // Above 60% threshold + return 0; + }), + } as any; + + mockChartRef.current = { + getEchartsInstance: () => mockEchartsInstance, + } as unknown as EChartsReact; + + const {result} = renderHook(() => + useChartXRangeSelection({ + chartRef: mockChartRef, + actionMenuRenderer: (_selection, _clearSelection) => ( +
Action Menu
+ ), + }) + ); + + act(() => { + result.current.onBrushEnd( + {areas: [{coordRange: [10, 90], panelId: 'test-panel-id'}]} as any, + mockEchartsInstance + ); + }); + + await waitFor(() => { + expect(result.current.ActionMenu).not.toBeNull(); + }); + }); + }); + + describe('outside click handling', () => { + it('should clear selection when clicking outside selection region', 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; + }), + }), + convertToPixel: jest.fn().mockReturnValue(100), + } as any; + + mockChartRef.current = { + getEchartsInstance: () => mockEchartsInstance, + } as unknown as EChartsReact; + + const {result} = renderHook(() => + useChartXRangeSelection({ + chartRef: mockChartRef, + }) + ); + + // Create a selection + act(() => { + result.current.onBrushEnd( + {areas: [{coordRange: [10, 90], panelId: 'test-panel-id'}]} as any, + mockEchartsInstance + ); + }); + + // Simulate outside click + const outsideElement = document.createElement('div'); + document.body.appendChild(outsideElement); + + act(() => { + const clickEvent = new MouseEvent('click', {bubbles: true}); + outsideElement.dispatchEvent(clickEvent); + }); + + await waitFor(() => { + expect(mockEchartsInstance.dispatchAction).toHaveBeenCalledWith({ + type: 'brush', + areas: [], + }); + }); + + document.body.removeChild(outsideElement); + }); + + it('should not clear selection when clicking inside selection region', 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; + }), + }), + convertToPixel: jest.fn().mockReturnValue(100), + } as any; + + mockChartRef.current = { + getEchartsInstance: () => mockEchartsInstance, + } as unknown as EChartsReact; + + const {result} = renderHook(() => + useChartXRangeSelection({ + chartRef: mockChartRef, + }) + ); + + // Create a selection + act(() => { + result.current.onBrushEnd( + {areas: [{coordRange: [10, 90], panelId: 'test-panel-id'}]} as any, + mockEchartsInstance + ); + }); + + // Create element inside selection region + const insideElement = document.createElement('div'); + insideElement.dataset.exploreChartSelectionRegion = ''; + document.body.appendChild(insideElement); + + const dispatchCallCount = mockEchartsInstance.dispatchAction.mock.calls.length; + + act(() => { + const clickEvent = new MouseEvent('click', {bubbles: true}); + insideElement.dispatchEvent(clickEvent); + }); + + // Should not call dispatchAction again (no clear) + expect(mockEchartsInstance.dispatchAction.mock.calls).toHaveLength( + dispatchCallCount + ); + + document.body.removeChild(insideElement); + }); + }); + + describe('brush mode activation', () => { + it('should activate brush mode on mount', async () => { + const mockEchartsInstance = { + ...mockChartInstance, + dispatchAction: jest.fn(), + } as any; + + mockChartRef.current = { + getEchartsInstance: () => mockEchartsInstance, + } as unknown as EChartsReact; + + renderHook(() => + useChartXRangeSelection({ + chartRef: mockChartRef, + }) + ); + + await waitFor(() => { + expect(mockEchartsInstance.dispatchAction).toHaveBeenCalledWith({ + type: 'takeGlobalCursor', + key: 'brush', + brushOption: expect.objectContaining({ + brushType: 'lineX', + brushMode: 'single', + }), + }); + }); + }); + + it('should re-activate brush mode when deps change', async () => { + const mockEchartsInstance = { + ...mockChartInstance, + dispatchAction: jest.fn(), + } as any; + + mockChartRef.current = { + getEchartsInstance: () => mockEchartsInstance, + } as unknown as EChartsReact; + + const {rerender} = renderHook( + ({deps}) => + useChartXRangeSelection({ + chartRef: mockChartRef, + deps, + }), + {initialProps: {deps: [1]}} + ); + + await waitFor(() => { + expect(mockEchartsInstance.dispatchAction).toHaveBeenCalled(); + }); + + const callCount = mockEchartsInstance.dispatchAction.mock.calls.length; + + rerender({deps: [2]}); + + await waitFor(() => { + expect(mockEchartsInstance.dispatchAction.mock.calls.length).toBeGreaterThan( + callCount + ); + }); + }); + }); +}); diff --git a/static/app/components/charts/useChartXRangeSelection.tsx b/static/app/components/charts/useChartXRangeSelection.tsx new file mode 100644 index 00000000000000..d78d86973a16f7 --- /dev/null +++ b/static/app/components/charts/useChartXRangeSelection.tsx @@ -0,0 +1,410 @@ +import { + useCallback, + useEffect, + useMemo, + useRef, + useState, + type DependencyList, +} from 'react'; +import {createPortal} from 'react-dom'; +import type {BrushComponentOption, EChartsOption, ToolboxComponentOption} from 'echarts'; +import * as echarts from 'echarts'; +import type EChartsReact from 'echarts-for-react'; +import type {EChartsInstance} from 'echarts-for-react'; + +import ToolBox from 'sentry/components/charts/components/toolBox'; +import type {EChartBrushEndHandler, EChartBrushStartHandler} from 'sentry/types/echarts'; + +export type Selection = { + /** + * The panel ID of the selection box that echarts assigns. It's a special encoded string, so we collect it here instead of hardcoding it. + * We need this to draw a selection box programmatically, like on load from the initial state in the url. + */ + panelId: string; + + range: [number, number]; +}; + +type State = { + actionMenuPosition: {left: number; position: 'left' | 'right'; top: number} | null; + selection: Selection; +} | null; + +export type BoxSelectionOptions = { + /** + * The brush option override for the chart, to enable brush mode. + */ + brush: EChartsOption['brush']; + + /** + * The callback that the chart calls when dragging ends. + */ + onBrushEnd: EChartBrushEndHandler; + + /** + * The callback that the chart calls when dragging starts. + */ + onBrushStart: EChartBrushStartHandler; + + /** + * The tool box override for the chart. Must be passed on to the chart to enable the brush mode. + */ + toolBox: ToolboxComponentOption | undefined; + + /** + * The floating action menu that is displayed when the user finishes dragging. + */ + ActionMenu?: React.ReactNode; +}; + +const CHART_X_RANGE_BRUSH_OPTION: BrushComponentOption = { + mainType: 'brush', + toolbox: ['rect', 'clear'], + brushMode: 'single', + brushType: 'lineX', + throttleType: 'debounce', + throttleDelay: 100, + xAxisIndex: 0, + brushStyle: {}, + removeOnClick: false, + transformable: true, +}; + +export type ChartXRangeSelectionProps = { + /** + * The ref to the chart component. + */ + chartRef: React.RefObject; + + /** + * The renderer for the action menu that is displayed when the selection/dragging ends. + */ + actionMenuRenderer?: ( + selection: Selection, + clearSelection: () => void + ) => React.ReactNode; + + /** + * In the case of multiple charts, this is the name of the chart's group. + */ + chartsGroupName?: string; + + /** + * The dependencies to be used to re-activate selection or re-paint the box. + */ + deps?: DependencyList; + + /** + * Whether selection is disabled. + */ + disabled?: boolean; + + /** + * The callback that is called when the selection is cleared. + */ + onClearSelection?: () => void; + + /** + * The callback that is called when the selection/dragging ends. + */ + onSelectionEnd?: (selection: Selection, clearSelection: () => void) => void; + + /** + * The callback that is called when the selection/dragging starts. + */ + onSelectionStart?: () => void; +}; + +export function useChartXRangeSelection({ + chartRef, + onSelectionEnd, + onSelectionStart, + onClearSelection, + actionMenuRenderer, + chartsGroupName, + disabled = false, + deps = [], +}: ChartXRangeSelectionProps): BoxSelectionOptions { + const [state, setState] = useState(); + const tooltipFrameRef = useRef(null); + const enabbleBrushModeFrameRef = useRef(null); + + const onBrushStart = useCallback( + (_evt: any, chartInstance: 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. + if (chartsGroupName) { + echarts?.disconnect(chartsGroupName); + } + + chartInstance.dispatchAction({type: 'hideTip'}); + + // Disable the tooltip as we start dragging, as it covers regions of the chart that the user + // may want to select. The tooltip remains hidden until the box is cleared. + if (tooltipFrameRef.current) cancelAnimationFrame(tooltipFrameRef.current); + + tooltipFrameRef.current = requestAnimationFrame(() => { + chartInstance.setOption( + { + tooltip: {show: false}, + }, + {silent: true} + ); + }); + + onSelectionStart?.(); + }, + [chartsGroupName, onSelectionStart] + ); + + const clearSelection = useCallback(() => { + const chartInstance = chartRef.current?.getEchartsInstance(); + + chartInstance?.dispatchAction({type: 'brush', areas: []}); + + // Restore the tooltip as we clear selection + if (tooltipFrameRef.current) cancelAnimationFrame(tooltipFrameRef.current); + + tooltipFrameRef.current = requestAnimationFrame(() => { + chartInstance?.setOption({tooltip: {show: true}}, {silent: true}); + }); + + setState(null); + + onClearSelection?.(); + }, [chartRef, onClearSelection]); + + const onBrushEnd = useCallback( + (evt: any, chartInstance: any) => { + if (!chartInstance) return; + + // Get the axis values of the chart + const xAxis = chartInstance.getModel().getComponent('xAxis', 0); + const yAxis = chartInstance.getModel().getComponent('yAxis', 0); + + // Get the minimum and maximum values of the x axis and y axis + const xMin = xAxis.axis.scale.getExtent()[0]; + const xMax = xAxis.axis.scale.getExtent()[1]; + const yMin = yAxis.axis.scale.getExtent()[0]; + + const xMaxPixel = chartInstance.convertToPixel({xAxisIndex: 0}, xMax); + const yMinPixel = chartInstance.convertToPixel({yAxisIndex: 0}, yMin); + + const area = evt.areas[0]; + + const [selected_xMin, selected_xMax] = area.coordRange; + + // Since we can keep dragging beyond the visible range, + // clamp the ranges to the minimum and maximum values of the visible x axis and y axis + const clampedCoordRange: [number, number] = [ + Math.max(xMin, selected_xMin), + Math.min(xMax, selected_xMax), + ]; + + const clampedXMaxPixel = chartInstance.convertToPixel( + {xAxisIndex: 0}, + clampedCoordRange[1] + ); + const clampedXMinPixel = chartInstance.convertToPixel( + {xAxisIndex: 0}, + clampedCoordRange[0] + ); + + const newSelection: Selection = { + range: clampedCoordRange, + panelId: area.panelId, + }; + + const actionMenuPosition = calculateActionMenuPosition({ + chartInstance, + clampedXMaxPixel, + clampedXMinPixel, + xMaxPixel, + yMinPixel, + }); + + setState({ + selection: newSelection, + actionMenuPosition, + }); + + onSelectionEnd?.(newSelection, clearSelection); + }, + [onSelectionEnd, clearSelection] + ); + + const handleOutsideClick = useCallback( + (event: MouseEvent) => { + let el = event.target as HTMLElement | null; + + // Propagate the click event to the parent elements until we find the element that has the + // data-explore-chart-selection-region attribute. This is used to prevent the selection from + // being cleared if the user clicks within an 'inbound' region. + while (el) { + if (el.dataset?.exploreChartSelectionRegion !== undefined) { + return; + } + el = el.parentElement; + } + + clearSelection(); + }, + [clearSelection] + ); + + useEffect(() => { + window.addEventListener('click', handleOutsideClick, {capture: true}); + + // eslint-disable-next-line consistent-return + return () => { + window.removeEventListener('click', handleOutsideClick, {capture: true}); + }; + }, [handleOutsideClick]); + + const enableBrushMode = useCallback(() => { + const chartInstance = chartRef.current?.getEchartsInstance(); + chartInstance?.dispatchAction({ + type: 'takeGlobalCursor', + key: 'brush', + brushOption: CHART_X_RANGE_BRUSH_OPTION, + }); + }, [chartRef]); + + useEffect(() => { + if (disabled) { + return; + } + + const chartInstance = chartRef.current?.getEchartsInstance(); + + // Re-draw the box in the chart when a new selection is made + if (state?.selection) { + chartInstance?.dispatchAction({ + type: 'brush', + areas: [ + { + brushType: 'lineX', + coordRange: state.selection.range, + coordRanges: [state.selection.range], + panelId: state.selection.panelId, + }, + ], + }); + + // 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. + if (chartsGroupName) { + echarts?.connect(chartsGroupName); + } + } + + // Activate brush mode on load and when we re-draw the box/clear the selection + enabbleBrushModeFrameRef.current = requestAnimationFrame(() => { + enableBrushMode(); + }); + + // eslint-disable-next-line consistent-return + return () => { + if (enabbleBrushModeFrameRef.current) + cancelAnimationFrame(enabbleBrushModeFrameRef.current); + if (tooltipFrameRef.current) cancelAnimationFrame(tooltipFrameRef.current); + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [state, disabled, enableBrushMode, chartRef.current, chartsGroupName, deps]); + + const brush: BrushComponentOption | undefined = useMemo(() => { + return disabled ? undefined : CHART_X_RANGE_BRUSH_OPTION; + }, [disabled]); + + const toolBox = useMemo(() => { + if (disabled) { + return undefined; + } + + return ToolBox( + { + show: false, // Prevent the toolbox from being shown, we enable selection on load + }, + { + brush: { + type: ['lineX'], + }, + } + ); + }, [disabled]); + + const renderedActionMenu = useMemo(() => { + if (!state?.actionMenuPosition || !actionMenuRenderer) return null; + + // We want the top right corner of the action menu to be aligned with the bottom left + // corner of the selection box, when the menu is positioned to the left. Using a transform, saves us + // form having to calculate the exact position of the menu. + const transform = + state.actionMenuPosition.position === 'left' ? 'translateX(-100%)' : 'none'; + + return createPortal( +
+ {actionMenuRenderer(state.selection, clearSelection)} +
, + document.body + ); + }, [state, actionMenuRenderer, clearSelection]); + + const options: BoxSelectionOptions = useMemo(() => { + return { + brush, + onBrushEnd, + onBrushStart, + toolBox, + ActionMenu: renderedActionMenu, + }; + }, [onBrushEnd, brush, toolBox, onBrushStart, renderedActionMenu]); + + return options; +} + +function calculateActionMenuPosition({ + chartInstance, + clampedXMaxPixel, + clampedXMinPixel, + xMaxPixel, + yMinPixel, +}: { + chartInstance: EChartsInstance; + clampedXMaxPixel: number; + clampedXMinPixel: number; + xMaxPixel: number; + yMinPixel: number; +}) { + let leftOffset: number; + let position: 'left' | 'right'; + const chartRect = chartInstance.getDom().getBoundingClientRect(); + + // If the point that we stop dragging is to the right of 60% of the width of the chart, + // position the action menu to the bottom-left of the box. Otherwise, position it to the + // bottom-right of the box. + if (clampedXMaxPixel > 0.6 * xMaxPixel) { + position = 'left'; + leftOffset = clampedXMinPixel; + } else { + position = 'right'; + leftOffset = clampedXMaxPixel; + } + + return { + position, + left: chartRect.left + leftOffset, + top: chartRect.top + yMinPixel + window.scrollY, + }; +} From bd08ba33a3a4357ccaba03e1c991b8a10dd2d769 Mon Sep 17 00:00:00 2001 From: "getsantry[bot]" <66042841+getsantry[bot]@users.noreply.github.com> Date: Thu, 20 Nov 2025 18:45:06 +0000 Subject: [PATCH 02/19] :hammer_and_wrench: apply pre-commit fixes --- static/app/components/charts/useChartXRangeSelection.spec.tsx | 2 +- static/app/components/charts/useChartXRangeSelection.tsx | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/static/app/components/charts/useChartXRangeSelection.spec.tsx b/static/app/components/charts/useChartXRangeSelection.spec.tsx index e641099ba4ed55..8672fa36c3ec05 100644 --- a/static/app/components/charts/useChartXRangeSelection.spec.tsx +++ b/static/app/components/charts/useChartXRangeSelection.spec.tsx @@ -276,7 +276,7 @@ describe('useChartXRangeSelection', () => { }; act(() => { - result.current.onBrushEnd(mockEvent as any, mockEchartsInstance as any); + result.current.onBrushEnd(mockEvent as any, mockEchartsInstance); }); // Wait for effect to run diff --git a/static/app/components/charts/useChartXRangeSelection.tsx b/static/app/components/charts/useChartXRangeSelection.tsx index d78d86973a16f7..ea98d306a47efa 100644 --- a/static/app/components/charts/useChartXRangeSelection.tsx +++ b/static/app/components/charts/useChartXRangeSelection.tsx @@ -257,7 +257,6 @@ export function useChartXRangeSelection({ useEffect(() => { window.addEventListener('click', handleOutsideClick, {capture: true}); - // eslint-disable-next-line consistent-return return () => { window.removeEventListener('click', handleOutsideClick, {capture: true}); }; From acd145349555c40740680badfb7d7fe446f208db Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Fri, 21 Nov 2025 10:22:40 -0500 Subject: [PATCH 03/19] feat(explore-attr-breakdowns): Using new useChartXRangeSelection hook --- .../timeSeriesWidgetVisualization.tsx | 184 +++++++++--------- .../chartSelectionContext.tsx | 6 +- .../cohortComparisonContent.tsx | 16 +- .../attributeBreakdowns/content.tsx | 2 +- .../attributeBreakdowns/floatingTrigger.tsx | 56 ++---- .../components/chart/chartVisualization.tsx | 20 +- .../hooks/useAttributeBreakdownComparison.tsx | 10 +- .../app/views/explore/spans/charts/index.tsx | 54 +++-- 8 files changed, 152 insertions(+), 196 deletions(-) diff --git a/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx b/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx index dfd1983d93b200..3d585bfbb2f561 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 */ @@ -175,6 +154,14 @@ export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizati saveOnZoom: true, }); + const {groupName} = useWidgetSyncContext(); + const {brush, onBrushEnd, onBrushStart, toolBox, ActionMenu} = useChartXRangeSelection({ + chartRef, + deps: [props.plottables], + 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..e252400f801c84 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; +export type ChartSelectionState = { 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..ba4f42fb14ea4b 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/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} ); } From f4b4f9cfadcd634bab4f7e5b92226d552a271548 Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Fri, 21 Nov 2025 10:27:27 -0500 Subject: [PATCH 04/19] feat(explore-attr-breakdowns): Removing old hook --- .../explore/hooks/useChartBoxSelect.spec.tsx | 306 ------------------ .../views/explore/hooks/useChartBoxSelect.tsx | 269 --------------- 2 files changed, 575 deletions(-) delete mode 100644 static/app/views/explore/hooks/useChartBoxSelect.spec.tsx delete mode 100644 static/app/views/explore/hooks/useChartBoxSelect.tsx 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; -} From 2be7db5900571fcae92dea6c79ce4e3a6ecd2848 Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Fri, 21 Nov 2025 10:40:09 -0500 Subject: [PATCH 05/19] feat(explore-attribute-breakdowns): Addressing cursor comments --- .../app/components/charts/useChartXRangeSelection.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/static/app/components/charts/useChartXRangeSelection.tsx b/static/app/components/charts/useChartXRangeSelection.tsx index ea98d306a47efa..b0d2c27e1c963c 100644 --- a/static/app/components/charts/useChartXRangeSelection.tsx +++ b/static/app/components/charts/useChartXRangeSelection.tsx @@ -160,6 +160,8 @@ export function useChartXRangeSelection({ ); const clearSelection = useCallback(() => { + if (!state?.selection) return; + const chartInstance = chartRef.current?.getEchartsInstance(); chartInstance?.dispatchAction({type: 'brush', areas: []}); @@ -174,7 +176,7 @@ export function useChartXRangeSelection({ setState(null); onClearSelection?.(); - }, [chartRef, onClearSelection]); + }, [chartRef, onClearSelection, state?.selection]); const onBrushEnd = useCallback( (evt: any, chartInstance: any) => { @@ -255,12 +257,15 @@ export function useChartXRangeSelection({ ); useEffect(() => { + if (disabled || !state?.selection) return; + window.addEventListener('click', handleOutsideClick, {capture: true}); + // eslint-disable-next-line consistent-return return () => { window.removeEventListener('click', handleOutsideClick, {capture: true}); }; - }, [handleOutsideClick]); + }, [handleOutsideClick, disabled, state?.selection]); const enableBrushMode = useCallback(() => { const chartInstance = chartRef.current?.getEchartsInstance(); @@ -345,6 +350,7 @@ export function useChartXRangeSelection({ return createPortal(
Date: Fri, 21 Nov 2025 10:47:58 -0500 Subject: [PATCH 06/19] feat(explore-attribute-breakdowns): Minor cleanup --- .../explore/components/attributeBreakdowns/floatingTrigger.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/views/explore/components/attributeBreakdowns/floatingTrigger.tsx b/static/app/views/explore/components/attributeBreakdowns/floatingTrigger.tsx index ba4f42fb14ea4b..b28b4faa511d14 100644 --- a/static/app/views/explore/components/attributeBreakdowns/floatingTrigger.tsx +++ b/static/app/views/explore/components/attributeBreakdowns/floatingTrigger.tsx @@ -60,7 +60,7 @@ export function FloatingTrigger({chartInfo, selection, clearSelection, setTab}: }, [selection, chartInfo, setChartSelection, setTab]); return ( - + {t('Zoom in')} {t('Compare Attribute Breakdowns')} From 0dd68973b6982cedcf1dc08bc24ed3b984b35ee7 Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Fri, 21 Nov 2025 10:55:27 -0500 Subject: [PATCH 07/19] feat(explore-attribute-breakdowns): Fixing knip errors --- static/app/components/charts/useChartXRangeSelection.tsx | 2 +- static/app/types/echarts.tsx | 2 +- .../components/attributeBreakdowns/chartSelectionContext.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/static/app/components/charts/useChartXRangeSelection.tsx b/static/app/components/charts/useChartXRangeSelection.tsx index b0d2c27e1c963c..3f27c9d265339b 100644 --- a/static/app/components/charts/useChartXRangeSelection.tsx +++ b/static/app/components/charts/useChartXRangeSelection.tsx @@ -30,7 +30,7 @@ type State = { selection: Selection; } | null; -export type BoxSelectionOptions = { +type BoxSelectionOptions = { /** * The brush option override for the chart, to enable brush mode. */ diff --git a/static/app/types/echarts.tsx b/static/app/types/echarts.tsx index 5cca2984d441f3..a140c277898681 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[][]; range: number[] | number[][]; }>; diff --git a/static/app/views/explore/components/attributeBreakdowns/chartSelectionContext.tsx b/static/app/views/explore/components/attributeBreakdowns/chartSelectionContext.tsx index e252400f801c84..8443b9efb96963 100644 --- a/static/app/views/explore/components/attributeBreakdowns/chartSelectionContext.tsx +++ b/static/app/views/explore/components/attributeBreakdowns/chartSelectionContext.tsx @@ -3,7 +3,7 @@ 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'; -export type ChartSelectionState = { +type ChartSelectionState = { chartInfo: ChartInfo; selection: Selection; } | null; From a6ed0deefbe6757b5ebd7b1c05aa39b0dd693249 Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Fri, 21 Nov 2025 11:04:51 -0500 Subject: [PATCH 08/19] feat(explore-attr-breakdowns): Fixing eslint errors --- .../app/components/charts/useChartXRangeSelection.spec.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/static/app/components/charts/useChartXRangeSelection.spec.tsx b/static/app/components/charts/useChartXRangeSelection.spec.tsx index 8672fa36c3ec05..97a15ba763a171 100644 --- a/static/app/components/charts/useChartXRangeSelection.spec.tsx +++ b/static/app/components/charts/useChartXRangeSelection.spec.tsx @@ -102,7 +102,7 @@ describe('useChartXRangeSelection', () => { }); describe('onBrushEnd handler', () => { - it('should set selection with clamped coordinates', async () => { + it('should set selection with clamped coordinates', () => { const onSelectionEnd = jest.fn(); const mockEchartsInstance = { @@ -171,7 +171,7 @@ describe('useChartXRangeSelection', () => { ); }); - it('should clamp coordinates that exceed axis bounds', async () => { + it('should clamp coordinates that exceed axis bounds', () => { const onSelectionEnd = jest.fn(); const mockEchartsInstance = { @@ -450,7 +450,7 @@ describe('useChartXRangeSelection', () => { document.body.removeChild(outsideElement); }); - it('should not clear selection when clicking inside selection region', async () => { + it('should not clear selection when clicking inside selection region', () => { const mockEchartsInstance = { ...mockChartInstance, getModel: jest.fn().mockReturnValue({ From 73d5e4b6f1835cebd1c03c0cd421eb52b09c1732 Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Fri, 21 Nov 2025 11:18:31 -0500 Subject: [PATCH 09/19] feat(explore-attr-breakdowns): Fixing tests --- .../widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx b/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx index 3d585bfbb2f561..faaae4cca8e433 100644 --- a/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx +++ b/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx @@ -158,6 +158,7 @@ export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizati const {brush, onBrushEnd, onBrushStart, toolBox, ActionMenu} = useChartXRangeSelection({ chartRef, deps: [props.plottables], + disabled: true, chartsGroupName: groupName, ...props.chartXRangeSelection, }); From c03426c1096060eecb50c3f1355bddbea68dcb1a Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Fri, 21 Nov 2025 14:59:17 -0500 Subject: [PATCH 10/19] feat(charts): Addressing PR comments --- .../charts/useChartXRangeSelection.tsx | 103 ++++++++++-------- static/app/types/echarts.tsx | 1 + 2 files changed, 58 insertions(+), 46 deletions(-) diff --git a/static/app/components/charts/useChartXRangeSelection.tsx b/static/app/components/charts/useChartXRangeSelection.tsx index b0d2c27e1c963c..52b36c717806d9 100644 --- a/static/app/components/charts/useChartXRangeSelection.tsx +++ b/static/app/components/charts/useChartXRangeSelection.tsx @@ -30,7 +30,7 @@ type State = { selection: Selection; } | null; -export type BoxSelectionOptions = { +type BoxSelectionOptions = { /** * The brush option override for the chart, to enable brush mode. */ @@ -127,10 +127,10 @@ export function useChartXRangeSelection({ }: ChartXRangeSelectionProps): BoxSelectionOptions { const [state, setState] = useState(); const tooltipFrameRef = useRef(null); - const enabbleBrushModeFrameRef = useRef(null); + const enableBrushModeFrameRef = useRef(null); const onBrushStart = useCallback( - (_evt: any, chartInstance: any) => { + (_evt, chartInstance) => { // 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, @@ -179,11 +179,15 @@ export function useChartXRangeSelection({ }, [chartRef, onClearSelection, state?.selection]); const onBrushEnd = useCallback( - (evt: any, chartInstance: any) => { + (evt, chartInstance) => { if (!chartInstance) return; - // Get the axis values of the chart + // @ts-expect-error TODO Abdullah Khan: chartInstance.getModel is a private method, but we access it to get the axis extremes + // could not find a better way, this works out perfectly for now. Passing down the entire series data to the hook is more gross. const xAxis = chartInstance.getModel().getComponent('xAxis', 0); + + // @ts-expect-error TODO Abdullah Khan: chartInstance.getModel is a private method, but we access it to get the axis extremes + // could not find a better way, this works out perfectly for now. Passing down the entire series data to the hook is more gross. const yAxis = chartInstance.getModel().getComponent('yAxis', 0); // Get the minimum and maximum values of the x axis and y axis @@ -196,43 +200,51 @@ export function useChartXRangeSelection({ const area = evt.areas[0]; - const [selected_xMin, selected_xMax] = area.coordRange; - - // Since we can keep dragging beyond the visible range, - // clamp the ranges to the minimum and maximum values of the visible x axis and y axis - const clampedCoordRange: [number, number] = [ - Math.max(xMin, selected_xMin), - Math.min(xMax, selected_xMax), - ]; - - const clampedXMaxPixel = chartInstance.convertToPixel( - {xAxisIndex: 0}, - clampedCoordRange[1] - ); - const clampedXMinPixel = chartInstance.convertToPixel( - {xAxisIndex: 0}, - clampedCoordRange[0] - ); - - const newSelection: Selection = { - range: clampedCoordRange, - panelId: area.panelId, - }; - - const actionMenuPosition = calculateActionMenuPosition({ - chartInstance, - clampedXMaxPixel, - clampedXMinPixel, - xMaxPixel, - yMinPixel, - }); - - setState({ - selection: newSelection, - actionMenuPosition, - }); + if ( + area && + Array.isArray(area.coordRange) && + area.coordRange.length === 2 && + typeof area.coordRange[0] === 'number' && + typeof area.coordRange[1] === 'number' + ) { + const [selected_xMin, selected_xMax] = area.coordRange; + + // Since we can keep dragging beyond the visible range, + // clamp the ranges to the minimum and maximum values of the visible x axis and y axis + const clampedCoordRange: [number, number] = [ + Math.max(xMin, selected_xMin), + Math.min(xMax, selected_xMax), + ]; + + const clampedXMaxPixel = chartInstance.convertToPixel( + {xAxisIndex: 0}, + clampedCoordRange[1] + ); + const clampedXMinPixel = chartInstance.convertToPixel( + {xAxisIndex: 0}, + clampedCoordRange[0] + ); - onSelectionEnd?.(newSelection, clearSelection); + const newSelection: Selection = { + range: clampedCoordRange, + panelId: area.panelId, + }; + + const actionMenuPosition = calculateActionMenuPosition({ + chartInstance, + clampedXMaxPixel, + clampedXMinPixel, + xMaxPixel, + yMinPixel, + }); + + setState({ + selection: newSelection, + actionMenuPosition, + }); + + onSelectionEnd?.(newSelection, clearSelection); + } }, [onSelectionEnd, clearSelection] ); @@ -305,18 +317,17 @@ export function useChartXRangeSelection({ } // Activate brush mode on load and when we re-draw the box/clear the selection - enabbleBrushModeFrameRef.current = requestAnimationFrame(() => { + enableBrushModeFrameRef.current = requestAnimationFrame(() => { enableBrushMode(); }); // eslint-disable-next-line consistent-return return () => { - if (enabbleBrushModeFrameRef.current) - cancelAnimationFrame(enabbleBrushModeFrameRef.current); + if (enableBrushModeFrameRef.current) + cancelAnimationFrame(enableBrushModeFrameRef.current); if (tooltipFrameRef.current) cancelAnimationFrame(tooltipFrameRef.current); }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [state, disabled, enableBrushMode, chartRef.current, chartsGroupName, deps]); + }, [state, disabled, enableBrushMode, chartRef, chartsGroupName, deps]); const brush: BrushComponentOption | undefined = useMemo(() => { return disabled ? undefined : CHART_X_RANGE_BRUSH_OPTION; diff --git a/static/app/types/echarts.tsx b/static/app/types/echarts.tsx index 5cca2984d441f3..d6746fe62813a7 100644 --- a/static/app/types/echarts.tsx +++ b/static/app/types/echarts.tsx @@ -164,6 +164,7 @@ export type EChartRenderedHandler = EChartEventHandler>; export type EchartBrushAreas = Array<{ coordRange: number[] | number[][]; + panelId: string; range: number[] | number[][]; }>; From 65fb51c764c22c4708f7d7260b23f469000195e5 Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Fri, 21 Nov 2025 15:58:07 -0500 Subject: [PATCH 11/19] feat(explore-attrs-breakdown): Addressing PR suggestions --- .../widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx b/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx index faaae4cca8e433..826429956b5dc3 100644 --- a/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx +++ b/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx @@ -138,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} = @@ -154,7 +154,6 @@ export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizati saveOnZoom: true, }); - const {groupName} = useWidgetSyncContext(); const {brush, onBrushEnd, onBrushStart, toolBox, ActionMenu} = useChartXRangeSelection({ chartRef, deps: [props.plottables], From 919b753d137a198433216bf5ee29c6be0f1f556e Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Sun, 23 Nov 2025 20:13:56 -0500 Subject: [PATCH 12/19] feat(explore-attrs-breakdown): Adding url persistence on absolute date queries --- .../charts/useChartXRangeSelection.tsx | 161 ++++++++++++------ .../chartSelectionContext.tsx | 3 +- .../cohortComparisonContent.tsx | 20 ++- .../attributeBreakdowns/content.tsx | 2 +- .../attributeBreakdowns/floatingTrigger.tsx | 9 +- .../hooks/useAttributeBreakdownComparison.tsx | 19 +-- .../app/views/explore/spans/charts/index.tsx | 6 +- 7 files changed, 139 insertions(+), 81 deletions(-) diff --git a/static/app/components/charts/useChartXRangeSelection.tsx b/static/app/components/charts/useChartXRangeSelection.tsx index 52b36c717806d9..6d293dad24c3a6 100644 --- a/static/app/components/charts/useChartXRangeSelection.tsx +++ b/static/app/components/charts/useChartXRangeSelection.tsx @@ -99,6 +99,11 @@ export type ChartXRangeSelectionProps = { */ disabled?: boolean; + /** + * Initial selection, used to draw the box on load. + */ + initialSelection?: Selection; + /** * The callback that is called when the selection is cleared. */ @@ -122,10 +127,23 @@ export function useChartXRangeSelection({ onClearSelection, actionMenuRenderer, chartsGroupName, + initialSelection, disabled = false, deps = [], }: ChartXRangeSelectionProps): BoxSelectionOptions { - const [state, setState] = useState(); + const initialState = useMemo(() => { + if (!initialSelection) { + return null; + } + + return { + selection: initialSelection, + actionMenuPosition: null, + }; + }, [initialSelection]); + + const [state, setState] = useState(initialState); + const tooltipFrameRef = useRef(null); const enableBrushModeFrameRef = useRef(null); @@ -182,22 +200,6 @@ export function useChartXRangeSelection({ (evt, chartInstance) => { if (!chartInstance) return; - // @ts-expect-error TODO Abdullah Khan: chartInstance.getModel is a private method, but we access it to get the axis extremes - // could not find a better way, this works out perfectly for now. Passing down the entire series data to the hook is more gross. - const xAxis = chartInstance.getModel().getComponent('xAxis', 0); - - // @ts-expect-error TODO Abdullah Khan: chartInstance.getModel is a private method, but we access it to get the axis extremes - // could not find a better way, this works out perfectly for now. Passing down the entire series data to the hook is more gross. - const yAxis = chartInstance.getModel().getComponent('yAxis', 0); - - // Get the minimum and maximum values of the x axis and y axis - const xMin = xAxis.axis.scale.getExtent()[0]; - const xMax = xAxis.axis.scale.getExtent()[1]; - const yMin = yAxis.axis.scale.getExtent()[0]; - - const xMaxPixel = chartInstance.convertToPixel({xAxisIndex: 0}, xMax); - const yMinPixel = chartInstance.convertToPixel({yAxisIndex: 0}, yMin); - const area = evt.areas[0]; if ( @@ -207,43 +209,17 @@ export function useChartXRangeSelection({ typeof area.coordRange[0] === 'number' && typeof area.coordRange[1] === 'number' ) { - const [selected_xMin, selected_xMax] = area.coordRange; - - // Since we can keep dragging beyond the visible range, - // clamp the ranges to the minimum and maximum values of the visible x axis and y axis - const clampedCoordRange: [number, number] = [ - Math.max(xMin, selected_xMin), - Math.min(xMax, selected_xMax), - ]; - - const clampedXMaxPixel = chartInstance.convertToPixel( - {xAxisIndex: 0}, - clampedCoordRange[1] - ); - const clampedXMinPixel = chartInstance.convertToPixel( - {xAxisIndex: 0}, - clampedCoordRange[0] - ); - - const newSelection: Selection = { - range: clampedCoordRange, - panelId: area.panelId, - }; - - const actionMenuPosition = calculateActionMenuPosition({ + const newState = calculateNewState({ chartInstance, - clampedXMaxPixel, - clampedXMinPixel, - xMaxPixel, - yMinPixel, + newRange: area.coordRange as [number, number], + panelId: area.panelId, }); - setState({ - selection: newSelection, - actionMenuPosition, - }); + setState(newState); - onSelectionEnd?.(newSelection, clearSelection); + if (newState) { + onSelectionEnd?.(newState.selection, clearSelection); + } } }, [onSelectionEnd, clearSelection] @@ -296,8 +272,8 @@ export function useChartXRangeSelection({ const chartInstance = chartRef.current?.getEchartsInstance(); // Re-draw the box in the chart when a new selection is made - if (state?.selection) { - chartInstance?.dispatchAction({ + if (state?.selection && chartInstance) { + chartInstance.dispatchAction({ type: 'brush', areas: [ { @@ -309,6 +285,21 @@ export function useChartXRangeSelection({ ], }); + // Synchronize the action menu position with the selection box + // only if the action menu position is not already set, this is used on load when we + // initialize the state, from the initialSelection prop. + if (!state.actionMenuPosition) { + const newState = calculateNewState({ + chartInstance, + newRange: state.selection.range, + panelId: state.selection.panelId, + }); + + if (newState) { + setState(newState); + } + } + // 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. if (chartsGroupName) { @@ -390,6 +381,69 @@ export function useChartXRangeSelection({ return options; } +function calculateNewState({ + chartInstance, + newRange, + panelId, +}: { + chartInstance: EChartsInstance; + newRange: [number, number]; + panelId: string; +}): State { + // @ts-expect-error TODO Abdullah Khan: chartInstance.getModel is a private method, but we access it to get the axis extremes + // could not find a better way, this works out perfectly for now. Passing down the entire series data to the hook is more gross. + const xAxis = chartInstance.getModel()?.getComponent?.('xAxis', 0); + + // @ts-expect-error TODO Abdullah Khan: chartInstance.getModel is a private method, but we access it to get the axis extremes + // could not find a better way, this works out perfectly for now. Passing down the entire series data to the hook is more gross. + const yAxis = chartInstance.getModel()?.getComponent?.('yAxis', 0); + + if (!xAxis || !yAxis) { + return null; + } + + // Get the minimum and maximum values of the x axis and y axis + const xMin = xAxis.axis.scale.getExtent()[0]; + const xMax = xAxis.axis.scale.getExtent()[1]; + const yMin = yAxis.axis.scale.getExtent()[0]; + + const xMaxPixel = chartInstance.convertToPixel({xAxisIndex: 0}, xMax); + const yMinPixel = chartInstance.convertToPixel({yAxisIndex: 0}, yMin); + const [selected_xMin, selected_xMax] = newRange; + + // Since we can keep dragging beyond the visible range, + // clamp the ranges to the minimum and maximum values of the visible x axis and y axis + const clampedCoordRange: [number, number] = [ + Math.max(xMin, selected_xMin), + Math.min(xMax, selected_xMax), + ]; + + const clampedXMaxPixel = chartInstance.convertToPixel( + {xAxisIndex: 0}, + clampedCoordRange[1] + ); + const clampedXMinPixel = chartInstance.convertToPixel( + {xAxisIndex: 0}, + clampedCoordRange[0] + ); + + const actionMenuPosition = calculateActionMenuPosition({ + chartInstance, + clampedXMaxPixel, + clampedXMinPixel, + xMaxPixel, + yMinPixel, + }); + + return { + actionMenuPosition, + selection: { + range: clampedCoordRange, + panelId, + }, + }; +} + function calculateActionMenuPosition({ chartInstance, clampedXMaxPixel, @@ -402,7 +456,8 @@ function calculateActionMenuPosition({ clampedXMinPixel: number; xMaxPixel: number; yMinPixel: number; -}) { +}): {left: number; position: 'left' | 'right'; top: number} { + // Calculate the position of the action menu let leftOffset: number; let position: 'left' | 'right'; const chartRect = chartInstance.getDom().getBoundingClientRect(); diff --git a/static/app/views/explore/components/attributeBreakdowns/chartSelectionContext.tsx b/static/app/views/explore/components/attributeBreakdowns/chartSelectionContext.tsx index 8443b9efb96963..5c8b845300eb93 100644 --- a/static/app/views/explore/components/attributeBreakdowns/chartSelectionContext.tsx +++ b/static/app/views/explore/components/attributeBreakdowns/chartSelectionContext.tsx @@ -1,10 +1,9 @@ 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'; type ChartSelectionState = { - chartInfo: ChartInfo; + chartIndex: number; selection: Selection; } | null; diff --git a/static/app/views/explore/components/attributeBreakdowns/cohortComparisonContent.tsx b/static/app/views/explore/components/attributeBreakdowns/cohortComparisonContent.tsx index c5bf31c8e32833..42ae17f5148fbb 100644 --- a/static/app/views/explore/components/attributeBreakdowns/cohortComparisonContent.tsx +++ b/static/app/views/explore/components/attributeBreakdowns/cohortComparisonContent.tsx @@ -18,8 +18,8 @@ import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; 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 {useQueryParamsVisualizes} from 'sentry/views/explore/queryParams/context'; import {prettifyAggregation} from 'sentry/views/explore/utils'; import {Chart} from './cohortComparisonChart'; @@ -33,14 +33,18 @@ const PERCENTILE_FUNCTION_PREFIXES = ['p50', 'p75', 'p90', 'p95', 'p99', 'avg']; export function CohortComparison({ selection, - chartInfo, + chartIndex, }: { - chartInfo: ChartInfo; + chartIndex: number; selection: Selection; }) { + const visualizes = useQueryParamsVisualizes(); + + const yAxis = visualizes[chartIndex]?.yAxis ?? ''; + const {data, isLoading, isError} = useAttributeBreakdownComparison({ - selection, - chartInfo, + aggregateFunction: yAxis, + range: selection.range, }); const [searchQuery, setSearchQuery] = useState(''); const sortingMethod: SortingMethod = 'rrr'; @@ -102,12 +106,12 @@ export function CohortComparison({ const endDate = moment.tz(endTimestamp, userTimezone).format('MMM D YYYY h:mm A z'); // Check if yAxis is a percentile function (only these functions should include "and is greater than or equal to") - const yAxisLower = chartInfo.yAxis.toLowerCase(); + const yAxisLower = yAxis.toLowerCase(); const isPercentileFunction = PERCENTILE_FUNCTION_PREFIXES.some(prefix => yAxisLower.startsWith(prefix) ); - const formattedFunction = prettifyAggregation(chartInfo.yAxis) ?? chartInfo.yAxis; + const formattedFunction = prettifyAggregation(yAxis) ?? yAxis; return { selection: isPercentileFunction @@ -120,7 +124,7 @@ export function CohortComparison({ : t(`Selection is data between %s - %s`, startDate, endDate), baseline: t('Baseline is all other spans from your query'), }; - }, [selection, chartInfo.yAxis]); + }, [selection, yAxis]); return ( diff --git a/static/app/views/explore/components/attributeBreakdowns/content.tsx b/static/app/views/explore/components/attributeBreakdowns/content.tsx index c0d8018dee76c7..a615b266a3f063 100644 --- a/static/app/views/explore/components/attributeBreakdowns/content.tsx +++ b/static/app/views/explore/components/attributeBreakdowns/content.tsx @@ -9,7 +9,7 @@ export function AttributeBreakdownsContent() { return ( ); } diff --git a/static/app/views/explore/components/attributeBreakdowns/floatingTrigger.tsx b/static/app/views/explore/components/attributeBreakdowns/floatingTrigger.tsx index b28b4faa511d14..0ad162fa1ea059 100644 --- a/static/app/views/explore/components/attributeBreakdowns/floatingTrigger.tsx +++ b/static/app/views/explore/components/attributeBreakdowns/floatingTrigger.tsx @@ -7,20 +7,19 @@ 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 {Tab} from 'sentry/views/explore/hooks/useTab'; import type {Mode} from 'sentry/views/explore/queryParams/mode'; import {useChartSelection} from './chartSelectionContext'; type Props = { - chartInfo: ChartInfo; + chartIndex: number; clearSelection: () => void; selection: Selection; setTab: (tab: Mode | Tab) => void; }; -export function FloatingTrigger({chartInfo, selection, clearSelection, setTab}: Props) { +export function FloatingTrigger({chartIndex, selection, clearSelection, setTab}: Props) { const router = useRouter(); const {setChartSelection} = useChartSelection(); @@ -54,10 +53,10 @@ export function FloatingTrigger({chartInfo, selection, clearSelection, setTab}: const handleFindAttributeBreakdowns = useCallback(() => { setChartSelection({ selection, - chartInfo, + chartIndex, }); setTab(Tab.ATTRIBUTE_BREAKDOWNS); - }, [selection, chartInfo, setChartSelection, setTab]); + }, [selection, chartIndex, setChartSelection, setTab]); return ( diff --git a/static/app/views/explore/hooks/useAttributeBreakdownComparison.tsx b/static/app/views/explore/hooks/useAttributeBreakdownComparison.tsx index 5b7f0b2f0a8942..852c0c281bf5d7 100644 --- a/static/app/views/explore/hooks/useAttributeBreakdownComparison.tsx +++ b/static/app/views/explore/hooks/useAttributeBreakdownComparison.tsx @@ -1,6 +1,5 @@ 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 +8,6 @@ import {MutableSearch} from 'sentry/utils/tokenizeSearch'; 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 {SAMPLING_MODE} from 'sentry/views/explore/hooks/useProgressiveQuery'; import {useSpansDataset} from 'sentry/views/explore/spans/spansQueryParams'; @@ -34,11 +32,11 @@ export type AttributeBreakdownsComparison = { }; function useAttributeBreakdownComparison({ - selection, - chartInfo, + aggregateFunction, + range, }: { - chartInfo: ChartInfo; - selection: Selection; + aggregateFunction: string; + range: [number, number]; }) { const location = useLocation(); const organization = useOrganization(); @@ -46,8 +44,7 @@ function useAttributeBreakdownComparison({ const {selection: pageFilters} = usePageFilters(); const aggregateExtrapolation = location.query.extrapolate ?? '1'; - const enableQuery = selection !== null; - const [x1, x2] = selection.range; + const [x1, x2] = range; // Ensure that we pass the existing queries in the search bar to the attribute breakdowns queries const currentQuery = location.query.query?.toString() ?? ''; @@ -77,12 +74,12 @@ function useAttributeBreakdownComparison({ query_1: query1, query_2: query2, dataset, - function: chartInfo.yAxis, + function: aggregateFunction, above: 1, sampling: SAMPLING_MODE.NORMAL, aggregateExtrapolation, }; - }, [query1, query2, pageFilters, dataset, chartInfo.yAxis, aggregateExtrapolation]); + }, [query1, query2, pageFilters, dataset, aggregateFunction, aggregateExtrapolation]); return useApiQuery( [ @@ -91,7 +88,7 @@ function useAttributeBreakdownComparison({ ], { staleTime: Infinity, - enabled: enableQuery, + enabled: !!aggregateFunction && !!range, } ); } diff --git a/static/app/views/explore/spans/charts/index.tsx b/static/app/views/explore/spans/charts/index.tsx index cff8be105372f1..1f719caac0c9b6 100644 --- a/static/app/views/explore/spans/charts/index.tsx +++ b/static/app/views/explore/spans/charts/index.tsx @@ -264,6 +264,10 @@ function Chart({ chartInfo={chartInfo} chartRef={chartRef} chartXRangeSelection={{ + initialSelection: { + range: [1763941159024.5266, 1763941508324.1692], + panelId: 'grid--\u0000series\u00000\u00000', + }, onClearSelection: () => { setChartSelection(null); }, @@ -273,7 +277,7 @@ function Chart({ actionMenuRenderer: (selection, clearSelection) => { return ( Date: Sun, 23 Nov 2025 20:46:26 -0500 Subject: [PATCH 13/19] feat(explore-attrs-breakdown): Iterating on functionality --- static/app/utils/url/urlParamBatchContext.tsx | 8 ++- .../chartSelectionContext.tsx | 66 +++++++++++++++++-- .../app/views/explore/spans/charts/index.tsx | 12 ++-- 3 files changed, 75 insertions(+), 11 deletions(-) diff --git a/static/app/utils/url/urlParamBatchContext.tsx b/static/app/utils/url/urlParamBatchContext.tsx index 3872ed0e07ebbc..e75c65ff52601f 100644 --- a/static/app/utils/url/urlParamBatchContext.tsx +++ b/static/app/utils/url/urlParamBatchContext.tsx @@ -39,7 +39,9 @@ export function UrlParamBatchProvider({children}: {children: React.ReactNode}) { {replace: true, preventScrollReset: true} ); pendingUpdates.current = {}; - }, [location, navigate]); + + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); // Debounced URL updater function const updateURL = useMemo( @@ -78,7 +80,9 @@ export function UrlParamBatchProvider({children}: {children: React.ReactNode}) { }, [updateURL]); return ( - {children} + + {children} + ); } diff --git a/static/app/views/explore/components/attributeBreakdowns/chartSelectionContext.tsx b/static/app/views/explore/components/attributeBreakdowns/chartSelectionContext.tsx index 5c8b845300eb93..a6af370c88961d 100644 --- a/static/app/views/explore/components/attributeBreakdowns/chartSelectionContext.tsx +++ b/static/app/views/explore/components/attributeBreakdowns/chartSelectionContext.tsx @@ -1,6 +1,8 @@ -import {createContext, useContext, useMemo, useState} from 'react'; +import {createContext, useContext, useMemo} from 'react'; import type {Selection} from 'sentry/components/charts/useChartXRangeSelection'; +import {UrlParamBatchProvider} from 'sentry/utils/url/urlParamBatchContext'; +import {useQueryParamState} from 'sentry/utils/url/useQueryParamState'; type ChartSelectionState = { chartIndex: number; @@ -20,13 +22,61 @@ interface ChartSelectionProviderProps { children: React.ReactNode; } -export function ChartSelectionProvider({children}: ChartSelectionProviderProps) { - const [chartSelection, setChartSelection] = useState(null); +function serializeChartSelection(state: ChartSelectionState): string { + if (!state) { + return ''; + } + + return JSON.stringify({ + chartIndex: state.chartIndex, + range: state.selection.range, + panelId: state.selection.panelId, + }); +} + +function deserializeChartSelection(value: string | undefined): ChartSelectionState { + if (!value) { + return null; + } + + try { + const parsed = JSON.parse(value); + + // Validate the parsed data + if ( + typeof parsed.chartIndex === 'number' && + Array.isArray(parsed.range) && + parsed.range.length === 2 && + typeof parsed.range[0] === 'number' && + typeof parsed.range[1] === 'number' && + typeof parsed.panelId === 'string' + ) { + return { + chartIndex: parsed.chartIndex, + selection: { + range: parsed.range as [number, number], + panelId: parsed.panelId, + }, + }; + } + } catch { + return null; + } + + return null; +} + +function ChartSelectionStateProvider({children}: ChartSelectionProviderProps) { + const [chartSelection, setChartSelection] = useQueryParamState({ + fieldName: 'chartSelection', + deserializer: deserializeChartSelection, + serializer: serializeChartSelection, + }); const value = useMemo( () => ({ + chartSelection: chartSelection ?? null, setChartSelection, - chartSelection, }), [chartSelection, setChartSelection] ); @@ -38,6 +88,14 @@ export function ChartSelectionProvider({children}: ChartSelectionProviderProps) ); } +export function ChartSelectionProvider({children}: ChartSelectionProviderProps) { + return ( + + {children} + + ); +} + export function useChartSelection(): ChartSelectionContextValue { const context = useContext(ChartSelectionContext); diff --git a/static/app/views/explore/spans/charts/index.tsx b/static/app/views/explore/spans/charts/index.tsx index 1f719caac0c9b6..82def5e78d118e 100644 --- a/static/app/views/explore/spans/charts/index.tsx +++ b/static/app/views/explore/spans/charts/index.tsx @@ -163,7 +163,7 @@ function Chart({ setTab, }: ChartProps) { const organization = useOrganization(); - const {setChartSelection} = useChartSelection(); + const {chartSelection, setChartSelection} = useChartSelection(); const [interval, setInterval, intervalOptions] = useChartInterval(); const chartHeight = visualize.visible ? CHART_HEIGHT : 50; @@ -254,6 +254,11 @@ function Chart({ ); + const initialChartSelection = + chartSelection && chartSelection.chartIndex === index + ? chartSelection.selection + : undefined; + const widget = ( { setChartSelection(null); }, From 499957d4e17a5583dbbf83b90d5ad4d79ec18093 Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Sun, 23 Nov 2025 21:43:48 -0500 Subject: [PATCH 14/19] feat(explore-attrs-breakdown): Iterating --- static/app/utils/url/urlParamBatchContext.tsx | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/static/app/utils/url/urlParamBatchContext.tsx b/static/app/utils/url/urlParamBatchContext.tsx index e75c65ff52601f..a5c2ba38bbc6c6 100644 --- a/static/app/utils/url/urlParamBatchContext.tsx +++ b/static/app/utils/url/urlParamBatchContext.tsx @@ -1,7 +1,7 @@ import {createContext, useCallback, useContext, useEffect, useMemo, useRef} from 'react'; import debounce from 'lodash/debounce'; +import * as qs from 'query-string'; -import {useLocation} from 'sentry/utils/useLocation'; import {useNavigate} from 'sentry/utils/useNavigate'; type BatchContextType = { @@ -13,7 +13,6 @@ const BatchContext = createContext(null); export function UrlParamBatchProvider({children}: {children: React.ReactNode}) { const navigate = useNavigate(); - const location = useLocation(); // Store the pending updates in a `ref`. This way, queuing more updates // doesn't update any state, so the context doesn't re-render and cause a @@ -27,9 +26,9 @@ export function UrlParamBatchProvider({children}: {children: React.ReactNode}) { navigate( { - ...location, + pathname: window.location.pathname, query: { - ...location.query, + ...qs.parse(window.location.search), ...pendingUpdates.current, }, }, @@ -39,9 +38,7 @@ export function UrlParamBatchProvider({children}: {children: React.ReactNode}) { {replace: true, preventScrollReset: true} ); pendingUpdates.current = {}; - - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [navigate]); // Debounced URL updater function const updateURL = useMemo( From a72ee2880175d98d50ad3c1dfa4d1a3a68dfc506 Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Mon, 24 Nov 2025 00:18:51 -0500 Subject: [PATCH 15/19] feat(explore-attrs-breakdown): Iterating --- .../charts/useChartXRangeSelection.tsx | 51 +++++++++++-------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/static/app/components/charts/useChartXRangeSelection.tsx b/static/app/components/charts/useChartXRangeSelection.tsx index 6d293dad24c3a6..3265563fe0c0fc 100644 --- a/static/app/components/charts/useChartXRangeSelection.tsx +++ b/static/app/components/charts/useChartXRangeSelection.tsx @@ -138,6 +138,8 @@ export function useChartXRangeSelection({ return { selection: initialSelection, + // Action menu position is not set on load. + // It's calculated from the selection selection range once brushing ends. actionMenuPosition: null, }; }, [initialSelection]); @@ -145,7 +147,7 @@ export function useChartXRangeSelection({ const [state, setState] = useState(initialState); const tooltipFrameRef = useRef(null); - const enableBrushModeFrameRef = useRef(null); + const brushStateSyncFrameRef = useRef(null); const onBrushStart = useCallback( (_evt, chartInstance) => { @@ -244,6 +246,8 @@ export function useChartXRangeSelection({ [clearSelection] ); + // This effect sets up the event listener for clearing of the selection + // when the user clicks outside the declared inbound regions. useEffect(() => { if (disabled || !state?.selection) return; @@ -264,15 +268,20 @@ export function useChartXRangeSelection({ }); }, [chartRef]); + // This effect fires whenever state changes. It: + // - Re-draws the selection box in the chart on state change enforcing persistence. + // - Populates the rest of the state from the optional `initialSelection` prop on load. + // - Activates brush mode on load and when we re-draw the box/clear the selection. useEffect(() => { - if (disabled) { + const chartInstance = chartRef.current?.getEchartsInstance(); + + if (disabled || !chartInstance) { return; } - const chartInstance = chartRef.current?.getEchartsInstance(); - - // Re-draw the box in the chart when a new selection is made - if (state?.selection && chartInstance) { + // Re-draw the box in the chart whenever state.selection changes, + // enforcing persistence. + if (state?.selection) { chartInstance.dispatchAction({ type: 'brush', areas: [ @@ -285,10 +294,19 @@ export function useChartXRangeSelection({ ], }); - // Synchronize the action menu position with the selection box - // only if the action menu position is not already set, this is used on load when we - // initialize the state, from the initialSelection prop. - if (!state.actionMenuPosition) { + // 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. + if (chartsGroupName) { + echarts?.connect(chartsGroupName); + } + } + + // Everything inside `requestAnimationFrame` is called only after the current render cycle completes, + // and this ensures ECharts has fully processed the dispatchAction above. + brushStateSyncFrameRef.current = requestAnimationFrame(() => { + // We only propagate the range of the selection box to the consumers, + // so we need to calculate the rest of the state from the range on load. + if (state && !state.actionMenuPosition) { const newState = calculateNewState({ chartInstance, newRange: state.selection.range, @@ -300,22 +318,13 @@ export function useChartXRangeSelection({ } } - // 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. - if (chartsGroupName) { - echarts?.connect(chartsGroupName); - } - } - - // Activate brush mode on load and when we re-draw the box/clear the selection - enableBrushModeFrameRef.current = requestAnimationFrame(() => { enableBrushMode(); }); // eslint-disable-next-line consistent-return return () => { - if (enableBrushModeFrameRef.current) - cancelAnimationFrame(enableBrushModeFrameRef.current); + if (brushStateSyncFrameRef.current) + cancelAnimationFrame(brushStateSyncFrameRef.current); if (tooltipFrameRef.current) cancelAnimationFrame(tooltipFrameRef.current); }; }, [state, disabled, enableBrushMode, chartRef, chartsGroupName, deps]); From 8c898e4bd1c049d8ee2c8fd858e99bdbb3be175d Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Mon, 24 Nov 2025 12:16:54 -0500 Subject: [PATCH 16/19] feat(explore-attrs-breakdown): Fixing tests --- .../charts/useChartXRangeSelection.tsx | 53 +++++++++++-------- .../utils/url/urlParamBatchContext.spec.tsx | 12 ++--- .../app/utils/url/useQueryParamState.spec.tsx | 8 +-- 3 files changed, 40 insertions(+), 33 deletions(-) diff --git a/static/app/components/charts/useChartXRangeSelection.tsx b/static/app/components/charts/useChartXRangeSelection.tsx index 3265563fe0c0fc..8655da7cc2344e 100644 --- a/static/app/components/charts/useChartXRangeSelection.tsx +++ b/static/app/components/charts/useChartXRangeSelection.tsx @@ -131,20 +131,7 @@ export function useChartXRangeSelection({ disabled = false, deps = [], }: ChartXRangeSelectionProps): BoxSelectionOptions { - const initialState = useMemo(() => { - if (!initialSelection) { - return null; - } - - return { - selection: initialSelection, - // Action menu position is not set on load. - // It's calculated from the selection selection range once brushing ends. - actionMenuPosition: null, - }; - }, [initialSelection]); - - const [state, setState] = useState(initialState); + const [state, setState] = useState(null); const tooltipFrameRef = useRef(null); const brushStateSyncFrameRef = useRef(null); @@ -273,9 +260,13 @@ export function useChartXRangeSelection({ // - Populates the rest of the state from the optional `initialSelection` prop on load. // - Activates brush mode on load and when we re-draw the box/clear the selection. useEffect(() => { + if (disabled) { + return; + } + const chartInstance = chartRef.current?.getEchartsInstance(); - if (disabled || !chartInstance) { + if (!chartInstance) { return; } @@ -301,16 +292,20 @@ export function useChartXRangeSelection({ } } + if (brushStateSyncFrameRef.current) { + cancelAnimationFrame(brushStateSyncFrameRef.current); + } + // Everything inside `requestAnimationFrame` is called only after the current render cycle completes, - // and this ensures ECharts has fully processed the dispatchAction above. + // and this ensures ECharts has fully processed all the dispatchActions like the one above. brushStateSyncFrameRef.current = requestAnimationFrame(() => { // We only propagate the range of the selection box to the consumers, - // so we need to calculate the rest of the state from the range on load. - if (state && !state.actionMenuPosition) { + // so we need to calculate the rest of the state from the `initialSelection` prop on load. + if (initialSelection && !state) { const newState = calculateNewState({ chartInstance, - newRange: state.selection.range, - panelId: state.selection.panelId, + newRange: initialSelection.range, + panelId: initialSelection.panelId, }); if (newState) { @@ -323,11 +318,23 @@ export function useChartXRangeSelection({ // eslint-disable-next-line consistent-return return () => { - if (brushStateSyncFrameRef.current) + if (brushStateSyncFrameRef.current) { cancelAnimationFrame(brushStateSyncFrameRef.current); - if (tooltipFrameRef.current) cancelAnimationFrame(tooltipFrameRef.current); + } + + if (tooltipFrameRef.current) { + cancelAnimationFrame(tooltipFrameRef.current); + } }; - }, [state, disabled, enableBrushMode, chartRef, chartsGroupName, deps]); + }, [ + state, + disabled, + enableBrushMode, + chartRef, + chartsGroupName, + initialSelection, + deps, + ]); const brush: BrushComponentOption | undefined = useMemo(() => { return disabled ? undefined : CHART_X_RANGE_BRUSH_OPTION; diff --git a/static/app/utils/url/urlParamBatchContext.spec.tsx b/static/app/utils/url/urlParamBatchContext.spec.tsx index 95741f963de47c..e16659281ec12d 100644 --- a/static/app/utils/url/urlParamBatchContext.spec.tsx +++ b/static/app/utils/url/urlParamBatchContext.spec.tsx @@ -1,18 +1,16 @@ import debounce from 'lodash/debounce'; -import {LocationFixture} from 'sentry-fixture/locationFixture'; import {renderHook} from 'sentry-test/reactTestingLibrary'; +import {setWindowLocation} from 'sentry-test/utils'; import { UrlParamBatchProvider, useUrlBatchContext, } from 'sentry/utils/url/urlParamBatchContext'; -import {useLocation} from 'sentry/utils/useLocation'; import {useNavigate} from 'sentry/utils/useNavigate'; import {testableDebounce} from './testUtils'; -jest.mock('sentry/utils/useLocation'); jest.mock('sentry/utils/useNavigate'); jest.mock('lodash/debounce'); @@ -33,7 +31,8 @@ describe('UrlParamBatchProvider', () => { }); it('should batch updates to the URL query params', () => { - jest.mocked(useLocation).mockReturnValue(LocationFixture()); + setWindowLocation('http://localhost/'); + const {result} = renderHook(() => useUrlBatchContext(), { wrapper: UrlParamBatchProvider, }); @@ -46,9 +45,10 @@ describe('UrlParamBatchProvider', () => { expect(mockNavigate).toHaveBeenCalledTimes(1); expect(mockNavigate).toHaveBeenCalledWith( - expect.objectContaining({ + { + pathname: '/', query: {foo: 'bar', potato: 'test'}, - }), + }, {replace: true, preventScrollReset: true} ); }); diff --git a/static/app/utils/url/useQueryParamState.spec.tsx b/static/app/utils/url/useQueryParamState.spec.tsx index a8644c6f514200..4e2d05eeda3b35 100644 --- a/static/app/utils/url/useQueryParamState.spec.tsx +++ b/static/app/utils/url/useQueryParamState.spec.tsx @@ -55,7 +55,7 @@ describe('useQueryParamState', () => { // The query param should not be updated yet expect(mockedNavigate).not.toHaveBeenCalledWith( { - ...LocationFixture(), + pathname: '/', query: {testField: 'initial state'}, }, {replace: true, preventScrollReset: true} @@ -67,7 +67,7 @@ describe('useQueryParamState', () => { // The query param should be updated expect(mockedNavigate).toHaveBeenCalledWith( { - ...LocationFixture(), + pathname: '/', query: {testField: 'newValue'}, }, {replace: true, preventScrollReset: true} @@ -120,7 +120,7 @@ describe('useQueryParamState', () => { expect(mockedNavigate).toHaveBeenCalledWith( { - ...LocationFixture(), + pathname: '/', query: {testField: 'newValue - 2 - true'}, }, {replace: true, preventScrollReset: true} @@ -153,7 +153,7 @@ describe('useQueryParamState', () => { expect(mockedNavigate).toHaveBeenCalledWith( { - ...LocationFixture(), + pathname: '/', query: {sort: ['testField']}, }, {replace: true, preventScrollReset: true} From 1491b44e7cf338ea24455205270efcc824d26106 Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Mon, 24 Nov 2025 12:46:11 -0500 Subject: [PATCH 17/19] feat(explore-attrs-breakdown): Resolving conflicts --- .../components/nameAndDescFields.spec.tsx | 23 ++++--------------- 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/static/app/views/dashboards/widgetBuilder/components/nameAndDescFields.spec.tsx b/static/app/views/dashboards/widgetBuilder/components/nameAndDescFields.spec.tsx index 94793ddecc4ba7..f9838fe8643bb1 100644 --- a/static/app/views/dashboards/widgetBuilder/components/nameAndDescFields.spec.tsx +++ b/static/app/views/dashboards/widgetBuilder/components/nameAndDescFields.spec.tsx @@ -1,25 +1,17 @@ import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; +import {setWindowLocation} from 'sentry-test/utils'; import WidgetBuilderNameAndDescription from 'sentry/views/dashboards/widgetBuilder/components/nameAndDescFields'; import {WidgetBuilderProvider} from 'sentry/views/dashboards/widgetBuilder/contexts/widgetBuilderContext'; describe('WidgetBuilder', () => { - const initialRouterConfig = { - route: '/organizations/:orgId/dashboard/:dashboardId/', - location: { - pathname: '/organizations/org-slug/dashboard/1/', - query: {project: '-1'}, - }, - }; - it('edits name and description', async () => { + setWindowLocation('http://localhost/organizations/org-slug/dashboard/1/?project=-1'); + const {router} = render( - , - { - initialRouterConfig, - } + ); await userEvent.type(await screen.findByPlaceholderText('Name'), 'some name'); @@ -28,7 +20,6 @@ describe('WidgetBuilder', () => { await userEvent.tab(); expect(router.location).toEqual( expect.objectContaining({ - ...initialRouterConfig.location, query: expect.objectContaining({title: 'some name'}), }) ); @@ -44,7 +35,6 @@ describe('WidgetBuilder', () => { await userEvent.tab(); expect(router.location).toEqual( expect.objectContaining({ - ...initialRouterConfig.location, query: expect.objectContaining({description: 'some description'}), }) ); @@ -56,10 +46,7 @@ describe('WidgetBuilder', () => { - , - { - initialRouterConfig, - } + ); expect( From 51114a0ed2113285eb7e135446c792425ff0ab99 Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Mon, 24 Nov 2025 15:18:31 -0500 Subject: [PATCH 18/19] feat(explore-attr-breakdowns): Persisting search --- .../attributeBreakdowns/attributeDistributionContent.tsx | 7 +++++-- .../attributeBreakdowns/cohortComparisonContent.tsx | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/static/app/views/explore/components/attributeBreakdowns/attributeDistributionContent.tsx b/static/app/views/explore/components/attributeBreakdowns/attributeDistributionContent.tsx index 3f1028e7513e77..ad882b2ca50074 100644 --- a/static/app/views/explore/components/attributeBreakdowns/attributeDistributionContent.tsx +++ b/static/app/views/explore/components/attributeBreakdowns/attributeDistributionContent.tsx @@ -15,6 +15,7 @@ import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; import type {NewQuery} from 'sentry/types/organization'; import EventView from 'sentry/utils/discover/eventView'; +import {useQueryParamState} from 'sentry/utils/url/useQueryParamState'; import {useDebouncedValue} from 'sentry/utils/useDebouncedValue'; import usePageFilters from 'sentry/utils/usePageFilters'; import {prettifyAttributeName} from 'sentry/views/explore/components/traceItemAttributes/utils'; @@ -35,7 +36,9 @@ export type AttributeDistribution = Array<{ }>; export function AttributeDistribution() { - const [searchQuery, setSearchQuery] = useState(''); + const [searchQuery, setSearchQuery] = useQueryParamState({ + fieldName: 'attributeBreakdownsSearch', + }); const [page, setPage] = useState(0); const query = useQueryParamsQuery(); @@ -75,7 +78,7 @@ export function AttributeDistribution() { // Debouncing the search query here to ensure smooth typing, by delaying the re-mounts a little as the user types. // query here to ensure smooth typing, by delaying the re-mounts a little as the user types. - const debouncedSearchQuery = useDebouncedValue(searchQuery, 100); + const debouncedSearchQuery = useDebouncedValue(searchQuery ?? '', 100); const filteredAttributeDistribution: AttributeDistribution = useMemo(() => { const attributeDistribution = diff --git a/static/app/views/explore/components/attributeBreakdowns/cohortComparisonContent.tsx b/static/app/views/explore/components/attributeBreakdowns/cohortComparisonContent.tsx index 42ae17f5148fbb..aba759a7f2cdcb 100644 --- a/static/app/views/explore/components/attributeBreakdowns/cohortComparisonContent.tsx +++ b/static/app/views/explore/components/attributeBreakdowns/cohortComparisonContent.tsx @@ -17,6 +17,7 @@ import {IconChevron} from 'sentry/icons/iconChevron'; import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; import {getUserTimezone} from 'sentry/utils/dates'; +import {useQueryParamState} from 'sentry/utils/url/useQueryParamState'; import {useDebouncedValue} from 'sentry/utils/useDebouncedValue'; import useAttributeBreakdownComparison from 'sentry/views/explore/hooks/useAttributeBreakdownComparison'; import {useQueryParamsVisualizes} from 'sentry/views/explore/queryParams/context'; @@ -46,14 +47,16 @@ export function CohortComparison({ aggregateFunction: yAxis, range: selection.range, }); - const [searchQuery, setSearchQuery] = useState(''); + const [searchQuery, setSearchQuery] = useQueryParamState({ + fieldName: 'attributeBreakdownsSearch', + }); const sortingMethod: SortingMethod = 'rrr'; const [page, setPage] = useState(0); const theme = useTheme(); // Debouncing the search query here to ensure smooth typing, by delaying the re-mounts a little as the user types. // query here to ensure smooth typing, by delaying the re-mounts a little as the user types. - const debouncedSearchQuery = useDebouncedValue(searchQuery, 100); + const debouncedSearchQuery = useDebouncedValue(searchQuery ?? '', 100); const filteredRankedAttributes = useMemo(() => { const attrs = data?.rankedAttributes; From 10e213096e854fd6990ecc80a40c2504547e71ad Mon Sep 17 00:00:00 2001 From: Abdullah Khan <60121741+Abdkhan14@users.noreply.github.com> Date: Tue, 25 Nov 2025 12:07:50 -0500 Subject: [PATCH 19/19] feat(explore-attr-breakdowns): Adding new loading state UI (#103953) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - I will be consolidating shared components between the 2 views (comparison view, total distribution view and loading view) in following PRs Screenshot 2025-11-24 at 9 08 59 PM --------- Co-authored-by: Abdullah Khan --- .../attributeDistributionContent.tsx | 132 ++++++++-------- .../cohortComparisonContent.tsx | 33 ++-- .../components/attributeBreakdowns/styles.tsx | 146 ++++++++++++++++++ 3 files changed, 226 insertions(+), 85 deletions(-) diff --git a/static/app/views/explore/components/attributeBreakdowns/attributeDistributionContent.tsx b/static/app/views/explore/components/attributeBreakdowns/attributeDistributionContent.tsx index ad882b2ca50074..e33917eda34ef7 100644 --- a/static/app/views/explore/components/attributeBreakdowns/attributeDistributionContent.tsx +++ b/static/app/views/explore/components/attributeBreakdowns/attributeDistributionContent.tsx @@ -7,7 +7,6 @@ import {ButtonBar} from '@sentry/scraps/button/buttonBar'; import {Flex} from '@sentry/scraps/layout'; import LoadingError from 'sentry/components/loadingError'; -import LoadingIndicator from 'sentry/components/loadingIndicator'; import Panel from 'sentry/components/panels/panel'; import BaseSearchBar from 'sentry/components/searchBar'; import {IconChevron} from 'sentry/icons/iconChevron'; @@ -126,76 +125,74 @@ export function AttributeDistribution() { setPage(0); }, [filteredAttributeDistribution]); + if (isAttributeBreakdownsError || isCohortCountError) { + return ; + } + return ( - {isAttributeBreakdownsLoading || isCohortCountLoading ? ( - - ) : isAttributeBreakdownsError || isCohortCountError ? ( - - ) : ( - - - { - setSearchQuery(q); - }} - query={debouncedSearchQuery} - size="sm" - /> - - - {filteredAttributeDistribution.length > 0 ? ( - - - {filteredAttributeDistribution - .slice(page * CHARTS_PER_PAGE, (page + 1) * CHARTS_PER_PAGE) - .map(attribute => ( - - ))} - - - -