Skip to content

Commit 8cc04a5

Browse files
ZylphrexZylphrex
authored
feat(explore): Support functions with no arguments (#94936)
We now have functions like epm and failure_rate that have no arguments.
1 parent 7c5215f commit 8cc04a5

File tree

8 files changed

+159
-43
lines changed

8 files changed

+159
-43
lines changed

‎static/app/components/arithmeticBuilder/grammar.pegjs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,20 @@ token
1010
}
1111

1212
func
13-
= func:name "(" spaces attr:attr spaces ")" {
14-
return tc.tokenFunction(func, attr, location());
13+
= func:name "(" attrs:attrs spaces ")" {
14+
return tc.tokenFunction(func, attrs, location());
15+
}
16+
17+
attrs = yes_attr / no_attr
18+
19+
yes_attr
20+
= spaces attr:attr spaces {
21+
return [attr];
22+
}
23+
24+
no_attr
25+
= spaces {
26+
return [];
1527
}
1628

1729
attr = typed_attr / untyped_attr

‎static/app/components/arithmeticBuilder/token/freeText.tsx

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import {IconParenthesis} from 'sentry/icons/iconParenthesis';
3434
import {IconSubtract} from 'sentry/icons/iconSubtract';
3535
import {t} from 'sentry/locale';
3636
import {space} from 'sentry/styles/space';
37+
import {defined} from 'sentry/utils';
3738
import type {AggregateParameter} from 'sentry/utils/fields';
3839

3940
interface ArithmeticTokenFreeTextProps {
@@ -56,6 +57,7 @@ export function ArithmeticTokenFreeText({
5657
item,
5758
ref,
5859
state,
60+
focusable: true,
5961
});
6062

6163
return (
@@ -118,6 +120,23 @@ function InternalInput({
118120

119121
const {dispatch, aggregations, getFieldDefinition} = useArithmeticBuilder();
120122

123+
const getNextFocusOverride = useCallback(
124+
(func?: string): string => {
125+
if (defined(func)) {
126+
const definition = getFieldDefinition(func);
127+
const parameterDefinitions: AggregateParameter[] = definition?.parameters ?? [];
128+
if (parameterDefinitions.length > 0) {
129+
// if they selected a function with arguments, move focus into the function argument
130+
return nextTokenKeyOfKind(state, token, TokenKind.FUNCTION);
131+
}
132+
}
133+
134+
// if they selected a function without arguments/parenthesis/operator, move focus into next free text
135+
return nextSimilarTokenKey(token.key);
136+
},
137+
[getFieldDefinition, state, token]
138+
);
139+
121140
const getFunctionDefault = useCallback(
122141
(func: string): string => {
123142
const definition = getFieldDefinition(func);
@@ -166,7 +185,11 @@ function InternalInput({
166185
type: 'REPLACE_TOKEN',
167186
token,
168187
text,
169-
focusOverride: {itemKey: nextSimilarTokenKey(token.key)},
188+
focusOverride: {
189+
itemKey: getNextFocusOverride(
190+
isTokenFunction(tok) ? tok.function : undefined
191+
),
192+
},
170193
});
171194
resetInputValue();
172195
return;
@@ -182,9 +205,7 @@ function InternalInput({
182205
type: 'REPLACE_TOKEN',
183206
token,
184207
text: `${input.substring(0, pos + 1)}${getFunctionDefault(maybeFunc)}`,
185-
focusOverride: {
186-
itemKey: nextTokenKeyOfKind(state, token, TokenKind.FUNCTION),
187-
},
208+
focusOverride: {itemKey: getNextFocusOverride(maybeFunc)},
188209
});
189210
resetInputValue();
190211
return;
@@ -199,10 +220,10 @@ function InternalInput({
199220
[
200221
aggregations,
201222
dispatch,
223+
getNextFocusOverride,
202224
getFunctionDefault,
203225
resetInputValue,
204226
setInputValue,
205-
state,
206227
token,
207228
]
208229
);
@@ -286,21 +307,17 @@ function InternalInput({
286307
const isFunction =
287308
typeof option.key === 'string' && option.key.startsWith(`${TokenKind.FUNCTION}:`);
288309

289-
const itemKey = isFunction
290-
? // if they selected a function, move focus into the function argument
291-
nextTokenKeyOfKind(state, token, TokenKind.FUNCTION)
292-
: // if they selected a parenthesis/operator, move focus into next free text
293-
nextSimilarTokenKey(token.key);
294-
295310
dispatch({
296311
type: 'REPLACE_TOKEN',
297312
token,
298313
text: isFunction ? getFunctionDefault(option.value) : option.value,
299-
focusOverride: {itemKey},
314+
focusOverride: {
315+
itemKey: getNextFocusOverride(isFunction ? option.value : undefined),
316+
},
300317
});
301318
resetInputValue();
302319
},
303-
[dispatch, getFunctionDefault, state, token, resetInputValue]
320+
[dispatch, getNextFocusOverride, getFunctionDefault, token, resetInputValue]
304321
);
305322

306323
const onPaste = useCallback((_evt: React.ClipboardEvent<HTMLInputElement>) => {

‎static/app/components/arithmeticBuilder/token/function.tsx

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,17 @@ export function ArithmeticTokenFunction({
3838
state,
3939
token,
4040
}: ArithmeticTokenFunctionProps) {
41-
if (token.attributes.length !== 1) {
42-
throw new Error('Only functions with 1 argument supported.');
41+
if (token.attributes.length > 1) {
42+
throw new Error('Only functions with at most 1 argument supported.');
4343
}
44-
const attribute = token.attributes[0]!;
44+
const attribute = token.attributes[0];
4545

4646
const ref = useRef<HTMLDivElement>(null);
4747
const {rowProps, gridCellProps} = useGridListItem({
4848
item,
4949
ref,
5050
state,
51+
focusable: defined(attribute), // if there are no attributes, it's not focusable
5152
});
5253

5354
const isFocused = item.key === state.selectionManager.focusedKey;
@@ -59,27 +60,29 @@ export function ArithmeticTokenFunction({
5960
{...rowProps}
6061
ref={ref}
6162
tabIndex={isFocused ? 0 : -1}
62-
aria-label={`${token.function}(${attribute.text})`}
63+
aria-label={`${token.function}(${attribute?.text ?? ''})`}
6364
aria-invalid={false}
6465
state={'valid'}
6566
>
6667
<FunctionGridCell {...gridCellProps}>{token.function}</FunctionGridCell>
6768
{'('}
68-
<BaseGridCell {...gridCellProps}>
69-
<InternalInput
70-
item={item}
71-
state={state}
72-
token={token}
73-
attribute={attribute}
74-
rowRef={ref}
75-
argumentIndex={0}
76-
/>
77-
{showUnfocusedState && (
78-
// Inject a floating span with the attribute name so when it's
79-
// not focused, it doesn't look like the placeholder text
80-
<FunctionArgumentOverlay>{attribute.attribute}</FunctionArgumentOverlay>
81-
)}
82-
</BaseGridCell>
69+
{defined(attribute) && (
70+
<BaseGridCell {...gridCellProps}>
71+
<InternalInput
72+
item={item}
73+
state={state}
74+
token={token}
75+
attribute={attribute}
76+
rowRef={ref}
77+
argumentIndex={0}
78+
/>
79+
{showUnfocusedState && (
80+
// Inject a floating span with the attribute name so when it's
81+
// not focused, it doesn't look like the placeholder text
82+
<FunctionArgumentOverlay>{attribute.attribute}</FunctionArgumentOverlay>
83+
)}
84+
</BaseGridCell>
85+
)}
8386
{')'}
8487
<BaseGridCell {...gridCellProps}>
8588
<DeleteFunction token={token} />

‎static/app/components/arithmeticBuilder/token/index.spec.tsx

Lines changed: 78 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
import {TokenGrid} from 'sentry/components/arithmeticBuilder/token/grid';
2121
import {FieldKind, getFieldDefinition} from 'sentry/utils/fields';
2222

23-
const aggregations = ['avg', 'sum', 'count', 'count_unique'];
23+
const aggregations = ['avg', 'sum', 'epm', 'count_unique'];
2424

2525
const functionArguments = [
2626
{name: 'span.duration', kind: FieldKind.MEASUREMENT},
@@ -93,7 +93,7 @@ describe('token', function () {
9393
expect(input).toHaveValue('');
9494
});
9595

96-
it('allow selecting function on mouse', async function () {
96+
it('allow selecting function using mouse', async function () {
9797
render(<Tokens expression="" />);
9898

9999
const input = screen.getByRole('combobox', {
@@ -143,7 +143,49 @@ describe('token', function () {
143143
});
144144
});
145145

146-
it('allows selection parenthesis using mouse', async function () {
146+
it('allows selecting function with no arguments using mouse', async function () {
147+
render(<Tokens expression="" />);
148+
149+
const input = screen.getByRole('combobox', {
150+
name: 'Add a term',
151+
});
152+
expect(input).toBeInTheDocument();
153+
154+
await userEvent.click(input);
155+
156+
// typing should reduce the options avilable in the autocomplete
157+
expect(screen.getAllByRole('option')).toHaveLength(5);
158+
await userEvent.type(input, 'epm');
159+
expect(screen.getAllByRole('option')).toHaveLength(1);
160+
161+
await userEvent.click(screen.getByRole('option', {name: 'epm'}));
162+
expect(await screen.findByLabelText('epm()')).toBeInTheDocument();
163+
164+
await waitFor(() => expect(getLastInput()).toHaveFocus());
165+
});
166+
167+
it('allows selecting function with no arguments using keyboard', async function () {
168+
render(<Tokens expression="" />);
169+
170+
const input = screen.getByRole('combobox', {
171+
name: 'Add a term',
172+
});
173+
expect(input).toBeInTheDocument();
174+
175+
await userEvent.click(input);
176+
177+
// typing should reduce the options avilable in the autocomplete
178+
expect(screen.getAllByRole('option')).toHaveLength(5);
179+
await userEvent.type(input, 'epm');
180+
expect(screen.getAllByRole('option')).toHaveLength(1);
181+
182+
await userEvent.type(input, '{ArrowDown}{Enter}');
183+
expect(await screen.findByLabelText('epm()')).toBeInTheDocument();
184+
185+
await waitFor(() => expect(getLastInput()).toHaveFocus());
186+
});
187+
188+
it('allows selecting parenthesis using mouse', async function () {
147189
render(<Tokens expression="" />);
148190

149191
const input = screen.getByRole('combobox', {
@@ -165,7 +207,7 @@ describe('token', function () {
165207
await waitFor(() => expect(getLastInput()).toHaveFocus());
166208
});
167209

168-
it('allows selection parenthesis using keyboard', async function () {
210+
it('allows selecting parenthesis using keyboard', async function () {
169211
render(<Tokens expression="" />);
170212

171213
const input = screen.getByRole('combobox', {
@@ -232,6 +274,27 @@ describe('token', function () {
232274
});
233275
});
234276

277+
it('autocompletes function token with no arguments when they reach the open parenthesis', async function () {
278+
render(<Tokens expression="" />);
279+
280+
const input = screen.getByRole('combobox', {
281+
name: 'Add a term',
282+
});
283+
expect(input).toBeInTheDocument();
284+
285+
await userEvent.click(input);
286+
await userEvent.type(input, 'epm(');
287+
288+
await waitFor(() => expect(getLastInput()).toHaveFocus());
289+
await userEvent.keyboard('{Escape}');
290+
291+
expect(
292+
await screen.findByRole('row', {
293+
name: 'epm()',
294+
})
295+
).toBeInTheDocument();
296+
});
297+
235298
it('autocompletes addition', async function () {
236299
render(<Tokens expression="" />);
237300

@@ -537,6 +600,17 @@ describe('token', function () {
537600
await userEvent.type(input, 'desc');
538601
expect(screen.getByRole('option')).toHaveTextContent('span.description');
539602
});
603+
604+
it('skips input when function has no arguments', async function () {
605+
render(<Tokens expression="epm()" />);
606+
await waitFor(() => {
607+
expect(
608+
screen.queryByRole('combobox', {
609+
name: 'Select an attribute',
610+
})
611+
).not.toBeInTheDocument();
612+
});
613+
});
540614
});
541615

542616
describe('ArithmeticTokenOperator', function () {

‎static/app/components/arithmeticBuilder/tokenizer.spec.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ describe('tokenizeExpression', function () {
7474
'avg( tags[foo, number] )',
7575
[s(0, ''), f(0, 'avg', [a(0, 'foo', 'number')]), s(1, '')],
7676
],
77+
['epm()', [s(0, ''), f(0, 'epm', []), s(1, '')]],
7778
])('tokenizes function `%s`', function (expression, expected) {
7879
expect(tokenizeExpression(expression)).toEqual(expected);
7980
});

‎static/app/components/arithmeticBuilder/tokenizer.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,10 @@ class TokenConverter {
185185

186186
tokenFunction(
187187
func: string,
188-
attribute: TokenAttribute,
188+
attributes: TokenAttribute[],
189189
location: LocationRange
190190
): TokenFunction {
191-
return new TokenFunction(location, func, [attribute]);
191+
return new TokenFunction(location, func, attributes);
192192
}
193193
}
194194

‎static/app/components/tokenizedInput/grid/useGridListItem.tsx

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,27 @@ import type {Node} from '@react-types/shared';
77
import {shiftFocusToChild} from 'sentry/components/tokenizedInput/token/utils';
88

99
interface UseGridListItemOptions<T> {
10+
focusable: boolean;
1011
item: Node<T>;
1112
ref: RefObject<HTMLDivElement | null>;
1213
state: ListState<T>;
1314
}
1415

15-
export function useGridListItem<T>({item, ref, state}: UseGridListItemOptions<T>) {
16+
export function useGridListItem<T>({
17+
focusable,
18+
item,
19+
ref,
20+
state,
21+
}: UseGridListItemOptions<T>) {
1622
const {rowProps, gridCellProps} = useGridListItemAria({node: item}, state, ref);
1723

1824
const onFocus = useCallback(
1925
(evt: FocusEvent<HTMLDivElement>) => {
20-
shiftFocusToChild(evt.currentTarget, item, state);
26+
if (focusable) {
27+
shiftFocusToChild(evt.currentTarget, item, state);
28+
}
2129
},
22-
[item, state]
30+
[focusable, item, state]
2331
);
2432

2533
return useMemo(() => {

‎static/app/components/tokenizedInput/token/deletableToken.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export function DeletableToken<T>({
3232
item,
3333
ref,
3434
state,
35+
focusable: true,
3536
});
3637

3738
const onKeyDownCapture = useCallback(

0 commit comments

Comments
 (0)