Skip to content

Commit a241f69

Browse files
Zylphrexandrewshie-sentry
Zylphrex
authored and
andrewshie-sentry
committed
feat(explore): Turn explore modes into tabs (#89437)
This introduces a tab style UI for switching to/from aggregates mode. It's a bit of a hack to avoid changing the underlying data model as this is temporary as we figure out next steps.
1 parent 628f367 commit a241f69

File tree

10 files changed

+216
-80
lines changed

10 files changed

+216
-80
lines changed

‎static/app/views/explore/contexts/pageParamsContext/groupBys.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {decodeList} from 'sentry/utils/queryString';
66
export const UNGROUPED = '';
77

88
export function defaultGroupBys(): string[] {
9-
return ['span.op'];
9+
return [''];
1010
}
1111

1212
export function getGroupBysFromLocation(location: Location): string[] {

‎static/app/views/explore/contexts/pageParamsContext/index.spec.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ describe('PageParamsProvider', function () {
111111
'transaction',
112112
'timestamp',
113113
],
114-
groupBys: ['span.op'],
114+
groupBys: [''],
115115
mode: Mode.SAMPLES,
116116
query: '',
117117
sortBys: [{field: 'timestamp', kind: 'desc'}],
@@ -199,7 +199,7 @@ describe('PageParamsProvider', function () {
199199
expect(pageParams).toEqual({
200200
dataset: DiscoverDatasets.SPANS_EAP_RPC,
201201
fields: ['id', 'timestamp'],
202-
groupBys: ['span.op'],
202+
groupBys: [''],
203203
mode: Mode.AGGREGATE,
204204
query: '',
205205
sortBys: [{field: 'count(span.self_time)', kind: 'asc'}],

‎static/app/views/explore/hooks/useAddToDashboard.spec.tsx‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -287,12 +287,12 @@ describe('AddToDashboardButton', () => {
287287
// For Open in Widget Builder
288288
widgetAsQueryParams: expect.objectContaining({
289289
dataset: WidgetType.SPANS,
290-
defaultTableColumns: ['span.op', 'count(span.duration)'],
290+
defaultTableColumns: ['count(span.duration)'],
291291
defaultTitle: 'Custom Widget',
292292
defaultWidgetQuery:
293293
'name=&aggregates=count(span.duration)&columns=&fields=count(span.duration)&conditions=&orderby=-count(span.duration)',
294294
displayType: DisplayType.BAR,
295-
field: ['span.op', 'count(span.duration)'],
295+
field: ['count(span.duration)'],
296296
}),
297297
})
298298
);
@@ -352,7 +352,6 @@ describe('AddToDashboardButton', () => {
352352
widgetAsQueryParams: expect.objectContaining({
353353
dataset: WidgetType.SPANS,
354354
defaultTableColumns: [
355-
'span.op',
356355
'avg(span.duration)',
357356
'max(span.duration)',
358357
'min(span.duration)',
@@ -361,12 +360,7 @@ describe('AddToDashboardButton', () => {
361360
defaultWidgetQuery:
362361
'name=&aggregates=avg(span.duration)%2Cmax(span.duration)%2Cmin(span.duration)&columns=&fields=avg(span.duration)%2Cmax(span.duration)%2Cmin(span.duration)&conditions=&orderby=-avg(span.duration)',
363362
displayType: DisplayType.LINE,
364-
field: [
365-
'span.op',
366-
'avg(span.duration)',
367-
'max(span.duration)',
368-
'min(span.duration)',
369-
],
363+
field: ['avg(span.duration)', 'max(span.duration)', 'min(span.duration)'],
370364
}),
371365
})
372366
);

‎static/app/views/explore/hooks/useTab.spec.tsx‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import {renderHook} from 'sentry-test/reactTestingLibrary';
2+
3+
import {useLocation} from 'sentry/utils/useLocation';
4+
import {useNavigate} from 'sentry/utils/useNavigate';
5+
import {Mode} from 'sentry/views/explore/contexts/pageParamsContext/mode';
6+
import {Tab, useTab} from 'sentry/views/explore/hooks/useTab';
7+
8+
jest.mock('sentry/utils/useLocation');
9+
jest.mock('sentry/utils/useNavigate');
10+
11+
const mockUseLocation = jest.mocked(useLocation);
12+
const mockUseNavigate = jest.mocked(useNavigate);
13+
14+
describe('useTab', function () {
15+
it('uses spans as default tab', function () {
16+
mockUseLocation.mockReturnValueOnce({
17+
pathname: '/',
18+
query: {},
19+
} as any);
20+
21+
const {result} = renderHook(useTab);
22+
expect(result.current).toEqual([Tab.SPAN, expect.any(Function)]);
23+
});
24+
25+
it('uses span tab', function () {
26+
mockUseLocation.mockReturnValueOnce({
27+
pathname: '/',
28+
query: {table: 'span'},
29+
} as any);
30+
31+
const {result} = renderHook(useTab);
32+
expect(result.current).toEqual([Tab.SPAN, expect.any(Function)]);
33+
});
34+
35+
it('uses trace tab', function () {
36+
mockUseLocation.mockReturnValueOnce({
37+
pathname: '/',
38+
query: {table: 'trace'},
39+
} as any);
40+
41+
const {result} = renderHook(useTab);
42+
expect(result.current).toEqual([Tab.TRACE, expect.any(Function)]);
43+
});
44+
45+
it('sets span tab', function () {
46+
mockUseLocation.mockReturnValueOnce({
47+
pathname: '/',
48+
query: {},
49+
} as any);
50+
const mockNavigate = jest.fn();
51+
mockUseNavigate.mockReturnValue(mockNavigate);
52+
53+
const {result} = renderHook(useTab);
54+
result.current[1](Tab.SPAN);
55+
expect(mockNavigate).toHaveBeenLastCalledWith(
56+
expect.objectContaining({
57+
query: {
58+
mode: Mode.SAMPLES,
59+
table: Tab.SPAN,
60+
},
61+
})
62+
);
63+
});
64+
65+
it('sets trace tab', function () {
66+
mockUseLocation.mockReturnValueOnce({
67+
pathname: '/',
68+
query: {},
69+
} as any);
70+
const mockNavigate = jest.fn();
71+
mockUseNavigate.mockReturnValue(mockNavigate);
72+
73+
const {result} = renderHook(useTab);
74+
result.current[1](Tab.TRACE);
75+
expect(mockNavigate).toHaveBeenLastCalledWith(
76+
expect.objectContaining({
77+
query: {
78+
mode: Mode.SAMPLES,
79+
table: Tab.TRACE,
80+
},
81+
})
82+
);
83+
});
84+
});

‎static/app/views/explore/hooks/useTab.tsx‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ import {useCallback, useMemo} from 'react';
33
import {decodeScalar} from 'sentry/utils/queryString';
44
import {useLocation} from 'sentry/utils/useLocation';
55
import {useNavigate} from 'sentry/utils/useNavigate';
6+
import {
7+
Mode,
8+
updateLocationWithMode,
9+
} from 'sentry/views/explore/contexts/pageParamsContext/mode';
610

711
export enum Tab {
812
SPAN = 'span',
@@ -23,14 +27,17 @@ export function useTab(): [Tab, (tab: Tab) => void] {
2327

2428
const setTab = useCallback(
2529
(newTab: Tab) => {
26-
navigate({
30+
const target = {
2731
...location,
2832
query: {
2933
...location.query,
3034
table: newTab,
3135
cursor: undefined,
3236
},
33-
});
37+
};
38+
// when switching tabs, we should land in samples mode
39+
updateLocationWithMode(target, Mode.SAMPLES);
40+
navigate(target);
3441
},
3542
[location, navigate]
3643
);

‎static/app/views/explore/spans/spansTab.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,9 @@ export function SpansTabContentImpl({
112112
...(organization?.features?.includes('visibility-explore-equations')
113113
? ['equations' as const]
114114
: []),
115+
...(organization?.features?.includes('visibility-explore-tabs')
116+
? ['tabs' as const]
117+
: []),
115118
];
116119

117120
const queryType: 'aggregate' | 'samples' | 'traces' =
@@ -295,6 +298,7 @@ export function SpansTabContentImpl({
295298
samplesTab={samplesTab}
296299
setSamplesTab={setSamplesTab}
297300
isProgressivelyLoading={tableIsProgressivelyLoading}
301+
useTabs={organization.features.includes('visibility-explore-tabs')}
298302
/>
299303
<Toggle>
300304
<StyledButton

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

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
useExploreFields,
1313
useExploreMode,
1414
useSetExploreFields,
15+
useSetExploreMode,
1516
} from 'sentry/views/explore/contexts/pageParamsContext';
1617
import {Mode} from 'sentry/views/explore/contexts/pageParamsContext/mode';
1718
import {useSpanTags} from 'sentry/views/explore/contexts/spanTagsContext';
@@ -36,9 +37,83 @@ interface ExploreTablesProps extends BaseExploreTablesProps {
3637
isProgressivelyLoading: boolean;
3738
spansTableResult: SpansTableResult;
3839
tracesTableResult: TracesTableResult;
40+
useTabs: boolean;
3941
}
4042

4143
export function ExploreTables(props: ExploreTablesProps) {
44+
if (props.useTabs) {
45+
return <ExploreTablesTabbed {...props} />;
46+
}
47+
return <ExploreTablesUntabbed {...props} />;
48+
}
49+
50+
function ExploreTablesTabbed(props: ExploreTablesProps) {
51+
const mode = useExploreMode();
52+
const setMode = useSetExploreMode();
53+
54+
const fields = useExploreFields();
55+
const setFields = useSetExploreFields();
56+
57+
const {tags: numberTags} = useSpanTags('number');
58+
const {tags: stringTags} = useSpanTags('string');
59+
60+
const openColumnEditor = useCallback(() => {
61+
openModal(
62+
modalProps => (
63+
<ColumnEditorModal
64+
{...modalProps}
65+
columns={fields}
66+
onColumnsChange={setFields}
67+
stringTags={stringTags}
68+
numberTags={numberTags}
69+
/>
70+
),
71+
{closeEvents: 'escape-key'}
72+
);
73+
}, [fields, setFields, stringTags, numberTags]);
74+
75+
// HACK: This is pretty gross but to not break anything in the
76+
// short term, we avoid introducing/removing any fields on the
77+
// query. So we continue using the existing `mode` value and
78+
// coalesce it with the `tab` value` to create a single tab.
79+
const tab = mode === Mode.AGGREGATE ? mode : props.samplesTab;
80+
const setTab = useCallback(
81+
(option: Tab | Mode) => {
82+
if (option === Mode.AGGREGATE) {
83+
setMode(Mode.AGGREGATE);
84+
} else if (option === Tab.SPAN || option === Tab.TRACE) {
85+
props.setSamplesTab(option);
86+
}
87+
},
88+
[setMode, props]
89+
);
90+
91+
return (
92+
<Fragment>
93+
<SamplesTableHeader>
94+
<Tabs value={tab} onChange={setTab}>
95+
<TabList hideBorder>
96+
<TabList.Item key={Tab.SPAN}>{t('Spans')}</TabList.Item>
97+
<TabList.Item key={Tab.TRACE}>{t('Traces')}</TabList.Item>
98+
<TabList.Item key={Mode.AGGREGATE}>{t('Aggregates')}</TabList.Item>
99+
</TabList>
100+
</Tabs>
101+
<Button
102+
disabled={tab !== Tab.SPAN}
103+
onClick={openColumnEditor}
104+
icon={<IconTable />}
105+
>
106+
{t('Edit Table')}
107+
</Button>
108+
</SamplesTableHeader>
109+
{tab === Tab.SPAN && <SpansTable {...props} />}
110+
{tab === Tab.TRACE && <TracesTable {...props} />}
111+
{tab === Mode.AGGREGATE && <AggregatesTable {...props} />}
112+
</Fragment>
113+
);
114+
}
115+
116+
function ExploreTablesUntabbed(props: ExploreTablesProps) {
42117
const mode = useExploreMode();
43118

44119
return (

‎static/app/views/explore/toolbar/index.spec.tsx‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ describe('ExploreToolbar', function () {
180180
// Add a group by, and leave one unselected
181181
await userEvent.click(aggregates);
182182
const groupBy = screen.getByTestId('section-group-by');
183-
await userEvent.click(within(groupBy).getByRole('button', {name: 'span.op'}));
183+
await userEvent.click(within(groupBy).getByRole('button', {name: '\u2014'}));
184184
await userEvent.click(within(groupBy).getByRole('option', {name: 'release'}));
185185
expect(groupBys).toEqual(['release']);
186186
await userEvent.click(within(groupBy).getByRole('button', {name: 'Add Group'}));
@@ -570,23 +570,29 @@ describe('ExploreToolbar', function () {
570570
})
571571
);
572572

573+
let options;
573574
const section = screen.getByTestId('section-group-by');
574575

575-
expect(within(section).getByRole('button', {name: 'span.op'})).toBeEnabled();
576-
await userEvent.click(within(section).getByRole('button', {name: 'span.op'}));
577-
const groupByOptions1 = await within(section).findAllByRole('option');
578-
expect(groupByOptions1.length).toBeGreaterThan(0);
576+
expect(groupBys).toEqual(['']);
577+
578+
await userEvent.click(within(section).getByRole('button', {name: '\u2014'}));
579+
options = await within(section).findAllByRole('option');
580+
expect(options.length).toBeGreaterThan(0);
581+
await userEvent.click(within(section).getByRole('option', {name: 'span.op'}));
582+
expect(groupBys).toEqual(['span.op']);
579583

584+
await userEvent.click(within(section).getByRole('button', {name: 'span.op'}));
585+
options = await within(section).findAllByRole('option');
586+
expect(options.length).toBeGreaterThan(0);
580587
await userEvent.click(within(section).getByRole('option', {name: 'project'}));
581588
expect(groupBys).toEqual(['project']);
582589

583590
await userEvent.click(within(section).getByRole('button', {name: 'Add Group'}));
584591
expect(groupBys).toEqual(['project', '']);
585592

586-
await userEvent.click(within(section).getByRole('button', {name: 'None'}));
587-
const groupByOptions2 = await within(section).findAllByRole('option');
588-
expect(groupByOptions2.length).toBeGreaterThan(0);
589-
593+
await userEvent.click(within(section).getByRole('button', {name: '\u2014'}));
594+
options = await within(section).findAllByRole('option');
595+
expect(options.length).toBeGreaterThan(0);
590596
await userEvent.click(
591597
within(section).getByRole('option', {name: 'span.description'})
592598
);

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {ToolbarSortBy} from 'sentry/views/explore/toolbar/toolbarSortBy';
2121
import {ToolbarSuggestedQueries} from 'sentry/views/explore/toolbar/toolbarSuggestedQueries';
2222
import {ToolbarVisualize} from 'sentry/views/explore/toolbar/toolbarVisualize';
2323

24-
type Extras = 'dataset toggle' | 'equations';
24+
type Extras = 'dataset toggle' | 'equations' | 'tabs';
2525

2626
interface ExploreToolbarProps {
2727
extras?: Extras[];
@@ -44,9 +44,9 @@ export function ExploreToolbar({extras, width}: ExploreToolbarProps) {
4444
{extras?.includes('dataset toggle') && (
4545
<ToolbarDataset dataset={dataset} setDataset={setDataset} />
4646
)}
47-
<ToolbarMode mode={mode} setMode={setMode} />
47+
{!extras?.includes('tabs') && <ToolbarMode mode={mode} setMode={setMode} />}
4848
<ToolbarVisualize equationSupport={extras?.includes('equations')} />
49-
{mode === Mode.AGGREGATE && <ToolbarGroupBy />}
49+
{(extras?.includes('tabs') || mode === Mode.AGGREGATE) && <ToolbarGroupBy />}
5050
<ToolbarSortBy
5151
fields={fields}
5252
groupBys={groupBys}

0 commit comments

Comments
 (0)