Skip to content

Commit 614b5d8

Browse files
narsaynorathnarsaynorath
authored
feat(explore): Convert buttons into context menu (#81219)
Convert the buttons for alerts and add to dashboards into a context menu. Shifts over a bunch of the code for both the alerts and dashboards buttons into a context menu component that's specific for explore charts. I needed to extract the logic for getting the sort option to a helper function I could use in my callback. This is because visualizes is a list of items and we map over it for charts, meaning I can't use a hook to reference the correct chart at the point of instantiation, it needs to be used in a callback that's defined during that mapping.
1 parent 25662e4 commit 614b5d8

File tree

6 files changed

+234
-200
lines changed

6 files changed

+234
-200
lines changed

‎static/app/views/explore/charts/index.tsx

+8-60
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,11 @@ import type {Dispatch, SetStateAction} from 'react';
22
import {Fragment, useCallback, useEffect, useMemo} from 'react';
33
import styled from '@emotion/styled';
44

5-
import Feature from 'sentry/components/acl/feature';
65
import {getInterval} from 'sentry/components/charts/utils';
76
import {CompactSelect} from 'sentry/components/compactSelect';
8-
import {DropdownMenu} from 'sentry/components/dropdownMenu';
97
import {Tooltip} from 'sentry/components/tooltip';
108
import {CHART_PALETTE} from 'sentry/constants/chartPalette';
11-
import {IconClock, IconGraph, IconSubscribed} from 'sentry/icons';
9+
import {IconClock, IconGraph} from 'sentry/icons';
1210
import {t} from 'sentry/locale';
1311
import {space} from 'sentry/styles/space';
1412
import {dedupeArray} from 'sentry/utils/dedupeArray';
@@ -18,12 +16,9 @@ import {
1816
prettifyParsedFunction,
1917
} from 'sentry/utils/discover/fields';
2018
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
21-
import useOrganization from 'sentry/utils/useOrganization';
2219
import usePageFilters from 'sentry/utils/usePageFilters';
23-
import useProjects from 'sentry/utils/useProjects';
2420
import {formatVersion} from 'sentry/utils/versions/formatVersion';
25-
import {Dataset} from 'sentry/views/alerts/rules/metric/types';
26-
import {AddToDashboardButton} from 'sentry/views/explore/components/addToDashboardButton';
21+
import ChartContextMenu from 'sentry/views/explore/components/chartContextMenu';
2722
import {useChartInterval} from 'sentry/views/explore/hooks/useChartInterval';
2823
import {useDataset} from 'sentry/views/explore/hooks/useDataset';
2924
import {useVisualizes} from 'sentry/views/explore/hooks/useVisualizes';
@@ -33,7 +28,6 @@ import Chart, {
3328
} from 'sentry/views/insights/common/components/chart';
3429
import ChartPanel from 'sentry/views/insights/common/components/chartPanel';
3530
import {useSortedTimeSeries} from 'sentry/views/insights/common/queries/useSortedTimeSeries';
36-
import {getAlertsUrl} from 'sentry/views/insights/common/utils/getAlertsUrl';
3731
import {CHART_HEIGHT} from 'sentry/views/insights/database/settings';
3832

3933
import {useGroupBys} from '../hooks/useGroupBys';
@@ -67,9 +61,6 @@ export const EXPLORE_CHART_GROUP = 'explore-charts_group';
6761
// TODO: Update to support aggregate mode and multiple queries / visualizations
6862
export function ExploreCharts({query, setError}: ExploreChartsProps) {
6963
const pageFilters = usePageFilters();
70-
const organization = useOrganization();
71-
const {projects} = useProjects();
72-
7364
const [dataset] = useDataset();
7465
const [visualizes, setVisualizes] = useVisualizes();
7566
const [interval, setInterval, intervalOptions] = useChartInterval();
@@ -184,29 +175,6 @@ export function ExploreCharts({query, setError}: ExploreChartsProps) {
184175
? 'area'
185176
: 'bar';
186177

187-
const project =
188-
projects.length === 1
189-
? projects[0]
190-
: projects.find(p => p.id === `${pageFilters.selection.projects[0]}`);
191-
const singleProject =
192-
(pageFilters.selection.projects.length === 1 || projects.length === 1) &&
193-
project;
194-
const alertsUrls = singleProject
195-
? visualizeYAxes.map(yAxis => ({
196-
key: yAxis,
197-
label: yAxis,
198-
to: getAlertsUrl({
199-
project,
200-
query,
201-
pageFilters: pageFilters.selection,
202-
aggregate: yAxis,
203-
orgSlug: organization.slug,
204-
dataset: Dataset.EVENTS_ANALYTICS_PLATFORM,
205-
interval,
206-
}),
207-
}))
208-
: undefined;
209-
210178
const data = getSeries(dedupedYAxes, formattedYAxes);
211179

212180
const outputTypes = new Set(
@@ -251,32 +219,12 @@ export function ExploreCharts({query, setError}: ExploreChartsProps) {
251219
options={intervalOptions}
252220
/>
253221
</Tooltip>
254-
<Feature features="organizations:alerts-eap">
255-
<Tooltip
256-
title={
257-
singleProject
258-
? t('Create an alert for this chart')
259-
: t('Cannot create an alert when multiple projects are selected')
260-
}
261-
>
262-
<DropdownMenu
263-
triggerProps={{
264-
'aria-label': t('Create Alert'),
265-
size: 'sm',
266-
borderless: true,
267-
showChevron: false,
268-
icon: <IconSubscribed />,
269-
}}
270-
position="bottom-end"
271-
items={alertsUrls ?? []}
272-
menuTitle={t('Create an alert for')}
273-
isDisabled={!alertsUrls || alertsUrls.length === 0}
274-
/>
275-
</Tooltip>
276-
</Feature>
277-
<Feature features="organizations:dashboards-eap">
278-
<AddToDashboardButton visualizeIndex={index} />
279-
</Feature>
222+
<ChartContextMenu
223+
visualizeYAxes={visualizeYAxes}
224+
query={query}
225+
interval={interval}
226+
visualizeIndex={index}
227+
/>
280228
</ChartHeader>
281229
<Chart
282230
height={CHART_HEIGHT}

‎static/app/views/explore/components/addToDashboardButton.tsx

-97
This file was deleted.

‎static/app/views/explore/components/chartContextMenu.tsx

+92
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import {DropdownMenu, type MenuItemProps} from 'sentry/components/dropdownMenu';
2+
import {IconEllipsis} from 'sentry/icons';
3+
import {t} from 'sentry/locale';
4+
import useOrganization from 'sentry/utils/useOrganization';
5+
import usePageFilters from 'sentry/utils/usePageFilters';
6+
import useProjects from 'sentry/utils/useProjects';
7+
import {Dataset} from 'sentry/views/alerts/rules/metric/types';
8+
import {useAddToDashboard} from 'sentry/views/explore/hooks/useAddToDashboard';
9+
import {getAlertsUrl} from 'sentry/views/insights/common/utils/getAlertsUrl';
10+
11+
function ChartContextMenu({
12+
visualizeIndex,
13+
visualizeYAxes,
14+
query,
15+
interval,
16+
}: {
17+
interval: string;
18+
query: string;
19+
visualizeIndex: number;
20+
visualizeYAxes: string[];
21+
}) {
22+
const {addToDashboard} = useAddToDashboard();
23+
const organization = useOrganization();
24+
25+
const {projects} = useProjects();
26+
const pageFilters = usePageFilters();
27+
28+
const project =
29+
projects.length === 1
30+
? projects[0]
31+
: projects.find(p => p.id === `${pageFilters.selection.projects[0]}`);
32+
const singleProject =
33+
(pageFilters.selection.projects.length === 1 || projects.length === 1) && project;
34+
35+
const alertsUrls = singleProject
36+
? visualizeYAxes.map(yAxis => ({
37+
key: yAxis,
38+
label: yAxis,
39+
to: getAlertsUrl({
40+
project,
41+
query,
42+
pageFilters: pageFilters.selection,
43+
aggregate: yAxis,
44+
orgSlug: organization.slug,
45+
dataset: Dataset.EVENTS_ANALYTICS_PLATFORM,
46+
interval,
47+
}),
48+
}))
49+
: undefined;
50+
51+
const items: MenuItemProps[] = [];
52+
53+
if (organization.features.includes('alerts-eap')) {
54+
items.push({
55+
key: 'create-alert',
56+
label: t('Create an alert for'),
57+
children: alertsUrls ?? [],
58+
tooltip: !singleProject
59+
? t('Cannot create an alert when multiple projects are selected')
60+
: undefined,
61+
disabled: !alertsUrls || alertsUrls.length === 0 || !singleProject,
62+
isSubmenu: true,
63+
});
64+
}
65+
66+
if (organization.features.includes('dashboards-eap')) {
67+
items.push({
68+
key: 'add-to-dashboard',
69+
label: t('Add to Dashboard'),
70+
onAction: () => addToDashboard(visualizeIndex),
71+
});
72+
}
73+
74+
if (items.length === 0) {
75+
return null;
76+
}
77+
78+
return (
79+
<DropdownMenu
80+
triggerProps={{
81+
size: 'sm',
82+
borderless: true,
83+
showChevron: false,
84+
icon: <IconEllipsis />,
85+
}}
86+
position="bottom-end"
87+
items={items}
88+
/>
89+
);
90+
}
91+
92+
export default ChartContextMenu;

‎static/app/views/explore/components/addToDashboardButton.spec.tsx renamed to ‎static/app/views/explore/hooks/useAddToDashboard.spec.tsx

+15-15
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
22

33
import {openAddToDashboardModal} from 'sentry/actionCreators/modal';
44
import {DisplayType, WidgetType} from 'sentry/views/dashboards/types';
5-
import {AddToDashboardButton} from 'sentry/views/explore/components/addToDashboardButton';
5+
import {useAddToDashboard} from 'sentry/views/explore/hooks/useAddToDashboard';
66
import {useResultMode} from 'sentry/views/explore/hooks/useResultsMode';
77
import {useVisualizes} from 'sentry/views/explore/hooks/useVisualizes';
88
import {ChartType} from 'sentry/views/insights/common/components/chart';
@@ -11,6 +11,11 @@ jest.mock('sentry/actionCreators/modal');
1111
jest.mock('sentry/views/explore/hooks/useVisualizes');
1212
jest.mock('sentry/views/explore/hooks/useResultsMode');
1313

14+
function TestPage({visualizeIndex}: {visualizeIndex: number}) {
15+
const {addToDashboard} = useAddToDashboard();
16+
return <button onClick={() => addToDashboard(visualizeIndex)}>Add to Dashboard</button>;
17+
}
18+
1419
describe('AddToDashboardButton', () => {
1520
beforeEach(() => {
1621
jest.clearAllMocks();
@@ -29,15 +34,10 @@ describe('AddToDashboardButton', () => {
2934
jest.mocked(useResultMode).mockReturnValue(['samples', jest.fn()]);
3035
});
3136

32-
it('renders', async () => {
33-
render(<AddToDashboardButton visualizeIndex={0} />);
34-
await userEvent.hover(screen.getByLabelText('Add to Dashboard'));
35-
expect(await screen.findByText('Add to Dashboard')).toBeInTheDocument();
36-
});
37-
3837
it('opens the dashboard modal with the correct query for samples mode', async () => {
39-
render(<AddToDashboardButton visualizeIndex={0} />);
40-
await userEvent.click(screen.getByLabelText('Add to Dashboard'));
38+
render(<TestPage visualizeIndex={0} />);
39+
40+
await userEvent.click(screen.getByText('Add to Dashboard'));
4141

4242
// The table columns are encoded as the fields for the defaultWidgetQuery
4343
expect(openAddToDashboardModal).toHaveBeenCalledWith(
@@ -107,8 +107,8 @@ describe('AddToDashboardButton', () => {
107107
jest.fn(),
108108
]);
109109

110-
render(<AddToDashboardButton visualizeIndex={1} />);
111-
await userEvent.click(screen.getByLabelText('Add to Dashboard'));
110+
render(<TestPage visualizeIndex={1} />);
111+
await userEvent.click(screen.getByText('Add to Dashboard'));
112112

113113
// The group by and the yAxes are encoded as the fields for the defaultTableQuery
114114
expect(openAddToDashboardModal).toHaveBeenCalledWith(
@@ -163,8 +163,8 @@ describe('AddToDashboardButton', () => {
163163
it('uses the yAxes for the aggregate mode', async () => {
164164
jest.mocked(useResultMode).mockReturnValue(['aggregate', jest.fn()]);
165165

166-
render(<AddToDashboardButton visualizeIndex={0} />);
167-
await userEvent.click(screen.getByLabelText('Add to Dashboard'));
166+
render(<TestPage visualizeIndex={0} />);
167+
await userEvent.click(screen.getByText('Add to Dashboard'));
168168

169169
expect(openAddToDashboardModal).toHaveBeenCalledWith(
170170
expect.objectContaining({
@@ -219,8 +219,8 @@ describe('AddToDashboardButton', () => {
219219
jest.fn(),
220220
]);
221221

222-
render(<AddToDashboardButton visualizeIndex={0} />);
223-
await userEvent.click(screen.getByLabelText('Add to Dashboard'));
222+
render(<TestPage visualizeIndex={0} />);
223+
await userEvent.click(screen.getByText('Add to Dashboard'));
224224

225225
expect(openAddToDashboardModal).toHaveBeenCalledWith(
226226
expect.objectContaining({

0 commit comments

Comments
 (0)