Skip to content

feat(explore-suspect-attrs): Positioning floating trigger to the bottom right #94939

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 7, 2025
Merged

feat(explore-suspect-attrs): Positioning floating trigger to the bottom right #94939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0cbd2a7
feat(explore-suspect-attrs): Positioning floating trogger to the bott…
Jul 6, 2025
9c66878
feat(explore-sus-attrs): Fixing tests
Jul 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions static/app/views/explore/components/suspectTags/floatingTrigger.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type Props = {

export function FloatingTrigger({boxSelectOptions, triggerWrapperRef, chartInfo}: Props) {
const router = useRouter();
const pageCoords = boxSelectOptions.pageCoords;
const triggerPosition = boxSelectOptions.floatingTriggerPosition;

const [isDrawerOpen, setIsDrawerOpen] = useState(false);
const {openDrawer} = useDrawer();
Expand Down Expand Up @@ -73,15 +73,15 @@ export function FloatingTrigger({boxSelectOptions, triggerWrapperRef, chartInfo}
}
}, [boxSelectOptions, chartInfo, isDrawerOpen, openDrawer]);

if (!pageCoords) return null;
if (!triggerPosition) return null;

return createPortal(
<div
ref={triggerWrapperRef}
style={{
position: 'absolute',
top: pageCoords.y,
left: pageCoords.x,
top: triggerPosition.top,
left: triggerPosition.left,
}}
>
<List>
Expand Down
7 changes: 7 additions & 0 deletions static/app/views/explore/hooks/useChartBoxSelect.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ describe('useChartBoxSelect', () => {
const mockChartInstance = {
getModel: jest.fn(),
dispatchAction: jest.fn(),
convertToPixel: jest.fn().mockReturnValue([100, 200]),
getDom: jest.fn().mockReturnValue({
getBoundingClientRect: jest.fn().mockReturnValue({
left: 50,
top: 100,
}),
}),
};

const mockAxis = {
Expand Down
68 changes: 37 additions & 31 deletions static/app/views/explore/hooks/useChartBoxSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ export type BoxSelectOptions = {
} | null;
brush: EChartsOption['brush'];
clearSelection: () => void;
floatingTriggerPosition: {left: number; top: number} | null;
onBrushEnd: EChartBrushEndHandler;
onBrushStart: EChartBrushStartHandler;
pageCoords: {x: number; y: number} | null;
reActivateSelection: () => void;
toolBox: ToolboxComponentOption | undefined;
};
Expand Down Expand Up @@ -64,7 +64,10 @@ export function useChartBoxSelect({

// 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 [pageCoords, setPageCoords] = useState<{x: number; y: number} | null>(null);
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
Expand Down Expand Up @@ -96,47 +99,50 @@ export function useChartBoxSelect({

const area = evt.areas[0];

const [x0, x1] = area.coordRange[0];
const [y0, y1] = area.coordRange[1];

const clampedCoordRange: [[number, number], [number, number]] = [
[Math.max(xMin, x0), Math.min(xMax, x1)],
[Math.max(yMin, y0), Math.min(yMax, y1)],
];

const newBrushArea: EchartBrushAreas = [
{
...area,
coordRange: [
[
Math.max(xMin, area.coordRange[0][0]),
Math.min(xMax, area.coordRange[0][1]),
],
[
Math.max(yMin, area.coordRange[1][0]),
Math.min(yMax, area.coordRange[1][1]),
],
],
coordRange: clampedCoordRange,
},
];

setBrushArea(newBrushArea);
},
[chartRef]
);

useEffect(() => {
const handleMouseUp = (e: MouseEvent) => {
if (brushArea) {
setPageCoords({x: e.clientX, y: e.clientY + window.scrollY});
} else {
setPageCoords(null);
}
};
// Get the bottom right coordinates of the box
const [clamped_x1, clamped_y0] = [clampedCoordRange[0][1], clampedCoordRange[1][0]];

const wrapper = chartWrapperRef.current;
if (!wrapper) return;
// Convert the bottom right coordinates to pixel coordinates, so that we can use them to
// absolutely position the floating CTAs.
const [clamped_x1_pixels, clamped_y0_pixels] = chart.convertToPixel(
{xAxisIndex: 0, yAxisIndex: 0},
[clamped_x1, clamped_y0]
);

wrapper.addEventListener('mouseup', handleMouseUp);
}, [brushArea, chartWrapperRef]);
const chartRect = chart.getDom().getBoundingClientRect();
Copy link
Contributor

Choose a reason for hiding this comment

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

iirc, getBoundingClientRect can be performance heavy. If this is for the size and position of the chart, would it be reasonable to cache this? My thinking is that having this called on every onBrushEnd might not be ideal, but this might just be nitpicking.


if (chartRect) {
setFloatingTriggerPosition({
left: chartRect.left + clamped_x1_pixels,
top: chartRect.top + clamped_y0_pixels + window.scrollY,
});
}
},
[chartRef]
);

const clearSelection = useCallback(() => {
const chartInstance = chartRef.current?.getEchartsInstance();
chartInstance?.dispatchAction({type: 'brush', areas: []});
setBrushArea(null);
setPageCoords(null);
setFloatingTriggerPosition(null);
}, [chartRef]);

const handleOutsideClick = useCallback(
Expand Down Expand Up @@ -199,7 +205,7 @@ export function useChartBoxSelect({
chartRef.current,
enableBrushMode,
handleOutsideClick,
pageCoords,
floatingTriggerPosition,
forceReActivateSelection,
]);

Expand Down Expand Up @@ -237,7 +243,7 @@ export function useChartBoxSelect({
onBrushEnd,
onBrushStart,
toolBox,
pageCoords,
floatingTriggerPosition,
reActivateSelection,
clearSelection,
};
Expand All @@ -247,7 +253,7 @@ export function useChartBoxSelect({
brush,
toolBox,
onBrushStart,
pageCoords,
floatingTriggerPosition,
reActivateSelection,
clearSelection,
]);
Expand Down
Loading